Commit 161f945e authored by Adam Barth's avatar Adam Barth

A blinking cursor should push only one frame (#3445)

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.
parent 7fe7de3f
...@@ -2,7 +2,6 @@ ...@@ -2,7 +2,6 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
import 'package:flutter/scheduler.dart';
import 'package:flutter/widgets.dart'; import 'package:flutter/widgets.dart';
import 'colors.dart'; import 'colors.dart';
...@@ -171,7 +170,6 @@ class DrawerControllerState extends State<DrawerController> { ...@@ -171,7 +170,6 @@ class DrawerControllerState extends State<DrawerController> {
} }
double get _width { double get _width {
assert(!Scheduler.debugInFrame); // we should never try to read the tree state while building or laying out
RenderBox drawerBox = _drawerKey.currentContext?.findRenderObject(); RenderBox drawerBox = _drawerKey.currentContext?.findRenderObject();
if (drawerBox != null) if (drawerBox != null)
return drawerBox.size.width; return drawerBox.size.width;
......
...@@ -86,7 +86,7 @@ abstract class Scheduler extends BindingBase { ...@@ -86,7 +86,7 @@ abstract class Scheduler extends BindingBase {
void initServiceExtensions() { void initServiceExtensions() {
super.initServiceExtensions(); super.initServiceExtensions();
registerNumericServiceExtension( registerNumericServiceExtension(
name: 'timeDilation', name: 'timeDilation',
getter: () => timeDilation, getter: () => timeDilation,
setter: (double value) { setter: (double value) {
timeDilation = value; timeDilation = value;
...@@ -94,9 +94,8 @@ abstract class Scheduler extends BindingBase { ...@@ -94,9 +94,8 @@ abstract class Scheduler extends BindingBase {
); );
} }
/// The strategy to use when deciding whether to run a task or not. /// The strategy to use when deciding whether to run a task or not.
/// ///
/// Defaults to [defaultSchedulingStrategy]. /// Defaults to [defaultSchedulingStrategy].
SchedulingStrategy schedulingStrategy = defaultSchedulingStrategy; SchedulingStrategy schedulingStrategy = defaultSchedulingStrategy;
...@@ -119,7 +118,6 @@ abstract class Scheduler extends BindingBase { ...@@ -119,7 +118,6 @@ abstract class Scheduler extends BindingBase {
_ensureEventLoopCallback(); _ensureEventLoopCallback();
} }
// Whether this scheduler already requested to be called from the event loop. // Whether this scheduler already requested to be called from the event loop.
bool _hasRequestedAnEventLoopCallback = false; bool _hasRequestedAnEventLoopCallback = false;
...@@ -155,11 +153,10 @@ abstract class Scheduler extends BindingBase { ...@@ -155,11 +153,10 @@ abstract class Scheduler extends BindingBase {
} else { } else {
// TODO(floitsch): we shouldn't need to request a frame. Just schedule // TODO(floitsch): we shouldn't need to request a frame. Just schedule
// an event-loop callback. // an event-loop callback.
ensureVisualUpdate(); _scheduleFrame();
} }
} }
int _nextFrameCallbackId = 0; // positive int _nextFrameCallbackId = 0; // positive
Map<int, _FrameCallbackEntry> _transientCallbacks = <int, _FrameCallbackEntry>{}; Map<int, _FrameCallbackEntry> _transientCallbacks = <int, _FrameCallbackEntry>{};
final Set<int> _removedIds = new HashSet<int>(); final Set<int> _removedIds = new HashSet<int>();
...@@ -187,7 +184,7 @@ abstract class Scheduler extends BindingBase { ...@@ -187,7 +184,7 @@ abstract class Scheduler extends BindingBase {
/// Callbacks registered with this method can be canceled using /// Callbacks registered with this method can be canceled using
/// [cancelFrameCallbackWithId]. /// [cancelFrameCallbackWithId].
int scheduleFrameCallback(FrameCallback callback, { bool rescheduling: false }) { int scheduleFrameCallback(FrameCallback callback, { bool rescheduling: false }) {
ensureVisualUpdate(); _scheduleFrame();
return addFrameCallback(callback, rescheduling: rescheduling); return addFrameCallback(callback, rescheduling: rescheduling);
} }
...@@ -270,7 +267,6 @@ abstract class Scheduler extends BindingBase { ...@@ -270,7 +267,6 @@ abstract class Scheduler extends BindingBase {
return true; return true;
} }
final List<FrameCallback> _persistentCallbacks = new List<FrameCallback>(); final List<FrameCallback> _persistentCallbacks = new List<FrameCallback>();
/// Adds a persistent frame callback. /// Adds a persistent frame callback.
...@@ -286,7 +282,6 @@ abstract class Scheduler extends BindingBase { ...@@ -286,7 +282,6 @@ abstract class Scheduler extends BindingBase {
_persistentCallbacks.add(callback); _persistentCallbacks.add(callback);
} }
final List<FrameCallback> _postFrameCallbacks = new List<FrameCallback>(); final List<FrameCallback> _postFrameCallbacks = new List<FrameCallback>();
/// Schedule a callback for the end of this frame. /// Schedule a callback for the end of this frame.
...@@ -306,10 +301,11 @@ abstract class Scheduler extends BindingBase { ...@@ -306,10 +301,11 @@ abstract class Scheduler extends BindingBase {
_postFrameCallbacks.add(callback); _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 // Whether this scheduler is currently producing a frame in handleBeginFrame.
// the next frame. bool _isProducingFrame = false;
bool _hasRequestedABeginFrameCallback = false;
/// If necessary, schedules a new frame by calling /// If necessary, schedules a new frame by calling
/// [ui.window.scheduleFrame]. /// [ui.window.scheduleFrame].
...@@ -319,19 +315,17 @@ abstract class Scheduler extends BindingBase { ...@@ -319,19 +315,17 @@ abstract class Scheduler extends BindingBase {
/// device's screen is turned off it will typically be delayed until /// device's screen is turned off it will typically be delayed until
/// the screen is on and the application is visible.) /// the screen is on and the application is visible.)
void ensureVisualUpdate() { void ensureVisualUpdate() {
if (_hasRequestedABeginFrameCallback) if (_hasScheduledFrame || _isProducingFrame)
return; return;
ui.window.scheduleFrame(); _scheduleFrame();
_hasRequestedABeginFrameCallback = true;
} }
/// Whether the scheduler is currently handling a "begin frame" void _scheduleFrame() {
/// callback. if (_hasScheduledFrame)
/// return;
/// True while [handleBeginFrame] is running in checked mode. False ui.window.scheduleFrame();
/// otherwise. _hasScheduledFrame = true;
static bool get debugInFrame => _debugInFrame; }
static bool _debugInFrame = false;
/// Called by the engine to produce a new frame. /// Called by the engine to produce a new frame.
/// ///
...@@ -342,11 +336,11 @@ abstract class Scheduler extends BindingBase { ...@@ -342,11 +336,11 @@ abstract class Scheduler extends BindingBase {
/// callbacks registered by [addPostFrameCallback]. /// callbacks registered by [addPostFrameCallback].
void handleBeginFrame(Duration rawTimeStamp) { void handleBeginFrame(Duration rawTimeStamp) {
Timeline.startSync('Begin frame'); Timeline.startSync('Begin frame');
assert(!_debugInFrame); assert(!_isProducingFrame);
assert(() { _debugInFrame = true; return true; }); _isProducingFrame = true;
_hasScheduledFrame = false;
Duration timeStamp = new Duration( Duration timeStamp = new Duration(
microseconds: (rawTimeStamp.inMicroseconds / timeDilation).round()); microseconds: (rawTimeStamp.inMicroseconds / timeDilation).round());
_hasRequestedABeginFrameCallback = false;
_invokeTransientFrameCallbacks(timeStamp); _invokeTransientFrameCallbacks(timeStamp);
for (FrameCallback callback in _persistentCallbacks) for (FrameCallback callback in _persistentCallbacks)
...@@ -358,7 +352,7 @@ abstract class Scheduler extends BindingBase { ...@@ -358,7 +352,7 @@ abstract class Scheduler extends BindingBase {
for (FrameCallback callback in localPostFrameCallbacks) for (FrameCallback callback in localPostFrameCallbacks)
_invokeFrameCallback(callback, timeStamp); _invokeFrameCallback(callback, timeStamp);
assert(() { _debugInFrame = false; return true; }); _isProducingFrame = false;
Timeline.finishSync(); Timeline.finishSync();
// All frame-related callbacks have been executed. Run lower-priority tasks. // All frame-related callbacks have been executed. Run lower-priority tasks.
...@@ -367,7 +361,7 @@ abstract class Scheduler extends BindingBase { ...@@ -367,7 +361,7 @@ abstract class Scheduler extends BindingBase {
void _invokeTransientFrameCallbacks(Duration timeStamp) { void _invokeTransientFrameCallbacks(Duration timeStamp) {
Timeline.startSync('Animate'); Timeline.startSync('Animate');
assert(_debugInFrame); assert(_isProducingFrame);
Map<int, _FrameCallbackEntry> callbacks = _transientCallbacks; Map<int, _FrameCallbackEntry> callbacks = _transientCallbacks;
_transientCallbacks = new Map<int, _FrameCallbackEntry>(); _transientCallbacks = new Map<int, _FrameCallbackEntry>();
callbacks.forEach((int id, _FrameCallbackEntry callbackEntry) { callbacks.forEach((int id, _FrameCallbackEntry callbackEntry) {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment