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

Provide a WebGPU build that re-exports from three #29156

Closed
isaac-mason opened this issue Aug 17, 2024 · 32 comments · Fixed by #29404
Closed

Provide a WebGPU build that re-exports from three #29156

isaac-mason opened this issue Aug 17, 2024 · 32 comments · Fixed by #29404
Labels
Milestone

Comments

@isaac-mason
Copy link
Contributor

isaac-mason commented Aug 17, 2024

Description

When using the WebGPU build introduced in r167 with vite, using recommended aliases configuration, I am unable to use renderer agnostic exports from libraries that also have webgl imports. Bundlers will have errors attempting to import webgl specific imports that are not exported by the webgpu build.

Solution

If the three/webgpu build re-exported from three, aliases would not be required to use the webgpu build with bundlers, and libraries could export both webgl and webgpu utilities that import from both three and three/webgpu.

Alternatives

A build that has both webgl and webgpu exports could solve some issues, but this is not as desirable as the proposed solution as bundler aliases would still be required to prevent duplication issues.

Additional context

https://twitter.com/onirenaud/status/1824688713542341119

@hybridherbst
Copy link
Contributor

+1 from me.

Besides the missing exports, I think what also should be clarified is how libraries are supposed to start adding support for WebGPU – not everyone will be able/willing to provide/maintain separate bundles for webgl vs. webgpu.

That unclarity includes three.js examples themself – an example:

  • GLTFExporter currently imports decompress, which is used to bring textures back from the GPU to the CPU
  • decompress (in TextureUtils) imports WebGLRenderer
  • is there a current plan for how that will work in WebGPURenderer, and how GLTFExporter will support both? Or will there be a hard cut at some point in time X, where GLTFExporter will switch over to WebGPURenderer?

@GitHubDragonFly
Copy link
Contributor

+1 from me as well but there is a forum topic which suggests that this might not be happening.

@donmccurdy
Copy link
Collaborator

is there a current plan for how that will work in WebGPURenderer, and how GLTFExporter will support both?

My feeling here would be that we may need to work out a way for GLTFExporter not to import a renderer. Either to require pre-processing the scene, or specifying some configuration on the exporter that provides a decompress implementation or renderer.

@Mugen87
Copy link
Collaborator

Mugen87 commented Sep 3, 2024

How about doing something like this:

const exporter = new THREE.GLTFExporter();
exporter.setTextureUtils( utils ); 

decompress() also relies on ShaderMaterial. According to the migration that has been done up to this point, it's probably best to create a separate TextureUtilsGPU module and implement all helper functions based on WebGPURenderer and node material.

USDZExporter requires the same change as well, btw.

@isaac-mason
Copy link
Contributor Author

isaac-mason commented Sep 3, 2024

To clarify the proposal in this issue with regards to the forums post
https://discourse.threejs.org/t/three-webgpu-min-js-to-provide-webglrenderer-export/69075

The idea of separate builds is to avoid having both renderers in one file. So I would say no, it is not possible to have WebGLRenderer included in three.webgpu.min.js.
When WebGPURenderer more matures, there will be no need for WebGLRenderer anymore so you can rely on the built-in WebGL fallback.

In my mind this issue is not in opposition to having another WebGPURenderer build that does not contain WebGLRenderer exports, this would primarily support use cases where bundlers are not used.

However, using that same build for bundler use cases is problematic, which is why this issue is asking for the three/webgpu entrypoint to re-export from three.

Something we can anticipate will be common during the transition period is libraries exporting both WebGLRenderer and WebGPURenderer versions of utilities. Without the proposed change, libraries cannot easily do this.

@Mugen87
Copy link
Collaborator

Mugen87 commented Sep 4, 2024

If the three/webgpu build re-exported from three,

How does this work? Does the project need a specific configuration in its package.json?

@isaac-mason
Copy link
Contributor Author

isaac-mason commented Sep 8, 2024

I imagine we could approach changing three/webgpu to re-export from three like this:

  1. WebGPURenderer related code would need to import from three, not from src

  2. The existing Three.WebGPU.js barrel file would change from exporting from src, to re-exporting from three, e.g. export { ... } from 'three' and exporting WebGPURenderer specific exports.

  3. The three/webgpu and three/tsl entrypoints would change from build/three.webgpu.js to the new Three.WebGPU.js barrel file, which imports and re-exports code common to both renderers from three. At this point imports to e.g. Mesh from both three and three/webgpu would now resolve to the same class.

  4. The existing rollup builds for three.webgpu.js and three.webgpu.min.js that don't contain WebGLRenderer code could remain, but entrypoints in package.json wouldn't refer to them. Usage of these webgpu-only builds in non-bundled projects wouldn't change, importmaps would still be used as they are now.

@mrdoob
Copy link
Owner

mrdoob commented Sep 12, 2024

How about we introduce a new package?

Package three: WebGLRenderer and WebGLRenderer-Supported Addons
Package three-next: WebGPURenderer and WebGPURenderer-Supported Addons

We'd have to figure out how to publish two packages from a single repo...

Then, maybe in 10 years (😶)... We could make both the same?

/cc @sunag

@hybridherbst
Copy link
Contributor

I think that would force each thing in the ecosystem to also provide multiple packages, instead of being able to support "three" with WebGL and WebGPU with one ecosystem package...

My opinion is that re-exporting with WebGPU, making sure the code in three supports both systems, and providing examples for how third-party code supports both systems would be better.

@donmccurdy
Copy link
Collaborator

donmccurdy commented Sep 13, 2024

Yes, I am worried three-next as a separate package would break a great many things, and then require another large break to consolidate later on.

If we had no other solution, then publishing two versions (not two packages) under three and three@next instead might be an option. A package that supports both can specify that in the peer dependencies ranges. But there are still a lot of implications to think through here.

The suggestion from @isaac-mason sounds workable to me, @mrdoob do you have
particular concerns with that? You're worried about making sure that WebGPURenderer can be used without a bundler, and without getting WebGL stuff by accident, I think?

@CodyJasonBennett
Copy link
Contributor

CodyJasonBennett commented Sep 13, 2024

Another idea is re-exporting from three.module.js instead of three (or some manner of code-splitting) since we know WebGPU code relies on top-level-await and is therefore ESM only. Ideally, this doesn't change how tools will resolve or bundle three and should work OOTB for import maps. I've been keeping an eye on tree-shaking in core so WebGL stuff isn't pulled in by accident for this exactly #28670.

@donmccurdy
Copy link
Collaborator

donmccurdy commented Sep 15, 2024

I like @CodyJasonBennett's proposal in #29404.

Possibly related... do we need/want the three/webgpu and three/tsl entrypoints to be separate and identical? Unless there's a longer-term plan for that, I think we should consider consolidating.

@seancheno
Copy link

seancheno commented Oct 1, 2024

I stumbled upon this post after upgrading our three version to 0.169 from 0.167 and switching around our exports along with our ts config paths and vite config alias. We currently are using the WebGPURenderer entirely throughout our project.

vite config:

        alias: {
          'three/examples/jsm': 'three/examples/jsm',
          'three/addons': 'three/examples/jsm',
          'three/tsl': 'three/webgpu',
          'three': 'three/src/three.webgpu.js',
          },

ts config:

      resolve: {
        alias: {
          'three/examples/jsm': 'three/examples/jsm',
          'three/addons': 'three/examples/jsm',
          'three/tsl': 'three/webgpu',
          'three': 'three/src/three.webgpu.js',
          },
      },

I wasn't sure if we were doing this entirely correct, but everything seemed to work fine. By doing this along with changing a few import statements, it also allowed us to get rid of the "multiple instances of webgpu being imported" that we had originally.

The only issue we face at the moment is when importing and using the GLTFExporter:

import { GLTFExporter } from "three/examples/jsm/exporters/GLTFExporter";

Since the GLTFExporter uses the decompress method of TextureUtils, we get the error

node_modules/three/examples/jsm/utils/TextureUtils.js (10:1): "WebGLRenderer" is not exported by "node_modules/three/src/Three.WebGPU.js", imported by "node_modules/three/examples/jsm/utils/TextureUtils.js".

Which stems from this line in the decompress method:

if ( renderer === null ) {

     renderer = _renderer = new WebGLRenderer( { antialias: false } );

}

If we comment out that line, it removes the error and we are then able to use the GLTFExporter without any issues.

I see that there is the TextureUtilsGPU.js file now, but that it's not used anywhere within the three code. I apologize if I'm missing something obvious here, but wanted to bump this thread to showcase how we are unable to use the GLTFExporter at the moment unless we comment out that line. For the time being, we have created a mod of the GLTFExporter where the only change we make to it is replacing:

import { decompress } from './../utils/TextureUtils.js';

with

import { decompress } from 'three/examples/jsm/utils/TextureUtilsGPU.js';

And this seems to work.

Anyways, thanks for taking another look into this.

@donmccurdy
Copy link
Collaborator

Hoping to fix GLTFExporter with:

But there are more issues still to resolve in that PR.

And, I think we do need to agree on #29404 or something along those lines, so that custom vite configs are not required.

@RenaudRohlinger
Copy link
Collaborator

RenaudRohlinger commented Oct 10, 2024

Bumping this as a solution like #29404 is crucial for us to move forward with the WebGPURenderer.

The approach I suggested in this comment is more of a temporary patch that sidesteps the core issue. A solid implementation is needed to ensure the WebGPURenderer integrates seamlessly with the Three.js ecosystem and its libraries.

Could we consider advancing with the PR of Cody? /cc @sunag @mrdoob @Mugen87

@Mugen87
Copy link
Collaborator

Mugen87 commented Oct 10, 2024

I'm okay with #29404. If the merge conflicts are resolved, we can make a merge from my point of view.

@LukeWood
Copy link

Not sure if this is still a point of discussion - but I was just considering how much nicer it would be to have a completely distinct package for the gpu stuff. It's a huge head-ache trying to make sure you don't accidentally import three when you want three/webgpu, and I think the best thing we could do is "stop pretending" i.e. just have a fully distinct package that only exports webgpu compatible things.

Obviously, this might be super hard to implement in practice - but figured I'd call out that I was just wishing for this (which is how I found this issue)

@isaac-mason
Copy link
Contributor Author

isaac-mason commented Oct 14, 2024

It's a huge head-ache trying to make sure you don't accidentally import three when you want three/webgpu

@LukeWood #29404 will address that head-ache by making it completely fine to use renderer agnostic exports from three when using three/webgpu. e.g. importing Vector3 from three and three/webgpu will resolve to the same class and have the same identity.

@LukeWood
Copy link

Oh fantastic - ok then problem solved from my perspective. Vector, Box, Frustrum and Sphere have been the sources of my woes in this area.

@trusktr
Copy link
Contributor

trusktr commented Nov 5, 2024

(Please note, my bold text below is only highlighting important details that I believe need high consideration)

However, using that same build for bundler use cases is problematic, which is why this issue is asking for the three/webgpu entrypoint to re-export from three.

Simpler solution:

  • three exports everything, easy to use, people can import WebGLRenderer or WebGPURenderer. Both of them will exist for quite a long time. This will not require people to update their build tooling like the current setup does.
  • make two new optional paths for people that don't have tree shaking build tools and want slimmer options: three/webgl and three/webgpu.

This approach

  • keeps things working very easily for anyone already using three (most people).
  • allows people who want a slimmer library bundle, who do not have a tree shaking build, to pick one of the other two options.
  • allows people who know how to configure build tools to map three to either three/webgl or three/webgpu.
  • is the best way to make it easiest for the huge Three.js ecosystem to migrate to WebGPU. One less thing to worry about when installing/importing/updating Threejs.

But the best thing about this approach is that the huge amounts of projects that already import from three will have a very simple way to start trying WebGPU without modifying any build systems by changing this,

import {WebGLRenderer} from 'three'

to this,

import {WebGPURenderer} from 'three'

or even to the following if they want to use both (f.e. libraries that provide multiple render modes):

import {WebGLRenderer, WebGPURenderer} from 'three'

Another thing is, we probably all expect the three export will eventually export the WebGPU stuff. The suggested approach aligns with that. People using the new APIs will not have to change their build setups again in the future to go back to three from three/webgpu. One day you might even want to delete three/webgpu and force another build setup update, for which there is no need.


I know a big software company (you might use their products every day) has a build step that ingests the three lib, and now it would need to be modified to be able to support WebGPU, which is a pain that wouldn't exist with the above approach (although they would be able to opt into updating the build to slim it down with the optional secondary paths)

@trusktr
Copy link
Contributor

trusktr commented Nov 5, 2024

@CodyJasonBennett The solution in #29404 still requires people to update their builds (f.e. at some-big-company-you-know, this is a pain right now). Some of those builds produce outputs for internal org use, so now they are forced to figure out how to handle the webgpu path, when really the whole issue could be 100% avoided without build changes anywhere.

What do you think about the approach of the previous comment? It would allow all build setups to continue working as-is.

@CodyJasonBennett
Copy link
Contributor

@CodyJasonBennett The solution in #29404 still requires people to update their builds (f.e. at some-big-company-you-know, this is a pain right now). Some of those builds produce outputs for internal org use, so now they are forced to figure out how to handle the webgpu path, when really the whole issue could be 100% avoided without build changes anywhere.

That is mistaken. #29404 is one of a few ways that require no build configuration. I suspect the issue is exactly they were relying on three/webgpu and three being the same, or more specifically, their configuration. I know this company, SpaceX, but I'd really appreciate it if they stopped trying to extort me and my colleagues around WebGPU and actually contribute or hold a conversation in an open forum. It shouldn't have to be said, but nothing is going to happen otherwise. It might be news to them that WebGPU is not a living standard, and all code you write can and will break until then. Unless you ship an implementation like wgpu/Dawn, it is extremely irresponsible to use in production, despite the WebGL fallback.

Another thing is, we probably all expect the three export will eventually export the WebGPU stuff. The suggested approach aligns with that. People using the new APIs will not have to change their build setups again in the future to go back to three from three/webgpu. One day you might even want to delete three/webgpu and force another build setup update, for which there is no need.

Merging everything to three is an extremely breaking change that the ecosystem is not yet prepared for (namely, top-level-await e.g., #26626). I've been doing health checks and preparations across three's ecosystem for the last few years in preparation, and there's been a lot of velocity lately in runtimes and standardization itself. This is something I've been meaning to track in an issue, but I'm hopeful it will naturally resolve itself in parallel. We should hopefully figure out tree-shaking of WebGPU/Nodes by then, but I opted to code-split in #29404 to sidestep that matter in the meantime, although the WebGL entrypoint tree-shakes out WebGL code since #28670. I think we can agree that this is where we want to go once WebGPU is a living standard and implementations catch up.

@trusktr
Copy link
Contributor

trusktr commented Nov 5, 2024

The company is not SpaceX.

And yes, the change in #29404 requires a build configuration update because the three module does not include WebGPURenderer.

No one is trying to extort you!

@trusktr
Copy link
Contributor

trusktr commented Nov 5, 2024

About top-level await, common bundlers support ESM spec nowadays, including top-level await. I agree though that if someone has a bundler or build setup that does not understand top-level await, then they will need to update their build.

Here's a question:

Can we replace top-level await with something else? For example, move it into a function, f.e. inside of a Loader. If so, then the problem will be solved in a better way.

@trusktr
Copy link
Contributor

trusktr commented Nov 5, 2024

And again, because I suggested an improvement, does not mean anyone is trying to extort you @CodyJasonBennett.

The purpose of this conversation is to resolve a very real issue.

@CodyJasonBennett
Copy link
Contributor

CodyJasonBennett commented Nov 5, 2024

Again, you are mistaken or conflating two issues. three not including WebGPURenderer is intentional and the agreed upon resolve for the current API. Merging WebGPU code into three is for another discussion, one this company should really not be shy of supporting. If that means you, then this is indeed the right place to start, and perhaps we should focus into a separate issue. I maintain ecosystem support is not there for TLA, which is separate from ESM support (you can support ESM but not TLA). I can create a tracking issue, but I will not settle for mere conjecture, which long plagues these discussions, notoriously around tree-shaking. I expect tree-shaking to be a recurrent point and outlast concerns over TLA.

@trusktr
Copy link
Contributor

trusktr commented Nov 6, 2024

I see top-level await (TLA) has already been fixed in WebGPUBackend. Is there another one in another file?

Can you please briefly outline the reasons that WebGPURenderer should not be exported from three?

@CodyJasonBennett
Copy link
Contributor

Although I have personal reservations about putting experimental code into core (which is part of the absolute mess that created this issue), the technical reasons are as follows, which I explained further above in context:

This list is exhaustive from a purely technical POV, but the general issue of migration and general API is ongoing, and it's a massive one. We've dealt with some of the scarier issues, like #26140, and now have EXT_clip_control which should remove asymmetry between backends (but still require migration). I'll leave it to the maintainers for specifics.

I will yet again maintain that it is not needed to resolve this issue, and #29404 or its outlined alternatives are sufficient. I maintain that these are two separate issues, as three API does not reflect the supposed configuration you were relying on, and the recommendation of configuration and even use in production is irresponsible and misleading. As I've seen trouble historically from unnamed companies and discourse devolve into useless conjecture, I'm willing to bet $10,000 this is and will remain the case and limit to the latest major of all build tools and runtimes (Webpack, Parcel, Rollup, Vite, ESBuild, Rspack, Turbo, SWC, Nx, Metro). As a simplification, this enables ES2021+ ESM-only output. I will leave this only until February, when I'm expecting WebGPU to become a living standard, and we can move to commit to this once implementations catch up. You will find that this is indeed very difficult, and what I'm offering is both a correction and stepping stone to a final API.

@trusktr
Copy link
Contributor

trusktr commented Nov 6, 2024

Although I have personal reservations about putting experimental code into core (which is part of the absolute mess that created this issue),

An API could be marked "experimental" in @types/three and in threejs.org docs. Wouldn't that be enough?

  • top-level-await (TLA) as WebGPU API is asynchronous for requesting a device/adapter or any sync points

I believe this was fixed already. This test repo shows that esbuild works with target es2020, indicating no TLA is used anywhere.

That's a good problem to solve. I thought tree shaking was irrespective of import/export paths. Is that not true?

#29404 is definitely better than what we have currently because it provides a path to having both webgl and webgpu instead of a difficult split, however it still requires build tool updates downstream.

I'm just expressing that if we can upgrade with no build changes required, it would be great: we'd just update the three version number, webgl APIs would be available as before, and new webgpu APIs would be available too.

@CodyJasonBennett
Copy link
Contributor

I have elaborated now several times in explicit detail, and it's clear you are simply manipulating the project into pulling the trigger on merging the WebGL/WebGPU entrypoints because you, or a company that cannot be named, has configuration that incidentally relied on this being the API, after a string of irresponsible messaging that created this problem in the first place. That is not the purpose of this issue nor the current state of three API or any surrounding implementation (be it browsers, implementations, tooling—all of it). I will no longer entertain this here and expect any further progress I have enumerated to be continued elsewhere. You should have the direction you need to migrate to stable API, or continue configuration like before.

@trusktr
Copy link
Contributor

trusktr commented Nov 6, 2024

@CodyJasonBennett I get the impression you don't want to discuss this, thanks for the input.


I made a pull request to show the concept:

If it is decided that this proposal is the desired path, and more work is needed, I'll be willing to update it.

@mrdoob
Copy link
Owner

mrdoob commented Nov 7, 2024

Lets focus on #29404

@mrdoob mrdoob closed this as completed Nov 7, 2024
@mrdoob mrdoob reopened this Nov 7, 2024
@Mugen87 Mugen87 added this to the r171 milestone Nov 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.