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

Reflector/Refractor/Water warn when renderer.outputEncoding is not Linear #19618

Merged
merged 1 commit into from
Jun 10, 2020

Conversation

mrdoob
Copy link
Owner

@mrdoob mrdoob commented Jun 10, 2020

While cleaning up webgl_shaders_ocean I bumped into the issue that programs were getting recompiled twice every frame when setting renderer.outputEncoding to sRGBEncoding.

The fact that Water uses LinearEncoding forces materials to be recompiled when rendering the reflection view. This applies to Reflector and Refractor too.

I think the right fix is to support multiple compiled programs per material (something I'm currently investigating). Until that's done I think it'll be better to warn the user.

@mrdoob mrdoob added this to the r118 milestone Jun 10, 2020
@mrdoob mrdoob merged commit 9110ec1 into dev Jun 10, 2020
@mrdoob mrdoob deleted the objects branch June 10, 2020 19:38
@mrdoob
Copy link
Owner Author

mrdoob commented Jun 10, 2020

Maybe Reflector and Refractor doesn't need this and we could just do renderTarget.texture.encoding = renderer.outputEncoding; before rendering, but I think we would be encoding twice? 🤔

@Mugen87
Copy link
Collaborator

Mugen87 commented Jun 10, 2020

I think the right fix is to support multiple compiled programs per material (something I'm currently investigating).

Another possibility to fix this particular issue is to provide the output encoding as a uniform. That would also avoid a recompile and is probably easier to implement. Multiple programs will also trigger program binds and thus WebGL state changes which is not the case when a uniform value is changed.

TBH, I'm not sure the warning is much of a help since the encoding parameter was a workaround that made the mirror and other objects usable with sRGB output encoding. This is now not the case anymore. I guess I would prefer to rollback the change if there is no other solution in the r118 dev cycle.

@mrdoob
Copy link
Owner Author

mrdoob commented Jun 10, 2020

Another possibility to fix this particular issue is to provide the output encoding as a uniform. That would also avoid a recompile and is probably easier to implement. Multiple programs will also trigger program binds and thus WebGL state changes which is not the case when a uniform value is changed.

We would also need a uniform for toneMapping then. Reflector and Refractor doesn't need it but Water does lighting and texture compositing in its shader.

@mrdoob
Copy link
Owner Author

mrdoob commented Jun 10, 2020

TBH, I'm not sure the warning is much of a help since the encoding parameter was a workaround that made the mirror and other objects usable with sRGB output encoding. This is now not the case anymore. I guess I would prefer to rollback the change if there is no other solution in the r118 dev cycle.

Happy to revert if this doesn't get solved in time.

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.

None yet

2 participants