From a2d9ff60b96428cd6e30bb0fe4c91372117c89cc Mon Sep 17 00:00:00 2001 From: Hixie Date: Mon, 6 Jul 2015 13:17:23 -0700 Subject: [PATCH] Make the menu items in the drawer change colour when tapped, per Material spec. This is done by the following steps: - Make Inherited classes notify their descendants when their state changes. - Make Components track which Inherited classes they looked at. - Make Components handle being notified of state changes by rebuilding if the changed ancestor is of a type we've looked at. - Refactor setState() and scheduleBuild(). - Make the "buildDirtyComponents" phase support incrementally building more components as each one finds it needs to update itself. The way this works is that when the menu item updates its color, it does so by changing a DefaultTextStyle (Inherited) node in its tree. That node then notifies its ancestors, which include in particular the Text component, which uses DefaultTextStyle. The notification is received, and, even though we're already in the rebuild phase, the Text queues itself up to be rebuilt as well. Since we now support adding people to the rebuild phase while it's active, it gets rebuilt as well, and all is well. This CL depends on the fix in https://codereview.chromium.org/1218183009 R=abarth@chromium.org Review URL: https://codereview.chromium.org/1222823004. --- sdk/lib/widgets/default_text_style.dart | 3 + sdk/lib/widgets/theme.dart | 3 + sdk/lib/widgets/widget.dart | 98 ++++++++++++++++++------- 3 files changed, 76 insertions(+), 28 deletions(-) diff --git a/sdk/lib/widgets/default_text_style.dart b/sdk/lib/widgets/default_text_style.dart index a8cc5f0d1..ea64c22da 100644 --- a/sdk/lib/widgets/default_text_style.dart +++ b/sdk/lib/widgets/default_text_style.dart @@ -23,4 +23,7 @@ class DefaultTextStyle extends Inherited { DefaultTextStyle result = component.inheritedOfType(DefaultTextStyle); return result == null ? null : result.style; } + + bool syncShouldNotify(DefaultTextStyle old) => style != old.style; + } diff --git a/sdk/lib/widgets/theme.dart b/sdk/lib/widgets/theme.dart index 8f7f4af17..529d76dea 100644 --- a/sdk/lib/widgets/theme.dart +++ b/sdk/lib/widgets/theme.dart @@ -27,4 +27,7 @@ class Theme extends Inherited { Theme theme = component.inheritedOfType(Theme); return theme == null ? _kFallbackTheme : theme.data; } + + bool syncShouldNotify(Theme old) => data != old.data; + } diff --git a/sdk/lib/widgets/widget.dart b/sdk/lib/widgets/widget.dart index 0cd31b07b..4d466c3bf 100644 --- a/sdk/lib/widgets/widget.dart +++ b/sdk/lib/widgets/widget.dart @@ -223,7 +223,27 @@ class ParentDataNode extends TagNode { } abstract class Inherited extends TagNode { + Inherited({ String key, Widget child }) : super._withKey(child, key); + + void _sync(Widget old, dynamic slot) { + if (old != null && syncShouldNotify(old)) { + final Type ourRuntimeType = runtimeType; + void notifyChildren(Widget child) { + if (child is Component && + child._dependencies != null && + child._dependencies.contains(ourRuntimeType)) + child._dependenciesChanged(); + if (child.runtimeType != ourRuntimeType) + child.walkChildren(notifyChildren); + } + walkChildren(notifyChildren); + } + super._sync(old, slot); + } + + bool syncShouldNotify(Inherited old); + } typedef void GestureEventListener(sky.GestureEvent e); @@ -354,33 +374,40 @@ abstract class Component extends Widget { _built.detachRoot(); } + Set _dependencies; Inherited inheritedOfType(Type targetType) { + if (_dependencies == null) + _dependencies = new Set(); + _dependencies.add(targetType); Widget ancestor = parent; while (ancestor != null && ancestor.runtimeType != targetType) ancestor = ancestor.parent; return ancestor; } + void _dependenciesChanged() { + // called by Inherited.sync() + scheduleBuild(); + } - bool _retainStatefulNodeIfPossible(Widget old) { + bool _retainStatefulNodeIfPossible(Component old) { assert(!_disqualifiedFromEverAppearingAgain); - Component oldComponent = old as Component; - if (oldComponent == null || !oldComponent._stateful) + if (old == null || !old._stateful) return false; - assert(runtimeType == oldComponent.runtimeType); - assert(key == oldComponent.key); + assert(runtimeType == old.runtimeType); + assert(key == old.key); // Make |this|, the newly-created object, into the "old" Component, and kill it _stateful = false; - _built = oldComponent._built; + _built = old._built; assert(_built != null); _disqualifiedFromEverAppearingAgain = true; - // Make |oldComponent| the "new" component - oldComponent._built = null; - oldComponent._dirty = true; - oldComponent.syncFields(this); + // Make |old| the "new" component + old._built = null; + old._dirty = true; + old.syncFields(this); return true; } @@ -394,6 +421,11 @@ abstract class Component extends Widget { assert(false); } + // order corresponds to _build_ order, not depth in the tree. + // All the Components built by a particular other Component will have the + // same order, regardless of whether one is subsequently inserted + // into another. The order is used to not tell a Component to + // rebuild if the Component that it built has itself been rebuilt. final int _order; static int _currentOrder = 0; @@ -446,20 +478,19 @@ abstract class Component extends Widget { } void scheduleBuild() { - setState(() {}); - } - - void setState(Function fn()) { assert(!_disqualifiedFromEverAppearingAgain); - assert(_stateful); - fn(); if (_isBuilding || _dirty || !_mounted) return; - _dirty = true; _scheduleComponentForRender(this); } + void setState(Function fn()) { + assert(_stateful); + fn(); + scheduleBuild(); + } + Widget build(); } @@ -470,24 +501,37 @@ bool _inRenderDirtyComponents = false; List _debugFrameTimes = []; +void _absorbDirtyComponents(List list) { + list.addAll(_dirtyComponents); + _dirtyComponents.clear(); + list.sort((Component a, Component b) => a._order - b._order); +} + void _buildDirtyComponents() { Stopwatch sw; if (_shouldLogRenderDuration) sw = new Stopwatch()..start(); + _inRenderDirtyComponents = true; try { sky.tracing.begin('Widgets._buildDirtyComponents'); - _inRenderDirtyComponents = true; - - List sortedDirtyComponents = _dirtyComponents.toList(); - sortedDirtyComponents.sort((Component a, Component b) => a._order - b._order); - for (var comp in sortedDirtyComponents) { - comp._buildIfDirty(); + List sortedDirtyComponents = new List(); + _absorbDirtyComponents(sortedDirtyComponents); + int index = 0; + while (index < sortedDirtyComponents.length) { + Component component = sortedDirtyComponents[index]; + component._buildIfDirty(); + if (_dirtyComponents.length > 0) { + // the following assert verifies that we're not rebuilding anyone twice in one frame + assert(_dirtyComponents.every((Component component) => !sortedDirtyComponents.contains(component))); + _absorbDirtyComponents(sortedDirtyComponents); + index = 0; + } else { + index += 1; + } } - - _dirtyComponents.clear(); - _buildScheduled = false; } finally { + _buildScheduled = false; _inRenderDirtyComponents = false; sky.tracing.end('Widgets._buildDirtyComponents'); } @@ -507,9 +551,7 @@ void _buildDirtyComponents() { } void _scheduleComponentForRender(Component c) { - assert(!_inRenderDirtyComponents); _dirtyComponents.add(c); - if (!_buildScheduled) { _buildScheduled = true; new Future.microtask(_buildDirtyComponents); -- GitLab