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

Fix implementation of screen node #1111

Conversation

rasmusbonnedal
Copy link
Contributor

The parenthesis were incorrectly placed in the implementation of the screen compositing node.

Changes from this:
(mix * ((1 - (1 - fg)) * (1 - bg))) + ((1 - mix) * bg)

to this:
(mix * ((1 - (1 - fg) * (1 - bg)))) + ((1 - mix) * bg)

@rasmusbonnedal
Copy link
Contributor Author

GLSL OSL
screen_color3_out_glsl.png screen_color3_out_osl.png
screen_color3_out_glsl screen_color3_out_osl
screen_color4_out_glsl.png screen_color4_out_osl.png
screen_color4_out_glsl screen_color4_out_osl
screen_float_out_glsl.png screen_float_out_osl.png
screen_float_out_glsl screen_float_out_osl

@rasmusbonnedal
Copy link
Contributor Author

This is what they look like before the fix:
image

After discussion with @niklasharrysson it was concluded that alpha is treated differently between GLSL and OSL in the tests. The difference between GLSL and OSL on screen_color4 is due to alpha=0.5 with the old screen implementation.

The parenthesis were incorrectly placed in the implementation of the
screen compositing node.

Changes from this:
(mix * ((1 - (1 - fg)) * (1 - bg))) + ((1 - mix) * bg)

to this:
(mix * ((1 - (1 - fg) * (1 - bg)))) + ((1 - mix) * bg)
@jstone-lucasfilm jstone-lucasfilm changed the title Correct impl of screen node Fix implementation of screen node Oct 22, 2022
Copy link
Member

@jstone-lucasfilm jstone-lucasfilm left a comment

Choose a reason for hiding this comment

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

Thanks for this fix, @rasmusbonnedal!

@jstone-lucasfilm jstone-lucasfilm merged commit 1b79589 into AcademySoftwareFoundation:main Oct 22, 2022
Michaelredaa pushed a commit to Michaelredaa/MaterialX that referenced this pull request Oct 21, 2023
The parentheses were incorrectly placed in the implementation of the screen compositing node.

Changes from this:
    (mix * ((1 - (1 - fg)) * (1 - bg))) + ((1 - mix) * bg)

to this:
    (mix * ((1 - (1 - fg) * (1 - bg)))) + ((1 - mix) * bg)
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.

2 participants