Add UpdateCtx::env_changed and UpdateCtx::env_key_changed

CHANGELOG.md

@@ -35,8 +35,9 @@ You can find its changes [documented below](#060---2020-06-01).
 - Implementation of `Data` trait for `i128` and `u128` primitive data types. ([#1214] by [@koutoftimer])
 - `LineBreaking` enum allows configuration of label line-breaking ([#1195] by [@cmyr])
 - `TextAlignment` support in `TextLayout` and `Label` ([#1210] by [@cmyr])`
+- `UpdateCtx` gets `env_changed` and `env_key_changed` methods ([#1207] by [@cmyr])
 - `Button::from_label` to construct a `Button` with a provided `Label`. ([#1226] by [@ForLoveOfCats])
-- Lens: Added Unit lens for type erased / display only widgets that do not need data. ([#1232] by [@rjwittams]) 
+- Lens: Added Unit lens for type erased / display only widgets that do not need data. ([#1232] by [@rjwittams])
 - `WindowLevel` to control system window Z order, with Mac and GTK implementations  ([#1231] by [@rjwittams])
 
 ### Changed
@@ -448,6 +449,7 @@ Last release without a changelog :(
 [#1195]: https://github.com/linebender/druid/pull/1195
 [#1204]: https://github.com/linebender/druid/pull/1204
 [#1205]: https://github.com/linebender/druid/pull/1205
+[#1207]: https://github.com/linebender/druid/pull/1207
 [#1210]: https://github.com/linebender/druid/pull/1210
 [#1214]: https://github.com/linebender/druid/pull/1214
 [#1226]: https://github.com/linebender/druid/pull/1226

druid/src/contexts.rs

@@ -21,11 +21,12 @@ use std::{
 };
 
 use crate::core::{CommandQueue, FocusChange, WidgetState};
+use crate::env::KeyLike;
 use crate::piet::{Piet, PietText, RenderContext};
 use crate::shell::Region;
 use crate::{
-    commands, Affine, Command, ContextMenu, Cursor, ExtEventSink, Insets, MenuDesc, Point, Rect,
-    SingleUse, Size, Target, TimerToken, WidgetId, WindowDesc, WindowHandle, WindowId,
+    commands, Affine, Command, ContextMenu, Cursor, Env, ExtEventSink, Insets, MenuDesc, Point,
+    Rect, SingleUse, Size, Target, TimerToken, WidgetId, WindowDesc, WindowHandle, WindowId,
 };
 
 /// A macro for implementing methods on multiple contexts.
@@ -91,6 +92,8 @@ pub struct LifeCycleCtx<'a, 'b> {
 pub struct UpdateCtx<'a, 'b> {
     pub(crate) state: &'a mut ContextState<'b>,
     pub(crate) widget_state: &'a mut WidgetState,
+    pub(crate) prev_env: Option<&'a Env>,
+    pub(crate) env: &'a Env,
 }
 
 /// A context provided to layout handling methods of widgets.
@@ -516,13 +519,41 @@ impl EventCtx<'_, '_> {
 
 impl UpdateCtx<'_, '_> {
     /// Returns `true` if this widget or a descendent as explicitly requested
-    /// an update call. This should only be needed in advanced cases;
-    /// See [`EventCtx::request_update`] for more information.
+    /// an update call.
+    ///
+    /// This should only be needed in advanced cases;
+    /// see [`EventCtx::request_update`] for more information.
     ///
     /// [`EventCtx::request_update`]: struct.EventCtx.html#method.request_update
     pub fn has_requested_update(&mut self) -> bool {
         self.widget_state.request_update
     }
+
+    /// Returns `true` if the current [`Env`] has changed since the previous
+    /// [`update`] call.
+    ///
+    /// [`Env`]: struct.Env.html
+    /// [`update`]: trait.Widget.html#tymethod.update
+    pub fn env_changed(&self) -> bool {
+        self.prev_env.is_some()
+    }
+
+    /// Returns `true` if the given key has changed since the last [`update`]
+    /// call.
+    ///
+    /// The argument can be anything that is resolveable from the [`Env`],
+    /// such as a [`Key`] or a [`KeyOrValue`].
+    ///
+    /// [`update`]: trait.Widget.html#tymethod.update
+    /// [`Env`]: struct.Env.html
+    /// [`Key`]: struct.Key.html
+    /// [`KeyOrValue`]: enum.KeyOrValue.html
+    pub fn env_key_changed<T>(&self, key: &impl KeyLike<T>) -> bool {
+        match self.prev_env.as_ref() {
+            Some(prev) => key.changed(prev, self.env),
+            None => false,
+        }
+    }
 }
 
 impl LifeCycleCtx<'_, '_> {

druid/src/core.rs

@@ -847,9 +847,13 @@ impl<T: Data, W: Widget<T>> WidgetPod<T, W> {
             }
         }
 
+        let prev_env = self.env.as_ref().filter(|p| !p.same(env));
+
         let mut child_ctx = UpdateCtx {
             state: ctx.state,
             widget_state: &mut self.state,
+            prev_env,
+            env,
         };
 
         self.inner

druid/src/env.rs

@@ -133,6 +133,35 @@ pub enum KeyOrValue<T> {
     Key(Key<T>),
 }
 
+/// A trait for anything that can resolve a value of some type from the [`Env`].
+///
+/// This is a generalization of the idea of [`KeyOrValue`], mostly motivated
+/// by wanting to improve the API used for checking if items in the [`Env`] have changed.
+///
+/// [`Env`]: struct.Env.html
+/// [`KeyOrValue`]: enum.KeyOrValue.html
+pub trait KeyLike<T> {
+    /// Returns `true` if this item has changed between the old and new [`Env`].
+    ///
+    /// [`Env`]: struct.Env.html
+    fn changed(&self, old: &Env, new: &Env) -> bool;
+}
+
+impl<T: ValueType> KeyLike<T> for Key<T> {
+    fn changed(&self, old: &Env, new: &Env) -> bool {
+        !old.get_untyped(self).same(new.get_untyped(self))
+    }
+}
+
+impl<T> KeyLike<T> for KeyOrValue<T> {
+    fn changed(&self, old: &Env, new: &Env) -> bool {
+        match self {
+            KeyOrValue::Concrete(_) => false,
+            KeyOrValue::Key(key) => !old.get_untyped(key).same(new.get_untyped(key)),
+        }
+    }
+}
+
 /// Values which can be stored in an environment.
 pub trait ValueType: Sized + Into<Value> {
     /// Attempt to convert the generic `Value` into this type.
@@ -244,7 +273,7 @@ impl Env {
     /// Panics if the key is not found.
     ///
     /// [`Value`]: enum.Value.html
-    pub fn get_untyped(&self, key: impl Borrow<Key<()>>) -> &Value {
+    pub fn get_untyped<V>(&self, key: impl Borrow<Key<V>>) -> &Value {
         match self.try_get_untyped(key) {
             Ok(val) => val,
             Err(err) => panic!("{}", err),
@@ -259,7 +288,7 @@ impl Env {
     /// e.g. for debugging, theme editing, and theme loading.
     ///
     /// [`Value`]: enum.Value.html
-    pub fn try_get_untyped(&self, key: impl Borrow<Key<()>>) -> Result<&Value, MissingKeyError> {
+    pub fn try_get_untyped<V>(&self, key: impl Borrow<Key<V>>) -> Result<&Value, MissingKeyError> {
         self.0.map.get(key.borrow().key).ok_or(MissingKeyError {
             key: key.borrow().key.into(),
         })

druid/src/lens/lens.rs

@@ -279,7 +279,7 @@ where
         let lens = &self.lens;
         lens.with(old_data, |old_data| {
             lens.with(data, |data| {
-                if ctx.has_requested_update() || !old_data.same(data) {
+                if ctx.has_requested_update() || !old_data.same(data) || ctx.env_changed() {
                     inner.update(ctx, old_data, data, env);
                 }
             })

druid/src/text/layout.rs

@@ -21,7 +21,7 @@ use crate::piet::{
     Color, PietText, PietTextLayout, Text as _, TextAlignment, TextAttribute, TextLayout as _,
     TextLayoutBuilder as _,
 };
-use crate::{ArcStr, Data, Env, FontDescriptor, KeyOrValue, PaintCtx, RenderContext};
+use crate::{ArcStr, Env, FontDescriptor, KeyOrValue, PaintCtx, RenderContext, UpdateCtx};
 
 /// A component for displaying text on screen.
 ///
@@ -31,28 +31,26 @@ use crate::{ArcStr, Data, Env, FontDescriptor, KeyOrValue, PaintCtx, RenderConte
 /// invalidating and rebuilding it as required.
 ///
 /// This object is not valid until the [`rebuild_if_needed`] method has been
-/// called. Additionally, this method must be called anytime the text or
-/// other properties have changed, or if any  items in the [`Env`] that are
-/// referenced in this layout change. In general, you should just call this
-/// method as part of your widget's `update` method.
+/// called. You should generally do this in your widget's [`layout`] method.
+/// Additionally, you should call [`needs_rebuild_after_update`]
+/// as part of your widget's [`update`] method; if this returns `true`, you will need
+/// to call [`rebuild_if_needed`] again, generally by scheduling another [`layout`]
+/// pass.
 ///
+/// [`layout`]: trait.Widget.html#tymethod.layout
+/// [`update`]: trait.Widget.html#tymethod.update
+/// [`needs_rebuild_after_update`]: #method.needs_rebuild_after_update
 /// [`rebuild_if_needed`]: #method.rebuild_if_needed
 /// [`Env`]: struct.Env.html
 #[derive(Clone)]
 pub struct TextLayout {
     text: ArcStr,
     font: KeyOrValue<FontDescriptor>,
-    text_size_override: Option<KeyOrValue<f64>>,
-    text_color: KeyOrValue<Color>,
-    //FIXME: all this caching stuff can go away when we have a simple way of
-    // checking if something has changed in the env.
-    cached_text_color: Color,
-    cached_font: FontDescriptor,
     // when set, this will be used to override the size in he font descriptor.
     // This provides an easy way to change only the font size, while still
     // using a `FontDescriptor` in the `Env`.
-    cached_text_size: Option<f64>,
-    // the underlying layout object. This is constructed lazily.
+    text_size_override: Option<KeyOrValue<f64>>,
+    text_color: KeyOrValue<Color>,
     layout: Option<PietTextLayout>,
     wrap_width: f64,
     alignment: TextAlignment,
@@ -69,11 +67,8 @@ impl TextLayout {
         TextLayout {
             text: text.into(),
             font: crate::theme::UI_FONT.into(),
-            cached_font: Default::default(),
             text_color: crate::theme::LABEL_COLOR.into(),
-            cached_text_color: Color::BLACK,
             text_size_override: None,
-            cached_text_size: None,
             layout: None,
             wrap_width: f64::INFINITY,
             alignment: Default::default(),
@@ -213,38 +208,47 @@ impl TextLayout {
     /// will check to see if any used environment items have changed,
     /// and invalidate itself as needed.
     ///
-    /// Returns `true` if an item has changed, indicating that the text object
-    /// needs layout.
+    /// Returns `true` if the text item needs to be rebuilt.
+    pub fn needs_rebuild_after_update(&mut self, ctx: &mut UpdateCtx) -> bool {
+        if ctx.env_changed() && self.layout.is_some() {
+            let rebuild = ctx.env_key_changed(&self.font)
+                || ctx.env_key_changed(&self.text_color)
+                || self
+                    .text_size_override
+                    .as_ref()
+                    .map(|k| ctx.env_key_changed(k))
+                    .unwrap_or(false);
+
+            if rebuild {
+                self.layout = None;
+            }
+        }
+        self.layout.is_none()
+    }
+
+    /// Rebuild the inner layout as needed.
     ///
-    /// # Note
+    /// This `TextLayout` object manages a lower-level layout object that may
+    /// need to be rebuilt in response to changes to the text or attributes
+    /// like the font.
     ///
-    /// After calling this method, the layout may be invalid until the next call
-    /// to [`rebuild_layout_if_needed`], [`layout`], or [`paint`].
+    /// This method should be called whenever any of these things may have changed.
+    /// A simple way to ensure this is correct is to always call this method
+    /// as part of your widget's [`layout`] method.
     ///
-    /// [`layout`]: #method.layout
-    /// [`paint`]: #method.paint
-    /// [`rebuild_layout_if_needed`]: #method.rebuild_layout_if_needed
+    /// [`layout`]: trait.Widget.html#method.layout
     pub fn rebuild_if_needed(&mut self, factory: &mut PietText, env: &Env) {
-        let new_font = self.font.resolve(env);
-        let new_color = self.text_color.resolve(env);
-        let new_size = self.text_size_override.as_ref().map(|key| key.resolve(env));
+        if self.layout.is_none() {
+            let font = self.font.resolve(env);
+            let color = self.text_color.resolve(env);
+            let size_override = self.text_size_override.as_ref().map(|key| key.resolve(env));
 
-        let needs_rebuild = !new_font.same(&self.cached_font)
-            || !new_color.same(&self.cached_text_color)
-            || new_size != self.cached_text_size
-            || self.layout.is_none();
-
-        self.cached_font = new_font;
-        self.cached_text_color = new_color;
-        self.cached_text_size = new_size;
-
-        if needs_rebuild {
-            let descriptor = if let Some(size) = &self.cached_text_size {
-                self.cached_font.clone().with_size(*size)
+            let descriptor = if let Some(size) = size_override {
+                font.with_size(size)
             } else {
-                self.cached_font.clone()
+                font
             };
-            let text_color = self.cached_text_color.clone();
+
             self.layout = Some(
                 factory
                     .new_text_layout(self.text.clone())
@@ -253,7 +257,7 @@ impl TextLayout {
                     .font(descriptor.family.clone(), descriptor.size)
                     .default_attribute(descriptor.weight)
                     .default_attribute(descriptor.style)
-                    .default_attribute(TextAttribute::ForegroundColor(text_color))
+                    .default_attribute(TextAttribute::ForegroundColor(color))
                     .build()
                     .unwrap(),
             )

druid/src/widget/label.rs

@@ -308,8 +308,7 @@ impl<T: Data> Widget<T> for Label<T> {
     fn lifecycle(&mut self, _ctx: &mut LifeCycleCtx, _event: &LifeCycle, _data: &T, _env: &Env) {}
 
     fn update(&mut self, ctx: &mut UpdateCtx, old_data: &T, data: &T, _env: &Env) {
-        //FIXME: this should also be checking if anything in the env has changed
-        if !old_data.same(data) {
+        if !old_data.same(data) | self.layout.needs_rebuild_after_update(ctx) {
             self.needs_rebuild = true;
             ctx.request_layout();
         }

druid/src/widget/widget.rs

@@ -118,16 +118,33 @@ pub trait Widget<T> {
     /// [`Command`]: struct.Command.html
     fn lifecycle(&mut self, ctx: &mut LifeCycleCtx, event: &LifeCycle, data: &T, env: &Env);
 
-    /// Handle a change of data.
+    /// Update the widget's appearance in response to a change in the app's
+    /// [`Data`] or [`Env`].
     ///
-    /// This method is called whenever the data changes. When the appearance of
-    /// the widget depends on data, call [`request_paint`] so that it's scheduled
-    /// for repaint.
+    /// This method is called whenever the data or environment changes.
+    /// When the appearance of the widget needs to be updated in response to
+    /// these changes, you can call [`request_paint`] or [`request_layout`] on
+    /// the provided [`UpdateCtx`] to schedule calls to [`paint`] and [`layout`]
+    /// as required.
     ///
     /// The previous value of the data is provided in case the widget wants to
-    /// compute a fine-grained delta.
+    /// compute a fine-grained delta; you should try to only request a new
+    /// layout or paint pass if it is actually required.
     ///
+    /// To determine if the [`Env`] has changed, you can call [`env_changed`]
+    /// on the provided [`UpdateCtx`]; you can then call [`env_key_changed`]
+    /// with any keys that are used in your widget, to see if they have changed;
+    /// you can then request layout or paint as needed.
+    ///
+    /// [`Data`]: trait.Data.html
+    /// [`Env`]: struct.Env.html
+    /// [`UpdateCtx`]: struct.UpdateCtx.html
+    /// [`env_changed`]: struct.UpdateCtx.html#method.env_changed
+    /// [`env_key_changed`]: struct.UpdateCtx.html#method.env_changed
     /// [`request_paint`]: struct.UpdateCtx.html#method.request_paint
+    /// [`request_layout`]: struct.UpdateCtx.html#method.request_layout
+    /// [`layout`]: #tymethod.layout
+    /// [`paint`]: #tymethod.paint
     fn update(&mut self, ctx: &mut UpdateCtx, old_data: &T, data: &T, env: &Env);
 
     /// Compute layout.

druid/src/window.rs

@@ -280,6 +280,8 @@ impl<T: Data> Window<T> {
         let mut update_ctx = UpdateCtx {
             widget_state: &mut widget_state,
             state: &mut state,
+            prev_env: None,
+            env,
         };
 
         self.root.update(&mut update_ctx, data, env);