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

Viewport background transparency not working with custom clear color in Mobile and Compatibility renderers #79778

Closed
LRFLEW opened this issue Jul 22, 2023 · 6 comments · Fixed by #79876

Comments

@LRFLEW
Copy link
Contributor

LRFLEW commented Jul 22, 2023

Godot version

4.1.1.stable, 4.2.dev [6588a4a]

System information

Godot v4.1.1.stable - Windows 10.0.19045 - Vulkan (Mobile) - dedicated NVIDIA GeForce GTX 1080 Ti (NVIDIA; 31.0.15.3640) - Intel(R) Core(TM) i7-8700K CPU @ 3.70GHz (12 Threads)

Issue description

This is related to #72602, but since this also affects the Mobile renderer, this is likely a separate issue.

Setting the "Transparent BG" setting on a viewport is supposed to always keep the background transparent. On the Forward+ renderer, this setting is respected for all Environment Background Modes . On both the Mobile and Compatibility renderers, using the "Clear Color" background mode works with the transparent background, but using a "Custom Color" background does not render it as transparent.

Using Custom Color:
bug

Using Clear Color:
clear

Steps to reproduce

  1. Add a SubViewport to render some 3D scene and a way to view it (eg. a Sprite2D using the viewport as its texture)
  2. Enable the "Transparent BG" option on the SubViewport
  3. Setup an Environment for the 3D scene being rendered to the SubViewport (either on the Camera3D node or on a WorldEnvironment node)
  4. Change the Background Mode of the Environment to Custom Color.

Minimal reproduction project

SkyTest.zip

@freshBakedPie314
Copy link

freshBakedPie314 commented Jul 23, 2023

would love to work on this as my first contribution!! But I have no idea where to start. Any suggestions would be helpful!
EDIT: well i just saw the related issue. Will try looking through that first.

@lostminds
Copy link

lostminds commented Jul 23, 2023

This could also be related to the existing issue #72602 where having a sky environment makes the viewport background opaque instead of transparent. Could very well be the same issue as when rendering just having a custom color environment.

Edit: Never mind, saw now that you had found this related issue already. But even if it's a separate issue, the fix might still fix both

@LRFLEW
Copy link
Contributor Author

LRFLEW commented Jul 24, 2023

Fixing this for the Mobile renderer can be its own PR most likely, and will likely be easier than fixing it for the Compatibility renderer. Fixing this for the Mobile renderer will be a matter of comparing RenderForwardMobile::_render_scene to its Forward+ counterpart RenderForwardClustered::_render_scene and figuring out which difference is responsible. The Compatibility equivalent to these functions, RasterizerSceneGLES3::render_scene, is different enough that comparing them is a bit more of a challenge. I actually discovered this issue (as well as the cause of #74187) while trying to figure out the cause of #72602 by comparing these functions. When I have some time again, I want to take another stab at figuring out #72602 and the Compatibility part of this issue, but if someone else figures it out first, then that will work too.

@LRFLEW
Copy link
Contributor Author

LRFLEW commented Jul 25, 2023

I made a PR that fixes the issue for the Mobile renderer. Turns out there was a change made to the Forward+ renderer that was never included in the Mobile renderer (perhaps because it was commented to have to do with subsurface scattering, not viewports), so making that change there fixed that part of the issue.

I'm starting to think that I should separate the Compatibility renderer and Mobile renderer parts of this. I can make a new issue report for the Compatibility renderer part of this issue, move the Compatibility renderer part to #72602, or just leave it as is. What do people here think I should do?

@clayjohn
Copy link
Member

I would just leave this open until the Compatibility backend is fixed as well.

@freshBakedPie314
Copy link

I know which comments you're referring to. I looked at it and didn't mess with it as I didn't know why they were commented out or what they did

And as clayjhon said I think you should leave this open till compatibility is fixed as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants