From a5e794ca42e22afda2c62c4e5bbb1a94f39c0416 Mon Sep 17 00:00:00 2001 From: Adam Barth Date: Fri, 22 Apr 2016 16:40:20 -0700 Subject: [PATCH] A blinking cursor should push only one frame (#3445) (#3506) Prior to this patch, we were pushing two frames each time the cursor blinked. In turning the cursor on or off, the markNeedsPaint call was triggering another frame to be scheduled because we cleared a bit in the scheduler at the beginning of the frame instead of at the end of the frame. To implement scheduling correctly, we actually need two bits: one for ensureVisualUpdate, which just promises to get to the end of the pipeline soon, and scheduleFrame, which promises to get to the beginning of the pipeline soon. (Reland) --- .../flutter/lib/src/scheduler/binding.dart | 49 +++++++++---------- packages/flutter/lib/src/widgets/binding.dart | 16 +++++- 2 files changed, 37 insertions(+), 28 deletions(-) diff --git a/packages/flutter/lib/src/scheduler/binding.dart b/packages/flutter/lib/src/scheduler/binding.dart index 231413c3cee..db3f46e7cd1 100644 --- a/packages/flutter/lib/src/scheduler/binding.dart +++ b/packages/flutter/lib/src/scheduler/binding.dart @@ -86,7 +86,7 @@ abstract class SchedulerBinding extends BindingBase { void initServiceExtensions() { super.initServiceExtensions(); registerNumericServiceExtension( - name: 'timeDilation', + name: 'timeDilation', getter: () => timeDilation, setter: (double value) { timeDilation = value; @@ -94,9 +94,8 @@ abstract class SchedulerBinding extends BindingBase { ); } - /// The strategy to use when deciding whether to run a task or not. - /// + /// /// Defaults to [defaultSchedulingStrategy]. SchedulingStrategy schedulingStrategy = defaultSchedulingStrategy; @@ -119,7 +118,6 @@ abstract class SchedulerBinding extends BindingBase { _ensureEventLoopCallback(); } - // Whether this scheduler already requested to be called from the event loop. bool _hasRequestedAnEventLoopCallback = false; @@ -155,11 +153,10 @@ abstract class SchedulerBinding extends BindingBase { } else { // TODO(floitsch): we shouldn't need to request a frame. Just schedule // an event-loop callback. - ensureVisualUpdate(); + scheduleFrame(); } } - int _nextFrameCallbackId = 0; // positive Map _transientCallbacks = {}; final Set _removedIds = new HashSet(); @@ -187,7 +184,7 @@ abstract class SchedulerBinding extends BindingBase { /// Callbacks registered with this method can be canceled using /// [cancelFrameCallbackWithId]. int scheduleFrameCallback(FrameCallback callback, { bool rescheduling: false }) { - ensureVisualUpdate(); + scheduleFrame(); return addFrameCallback(callback, rescheduling: rescheduling); } @@ -270,7 +267,6 @@ abstract class SchedulerBinding extends BindingBase { return true; } - final List _persistentCallbacks = new List(); /// Adds a persistent frame callback. @@ -286,7 +282,6 @@ abstract class SchedulerBinding extends BindingBase { _persistentCallbacks.add(callback); } - final List _postFrameCallbacks = new List(); /// Schedule a callback for the end of this frame. @@ -306,10 +301,11 @@ abstract class SchedulerBinding extends BindingBase { _postFrameCallbacks.add(callback); } + // Whether this scheduler as requested that handleBeginFrame be called soon. + bool _hasScheduledFrame = false; - // Whether this scheduler already requested to be called at the beginning of - // the next frame. - bool _hasRequestedABeginFrameCallback = false; + // Whether this scheduler is currently producing a frame in handleBeginFrame. + bool _isProducingFrame = false; /// If necessary, schedules a new frame by calling /// [ui.window.scheduleFrame]. @@ -319,19 +315,17 @@ abstract class SchedulerBinding extends BindingBase { /// device's screen is turned off it will typically be delayed until /// the screen is on and the application is visible.) void ensureVisualUpdate() { - if (_hasRequestedABeginFrameCallback) + if (_isProducingFrame) return; - ui.window.scheduleFrame(); - _hasRequestedABeginFrameCallback = true; + scheduleFrame(); } - /// Whether the scheduler is currently handling a "begin frame" - /// callback. - /// - /// True while [handleBeginFrame] is running in checked mode. False - /// otherwise. - static bool get debugInFrame => _debugInFrame; - static bool _debugInFrame = false; + void scheduleFrame() { + if (_hasScheduledFrame) + return; + ui.window.scheduleFrame(); + _hasScheduledFrame = true; + } /// Called by the engine to produce a new frame. /// @@ -342,23 +336,24 @@ abstract class SchedulerBinding extends BindingBase { /// callbacks registered by [addPostFrameCallback]. void handleBeginFrame(Duration rawTimeStamp) { Timeline.startSync('Frame'); - assert(!_debugInFrame); - assert(() { _debugInFrame = true; return true; }); + assert(!_isProducingFrame); + _isProducingFrame = true; + _hasScheduledFrame = false; Duration timeStamp = new Duration( microseconds: (rawTimeStamp.inMicroseconds / timeDilation).round()); - _hasRequestedABeginFrameCallback = false; _invokeTransientFrameCallbacks(timeStamp); for (FrameCallback callback in _persistentCallbacks) _invokeFrameCallback(callback, timeStamp); + _isProducingFrame = false; + List localPostFrameCallbacks = new List.from(_postFrameCallbacks); _postFrameCallbacks.clear(); for (FrameCallback callback in localPostFrameCallbacks) _invokeFrameCallback(callback, timeStamp); - assert(() { _debugInFrame = false; return true; }); Timeline.finishSync(); // All frame-related callbacks have been executed. Run lower-priority tasks. @@ -367,7 +362,7 @@ abstract class SchedulerBinding extends BindingBase { void _invokeTransientFrameCallbacks(Duration timeStamp) { Timeline.startSync('Animate'); - assert(_debugInFrame); + assert(_isProducingFrame); Map callbacks = _transientCallbacks; _transientCallbacks = new Map(); callbacks.forEach((int id, _FrameCallbackEntry callbackEntry) { diff --git a/packages/flutter/lib/src/widgets/binding.dart b/packages/flutter/lib/src/widgets/binding.dart index b63fc423ca0..685e7d8d7c7 100644 --- a/packages/flutter/lib/src/widgets/binding.dart +++ b/packages/flutter/lib/src/widgets/binding.dart @@ -54,7 +54,7 @@ abstract class WidgetsBinding extends BindingBase implements GestureBinding, Ren void initInstances() { super.initInstances(); _instance = this; - buildOwner.onBuildScheduled = ensureVisualUpdate; + buildOwner.onBuildScheduled = _handleBuildScheduled; ui.window.onLocaleChanged = handleLocaleChanged; ui.window.onPopRoute = handlePopRoute; ui.window.onAppLifecycleStateChanged = handleAppLifecycleStateChanged; @@ -183,9 +183,23 @@ abstract class WidgetsBinding extends BindingBase implements GestureBinding, Ren _didFirstFrame = true; } + void _handleBuildScheduled() { + // If we're in the process of building dirty elements, we're know that any + // builds that are scheduled will be run this frame, which means we don't + // need to schedule another frame. + if (_buildingDirtyElements) + return; + scheduleFrame(); + } + + bool _buildingDirtyElements = false; + @override void beginFrame() { + assert(!_buildingDirtyElements); + _buildingDirtyElements = true; buildOwner.buildDirtyElements(); + _buildingDirtyElements = false; super.beginFrame(); buildOwner.finalizeTree(); // TODO(ianh): Following code should not be included in release mode, only profile and debug mode