Skip to content

Conversation

@sunag
Copy link
Collaborator

@sunag sunag commented Jun 3, 2023

Related issue: Fixes #26184

Description

This PR fix the redundancy in the attributes and index set state.

webgpu_materials webgpu_sprites
image image

@sunag sunag added this to the r154 milestone Jun 3, 2023
@github-actions
Copy link

github-actions bot commented Jun 3, 2023

📦 Bundle size

Full ESM build, minified and gzipped.

Filesize dev Filesize PR Diff
644.1 kB (159.5 kB) 644.2 kB (159.5 kB) +117 B

🌳 Bundle size after tree-shaking

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

Filesize dev Filesize PR Diff
434.1 kB (105.1 kB) 434.2 kB (105.1 kB) +47 B

@@ -1,3 +1,7 @@
/*// debugger tools
import 'https://greggman.github.io/webgpu-avoid-redundant-state-setting/webgpu-check-redundant-state-setting.js';
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@greggman I've added its lib here, so it can inspire other developers to build good tools for it :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can add some debug toggle (for example, global variable like window.DEBUG), which will dynamic-import the lib if enabled?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Or maybe THREE.Renderer( { debug: true } )

Copy link
Contributor

@LeviPesin LeviPesin Jun 3, 2023

Choose a reason for hiding this comment

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

Yeah... And maybe place renderer.info also under that toggle? Like, it isn't updating (and there is no such property) when it is disabled.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah... And maybe place renderer.info also under that toggle? Like, it isn't updating (and there is no such property) when it is disabled.

I think debug would be a property for development, info can still be useful for inspection.

@sunag sunag requested a review from Mugen87 June 3, 2023 20:59
@sunag
Copy link
Collaborator Author

sunag commented Jun 3, 2023

@Mugen87 I would like to know if it's ok for you to add uuid in the BufferAttribute?

@greggman
Copy link
Contributor

greggman commented Jun 3, 2023

forgive me if there are good reasons to use UUID in a cache bit just in case you didn't know you can use objects as keys in Map and WeakMap

@sunag
Copy link
Collaborator Author

sunag commented Jun 3, 2023

forgive me if there are good reasons to use UUID in a cache bit just in case you didn't know you can use objects as keys in Map and WeakMap

I don't think *Map would fit in this case, the UUID was an exception to the Hash because it was comparing the Geometry and not the attributes individually, looking closer I think that just simple comparisons should solve it, but I'll have to test it.

@sunag
Copy link
Collaborator Author

sunag commented Jun 3, 2023

Now it looks good :) . Merging...

@sunag sunag merged commit 52bc19c into mrdoob:dev Jun 3, 2023
@sunag sunag deleted the dev-webgpu-optmization-1 branch June 3, 2023 22:53
@Mugen87
Copy link
Collaborator

Mugen87 commented Jun 4, 2023

The UUIDs are mainly intended for serialization/deserialization in order to identify shared resources so it's good that you've found a solution without using them^^.

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.

Don't set redundant state in the WebGPURenderer

4 participants