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

WebGLTextures: Remove render target texture fallback. #23470

Merged
merged 1 commit into from
Feb 14, 2022
Merged

Conversation

Mugen87
Copy link
Collaborator

@Mugen87 Mugen87 commented Feb 12, 2022

Related issue: #8658

Description

Almost six years ago a policy was introduced that disallows the usage of render target as texture.

I think it's okay to remove the respective fallback in WebGLTextures which saves unnecessary function calls and if statements evaluations for each uniforms update.

@gkjohnson
Copy link
Collaborator

What happens now if you set a render target as a texture? Does it throw an error or just not render anything? Tbh I still get this wrong some times and the warning is helpful. Would it be possible to just log a warning if the render target is used?

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Feb 12, 2022

I do not vote to do that. The idea is to remove the overhead of such checks. The general policy in the engine is to not evaluate user input for better performance. The current fallback was never intended to be permanent.

@WestLangley
Copy link
Collaborator

Is there a compelling reason why WebGLRenderTarget has a texture, rather than extends texture?

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Feb 12, 2022

The design matches the framebuffer/attachments pattern. You can have more than one texture per render target.

@WestLangley
Copy link
Collaborator

The design matches the framebuffer/attachments pattern. You can have more than one texture per render target.

You can have more than one THREE.Texture per THREE.WebGLRenderTarget?

@Mugen87
Copy link
Collaborator Author

Mugen87 commented Feb 12, 2022

Yes. For example a render target can have a depth texture attached. And there is MRT.

@mrdoob mrdoob added this to the r138 milestone Feb 14, 2022
@mrdoob mrdoob merged commit b6e02c2 into mrdoob:dev Feb 14, 2022
@mrdoob
Copy link
Owner

mrdoob commented Feb 14, 2022

Thanks!

donmccurdy pushed a commit to donmccurdy/three.js that referenced this pull request Mar 10, 2022
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