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 engine option for using exact sRGB conversions in the shader. #12750

Merged

Conversation

kircher1
Copy link
Contributor

@kircher1 kircher1 commented Jul 15, 2022

This provides an option to use the more expensive but accurate conversion for transforming colors in the shader to and from linear and sRGB color spaces. The conversion constants and formulas are described in the Wikipedia article for sRGB: https://en.wikipedia.org/wiki/SRGB

I opted to make this option a property passed into the engine, as it feels somewhat similar functionally to the engine option for high precision floats i.e., it affects many things (in this case, many shaders). IMO it would be odd to use the approximation in one shader and the exact formula in another. I think most of the time, you'd either want it always or never.

@kircher1
Copy link
Contributor Author

kircher1 commented Jul 15, 2022

Not in this change, but in the future, I think the engine option should also affect the CPU-side math implementations of toLinear/toGamma on the Color classes.

@kircher1 kircher1 changed the title Add option to engine to use exact sRGB conversions in the shader. Add engine option for using exact sRGB conversions in the shader. Jul 15, 2022
@azure-pipelines
Copy link

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.

@azure-pipelines
Copy link

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.

@azure-pipelines
Copy link

@azure-pipelines
Copy link

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.

@azure-pipelines
Copy link

@azure-pipelines
Copy link

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.

@azure-pipelines
Copy link

Copy link
Contributor

@Popov72 Popov72 left a comment

Choose a reason for hiding this comment

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

All good for me but the implementation can be simplified a lot, see my comment.

@azure-pipelines
Copy link

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.

@azure-pipelines
Copy link

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.

Love it !!!

@sebavan sebavan merged commit 5e101da into BabylonJS:master Jul 18, 2022
@kircher1 kircher1 deleted the users/briank/use-exact-srgb-conversion-engine branch August 26, 2022 17:41
RaananW pushed a commit that referenced this pull request Dec 9, 2022
…onversion-engine

Add engine option for using exact sRGB conversions in the shader.

Former-commit-id: b4a78869824343248e242185a30d31986b635765
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.

4 participants