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

Stop visual artifacts from appearing when using input fields #2381

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

Conversation

matthewgapp
Copy link
Contributor

@matthewgapp matthewgapp commented Jun 3, 2023

This PR fills invalid rects with a reset color instead of clearing them to fix #2367 which is is easily reproducible on Mac almost anywhere that uses an input field (I'm on M1 Macbook).

This PR partially reverts #1617 to restore the invalidation painting to use piet.fill instead of piet.clear.

I tracked down the issue by bisecting but I don't know why this fixes the issue or if the invalidation painting should have ever used clear in the first place. Perhaps there is a bug with the clear method.

🐛 Bug:
Repro steps in issue
Boredom warning: the GIF below is in like slowmo for some reason.
Screen Recording 2023-06-03 at 10 48 13 AM

how tested

Manually, both examples in 2367 appear resolved and the transparency.rs example remains unchanged after the fix.

@matthewgapp matthewgapp changed the title partially "reverting" PR #1617 to fix invalid rectangle paint issue which is causing visual artifacts almost anywhere an input field is used. Stop visual artifacts from appearing when using input fields Jun 3, 2023
@matthewgapp
Copy link
Contributor Author

@cmyr and @Ciantic, perhaps you have more insight here given @Ciantic wrote #1617 and @cmyr reviewed it.

@xStrom
Copy link
Member

xStrom commented Jun 15, 2023

This does indeed seem to solve the issue on macOS.

However it breaks stuff (e.g. transparency example) on Windows, Linux GTK, Linux X11. Instead of clearing, the new fills are just laid on top. So transparent areas become increasingly opaque with every redraw.

In addition to #1617 there are also some piet issues that might be of interest - piet#401 & piet#403.

Of special interest is the note on the piet clear method:

/// You probably don't want to call this. It is essentially a specialized
/// fill method that can be used in GUI contexts for things like clearing
/// damage regions. It does not have a good cross-platform implementation,
/// and eventually should be deprecated when support is added for blend
/// modes, at which point it will be easier to just use [`fill`] for
/// everything.

The does not have a good cross-platform implementation part suggests that the macOS implementation of Piet's clear is just broken. Maybe not too hard to fix? Interestingly the problem doesn't show up in Piet's own test cases, so a next step might be to figure out how to reproduce the issue with Piet. Not absolutely required though, we could also merge a fix that solves our heavy Druid repro.

The note also suggests that the alternative solution is to have blend modes so that we could actually use fill. Piet does not currently have blend modes. @raphlinus has mentioned during office hours that he now sees a path forward for adding blend modes to Piet. However I don't think he has any time to actually do any of that work. Perhaps we only need a specific blend mode that isn't too difficult to add? Not sure, I know little about blend modes.

Given that the macOS issue (#2367) is quite severe I will spend a bit more time myself looking into this, however no promises of a solution. So I wanted to give this status report in the meantime.

@xStrom xStrom added the S-blocked waits for another PR or dependency label Jun 15, 2023
@xStrom
Copy link
Member

xStrom commented Jun 16, 2023

Also worth noting that because it's not actually the clear operation itself that's failing, but the subsequent redrawing of elements on top of that - that perhaps the issue isn't really with clear but with our invalidation logic on macOS.

#1593 might be related.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-blocked waits for another PR or dependency
Projects
None yet
Development

Successfully merging this pull request may close these issues.

OSX Tab change input field focus breaks rendering
2 participants