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

WebGL2: Added support for Uniform Buffer Objects. #21558

Merged
merged 8 commits into from
Jul 7, 2022
Merged

Conversation

Mugen87
Copy link
Collaborator

@Mugen87 Mugen87 commented Apr 1, 2021

Related issue: Fixed #13700

Description

Successor of #15562.

@mrdoob mrdoob added this to the r129 milestone Apr 22, 2021
@mrdoob mrdoob modified the milestones: r129, r130 May 27, 2021
@mrdoob mrdoob modified the milestones: r130, r131 Jun 30, 2021
@mrdoob mrdoob modified the milestones: r131, r132 Jul 28, 2021
@mrdoob mrdoob modified the milestones: r132, r133 Aug 26, 2021
@mrdoob mrdoob modified the milestones: r133, r134 Sep 30, 2021
@MajorSquirrelTVS
Copy link

@Mugen87 Any chance this awesome feature will be added in a future release ?

@mrdoob
Copy link
Owner

mrdoob commented Oct 8, 2021

@Mugen87 Should we give it a go this month?

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Oct 8, 2021

@mrdoob Yes, I would give this a try so devs can start using UBOs with their custom materials. The PR should be a safe change since the additions should no affect existing apps.

We should just keep in mind that UniformsGroup will be a WebGL thing. With WebGPURenderer and node material, we don't need it since UBOs are used internally by default.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Oct 11, 2021

BTW: I'm adding documentation if the PR is going to be merged.

@mrdoob mrdoob modified the milestones: r134, r135 Oct 28, 2021
@mrdoob mrdoob modified the milestones: r135, r136 Nov 26, 2021
@erichlof
Copy link
Contributor

erichlof commented Dec 7, 2021

Hello @Mugen87 ,
Thank you for adding this! Is this feature available to the current WebGL2 renderer as of yet?

I am needing this feature because I want to send to my fragment shader 64 4x4 invMatrices as well as a top level dynamic BVH that is packed into 256 vec4's. The 64 3D game objects that these sets of uniforms belong to are constantly moving every frame, so writing to and reading from data textures (like I did for static path traced scenes) is too slow.

Also, writing 64 4x4 matrices and 256 vec4's as normal old-style individual uniforms works on my laptop, but fails on mobile because there is a lower limit of how many uniforms you can send. So I end up blowing out my uniform slots on my phone for instance.

This new UBO feature sounds like the perfect solution for my uniforms requirements! Thanks again for working on this.

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Dec 7, 2021

Thank you for adding this! Is this feature available to the current WebGL2 renderer as of yet?

No sorry. Since the PR is not merged it's not yet possible to use UBOs with custom materials.

@mrdoob mrdoob modified the milestones: r137, r138 Jan 26, 2022
@mrdoob mrdoob modified the milestones: r138, r139 Feb 23, 2022
@RenaudRohlinger
Copy link
Collaborator

UBO is a great feature. It would be awesome to see this one merged! 😊

@mrdoob mrdoob modified the milestones: r139, r140 Mar 24, 2022
@robertlong
Copy link
Contributor

Is this mainly blocked on documentation at this point? How do you feel about this implementation?

@mrdoob mrdoob modified the milestones: r140, r141 Apr 30, 2022
@mrdoob mrdoob modified the milestones: r141, r142 May 26, 2022
@snagy
Copy link
Contributor

snagy commented Jun 21, 2022

What needs to get done before this gets merged? Is there anything we can help with?

@robertlong
Copy link
Contributor

Yup. Been waiting a while for this. Pretty sure we have the interest to get this done. We need details on what's holding it up. Is the design alright @mrdoob ? Anything you would like changed?

@mrdoob mrdoob modified the milestones: r142, r143 Jun 29, 2022
@mrdoob
Copy link
Owner

mrdoob commented Jul 7, 2022

Sorry for the delay. Was waiting for things to get a bit under control 😅

@mrdoob mrdoob merged commit bd61eb4 into mrdoob:dev Jul 7, 2022
@mrdoob
Copy link
Owner

mrdoob commented Jul 7, 2022

Thanks!

@CodyJasonBennett
Copy link
Contributor

CodyJasonBennett commented Jul 7, 2022

Nit: on the topic of documentation, grammatically I believe this would be uniformGroups rather than uniformsGroups.

Edit: I see this is more consistent with other utils (#15562 (comment)).

@joshuaellis joshuaellis mentioned this pull request Jul 18, 2022
4 tasks

if ( material.isShaderMaterial || material.isRawShaderMaterial ) {

const groups = material.uniformsGroups;
Copy link
Contributor

Choose a reason for hiding this comment

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

@Mugen87 I had an exception on this line using a package which had an old TypeScript types of three.js (134)
I'm not sure how it works but seems like something along the way stripped this member from the RawSahderMaterial instance (internally in package this member was initialized correctly to an [])
uniformsGroups didn't exist on the instance at all, so the groups.length was the actual exception
So I feel like there's a backwards compatibility issue here but I can't create a JSFiddle with an example

Copy link
Collaborator Author

@Mugen87 Mugen87 Jul 31, 2022

Choose a reason for hiding this comment

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

That is strange since UBO support was added with r143 and all related classes and properties were added with this release.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll clarify, I use threejs 143 (no typescript in the project)
I import a package that's using 134+ts
The package somehow creates ShaderMaterials without this property
So the package is not ahead of my project - which I think should be supported
I'll try to work tomorrow on a jsfiddle, but I'm really not sure how I'll import 2 versions of threejs :)

Copy link
Collaborator Author

@Mugen87 Mugen87 Jul 31, 2022

Choose a reason for hiding this comment

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

but I'm really not sure how I'll import 2 versions of threejs :)

That would be an invalid workflow anyway. Try using the types from https://github.com/three-types/three-ts-types. They have a r143 release.

Copy link
Contributor

Choose a reason for hiding this comment

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

I patched the package on our private copy, so I don't have the issue anymore (doing what you mentioned)
But I believe that given 2 packages A and B where:

  • A uses typescript and is using three.js r134 (or anything below 143), and if creating objects to be rendered by B.
  • B uses three.js r143 or up (and is not using typescript? not sure it matters), B is running the main rendering loop and is responsible to create the canvas and renderer etc.
    A will produce objects which are illegal in B (since this field doesn't exist in r134).
    I have very little knowledge on how A and B are interacting with one another, might be a misconfiguration on that particular package and how it's imported.

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like I'm wrong, this example shows the same scenario:
https://codesandbox.io/s/pix4d-three-potree-loader-example-forked-s6i6i1?file=/package.json

The imported package uses r139
But the example uses r143 without a problem

Copy link
Collaborator Author

@Mugen87 Mugen87 Jul 31, 2022

Choose a reason for hiding this comment

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

It might work to combine three.js from different releases but officially it is not supported. If library A creates an instance of THREE.Mesh from r100 there is absolutely no guarantee that application B using r143 can process it. Let's be honest, it's quite unrealistic to support such a scenario.

abernier pushed a commit to abernier/three.js that referenced this pull request Sep 16, 2022
* WebGL2: Added support for Uniform Buffer Objects.

* Update WebGLState.js

Clean up.

* Fix example.
snagy pushed a commit to snagy/three.js-1 that referenced this pull request Sep 21, 2022
* WebGL2: Added support for Uniform Buffer Objects.

* Update WebGLState.js

Clean up.

* Fix example.
@unphased
Copy link
Contributor

unphased commented Oct 5, 2024

Hi, it seems like this feature exists, but is totally undocumented. Is that expected?

I had a question about usage. I'm working on something that may benefit from UBOs, but I don't really have more than around 384 8 bit components I need my shader to be able to reference. This would appear to still fit in the 1024~4096 or so components that I could expect WebGL2 to give me on most devices, so I most likely do not need to transition to using UBOs for such a need until I have 10x or more of this kind of data. Since I do not update this data except periodically there should be no performance benefit in switching to uniform buffers from uniform arrays.

@mrdoob
Copy link
Owner

mrdoob commented Oct 22, 2024

Please use one of the links below for help / support:

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Oct 22, 2024

Hi, it seems like this feature exists, but is totally undocumented. Is that expected?

@unphased The UBO support in WebGLRenderer is considered as experimental. That's why the built-in materials don't use UBOs and there is no documentation. This will be different in WebGPURenderer where UBOs are a more integral part of the renderer.

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 this pull request may close these issues.

Explore the benefits of using Uniform Buffer Objects (UBO) for WebGL2 renderer