Revisit the druid <-> druid-shell animation and invalidation interfac…

…e. (#1057)

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-shell calls prepare_paint, which is allowed to invalidate whatever regions that it wants. Immediately after that, druid-shell calls paint and provides an invalidation region that must be filled. In the discussion on zulip, I proposed having prepare_paint return the region that it wants to be invalidated, but in fact it was simpler to just ask prepare_paint to use the existing invalidation methods.

Note that this is a breaking change, in that widgets that previously relied on request_anim_frame doing the invalidation will now need to do their own.

CHANGELOG.md

@@ -24,6 +24,7 @@ You can find its changes [documented below](#060---2020-06-01).
 - `Scale::from_scale` to `Scale::new`, and `Scale` methods `scale_x` / `scale_y` to `x` / `y`. ([#1042] by [@xStrom])
 - Major rework of keyboard event handling. ([#1049] by [@raphlinus])
 - `Container::rounded` takes `KeyOrValue<f64>` instead of `f64`. ([#1054] by [@binomial0])
+- `request_anim_frame` no longer invalidates the entire window. ([#1057] by [@jneem])
 
 ### Deprecated
 
@@ -375,6 +376,7 @@ Last release without a changelog :(
 [#1049]: https://github.com/linebender/druid/pull/1049
 [#1050]: https://github.com/linebender/druid/pull/1050
 [#1054]: https://github.com/linebender/druid/pull/1054
+[#1057]: https://github.com/linebender/druid/pull/1057
 [#1058]: https://github.com/linebender/druid/pull/1058
 [#1061]: https://github.com/linebender/druid/pull/1061
 [#1062]: https://github.com/linebender/druid/pull/1062

druid-shell/examples/invalidate.rs

@@ -19,7 +19,7 @@ use std::time::{Duration, Instant};
 use druid_shell::kurbo::{Point, Rect, Size};
 use druid_shell::piet::{Color, Piet, RenderContext};
 
-use druid_shell::{Application, TimerToken, WinHandler, WindowBuilder, WindowHandle};
+use druid_shell::{Application, Region, TimerToken, WinHandler, WindowBuilder, WindowHandle};
 
 struct InvalidateTest {
     handle: WindowHandle,
@@ -58,9 +58,10 @@ impl WinHandler for InvalidateTest {
         self.handle.request_timer(Duration::from_millis(60));
     }
 
-    fn paint(&mut self, piet: &mut Piet, rect: Rect) -> bool {
-        piet.fill(rect, &self.color);
-        false
+    fn prepare_paint(&mut self) {}
+
+    fn paint(&mut self, piet: &mut Piet, region: &Region) {
+        piet.fill(region.bounding_box(), &self.color);
     }
 
     fn size(&mut self, size: Size) {

druid-shell/examples/perftest.rs

@@ -16,10 +16,10 @@ use std::any::Any;
 
 use time::Instant;
 
-use piet_common::kurbo::{Line, Rect, Size};
+use piet_common::kurbo::{Line, Size};
 use piet_common::{Color, FontBuilder, Piet, RenderContext, Text, TextLayoutBuilder};
 
-use druid_shell::{Application, KeyEvent, WinHandler, WindowBuilder, WindowHandle};
+use druid_shell::{Application, KeyEvent, Region, WinHandler, WindowBuilder, WindowHandle};
 
 const BG_COLOR: Color = Color::rgb8(0x27, 0x28, 0x22);
 const FG_COLOR: Color = Color::rgb8(0xf0, 0xf0, 0xea);
@@ -39,7 +39,11 @@ impl WinHandler for PerfTest {
         self.handle = handle.clone();
     }
 
-    fn paint(&mut self, piet: &mut Piet, _: Rect) -> bool {
+    fn prepare_paint(&mut self) {
+        self.handle.invalidate();
+    }
+
+    fn paint(&mut self, piet: &mut Piet, _: &Region) {
         let rect = self.size.to_rect();
         piet.fill(rect, &BG_COLOR);
 
@@ -103,8 +107,7 @@ impl WinHandler for PerfTest {
             let y = y0 + (i as f64) * dy;
             piet.draw_text(&layout, (x0, y), &FG_COLOR);
         }
-
-        true
+        self.handle.request_anim_frame();
     }
 
     fn command(&mut self, id: u32) {

druid-shell/examples/quit.rs

@@ -14,10 +14,12 @@
 
 use std::any::Any;
 
-use druid_shell::kurbo::{Line, Rect, Size};
+use druid_shell::kurbo::{Line, Size};
 use druid_shell::piet::{Color, RenderContext};
 
-use druid_shell::{Application, HotKey, Menu, SysMods, WinHandler, WindowBuilder, WindowHandle};
+use druid_shell::{
+    Application, HotKey, Menu, Region, SysMods, WinHandler, WindowBuilder, WindowHandle,
+};
 
 const BG_COLOR: Color = Color::rgb8(0x27, 0x28, 0x22);
 const FG_COLOR: Color = Color::rgb8(0xf0, 0xf0, 0xea);
@@ -34,11 +36,12 @@ impl WinHandler for QuitState {
         self.handle = handle.clone();
     }
 
-    fn paint(&mut self, piet: &mut piet_common::Piet, _: Rect) -> bool {
+    fn prepare_paint(&mut self) {}
+
+    fn paint(&mut self, piet: &mut piet_common::Piet, _: &Region) {
         let rect = self.size.to_rect();
         piet.fill(rect, &BG_COLOR);
         piet.stroke(Line::new((10.0, 50.0), (90.0, 90.0)), &FG_COLOR, 1.0);
-        false
     }
 
     fn size(&mut self, size: Size) {

druid-shell/examples/shello.rs

@@ -14,12 +14,12 @@
 
 use std::any::Any;
 
-use druid_shell::kurbo::{Line, Rect, Size};
+use druid_shell::kurbo::{Line, Size};
 use druid_shell::piet::{Color, RenderContext};
 
 use druid_shell::{
-    Application, Cursor, FileDialogOptions, FileSpec, HotKey, KeyEvent, Menu, MouseEvent, SysMods,
-    TimerToken, WinHandler, WindowBuilder, WindowHandle,
+    Application, Cursor, FileDialogOptions, FileSpec, HotKey, KeyEvent, Menu, MouseEvent, Region,
+    SysMods, TimerToken, WinHandler, WindowBuilder, WindowHandle,
 };
 
 const BG_COLOR: Color = Color::rgb8(0x27, 0x28, 0x22);
@@ -36,11 +36,12 @@ impl WinHandler for HelloState {
         self.handle = handle.clone();
     }
 
-    fn paint(&mut self, piet: &mut piet_common::Piet, _: Rect) -> bool {
+    fn prepare_paint(&mut self) {}
+
+    fn paint(&mut self, piet: &mut piet_common::Piet, _: &Region) {
         let rect = self.size.to_rect();
         piet.fill(rect, &BG_COLOR);
         piet.stroke(Line::new((10.0, 50.0), (90.0, 90.0)), &FG_COLOR, 1.0);
-        false
     }
 
     fn command(&mut self, id: u32) {

druid-shell/src/lib.rs

@@ -46,6 +46,7 @@ mod keyboard;
 mod menu;
 mod mouse;
 mod platform;
+mod region;
 mod scale;
 mod window;
 
@@ -58,6 +59,7 @@ pub use hotkey::{HotKey, RawMods, SysMods};
 pub use keyboard::{Code, IntoKey, KbKey, KeyEvent, KeyState, Location, Modifiers};
 pub use menu::Menu;
 pub use mouse::{Cursor, MouseButton, MouseButtons, MouseEvent};
+pub use region::Region;
 pub use scale::{Scalable, Scale, ScaledArea};
 pub use window::{
     IdleHandle, IdleToken, Text, TimerToken, WinHandler, WindowBuilder, WindowHandle,

druid-shell/src/platform/gtk/window.rs

@@ -26,6 +26,7 @@ use std::sync::{Arc, Mutex, Weak};
 use std::time::Instant;
 
 use anyhow::anyhow;
+use cairo::Surface;
 use gdk::{EventKey, EventMask, ModifierType, ScrollDirection, WindowExt};
 use gio::ApplicationExt;
 use gtk::prelude::*;
@@ -39,6 +40,7 @@ use crate::dialog::{FileDialogOptions, FileDialogType, FileInfo};
 use crate::error::Error as ShellError;
 use crate::keyboard::{KbKey, KeyEvent, KeyState, Modifiers};
 use crate::mouse::{Cursor, MouseButton, MouseButtons, MouseEvent};
+use crate::region::Region;
 use crate::scale::{Scalable, Scale, ScaledArea};
 use crate::window::{IdleToken, Text, TimerToken, WinHandler};
 
@@ -120,6 +122,21 @@ pub(crate) struct WindowState {
     /// this is true, and this gets set to true when our client requests a close.
     closing: Cell<bool>,
     drawing_area: DrawingArea,
+    // A cairo surface for us to render to; we copy this to the drawing_area whenever necessary.
+    // This extra buffer is necessitated by DrawingArea's painting model: when our paint callback
+    // is called, we are given a cairo context that's already clipped to the invalid region. This
+    // doesn't match up with our painting model, because we need to call `prepare_paint` before we
+    // know what the invalid region is.
+    //
+    // The way we work around this is by always invalidating the entire DrawingArea whenever we
+    // need repainting; this ensures that GTK gives us an unclipped cairo context. Meanwhile, we
+    // keep track of the actual invalid region. We use that region to render onto `surface`, which
+    // we then copy onto `drawing_area`.
+    surface: RefCell<Option<Surface>>,
+    // The size of `surface` in pixels. This could be bigger than `drawing_area`.
+    surface_size: RefCell<(i32, i32)>,
+    // The invalid region, in display points.
+    invalid: RefCell<Region>,
     pub(crate) handler: RefCell<Box<dyn WinHandler>>,
     idle_queue: Arc<Mutex<Vec<IdleKind>>>,
     current_keycode: RefCell<Option<u16>>,
@@ -203,6 +220,9 @@ impl WindowBuilder {
             area: Cell::new(area),
             closing: Cell::new(false),
             drawing_area,
+            surface: RefCell::new(None),
+            surface_size: RefCell::new((0, 0)),
+            invalid: RefCell::new(Region::EMPTY),
             handler: RefCell::new(handler),
             idle_queue: Arc::new(Mutex::new(vec![])),
             current_keycode: RefCell::new(None),
@@ -287,29 +307,56 @@ impl WindowBuilder {
                     let area = ScaledArea::from_px(size_px, scale);
                     let size_dp = area.size_dp();
                     state.area.set(area);
+                    if let Err(e) = state.resize_surface(extents.width, extents.height) {
+                        log::error!("Failed to resize surface: {}", e);
+                    }
                     if let Ok(mut handler_borrow) = state.handler.try_borrow_mut() {
                         handler_borrow.size(size_dp);
                     } else {
                         log::warn!("Failed to inform the handler of a resize because it was already borrowed");
                     }
+                    state.invalidate_rect(size_dp.to_rect());
                 }
 
                 if let Ok(mut handler_borrow) = state.handler.try_borrow_mut() {
-                    // For some reason piet needs a mutable context, so give it one I guess.
-                    let mut context = context.clone();
-                    context.scale(scale.x(), scale.y());
-                    let (x0, y0, x1, y1) = context.clip_extents();
-                    let invalid_rect = Rect::new(x0, y0, x1, y1);
-
-                    let mut piet_context = Piet::new(&mut context);
-                    let anim = handler_borrow
-                        .paint(&mut piet_context, invalid_rect);
-                    if let Err(e) = piet_context.finish() {
-                        eprintln!("piet error on render: {:?}", e);
-                    }
-
-                    if anim {
-                        widget.queue_draw();
+                    // Note that we aren't holding any RefCell borrows here (except for the
+                    // WinHandler itself), because prepare_paint can call back into our WindowHandle
+                    // (most likely for invalidation).
+                    handler_borrow.prepare_paint();
+
+                    let surface = state.surface.try_borrow();
+                    if let Ok(Some(surface)) = surface.as_ref().map(|s| s.as_ref()) {
+                        if let Ok(mut invalid) = state.invalid.try_borrow_mut() {
+                            let mut surface_context = cairo::Context::new(surface);
+
+                            // Clip to the invalid region, in order that our surface doesn't get
+                            // messed up if there's any painting outside them.
+                            for rect in invalid.rects() {
+                                surface_context.rectangle(rect.x0, rect.y0, rect.width(), rect.height());
+                            }
+                            surface_context.clip();
+
+                            surface_context.scale(scale.x(), scale.y());
+                            let mut piet_context = Piet::new(&mut surface_context);
+                            handler_borrow.paint(&mut piet_context, &invalid);
+                            if let Err(e) = piet_context.finish() {
+                                eprintln!("piet error on render: {:?}", e);
+                            }
+
+                            // Copy the entire surface to the drawing area (not just the invalid
+                            // region, because there might be parts of the drawing area that were
+                            // invalidated by external forces).
+                            let alloc = widget.get_allocation();
+                            context.set_source_surface(&surface, 0.0, 0.0);
+                            context.rectangle(0.0, 0.0, alloc.width as f64, alloc.height as f64);
+                            context.fill();
+
+                            invalid.clear();
+                        } else {
+                            log::warn!("Drawing was skipped because the invalid region was borrowed");
+                        }
+                    } else {
+                        log::warn!("Drawing was skipped because there was no surface");
                     }
                 } else {
                     log::warn!("Drawing was skipped because the handler was already borrowed");
@@ -549,6 +596,58 @@ impl WindowBuilder {
     }
 }
 
+impl WindowState {
+    fn resize_surface(&self, width: i32, height: i32) -> Result<(), anyhow::Error> {
+        fn next_size(x: i32) -> i32 {
+            // We round up to the nearest multiple of `accuracy`, which is between x/2 and x/4.
+            // Don't bother rounding to anything smaller than 32 = 2^(7-1).
+            let accuracy = 1 << ((32 - x.leading_zeros()).max(7) - 2);
+            let mask = accuracy - 1;
+            (x + mask) & !mask
+        }
+
+        let mut surface = self.surface.borrow_mut();
+        let mut cur_size = self.surface_size.borrow_mut();
+        let (width, height) = (next_size(width), next_size(height));
+        if surface.is_none() || *cur_size != (width, height) {
+            *cur_size = (width, height);
+            if let Some(s) = surface.as_ref() {
+                s.finish();
+            }
+            *surface = None;
+
+            if let Some(w) = self.drawing_area.get_window() {
+                *surface = w.create_similar_surface(cairo::Content::Color, width, height);
+                if surface.is_none() {
+                    return Err(anyhow!("create_similar_surface failed"));
+                }
+            } else {
+                return Err(anyhow!("drawing area has no window"));
+            }
+        }
+        Ok(())
+    }
+
+    /// Queues a call to `prepare_paint` and `paint`, but without marking any region for
+    /// invalidation.
+    fn request_anim_frame(&self) {
+        self.window.queue_draw();
+    }
+
+    /// Invalidates a rectangle, given in display points.
+    fn invalidate_rect(&self, rect: Rect) {
+        if let Ok(mut region) = self.invalid.try_borrow_mut() {
+            let scale = self.scale.get();
+            // We prefer to invalidate an integer number of pixels.
+            let rect = rect.to_px(scale).expand().to_dp(scale);
+            region.add_rect(rect);
+            self.window.queue_draw();
+        } else {
+            log::warn!("Not invalidating rect because region already borrowed");
+        }
+    }
+}
+
 impl WindowHandle {
     pub fn show(&self) {
         if let Some(state) = self.state.upgrade() {
@@ -585,25 +684,25 @@ impl WindowHandle {
         }
     }
 
-    // Request invalidation of the entire window contents.
+    /// Request a new paint, but without invalidating anything.
+    pub fn request_anim_frame(&self) {
+        if let Some(state) = self.state.upgrade() {
+            state.request_anim_frame();
+        }
+    }
+
+    /// Request invalidation of the entire window contents.
     pub fn invalidate(&self) {
         if let Some(state) = self.state.upgrade() {
-            state.window.queue_draw();
+            self.invalidate_rect(state.area.get().size_dp().to_rect());
         }
     }
 
-    /// Request invalidation of one rectangle, which is given relative to the drawing area.
+    /// Request invalidation of one rectangle, which is given in display points relative to the
+    /// drawing area.
     pub fn invalidate_rect(&self, rect: Rect) {
         if let Some(state) = self.state.upgrade() {
-            // GTK takes rects with non-negative integer width/height.
-            let r = rect.abs().to_px(state.scale.get()).expand();
-            let origin = state.drawing_area.get_allocation();
-            state.window.queue_draw_area(
-                r.x0 as i32 + origin.x,
-                r.y0 as i32 + origin.y,
-                r.width() as i32,
-                r.height() as i32,
-            );
+            state.invalidate_rect(rect);
         }
     }