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

GRL OIT support #15966

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

GRL OIT support #15966

wants to merge 2 commits into from

Conversation

RolandCsibrei
Copy link
Contributor

No description provided.

@bjsplat
Copy link
Collaborator

bjsplat commented Dec 8, 2024

Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s).
To prevent this PR from going to the changelog marked it with the "skip changelog" label.

@bjsplat
Copy link
Collaborator

bjsplat commented Dec 8, 2024

@bjsplat
Copy link
Collaborator

bjsplat commented Dec 8, 2024

@bjsplat
Copy link
Collaborator

bjsplat commented Dec 8, 2024

@RolandCsibrei RolandCsibrei marked this pull request as draft December 8, 2024 18:13
@RolandCsibrei RolandCsibrei marked this pull request as draft December 8, 2024 18:13
@RolandCsibrei
Copy link
Contributor Author

RolandCsibrei commented Dec 8, 2024

@Popov72 Hello! Is this as simpke as changing the fragmentOutputs output color name to support OIT or do I need to do more? Thanks!

EDIT: Test PG:
https://playground.babylonjs.com/?webgpu&snapshot=refs/pull/15966/merge#H1LRZ3#690

@Popov72
Copy link
Contributor

Popov72 commented Dec 9, 2024

It's a bit more involved:

  • you have to add two includes in your shader code (look for "oit" in default.fragment.fx for an example).
  • you have to add some JS code in your material. Look for "oit" in standardMaterial.ts to know what to do (you don't need the PrepareDefinesForPrePass call).

Also, the PG test link is for WebGL, whereas you only updated the WGSL shaders, so it doesn't show your changes. Another thing is that there's a "Cannot add an uniform after UBO has been created. uniformName=viewProjection" error with this link. It probably doesn't come from your PR, but it should be fixed nonetheless.

@RolandCsibrei
Copy link
Contributor Author

@Popov72

Since the GRL material is a material plugin are those two additional steps really needed? Isn't OIT handled by the standard and pbr material base shaders?

@Popov72
Copy link
Contributor

Popov72 commented Dec 9, 2024

No, it's not needed, but I think greasedLine.fragment.fx is used by the standalone material, not the plugin?

@RolandCsibrei
Copy link
Contributor Author

RolandCsibrei commented Dec 9, 2024

Yes, it is a ShaderMaterial. I just added the OIT "support" shader code to a random GRL shader to be able to show the changes I'm going to make to all GRL shaders involved in OIT and I didn't really choose the best one. Sorry to confuse you.

On the other hand since the GrasedLineSimpleMaterial doesn't support alpha I believe there is no need to implement OIT support in this material. Is this correct?

@Popov72
Copy link
Contributor

Popov72 commented Dec 9, 2024

Yes, in that case you don't have to modify greasedLine.fragment.fx.

Regarding the plugin, you should inject your code in CUSTOM_FRAGMENT_BEFORE_FRAGCOLOR and not CUSTOM_FRAGMENT_MAIN_END. That way, it will automatically work with OIT/the pre-pass renderer.

With this change, instead of gl_FragColor, you should update color for standard materials, and finalColor for PBR materials. The easiest way to do this is to add something like that at the start of the shader code injected at CUSTOM_FRAGMENT_BEFORE_FRAGCOLOR:

#ifdef PBR
    #define grlFinalColor finalColor
#else
    #define grlFinalColor color
#endif

and use grlFinalColor instead of gl_FragColor.

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.

3 participants