Commit 194bf41e authored by Ian Hickson's avatar Ian Hickson Committed by GitHub

Don't relayout a Text if only its color changed. (#11313)

parent 6dbf2269
...@@ -52,3 +52,41 @@ export 'dart:ui' show ...@@ -52,3 +52,41 @@ export 'dart:ui' show
// - window, WindowPadding // - window, WindowPadding
// These are generally wrapped by other APIs so we always refer to them directly // These are generally wrapped by other APIs so we always refer to them directly
// as ui.* to avoid making them seem like high-level APIs. // as ui.* to avoid making them seem like high-level APIs.
/// The description of the difference between two objects, in the context of how
/// it will affect the rendering.
///
/// Used by [TextSpan.compareTo] and [TextStyle.compareTo].
///
/// The values in this enum are ordered such that they are in increasing order
/// of cost. A value with index N implies all the values with index less than N.
/// For example, [layout] (index 3) implies [paint] (2).
enum RenderComparison {
/// The two objects are identical (meaning deeply equal, not necessarily
/// [identical]).
identical,
/// The two objects are identical for the purpose of layout, but may be different
/// in other ways.
///
/// For example, maybe some event handlers changed.
metadata,
/// The two objects are different but only in ways that affect paint, not layout.
///
/// For example, only the color is changed.
///
/// [RenderObject.markNeedsPaint] would be necessary to handle this kind of
/// change in a render object.
paint,
/// The two objects are different in ways that affect layout (and therefore paint).
///
/// For example, the size is changed.
///
/// This is the most drastic level of change possible.
///
/// [RenderObject.markNeedsLayout] would be necessary to handle this kind of
/// change in a render object.
layout,
}
...@@ -317,6 +317,39 @@ class TextSpan { ...@@ -317,6 +317,39 @@ class TextSpan {
return true; return true;
} }
/// Describe the difference between this text span and another, in terms of
/// how much damage it will make to the rendering. The comparison is deep.
///
/// See also:
///
/// * [TextStyle.compareTo], which does the same thing for [TextStyle]s.
RenderComparison compareTo(TextSpan other) {
if (identical(this, other))
return RenderComparison.identical;
if (other.text != text ||
children?.length != other.children?.length ||
(style == null) != (other.style == null))
return RenderComparison.layout;
RenderComparison result = recognizer == other.recognizer ? RenderComparison.identical : RenderComparison.metadata;
if (style != null) {
final RenderComparison candidate = style.compareTo(other.style);
if (candidate.index > result.index)
result = candidate;
if (result == RenderComparison.layout)
return result;
}
if (children != null) {
for (int index = 0; index < children.length; index += 1) {
final RenderComparison candidate = children[index].compareTo(other.children[index]);
if (candidate.index > result.index)
result = candidate;
if (result == RenderComparison.layout)
return result;
}
}
return result;
}
@override @override
bool operator ==(dynamic other) { bool operator ==(dynamic other) {
if (identical(this, other)) if (identical(this, other))
......
...@@ -387,6 +387,33 @@ class TextStyle { ...@@ -387,6 +387,33 @@ class TextStyle {
); );
} }
/// Describe the difference between this style and another, in terms of how
/// much damage it will make to the rendering.
///
/// See also:
///
/// * [TextSpan.compareTo], which does the same thing for entire [TextSpan]s.
RenderComparison compareTo(TextStyle other) {
if (identical(this, other))
return RenderComparison.identical;
if (inherit != other.inherit ||
fontFamily != other.fontFamily ||
fontSize != other.fontSize ||
fontWeight != other.fontWeight ||
fontStyle != other.fontStyle ||
letterSpacing != other.letterSpacing ||
wordSpacing != other.wordSpacing ||
textBaseline != other.textBaseline ||
height != other.height)
return RenderComparison.layout;
if (color != other.color ||
decoration != other.decoration ||
decorationColor != other.decorationColor ||
decorationStyle != other.decorationStyle)
return RenderComparison.paint;
return RenderComparison.identical;
}
@override @override
bool operator ==(dynamic other) { bool operator ==(dynamic other) {
if (identical(this, other)) if (identical(this, other))
......
...@@ -1559,6 +1559,8 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget { ...@@ -1559,6 +1559,8 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget {
/// to condition their runtime behavior on whether they are dirty or not, /// to condition their runtime behavior on whether they are dirty or not,
/// since they should only be marked dirty immediately prior to being laid /// since they should only be marked dirty immediately prior to being laid
/// out and painted. /// out and painted.
///
/// It is intended to be used by tests and asserts.
bool get debugNeedsLayout { bool get debugNeedsLayout {
bool result; bool result;
assert(() { assert(() {
...@@ -2133,6 +2135,28 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget { ...@@ -2133,6 +2135,28 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget {
_needsCompositingBitsUpdate = false; _needsCompositingBitsUpdate = false;
} }
/// Whether this render object's paint information is dirty.
///
/// This is only set in debug mode. In general, render objects should not need
/// to condition their runtime behavior on whether they are dirty or not,
/// since they should only be marked dirty immediately prior to being laid
/// out and painted.
///
/// It is intended to be used by tests and asserts.
///
/// It is possible (and indeed, quite common) for [debugNeedsPaint] to be
/// false and [debugNeedsLayout] to be true. The render object will still be
/// repainted in the next frame when this is the case, because the
/// [markNeedsPaint] method is implicitly called by the framework after a
/// render object is laid out, prior to the paint phase.
bool get debugNeedsPaint {
bool result;
assert(() {
result = _needsPaint;
return true;
});
return result;
}
bool _needsPaint = true; bool _needsPaint = true;
/// Mark this render object as having changed its visual appearance. /// Mark this render object as having changed its visual appearance.
...@@ -2145,6 +2169,10 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget { ...@@ -2145,6 +2169,10 @@ abstract class RenderObject extends AbstractNode implements HitTestTarget {
/// ///
/// This mechanism batches the painting work so that multiple sequential /// This mechanism batches the painting work so that multiple sequential
/// writes are coalesced, removing redundant computation. /// writes are coalesced, removing redundant computation.
///
/// Once [markNeedsPaint] has been called on a render object,
/// [debugNeedsPaint] returns true for that render object until just after
/// the pipeline owner has called [paint] on the render object.
void markNeedsPaint() { void markNeedsPaint() {
assert(owner == null || !owner.debugDoingPaint); assert(owner == null || !owner.debugDoingPaint);
if (_needsPaint) if (_needsPaint)
......
...@@ -64,11 +64,20 @@ class RenderParagraph extends RenderBox { ...@@ -64,11 +64,20 @@ class RenderParagraph extends RenderBox {
TextSpan get text => _textPainter.text; TextSpan get text => _textPainter.text;
set text(TextSpan value) { set text(TextSpan value) {
assert(value != null); assert(value != null);
if (_textPainter.text == value) switch (_textPainter.text.compareTo(value)) {
return; case RenderComparison.identical:
_textPainter.text = value; case RenderComparison.metadata:
_overflowShader = null; return;
markNeedsLayout(); case RenderComparison.paint:
_textPainter.text = value;
markNeedsPaint();
break;
case RenderComparison.layout:
_textPainter.text = value;
_overflowShader = null;
markNeedsLayout();
break;
}
} }
/// How the text should be aligned horizontally. /// How the text should be aligned horizontally.
......
...@@ -177,6 +177,36 @@ void main() { ...@@ -177,6 +177,36 @@ void main() {
expect(paragraph.size.height, 30.0); expect(paragraph.size.height, 30.0);
}); });
test('changing color does not do layout', () {
final RenderParagraph paragraph = new RenderParagraph(
const TextSpan(
text: 'Hello',
style: const TextStyle(color: const Color(0xFF000000)),
),
);
layout(paragraph, constraints: const BoxConstraints(maxWidth: 100.0), phase: EnginePhase.paint);
expect(paragraph.debugNeedsLayout, isFalse);
expect(paragraph.debugNeedsPaint, isFalse);
paragraph.text = const TextSpan(
text: 'Hello World',
style: const TextStyle(color: const Color(0xFF000000)),
);
expect(paragraph.debugNeedsLayout, isTrue);
expect(paragraph.debugNeedsPaint, isFalse);
pumpFrame(phase: EnginePhase.paint);
expect(paragraph.debugNeedsLayout, isFalse);
expect(paragraph.debugNeedsPaint, isFalse);
paragraph.text = const TextSpan(
text: 'Hello World',
style: const TextStyle(color: const Color(0xFFFFFFFF)),
);
expect(paragraph.debugNeedsLayout, isFalse);
expect(paragraph.debugNeedsPaint, isTrue);
pumpFrame(phase: EnginePhase.paint);
expect(paragraph.debugNeedsLayout, isFalse);
expect(paragraph.debugNeedsPaint, isFalse);
});
test('toStringDeep', () { test('toStringDeep', () {
final RenderParagraph paragraph = new RenderParagraph( final RenderParagraph paragraph = new RenderParagraph(
const TextSpan(text: _kText), const TextSpan(text: _kText),
......
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