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

Fix weird flickering issue #3356

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

Cheemsandfriends
Copy link
Contributor

Apparently, there's this issue that's been for too long where the specifically White color would just disappear for no apparent reason.
Fixes #3354

@Geokureli
Copy link
Member

Geokureli commented Feb 2, 2025

Can I get some simple code that reproduces the issue?

I also don't see the relevance to 3345. both mention flickering but the issue specifically references colored quads where yours specifically references non-colored quads

@MaybeMaru you should test this, in case I'm missing something

@Cheemsandfriends
Copy link
Contributor Author

Cheemsandfriends commented Feb 2, 2025

Can I get some simple code that reproduces the issue?

It's using the same bit of code that the user has provided in #3354 (comment)

I also don't see the relevance to 3345. both mention flickering but the issue specifically references colored quads where yours specifically references non-colored quads

I define "colored" as the sprite that has has a color modification that actually differs from the normal WHITE color, "non-colored" is basically that it doesn't make any difference (pretty much an identity ColorTransform, or if you forcefully set the color as FlxColor.WHITE for that matter)
I think Maru defines it whether there's a "colored" flag set, that could've caused the misconception

@Geokureli
Copy link
Member

I completely missed their second comment, thanks

@Geokureli
Copy link
Member

Geokureli commented Feb 2, 2025

Questions:

  • What does this do and why does it fix this
  • Why hasRGBMultipliers but not hasRGBAOffsets
  • Is there any loss of functionality from this, if so, what? If not, why not do this on all targets?

This seems to draw them all in one quad batch, is that the fix? is separating batches necessary on other targets? I thought it was neccessary on all renderTile targets, but maybe we should revisit this?

@Geokureli
Copy link
Member

Geokureli commented Feb 2, 2025

Tried this change on the PixelPerfectCollision demo in a previous version this would set alien.color to red or white, causing the flickering on html5, so we baked separate red and white animations into the image. With this change I saw no issues when reverting it to use color, but also no flicker without this change.

More importantly, performance was improved with your change, due to batching, and there was no difference in appearance. This was also true on hashlink. I've been meaning to test batching colored sprites, for a minute, ever since I noticed that Shaders handle color on the vertex shader. I think we should test this on more targets, and try batching other things, like scale and rotation which seem to prevent batching.

once we have a comprehensive list of which fields prevent batching on each target we should add compile flags for each, eg: FLX_QUAD_BATCH.ANGLE, FLX_QUAD_BATCH.SCALE and FLX_QUAD_BATCH.COLOR

@Cheemsandfriends
Copy link
Contributor Author

* What does this do and why does it fix this

Basically remove a conditional that checks for the ColorTransform's multipliers, basically how color
works and does it regardless lol

* Why `hasRGBMultipliers` but not `hasRGBAOffsets`

Multipliers check for the percentage of the RGBA, like its name says, while Offsets are additive.

* Is there any loss of functionality from this, if so, what? If not, why not do this on all targets?

I don't think so? It's removes the conditional which checks for the calculation but to be honest, I kept it as not changing for other platforms, but I consider it quite redundant. Here's a blog in AS explaining how redundant if statements CAN BE EXPENSIVE, this is AS, but Im applying this logic the same to other targets like desktop, web and mobile

More importantly, performance was improved with your change

I'm basing on the fact that, multiplications tend to be veeryy cheap, to the point that it can do it eyes closed, im just cleaning the redundancy that the original flixel might've had to do due to how it could've been taxing back in the day, but nowadays, since the conditional not only gives us rendering issues, it decreases performance, it's safe to say it would be pretty much the same?

Tl;DR: I removed a kinda redundant conditional that was causing issues, but kept it in other targets cos I haven't tested it in depth if I removed it, but I believe it's redundant overall.

This seems to draw them all in one quad batch, is that the fix? is separating batches necessary on other targets?

This does not affect in any way how it rendered, everything's pretty much the same! same quad batches and same rendering stuff.
Even though it would be interesting to see batched colors, I think it'd be impossible due to them being quite different and it would be taxing, as a rule of thumb, smaller but numerous renders tend to work better than a massive singular one

@Cheemsandfriends
Copy link
Contributor Author

I'd say to check over how the rendering stuff is set, and then check what is redundant and what not, sometimes too much polishing results in erosion!

@Geokureli Geokureli added this to the Next Patch milestone Feb 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Colored quads not rendering on Web
2 participants