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

WebGPURenderer: Build - three.webgpu.js #28650

Merged
merged 36 commits into from
Jun 30, 2024
Merged

Conversation

sunag
Copy link
Collaborator

@sunag sunag commented Jun 13, 2024

Related issue: #28328, #28328 (comment)

Description

TSL

I did some tests to include TSL in the build, the first attempt was THREE.TSL and use const { .. } = to import, which works well but is incompatible with tree shaking, the second attempt was to preserve three/tls but that would prevent having a single build file for everything. I think that since TSL will be part of the agnostic rendering system, it seems correct for it to be an essential central part of the file, just like other materials are.

To import a TSL, just use:

import { float, vec3, color } from 'three/tsl';

For class creation this is more familiar:

const material = THREE.MeshStandardNodeMaterial();

Renderer

Using the Renderer has become simpler too with THREE.:

const renderer = new THREE.WebGPURenderer();

One possibility is that we can keep a WebGLRenderer class using forceWebGL automatically.

PMREMGenerator

Previously it was necessary to reimport PMREMGenerator from a new path for webgpu, now it is possible to use the original THREE.PMREMGenerator syntax.

const sceneRT = new THREE.PMREMGenerator( renderer ).fromScene( scene );

RectAreaLightTexturesLib

I created a base LTC for use on WebGPU, WebGLRenderer also relies on this same class.

QuadMesh, PostProcessing

QuadMesh and PostProcessing can be used with THREE.QuadMesh and THREE.PostProcessing.

Lines2

We have a separate jsm/lines/webgpu class now to handle just TSL, previously we had a lot of GLSL mixed in between the class extensions.

Imports

{
	"imports": {
		"three": "../build/three.webgpu.js",
		"three/debug": "../src/Three.WebGPU.js",
		"three/addons/": "./jsm/"
	}
}

If you want to edit the files without having to recompile them, you can use the imports above.

Next steps

This simplifies the process for creating the tree shaking that will come with the next PR. Do you just need to update from three.webgpu.js to three.webgpu.nodes.js or three.nodes.js?

Copy link

github-actions bot commented Jun 13, 2024

📦 Bundle size

Full ESM build, minified and gzipped.

Filesize dev Filesize PR Diff
682.2 kB (169 kB) 682.2 kB (169 kB) +0 B

🌳 Bundle size after tree-shaking

Minimal build including a renderer, camera, empty scene, and dependencies.

Filesize dev Filesize PR Diff
459.4 kB (110.9 kB) 459.4 kB (110.9 kB) +0 B

@sunag
Copy link
Collaborator Author

sunag commented Jun 13, 2024

Seems that an update in puppeteer will be necessary.

@RenaudRohlinger
Copy link
Collaborator

RenaudRohlinger commented Jun 14, 2024

Could we also use this PR to move the WebGPURenderer and TSL files to a directory like /webgpu/? 😄

It feels a bit odd to point to the addons such as examples/jsm/renderers/webgpu/Three.js for the build. This change would also make it easier for maintainers to work on new the Renderer by separate the TSL and WebGPURenderer files from the hundreds of existing WebGLRenderer examples blended in.

The import map would then look like this:

{
	"imports": {
		"three": "../build/three.webgpu.js",
		"three/debug": "./webgpu/renderers/webgpu/Three.js",
		"three/addons/": "./jsm/"
	}
}

/cc @mrdoob @sunag @Mugen87

@Mugen87
Copy link
Collaborator

Mugen87 commented Jun 14, 2024

It wouldn't be examples/jsm/renderers/webgpu/Three.js but three/addons/renderers/webgpu/Three.js when importing from the npm package.

TBH, I did not have the impression so far that import paths containing addons are an issue.

This change would also make it easier for maintainers to work on new the Renderer by separate the TSL and WebGPURenderer files from the hundreds of existing WebGLRenderer examples blended in.

Um, I'm not sure I agree. What would be exactly easier?

@RenaudRohlinger
Copy link
Collaborator

RenaudRohlinger commented Jun 14, 2024

TBH, I did not have the impression so far that import paths containing addons are an issue.

It's a matter of best practice and trying to organize the repository more effectively. Why should the new Renderer and Node System be under the "examples/jsm" folder?
I understand that historically it was done this way, and while I don't mind maintaining that approach, it does seem counterintuitive.

It feels like anything that is not the WebGLRenderer is placed under examples/jsm by default.

I just feel that this PR will bring some fresh air to the repository and maybe we could take that opportunity to reorganize a bit! 😄

@Mugen87
Copy link
Collaborator

Mugen87 commented Jun 14, 2024

It feels like anything that is not the WebGLRenderer is placed under examples/jsm by default.

Um, why not moving Renderer, WebGPURenderer and TSL related modules into src? Wouldn't be that the more obvious approach?

The directory size of src or its number of files is not the critical metric. It's the size of the build files that matters. However, it should be possible to manage more code in src while keeping the sizes of files like three.module.js, three.module.min.js and three.cjs consistent.

@RenaudRohlinger
Copy link
Collaborator

Indeed, even better! 👍

@sunag sunag marked this pull request as ready for review June 14, 2024 19:00
@sunag sunag added this to the r166 milestone Jun 14, 2024
"three/addons/": "./jsm/",
"three/nodes": "./jsm/nodes/Nodes.js"
"three": "../build/three.webgpu.js",
"three/debug": "../src/Three.WebGPU.js",
Copy link
Collaborator

Choose a reason for hiding this comment

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

debug does not appear to be needed. Should we have an example in which it is?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I purposely put it in sandbox because I thought it could convey the idea of ​​debug since I can't make simple comments in the JSON, maybe there is a more suitable way to do this...

Copy link
Collaborator

Choose a reason for hiding this comment

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

Your choice... I'd vote to merge this PR ASAP if @Mugen87 approves.

Copy link
Owner

Choose a reason for hiding this comment

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

Let me think about the naming for all this for a couple of days 🙏

Copy link
Collaborator Author

@sunag sunag Jun 17, 2024

Choose a reason for hiding this comment

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

I replaced three/nodes to three/webgpu in package. I imagined that developers could use three alias in vite like:

// vite.config.js

import { defineConfig } from 'vite';

export default defineConfig({
  resolve: {
    alias: {
      'three': 'three/webgpu'
    }
  }
});

@mrdoob
Copy link
Owner

mrdoob commented Jun 22, 2024

I did some tests to include TSL in the build, the first attempt was THREE.TSL and use const { .. } = to import, which works well but is incompatible with tree shaking

Hmm, that would definitely more clean than import { float, vec3, color } from 'three';...

Why is it incompatible with tree shaking?

@sunag
Copy link
Collaborator Author

sunag commented Jun 22, 2024

Why is it incompatible with tree shaking?

I created a mini example in the link below, when I used const { .. } = , it imports all the values ​​from TSL, unlike when I use THREE.TSL.vec3() for example.

https://shorturl.at/x6AKZ

@sunag
Copy link
Collaborator Author

sunag commented Jun 27, 2024

I created a mini example in the link below, when I used const { .. } = , it imports all the values ​​from TSL, unlike when I use THREE.TSL.vec3() for example.

Same thing with vite.

Maybe we could add a three/tsl alias and advance? I'm a little worried about revisiting this PR every time we merge some PR, it feels like it's going to break at some point.

@mrdoob
Copy link
Owner

mrdoob commented Jun 28, 2024

Maybe we could add a three/tsl alias and advance?

Yes, let's try that first 👍

@mrdoob mrdoob modified the milestones: r166, r167 Jun 28, 2024
@Mugen87 Mugen87 merged commit 2c13adc into mrdoob:dev Jun 30, 2024
12 checks passed
@RenaudRohlinger
Copy link
Collaborator

For documentation purpose here is the correct way to use the new build:

// vite.config.js

import { defineConfig } from 'vite';

export default defineConfig({
  resolve: {
    alias: {
       'three/addons': 'three/examples/jsm',
       'three/tsl': 'three/webgpu',
       'three': 'three/webgpu',
    }
  }
});

The order is important otherwise three/tsl will try to resolve three/webgpu/tsl for example.

/cc @sunag

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.

None yet

5 participants