Revisit the druid <-> druid-shell animation and invalidation interface. #1057
Conversation
There was a problem hiding this comment.
Generally this looks good to me, thanks. I skimmed it, and didn't go into details of, for example, the Region impl. The use of "viewWillDraw" on mac looks right to me, a detail I hadn't personally explored. I'd be happy merging a more polished version of this (CI looks like it's failing a bit).
| @@ -127,6 +129,15 @@ pub(crate) fn recti_to_rect(rect: RECT) -> Rect { | |||
| ) | |||
| } | |||
|
|
|||
| /// Converts a `Region` into a vec of winapi `RECT`, with a scaling applied. | |||
| pub(crate) fn region_to_rectis(region: &Region, scale: Scale) -> Vec<RECT> { | |||
There was a problem hiding this comment.
"rectis" seems an odd name.
There was a problem hiding this comment.
I'm not in love with it either, but I copied it from rect_to_recti...
|
I think this is ready for another look. I've tested it on all platforms but mac, and I've been dogfooding it for a couple of weeks in my own druid project. |
There was a problem hiding this comment.
I tried this on macOS and it seems to work. I also took another quick pass through the code and didn't find any issue. I think it should be merged. Thanks for the work!
It looks like there are some merge conflicts on Linux, but hopefully not too bad. Do you mind fixing?
This will probably have merge conflicts with other work in flight; I'm particularly concerned about #1037, and I also strongly expect it to conflict with my simpler_d2d branch. In the latter case, I'm happy for this to go in first.
|
I checked the conflict with #1037: merging this PR into #1037 results in just two conflicts. There's a trivial one in |
* Naive first step to refactor out scroll logic into separate component * Move a bunch of event logic to `scroll_component` from `Scroll` * Add method to coordinate painting with `scroll_component` * Move a bunch of lifecycle logic to `scroll_component` from `Scroll` * Documentation pass on `scroll_component` * Rename `Scroll` to `AbsoluteScroll` & add scroll functionality to `List` * Add ability to change layout axis for `List` with builder fn * Scroll behavior component changelog * Pass `BarHoveredState` by value and add impl Default for ScrollComponent * `ScrollComponent` review changes * Add some missing information to `ScrollComponent` documentation * `ScrollContainer` review take 2 * Changes from #1057 * ScrollComponent clippy * Remove List changes (updated to master) & fix Scroll regression * Add missing doc comments * Be a bit more careful about event consuming * Fix dup changelog entry & doc comment misspelling
This implements (only for GTK so far) the invalidation scheme discussed on zulip, with a few small differences. I'm posting it in the hope of getting a little feedback before I dig into the other backends. (I went with GTK first because I think it'll be the hardest one, because there's a mismatch between what we want to do and what GTK wants us to do.)
The main issue with the current setup is that animation frames are propagated in the paint callback. This means that they can't invalidate any regions, which in turn means that when druid requests an animation frame, it has to invalidate the whole window just to be sure. So this patch introduces a two-phase paint protocol: first,
druid-shellcallsprepare_paint, which is allowed to invalidate whatever regions that it wants. Immediately after that,druid-shellcallspaintand provides an invalidation region that must be filled. In the discussion on zulip, I proposed havingprepare_paintreturn the region that it wants to be invalidated, but in fact it was simpler to just askprepare_paintto use the existing invalidation methods.Note that this is a breaking change, in that widgets that previously relied on
request_anim_framedoing the invalidation will now need to do their own.Here's a little gif showing (with debug_invalidation enabled) a scroll bar that doesn't repaint the whole window when it fades out.
