Skip to content

Conversation

@Mugen87
Copy link
Collaborator

@Mugen87 Mugen87 commented Jan 28, 2022

Related issue: #23255

Description

It seems the spec of Import Maps has not yet defined how they should work with Web Workers (see WICG/import-maps#2). Hence webgl_worker_offscreencanvas is currently broken.

This PR is a hotfix to make webgl_worker_offscreencanvas work again. However, the PR is essentially wrong since using import * as THREE from '../../../build/three.module.js'; does not allow the usage of example modules.

I'm afraid the missing import maps support in workers was not discussed in #23255.

/cc @marcofugaro

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Jan 28, 2022

@marcofugaro
Copy link
Contributor

This PR looks good to me.

examples/jsm/offscreen/ is not supposed to be imported by users anyway.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Jan 28, 2022

Yeah, but the problem is the introduction of import maps breaks all usage of example modules in workers. For example it's not possible to use OrbitControls in a worker anymore like in the following demo:

HTML: https://github.com/mrdoob/three.js/blob/dev/manual/examples/offscreencanvas-w-orbitcontrols.html
Worker Script: https://github.com/mrdoob/three.js/blob/dev/manual/examples/shared-orbitcontrols.js

@marcofugaro
Copy link
Contributor

Hmmm, might be worth looking into an import map polyfill for web workers, see guybedford/es-module-shims#206

@marcofugaro
Copy link
Contributor

Looks like polyfill still needs some work to be able to funciton in web workers.

@Mugen87 do you think we could use skypack for that particular example in the meantime? Like this: https://jsfiddle.net/b9fcn3dL/

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Jan 28, 2022

Yes, there is no other choice anyway. When the manual's editor is fixed (see #23363 (comment)), this approach can be applied to the worker demos.

However, I'm not sure what to tell devs who run into this issue with their app. Asking to stay with r136 is only acceptable if the polyfill works with workers soon (e.g. in one or two month). If not, we need a different solution.

I think we have to clarify if #23255 would have been merged if we would be aware of this restriction. If we say yes, we have to figure out something for the worker scenario.

@mrdoob
Copy link
Owner

mrdoob commented Jan 28, 2022

Cherry-picked in master.

@mrdoob mrdoob added this to the r137 milestone Jan 28, 2022
@mrdoob mrdoob closed this Jan 28, 2022
@mrdoob
Copy link
Owner

mrdoob commented Jan 28, 2022

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants