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

Transparent GUI's controls are incorrectly blending with the parent GUI #1838

Closed
AlanDrake opened this issue Nov 28, 2022 · 25 comments · Fixed by #1874
Closed

Transparent GUI's controls are incorrectly blending with the parent GUI #1838

AlanDrake opened this issue Nov 28, 2022 · 25 comments · Fixed by #1874
Labels
context: graphics type: bug unexpected/erroneous behavior in the existing functionality
Milestone

Comments

@AlanDrake
Copy link
Contributor

AlanDrake commented Nov 28, 2022

Describe the bug
In hardware accelerated rendering, semi-transparent GUIs are being rendered incorrectly because controls are being blended with the underlying GUI graphics.

AGS Version
AGS 3.6 and AGS4 (D3D9 and OpenGL)

To Reproduce
Steps to reproduce the behavior:

  1. Create a GUI with a black background and colored border
  2. Create a control, like a text box with some colors and borders so it overlaps with other visible elements
  3. Make the GUI 50% transparent or so
  4. Controls are drawn on top of the semi-transparent GUI mixing with the underlying content

Expected behavior
immagine
(correct, software rendering)

Screenshots
immagine
(wrong, D3D/OGL)
The controls shouldn't be mixing with what's underneath that is part fo the GUI.

@AlanDrake AlanDrake added type: bug unexpected/erroneous behavior in the existing functionality ags 4 related to the ags4 development labels Nov 28, 2022
@ivan-mogilko
Copy link
Contributor

Also reproduced in 3.6.0, so it's a 3.6.0 bug.

@ivan-mogilko ivan-mogilko added context: graphics and removed ags 4 related to the ags4 development labels Nov 29, 2022
@ivan-mogilko ivan-mogilko added this to the 3.6.0 milestone Nov 29, 2022
@ivan-mogilko ivan-mogilko changed the title AGS4: Issue with transparent GUIs Transparent GUI's controls are incorrectly blending with the parent GUI Nov 29, 2022
@ivan-mogilko
Copy link
Contributor

ivan-mogilko commented Dec 8, 2022

@AlanDrake
Looks like I made a successful experiment:
https://github.com/ivan-mogilko/ags-refactoring/tree/360--guitexturerenderfix2

Basically, this lets to pass a texture when you call BeginSpriteBatch, and the whole batch and all the nested sprites will be rendered on that texture target instead. You just have to remember that and adjust the transformation parameters for the batch accordingly: that is set batch offset 0,0 instead of gui.X,Y and so on.
The nice thing is that then this texture may be added just like a regular sprite, with any transforms and effects.
Unfortunate thing is that the code organization is clumsy. But I am aiming at keeping changes to minimum atm, for 3.6.0 release.

Made it work with Direct3D for now, next will do for OpenGL, and try to tidy the code.

@AlanDrake
Copy link
Contributor Author

I tried it, but I see the GUI transparency is not being applied somehow, though it's being set to the new texture.

@ivan-mogilko
Copy link
Contributor

I tried it, but I see the GUI transparency is not being applied somehow, though it's being set to the new texture.

This is strange, it works in a test game I've been using. Can you give an example of how do you test this?

@AlanDrake
Copy link
Contributor Author

The 3.6 version of the Test Wretcher project, if you still have it.
Just press CTRL-Z to open the console, it will open at full opacity, in all rendering modes, even software which worked before.

Current master applies transparency just fine to the gConsole GUI.

@ivan-mogilko
Copy link
Contributor

ivan-mogilko commented Dec 8, 2022

The 3.6 version of the Test Wretcher project, if you still have it.

Apparently the 3.5 (or 3.6?) version of the Wretcher had "Classic" gui render style set, which makes GUI draw in software mode, because it uses incorrect alpha blending. I haven't tried the ags4 version yet.
Pushed a fix that broke the software mode now.

But this also means that 3.* version of Wretcher cannot be used to test this change, unless you change "GUI rendering style" to "Proper alpha blending".

@AlanDrake
Copy link
Contributor Author

Ah, that explains why I had never noticed the blending working differently on 3.x... I just wasn't using proper alpha blending.
Now it appears to work as expected with this branch, in both modes. 👍

@ivan-mogilko
Copy link
Contributor

Updated the test branch with OpenGL supporting render targets; but I'm probably missing something, because the GUI images are now vertically mirrored. Either the sprites are drawn inverted along the y axis on the texture, or the texture is drawn inverted itself, not sure yet.

Another thing that's missing is that Direct3D must release these "buffer" textures after window is resized, otherwise IDirect3DDevice9::Reset will fail. On another hand, I'm curious whether there's a way around that.

@ivan-mogilko
Copy link
Contributor

ivan-mogilko commented Dec 13, 2022

Unfortunately current code does not work correctly. This is not seen while only GUI has transparency, because that transparency is applied to the final texture (used as a render target). But if you set transparencies to individual gui controls, this produces a weird effect where the control's image would blend with contents of a main backbuffer, and then pasted on top of the render target texture (or so it seems).

Same effect takes place with both Direct3D and OpenGL renderers, which makes me think that I do exact same mistake in both cases, related to the use of the render targets.

This is an example of how this looks like (direct3d):
test-screenshot01

@ericoporto
Copy link
Member

ericoporto commented Dec 13, 2022

You bind to the framebuffer that is the GUI texture, do the drawing, and later you bind to the framebuffer that is the top on stack (screen). It looks correct following this tutorial. Maybe add an assert(current_fbo > 0) in the first bind, just in case.

@ivan-mogilko
Copy link
Contributor

@ericoporto have found that there are alpha blend settings that may fix this problem. The code is experimental, but pushed to this branch for a test. It's for Direct3D at the moment, but supposedly similar thing may be done for OpenGL.

In regards to the OpenGL inverting GUI, I suspect this may be related to the sprite rendering relying on either native-res surface size or window size when applying positions. This should be rewritten to use sprite batch's matrix instead.
(There's another related thing that potentially may be optimized, it's the choice of a scaling filter, which also depends on "render in native res" / "render in screen res" setting, and will conflict with the new code.)

@ericoporto
Copy link
Member

ericoporto commented Dec 14, 2022

I believe the equivalent in OpenGL is something like glBlendFuncSeparate, but one thing I am not sure is if it's possible to find a blend function combination that will work for all, or if it would need a different algorithm say for sprites to the renderbuffer and the renderbuffer texture to the "gamebuffer". From the final conclusion here, I understood we would need to use different blends for each.

@ivan-mogilko
Copy link
Contributor

ivan-mogilko commented Dec 15, 2022

Hm, on another hand, I realized that #1708 will be necessary for this regardless, because right now there are some global sprite batch settings being applied in every batch, that will conflict with ones rendered onto a texture. So I will try resolving that first. Hopefully that will also fix the OpenGL inversion problem, or at least make it simpler to deal with.

@ivan-mogilko
Copy link
Contributor

ivan-mogilko commented Dec 23, 2022

So, back at this (after #1708 is resolved).

The updated test branch: https://github.com/ivan-mogilko/ags-refactoring/tree/360--guitexturerenderfix3

Fixed inverted GUIs with OpenGL, and few minor things.

Remaining problems:

  1. Blending bug, described above in a comment with a screenshot. There's a fix suggested by @ericoporto, which seem to fix this problem, but it in turn breaks anti-aliasing, which means that the blending settings are still not fully suitable, and have to be adjusted further. This is what I will be investigating next.
  2. All render targets must be released and recreated under Direct3D when the screen mode changes.
  3. Tidy code...

@ivan-mogilko
Copy link
Contributor

ivan-mogilko commented Dec 25, 2022

I found a source of another error, which I initially attributed to the aforementioned alpha blending settings. But in the end I realized it's not that. The problem was that the GUI background with alpha channel and translucent parts was rendered as if alpha value was additionally decreased (more transparent than necessary). I figured out this is because the GUI background is still being blended with the texture buffer (which has some clear color), while what we need is to get it copied.

The solution which I decided to try was to introduce "blend mode" for the sprite, similar to what @AlanDrake did in ags4, only with 1 special blend mode that does plain src copy: ivan-mogilko@c5aa232
This seem to fix that erroneous behavior.

So far from what I see there's one issue remaining, which we noticed randomly testing Dave Gilbert's new game "Old Skies". With this branch the options of the main menu appear missing a greenish "glow" (or have less of it); so they appear too sharp, or having more contrast than necessary. Following are two screenshots comparing the original and new look:

oldskies--normal
oldskies--sharp

What's interesting, this does not depend on whether the previous alpha blending settings are done or not. I thought this is too going to be fixed with this latest blend mode fix, but it was not, so there's something else missing or wrong.

@ericoporto
Copy link
Member

When reading the blending stuff, something that went over my head, but maybe you can make sense of, is the premultiplication of alpha with the colors, I am not sure how it works, but this seems to darken things when done incorrectly, just thought to mention in case it this helps.

@ivan-mogilko
Copy link
Contributor

ivan-mogilko commented Dec 25, 2022

The above appears to be the same problem I was trying to fix with blend mode change; except I was thinking that copying gui background is enough. But same issue reoccurs if gui background is fully transparent while the buttons are translucent.

EDIT: everything seem to be fine if the gui background has at least some color, that is - pixels with alpha above 0.

@ericoporto
Copy link
Member

ericoporto commented Dec 25, 2022

Does using white vs black makes any difference? I mean because white is all 255, so the blending I think would maintain all range if this is the observed behavior, in case the premultiplication is impacting.

@ivan-mogilko
Copy link
Contributor

ivan-mogilko commented Dec 25, 2022

Does using white vs black makes any difference? I mean because white is all 255, so the blending I think would maintain all range if this is the observed behavior, in case the premultiplication is impacting.

Do you mean the color which is used to clear the surface? If so, then the white color gives an opposite, "lighter", effect.

Overall, the root of the problem is that the sprite's color is blended with the surface, but we don't need it to blend with the surface at all, we need it to plain copy onto the surface. But at the same time blend with each other...
LATER EDIT: ok, I realized that I had a very wrong idea about how multiple texture blending is done; above is actually done using the separate alpha operations. What remains is the problem of colors being multiplied twice, if i understand correctly.
EDIT: that's probably the same thing as the "premultiplied alpha" issue you were speaking of.
I found numerous discussions of this or similar problem on the web, but probably was not capable of fully understanding the answers yet.

Here's the test game I am using:
test--351guihires.zip

It has a GUI with a button, both with the same "gradient" sprites. Key 1 toggle the GUI's graphic, Key 2 toggles the button. It's very easy to see the difference between expected and the current branch.

@ericoporto
Copy link
Member

ericoporto commented Dec 25, 2022

Uhm, I wonder if it makes sense testing this in AGS4 context, just to see

  • rotation is indeed working (from previous PR)
  • existing blendmodes are working (we can fix them later, but just to be aware)

@ivan-mogilko
Copy link
Contributor

ivan-mogilko commented Dec 25, 2022

Uhm, I wonder if it makes sense testing this in AGS4 context, just to see

  • rotation is indeed working (from previous PR)
  • existing blendmodes are working (we can fix them later, but just to be aware)

I don't see why rotation would not work, if it would be about rotating a single texture - that is what it currently does easily.
As for blendmodes, tbh I would ignore these for now, if custom blend settings would allow me to fix this problem in 3.6.0. Perhaps they may be reimplemented using shaders in ags4 (iirc some of them do not work properly in Direct3D anyway, and require some extra hacks).

@AlanDrake
Copy link
Contributor Author

AlanDrake commented Dec 25, 2022

iirc some of them do not work properly

Correct, one can only do so much with a single pass of blend modes. For perfect implementation we need to turn them into shaders.

@ivan-mogilko
Copy link
Contributor

ivan-mogilko commented Dec 25, 2022

Hmm, I finally realized that was thinking about whole blending of multiple textures in a wrong way (maybe a case of a mental block), and now fully understand what that separate alpha operation setting was about, and possibly what the "premultiplied alpha" case is.

So, now I get it, these settings make it so that the render target has the alpha component corresponding to all the sprites summed together, which makes the final alpha applied to the render target's pixels correct. (Assuming the render target itself was cleared to alpha 0.)

But, because the sprite colors (RGB component) were combined using sprite's src alphas along the way, including the first pixels getting onto the render target itself, these colors, so to speak, already include alpha factor. This is what "premultiplied alpha" is, if i understand correctly.

And that's a problem, because now, when we finally draw the texture, previously used as a render target, on screen itself, then all the colors will be multiplied by the alpha again, even though they were already multiplied by it.

Previously I found a solution for this: use a special blend mode for this texture, where the source color is taken as is (not multiplied by its source alpha), but the destination color is still multiplied by inverse of a source alpha:

direct3ddevice->SetRenderState(D3DRS_BLENDOP, D3DBLENDOP_ADD);
direct3ddevice->SetRenderState(D3DRS_SRCBLEND, D3DBLEND_ONE);
direct3ddevice->SetRenderState(D3DRS_DESTBLEND, D3DBLEND_INVSRCALPHA);

This works perfectly, but right until you apply GUI's Transparency to this texture: because it was not used when drawing onto the render target, its colors do not have it in them, and basically do not change, while the background colors increase. The result looks like the background begins to shine through the gui... sort of... anyway, it's wrong.

Hypothetically, it might work if there were a way to do this:

  • multiply src color with transparency alpha
  • multiply dst color with (1 - transparency alpha * pixel alpha)

but I do not know if such method even exists.

So, this is where I'm currently at.

NOTE: I cannot apply this GUI Transparency to the sprites I draw onto render target, because then they will use it for blending between each other, and I will be back at the problem I am trying to fix with render targets in the first place.

@ivan-mogilko
Copy link
Contributor

ivan-mogilko commented Dec 25, 2022

Hypothetically, it might work if there were a way to do this:

multiply src color with transparency alpha
multiply dst color with (1 - transparency alpha * pixel alpha)

Oh, wow, looks like this is possible:

direct3ddevice->SetRenderState(D3DRS_BLENDFACTOR, D3DCOLOR_RGBA(alpha, alpha, alpha, 255));
direct3ddevice->SetRenderState(D3DRS_BLENDOP, D3DBLENDOP_ADD);
direct3ddevice->SetRenderState(D3DRS_SRCBLEND, D3DBLEND_BLENDFACTOR /*D3DBLEND_ONE*/);
direct3ddevice->SetRenderState(D3DRS_DESTBLEND, D3DBLEND_INVSRCALPHA);

EDIT: tested, and this seem to actually work. Updated the branch now:
ivan-mogilko@4385ad1

Now only need to code same thing for OpenGL.

@ivan-mogilko
Copy link
Contributor

Alright, I updated with fixing OpenGL's blending, and fixing Direct3D failing to reset device when window size/mode changes.
https://github.com/ivan-mogilko/ags-refactoring/tree/360--guitexturerenderfix3

I think this resolves the issue, except maybe some minor things that we did not notice yet.

I will be tidying the code and making proper PR now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
context: graphics type: bug unexpected/erroneous behavior in the existing functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants