Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WebGPUBackend: Request static adapter in init async. #26242

Closed
wants to merge 2 commits into from

Conversation

danrossi
Copy link

When bundling the WebGPURenderer the external await breaks bundling when not an es6 module bundle. When bundling umd/iife. Therefore create the static adapter in the async init method works.

The error produced is

[!] Error: Module format iife does not support top-level await. Use the "es" or "system" output formats rather.

Modules will end up as these bundles.

@sunag sunag changed the title request static adapter in init async WebGPUBackend: Request static adapter in init async. Jun 12, 2023
@sunag
Copy link
Collaborator

sunag commented Jun 12, 2023

Maybe the challenge here is that we need something that is also compatible with WebGPUBackend.hasFeature().
See webgpu_loader_gltf_compressed example.

Repository owner deleted a comment from Alexander-103 Jun 13, 2023
@danrossi
Copy link
Author

I'm sorry. renderer.hasFeature( 'texture-compression-astc' ) has a null error. Adapter not available. I'll check if init is set.

@danrossi
Copy link
Author

danrossi commented Jun 13, 2023

I see why there is a static adapter init is called after. I'm looking to see if rollup can bundle with an async wrapper, there is a ticket for that. Maybe this change isn't necessary. The static adapter is a work around to the required async in init. So maybe the hasfeature needs to be async.

@danrossi
Copy link
Author

A deferred futures promise if no adapter is ready is possibly the solution. So they are resolved in init.

@danrossi
Copy link
Author

I've done this many times but I am stumped trying to return the value to the stored resolve. I'm getting stack issues. Trying a few ways

 new Promise((resolve, reject) => {
			if (!this.adapter) {
				_deferFeatures.push(async () => this.hasFeature( name ).resolve);
			} else {
				const adapter = this.adapter || _staticAdapter;

				//

				const features = Object.values( GPUFeatureName );

				if ( features.includes( name ) === false ) {

					reject( 'THREE.WebGPURenderer: Unknown WebGPU GPU feature: ' + name );
					//throw new Error( 'THREE.WebGPURenderer: Unknown WebGPU GPU feature: ' + name );

				}

				//

				resolve(adapter.features.has( name ));
			}
		});

@danrossi
Copy link
Author

This is the best I could come up with. https://jsfiddle.net/danrossi303/jouqz314/46/

@danrossi
Copy link
Author

Ive tested the example works. Resolving the deferred promises is the best I could come up with but works.

@@ -110,15 +110,16 @@ class KTX2Loader extends Loader {

}

detectSupport( renderer ) {
async detectSupport( renderer ) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has already been considered, related #26006

WebGPURenderer proposes not to modify the examples created in WebGLRenderer to preserve the API.
This can be a challenge for certain changes, note that this change in the KTX2Loader we have to update all WebGL examples and documentation related.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That change moved the async top level and caused the problem. So the examples need to be updated now with this change or should I close ? I had a duplicate in my gpu renderer module build.

@marcofugaro
Copy link
Contributor

The three.js iife builds have been already deprecated, are we sure we want to support iife build for the WebGPU Renderer?

@Mugen87
Copy link
Collaborator

Mugen87 commented Jun 21, 2023

Yes, the builds files are deprecated but the user can still create its own custom UMD (app) build. This is option is supported and has not been deprecated.

@marcofugaro
Copy link
Contributor

marcofugaro commented Jun 21, 2023

Maybe we should add a build step for the WebGPU renderer then. So the issue with top-level await will be solved during the transpialtion step.
But not sure what is the plan for this.

@LeviPesin
Copy link
Contributor

@danrossi Can the builder that you are using generate async IIFEs? I.e. like (async function() {code})()?

@danrossi
Copy link
Author

rollup complains and fails to build.

The end bundles are not es6 imports. I have an example project showing how it's bundled. More so Terser is completely removing some of the class names when mangling and needs a filter.

https://github.com/danrossi/three-troika/blob/main/build/troika-lib.js
https://github.com/danrossi/three-webgpu-renderer/blob/master/rollup.config.js#L91

I wouldnt be able to bundle my projects as importable es6 files into the page. They have to be loaded as an external script in an iife bundle. The end bundle I presume will always be an iife. Unless there is other methods ?

@danrossi
Copy link
Author

danrossi commented Jun 21, 2023

I apologise this is possible I will experiment with this and maybe iife can be deprecated on the end result bundles.

<script type="module" src="index.js" />

It needs to be pre-exported into a namespace somehow so doesnt need imports into the page.

@danrossi
Copy link
Author

Just another thought. It should be done in the renderer init so its not called when not using the renderer. I have it setup to use webgpu renderer or fallback to webgl render. It shouldn't be doing adapter requests outside of this until a webgpu check is made.

https://github.com/danrossi/three-webgpu-renderer/blob/master/tests/webgpu_video_panorama_equirectangular.html#L128

if (await WebGPU.isAvailable()) {
				console.log("Using WebGPU");
				renderer = new WebGPURenderer();
				texture.colorSpace = THREE.SRGBColorSpace;
				material = new THREE.MeshBasicMaterial({ map: texture });
			} else {
				console.log("Using WebGL");
				renderer = new THREE.WebGLRenderer();
				texture.colorSpace = THREE.LinearSRGBColorSpace;

				//use shader material for performance srgb decode fix
				material = new THREE.ShaderMaterial( {

The async is on isAvailable. I didn't try and include this one. I'm just overriding the path resolving.

https://github.com/danrossi/three-webgpu-renderer/blob/master/src/capabilities/WebGPU.js#L3

@danrossi
Copy link
Author

danrossi commented Jul 4, 2023

I don't know what to do. I have to create a copy of the file and merge method changes right now. Logically the async code should be run after checking for webgpu support when falling back to webgl. Like my example.

To bundle it correctly. modules is not the standard delivery use case for applications using it. I contacted rollup and they suggested hacking to export a namespace to globalThis to replicate iffe in a module. But I personally deliver files with just in time code run in the included script and nothing is imported and run on the page as end users have different technical abilities. I've yet to experiment with the work around.

https://github.com/danrossi/three-webgpu-renderer/blob/master/src/renderers/webgpu/WebGPUBackend.js

@danrossi
Copy link
Author

danrossi commented Jul 25, 2023

I don't know how to easily resolve this. I don't know what the tests need. I have to keep a copy of the patch and keep merging changes. I tested out a globalThis.THREE = THREE hack it worked with no top level async there. No need to run import code on the page just include like

 <script type="module" src="../build/three-webgpu.js"></script>

However the await in the top level prevents the code in the rest of the page from seeing global THREE when loading. Tonnes of application bundles out there are loaded onto the page in an iife wrapper rather than module import. I can't see how to depricate that method yet it's premature. Rollup can't bundle top level async

see rollup/rollup#3623 (comment)

When working with something like videojs as I do. It can't see plugins registered within modules unless its exported back to the page inline and registered inline which is not normal use case. I just checked. Inside an iiife is fine. But I still believe even an async top level iife will block the rest of the inline code from running. If the async is done when initializing the renderer after a webgpu check this problem will go away, the check isn't needed if using webgl instead as fallback.

I'll have to keep patching the file to keep it up to date.

@Mugen87
Copy link
Collaborator

Mugen87 commented Aug 28, 2023

The project needs a static adapter so we can't merge this PR. The issue needs to be solved on build tool level (see #26626) or via a single build file (#25947).

@Mugen87 Mugen87 closed this Aug 28, 2023
@danrossi
Copy link
Author

I'm making hacks in the external module until I have time to figure it out. There is a rollup plugin for iife I've just been alerted about but yet to test it.

However still this code shouldn't be running when trying to support both renderers. So wrapped in a gpu check. That is a logical issue. It will be run on page load.

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.

5 participants