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

Film grain shader bug when switching to full screen #529

Closed
eugeneloza opened this issue Jul 31, 2023 · 6 comments
Closed

Film grain shader bug when switching to full screen #529

eugeneloza opened this issue Jul 31, 2023 · 6 comments

Comments

@eugeneloza
Copy link
Member

This specific shader breaks (behaves unexpectedly: just appears as dark overlay in Vinculike project, displays garbage in this project) when the app goes to full screen by Window.FullScreen := true or rather when changing Window.FullScreen from the value the app initially started.

I couldn't reproduce this effect on a different shader, so most likely it's something in the way this shader is written or how it is handled from the code.

The version of CGE is close to the latest (updated on Weekends if I remember correctly - most likely a day before I've written the post on Discord, I wanted to check if upgrading CGE won't fix the issue). Note, that this bug started appearing relatively recently, I'll try a bisection and see if I could pinpoint it.

Minimal reproduction example:
full-screen-bug.zip

Steps to reproduce:

  1. Run.
  2. Press F11 to go full screen.
  3. The bug appears.
  4. Pressing F11 again to go back doesn't fix the bug.

Video:
https://github.com/castle-engine/castle-engine/assets/10726162/0310b30e-8797-45a7-b8e3-d57334b1cd11
(when everything disappears the app is in fullscreen and the picture is the same as when returning from full screen mode)

@eugeneloza
Copy link
Member Author

ce45377 (9.06) - no bug
3661044 (22.06) - no bug
74f18e2 (22.06) - no bug
40e39f6 (23.06) - no bug(?) / Access Violation (null pointer exception)
9590884 (23.06) - Fails to compile
1281fb1 (23.06) - Fails to compile
c949545 (23.06) - BUG
11f4d9f (23.06) - BUG and much worse than other variants, similar to the minimal example in Vinculike
3d17a33 (24.06) - BUG
4bdb2a1 (6.07) - BUG
70c0812 (28.07) - BUG

Note that this bug affects latest GitLab CI/CD builds too - i.e. it's unrelated to build environment.

@eugeneloza
Copy link
Member Author

So, the culprit is most likely one of the three commits : 9590884 1281fb1 c949545 - and those commits indeed deal with a big rendering refactor, which looks like a good candidate for such an issue.

As according to the symptoms it may look like the film grain texture stored on GPU becomes corrupt - and therefore considering commit message the first one of the three may be the most viable candidate to blame.

With commit 40e39f6 the game crashes with some null pointer exception when launching the app or trying to go full screen - but after pressing "YES" (continue running) the image is correct, no issue. So technically it still may be related but some additional environment reinitialization happens due to a crash.

I didn't try other commits between the one above and 74f18e2 - because according to commit messages they are unrelated. However I may be wrong.

@eugeneloza
Copy link
Member Author

And just a note from Discord, this bug doesn't affect Linux - seems like Windows-only thing. Everything works well on Linux with ab66b5a (29.07)

@eugeneloza
Copy link
Member Author

Ah, and one more thing to think about: don't trust my video but test for yourself. It may be again the bug of my weird Radeon drivers for Windows (just like the one with unlit one-sided materials).

@michaliskambi
Copy link
Member

Sorry for delay! I can reproduce the problem, on a Windows with NVidia. So this is not specific to your "weird Radeon drivers for Windows" :)

My system:

Version string: 4.6.0 NVIDIA 537.42
Vendor string: NVIDIA Corporation
Renderer: NVIDIA GeForce RTX 3050 Ti Laptop GPU/PCIe/SSE2

It's my regular laptop where I do most CGE development in 2023, so I would expect everything to work perfectly here :) Yet I can reproduce the issue you show.

Looking into it.

@michaliskambi
Copy link
Member

  1. Found and fixed.

    This was the same issue as Warning: Textures: Releasing texture id 2 after OpenGL context is closed. This means that you free some OpenGL resources (like TDrawableImage) too late, you should move their destruction to an event like TCastleWindow.OnClose or TCastleControl.OnGLContextClose. #551 . Because of a "rookie mistake" (not using setter) on my part: a texture resource was not released, so also not recreated, when it should be. This explains both bugs, and they are both fixed now.

    It also makes sense that a refactor you pointed out was the moment when it started happening. This is when I changed the texture management, and indeed this is when I forgot to call the setter.

    Note that the bug was generally cross-platform, cross-compiler, and could matter on any GPU. E.g. it is possible it also occurs on Linux and everywhere else, although in the end today I only tested on Windows. It's a matter of internals, whether a given texture resource may, by accident, point to something sensible (or at least a garbage that looks like grain :) ) or not. It could go wrong on any system.

  2. Note: There is an independent issue in your testcase (also in testcase for Warning: Textures: Releasing texture id 2 after OpenGL context is closed. This means that you free some OpenGL resources (like TDrawableImage) too late, you should move their destruction to an event like TCastleWindow.OnClose or TCastleControl.OnGLContextClose. #551 ), visible in the video and when running the example: the screen contents outside of the white rectangle, in particular "FPS:..." label display, are rendered incorrectly. They are rendered on top of previous frames.

    This issue is independent, and it is not a CGE issue, it is caused because the contents you insert into TCastleScreenEffect instance do not completely render the TCastleScreenEffect area. That is, your RectangleControl1 has FullSize = false.

    When rendered without screen effects (if one removed the ScreenEffect.Inject(Self)) then everything is OK, because TCastleContainer clears the background each time before rendering everything (following TCastleContainer.BackgroundColor, TCastleContainer.BackgroundEnable). But when you move Group1 to be a child of TCastleScreenEffect (which happens at ScreenEffect.Inject(Self)) then the screen effects area is not completely cleared by the TCastleScreenEffect children or descendants. This is documented at TCastleScreenEffects,

     > Note that the UI controls rendered for the screen effects
     > (our children and descendants) must always initialize and fill
     > colors of the entire rectangle (@link(RenderRect)) of this control.
     > Otherwise, the results are undefined, as an internal texture that is used
     > for screen effects is initially undefined.
     > You may use e.g. @link(TCastleRectangleControl) to fill the background with a solid color
     > from @link(TCastleRectangleControl.Color).
     > Or use @link(TCastleViewport) with @link(TCastleViewport.Transparent) = @false (default)
     > which fills background with @link(TCastleViewport.BackgroundColor).
    

    Following that advise, I tested by adding Bg: TCastleRectangleControl with FullSize=true, with Blue color (with alpha=1, i.e. opaque) to Group1 and then it works OK. See screenshot.

I emphasize that AD 1 ("screen effects doing something weird when switched fullscreen") and AD 2 ("FPS display broken after a few frames") are completely indedepedent. Merely adding that Bg: TCastleRectangleControl makes the "FPS:..." display correct, but it doesn't fix the broken grain effect on fullscreen toggle. The grain effect is only fixed by CGE commit referenced above.

Zrzut ekranu 2023-12-05 075807

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

No branches or pull requests

2 participants