linebender / druid

A data-first Rust-native UI design toolkit.
https://linebender.org/druid/
Apache License 2.0
9.58k stars 566 forks source link

Stop visual artifacts from appearing when using input fields #2381

Open matthewgapp opened 1 year ago

matthewgapp commented 1 year ago

This PR fills invalid rects with a reset color instead of clearing them to fix https://github.com/linebender/druid/issues/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 commented 1 year ago

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

xStrom commented 1 year ago

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 commented 1 year ago

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.