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

Screen effects blending #316

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open

Screen effects blending #316

wants to merge 16 commits into from

Conversation

eugeneloza
Copy link
Member

Implement and example for Screen Effects Blending according to @michaliskambi suggestions at https://discord.com/channels/389676745957310465/389676745957310467/861898173034397697

Fixes #315

@eugeneloza
Copy link
Member Author

I wonder if un-deprecating TCastleSimpleBackground in this case makes sense?

@@ -696,7 +743,14 @@ procedure TCastleScreenEffects.RenderOverChildren;
AttribTexCoord.EnableArrayVector2(SizeOf(TScreenPoint),
OffsetUInt(ScreenPoint[0].TexCoord, ScreenPoint[0]));

glDrawArrays(GL_TRIANGLE_FAN, 0, 4);
if Blending then
Copy link
Member

Choose a reason for hiding this comment

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

I realized that the condition for this should be if Blending and "this is the last effect, rendered on screen" then ...

I.e. the intermediate effects, that reuse FBOs, should not use blending (instead, they should rely on TCastleSimpleBackground clearning the contents).

Please extend the RenderOneEffect routine with additional parameter const RenderToScreen: Boolean.

Then it should be used with false in the loop, RenderOneEffect(GetScreenEffect(I), false);

And used with true for final render, RenderOneEffect(GetScreenEffect(CurrentScreenEffectsCount - 1), true);

Copy link
Member Author

Choose a reason for hiding this comment

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

Done in bebc407 !

@michaliskambi
Copy link
Member

  1. Code (CGE and example) looks cool, thank you! I found one thing that I didn't think of earlier -- see my comment above, the Blending should only be used for the last effect.

  2. Please add a README.md inside, I propose contents like this:

# Screen Effects With Blending

Demo of using `TCastleScreenEffects` that sets non-trivial (not fully opaque) alpha values for the generated screen effect contents. See the [TCastleScreenEffects.Blending](https://castle-engine.io/apidoc-unstable/html/CastleScreenEffects.TCastleScreenEffects.html#Blending) API docs for a documentation.

Using [Castle Game Engine](https://castle-engine.io/).

## Building

Compile by:

- [CGE editor](https://castle-engine.io/manual_editor.php). Just use menu item _"Compile"_.

- Or use [CGE command-line build tool](https://github.com/castle-engine/castle-engine/wiki/Build-Tool). Run `castle-engine compile` in this directory.

- Or use [Lazarus](https://www.lazarus-ide.org/). Open in Lazarus `game_3d_sound_standalone.lpi` file and compile / run from Lazarus. Make sure to first install [CGE Lazarus packages](https://castle-engine.io/documentation.php).

Going forward, I want to add similar README.md to all examples:)

I wonder if un-deprecating TCastleSimpleBackground in this case makes sense?

Yes, but I hesitate about the name. It was originally deprecated, because in almost all cases it is simpler to use TCastleRectangleControl (that does blending when it draws rectangle with alpha, and it correctly honors it's size -- with TCastleSimpleBackground it, by default, just clears whole screen, wherever it is). You have found one case when TCastleSimpleBackground is better (it overwrites pixels, never caring about previous screen contents). But the name TCastleSimpleBackground may be confusing. Hm, maybe TCastleClearPixels? Sounds convoluted...

@eugeneloza
Copy link
Member Author

Michalis, note there seems to be one more issue with the current approach https://discord.com/channels/389676745957310465/389676745957310467/862193035109466132 - it doesn't harm my usecase (monochrome effect) in any way, but may be unexpected for other users. TL;DR - incoming color in the shader has always alpha = 1 (i.e. the alpha from the unaltered image is discarded).

@eugeneloza
Copy link
Member Author

1 - done in bebc407

2 - done in b0dfda8

3 - As in https://castle-engine.io/apidoc-unstable/html/CastleRenderContext.TRenderContext.html#Clear - I believe something like TCastleClearPixels is though not perfectly clear, but very consistent. Alterntatively I can propose something like

  • TCastleClearRectangle
  • TCastleClearBackground
  • TCastleRectangleControlClear
  • TCastleRectangleClear (as in TCastleRectangleControl)
  • TCastleRectangleSimple
  • TCastleRectangleBackground
  • TCastleOverwriteBackground
  • TCastleClearBufferRectangle

@eugeneloza
Copy link
Member Author

eugeneloza commented Sep 18, 2021

Tested on an ancient Android device - the current solution (together with Window.AlphaBits := 8;) doesn't seem to work in Android 5.0. But I didn't do the logcat, so it might be an unrelated Android-specific issue (like sounds). Same APK works flawlessly on Android 11. (this issue was unrelated and was resolved)

@eugeneloza
Copy link
Member Author

eugeneloza commented Oct 16, 2021

Note that the issue above is https://trello.com/c/wmxWrx8d/190-android-50-libpng-not-found - and it doesn't have anything to do with this commit. Project built with this PR doesn't have any issues and works on Android 4.2.2 flawlessly. It only crashes on my weird Lenovo with Android 5.0. (this issue has been resolved)

Fix merge conflict

# Conflicts:
#	examples/screen_effects/screen_effects_demo/screen_effects_demo_standalone.lpr
#	examples/screen_effects_demo/screen_effects_demo_standalone.dpr
#	examples/screen_effects_demo/screen_effects_demo_standalone.lpr
#	src/x3d/opengl/castlescreeneffects.pas
(fix merge conflict)

# Conflicts:
#	.gitignore
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.

Investigate TCastleScreenEffect interaction with alpha channel
2 participants