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

Add OpenPBR's base_weight material parameter #16085

Merged
merged 13 commits into from
Jan 24, 2025

Conversation

virtualzavie
Copy link
Contributor

@virtualzavie virtualzavie commented Jan 17, 2025

This PR implements the base_weight parameter from the OpenPBR specification.
This parameter simply scales linearly the base_color (named albedo in Babylon.js).

Test scene:
https://playground.babylonjs.com/?snapshot=refs/pull/16085/merge#DT1XPP#4

@bjsplat
Copy link
Collaborator

bjsplat commented Jan 17, 2025

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.

@virtualzavie virtualzavie changed the title Add OpenPBR's base_weight material paramener Add OpenPBR's base_weight material parameter Jan 17, 2025
@bjsplat
Copy link
Collaborator

bjsplat commented Jan 17, 2025

@bjsplat
Copy link
Collaborator

bjsplat commented Jan 17, 2025

@bjsplat
Copy link
Collaborator

bjsplat commented Jan 17, 2025

@bjsplat
Copy link
Collaborator

bjsplat commented Jan 17, 2025

@bjsplat
Copy link
Collaborator

bjsplat commented Jan 17, 2025

@bjsplat
Copy link
Collaborator

bjsplat commented Jan 17, 2025

@bjsplat
Copy link
Collaborator

bjsplat commented Jan 20, 2025

@virtualzavie virtualzavie marked this pull request as ready for review January 21, 2025 18:27
@bjsplat
Copy link
Collaborator

bjsplat commented Jan 21, 2025

@sebavan sebavan enabled auto-merge (squash) January 21, 2025 19:04
@bjsplat
Copy link
Collaborator

bjsplat commented Jan 21, 2025

Copy link
Member

@sebavan sebavan left a comment

Choose a reason for hiding this comment

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

WebGPU seems to fail here
image

auto-merge was automatically disabled January 22, 2025 10:16

Head branch was pushed to by a user without write access

@bjsplat
Copy link
Collaborator

bjsplat commented Jan 22, 2025

@virtualzavie
Copy link
Contributor Author

What's the most straightforward way to reproduce locally a failing test?

@sebavan
Copy link
Member

sebavan commented Jan 22, 2025

finding the failling test in the config.json and running the associated playground id locally

@sebavan
Copy link
Member

sebavan commented Jan 22, 2025

But in your case the webgpu test link above in the chat here let you access this:
https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/refs/pull/16085/merge/testResults/webgpuplaywright/index.html#?testId=3a254ccdc8803e849057-08ae18e069e61964c18d

where you can see the stdout:
image

Meaning there is probably a missing/not working part in the NME code for pbr calling into the block around here https://github.com/BabylonJS/Babylon.js/pull/16085/files#diff-e3d517f82735473032d4378b87b2ba60963f75b95023eb944446a79b9702257bR977

@sebavan
Copy link
Member

sebavan commented Jan 22, 2025

cc @Popov72 :-)

It seems the preprocessor removes the entire line when it finds a // comment.
@sebavan sebavan enabled auto-merge (squash) January 24, 2025 14:19
@bjsplat
Copy link
Collaborator

bjsplat commented Jan 24, 2025

@bjsplat
Copy link
Collaborator

bjsplat commented Jan 24, 2025

@sebavan sebavan merged commit 7fa12bb into BabylonJS:master Jan 24, 2025
12 of 14 checks passed
@virtualzavie virtualzavie deleted the openpbr/base_weight branch January 24, 2025 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants