From 2e0af13eb29419548a9044d9f554b31968c62759 Mon Sep 17 00:00:00 2001 From: ferhatb Date: Tue, 3 Nov 2020 10:32:57 -0800 Subject: [PATCH 1/3] Fix repaint logic for cullrect,transform changes --- lib/web_ui/lib/src/engine/html/picture.dart | 42 ++++++++++----------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/lib/web_ui/lib/src/engine/html/picture.dart b/lib/web_ui/lib/src/engine/html/picture.dart index a6d1867713e77..504524d65dd2a 100644 --- a/lib/web_ui/lib/src/engine/html/picture.dart +++ b/lib/web_ui/lib/src/engine/html/picture.dart @@ -91,6 +91,10 @@ class PersistedPicture extends PersistedLeafSurface { final ui.Rect? localPaintBounds; final int hints; double _density = 1.0; + /// Cull rect changes and density changes due to transforms should + /// call applyPaint for picture when retain() or update() is called after + /// preroll is complete. + bool _requiresRepaint = false; /// Cache for reusing elements such as images across picture updates. CrossFrameCache? _elementCache = @@ -114,16 +118,7 @@ class PersistedPicture extends PersistedLeafSurface { ? 1.0 : _computePixelDensity(_transform, paintWidth, paintHeight); if (newDensity != _density) { _density = newDensity; - if (_canvas != null) { - // If cull rect and density hasn't changed, this will only repaint. - // If density doesn't match canvas, a new canvas will be created - // and paint queued. - // - // Similar to preroll for transform where transform is updated, for - // picture this means we need to repaint so pixelation doesn't occur - // due to transform changing overall dpi. - applyPaint(_canvas); - } + _requiresRepaint = true; } _computeExactCullRects(); } @@ -204,13 +199,14 @@ class PersistedPicture extends PersistedLeafSurface { } } - bool _computeOptimalCullRect(PersistedPicture? oldSurface) { + void _computeOptimalCullRect(PersistedPicture? oldSurface) { assert(_exactLocalCullRect != null); if (oldSurface == null || !oldSurface.picture.recordingCanvas!.didDraw) { // First useful paint. _optimalLocalCullRect = _exactLocalCullRect; - return true; + _requiresRepaint = true; + return; } assert(oldSurface._optimalLocalCullRect != null); @@ -224,7 +220,10 @@ class PersistedPicture extends PersistedLeafSurface { // The clip collapsed into a zero-sized rectangle. If it was already zero, // no need to signal cull rect change. _optimalLocalCullRect = ui.Rect.zero; - return oldOptimalLocalCullRect != ui.Rect.zero; + if (oldOptimalLocalCullRect != ui.Rect.zero) { + _requiresRepaint = true; + } + return; } if (rectContainsOther(oldOptimalLocalCullRect!, _exactLocalCullRect!)) { @@ -233,7 +232,7 @@ class PersistedPicture extends PersistedLeafSurface { // a clip when it is scrolled out of the screen. In this case we do not // repaint the picture. We just let it be shrunk by the outer clip. _optimalLocalCullRect = oldOptimalLocalCullRect; - return false; + return; } // The new cull rect contains area not covered by a previous rect. Perhaps @@ -270,9 +269,9 @@ class PersistedPicture extends PersistedLeafSurface { _predictTrend(bottomwardDelta, _exactLocalCullRect!.height), ).intersect(localPaintBounds!); - final bool localCullRectChanged = _optimalLocalCullRect != newLocalCullRect; + _requiresRepaint = _optimalLocalCullRect != newLocalCullRect; _optimalLocalCullRect = newLocalCullRect; - return localCullRectChanged; + return; } /// Predicts the delta a particular side of a clip rect will move given the @@ -540,6 +539,7 @@ class PersistedPicture extends PersistedLeafSurface { @override void build() { _computeOptimalCullRect(null); + _requiresRepaint = true; super.build(); } @@ -556,15 +556,14 @@ class PersistedPicture extends PersistedLeafSurface { _applyTranslate(); } - final bool cullRectChangeRequiresRepaint = - _computeOptimalCullRect(oldSurface); + _computeOptimalCullRect(oldSurface); if (identical(picture, oldSurface.picture)) { bool densityChanged = (_canvas is BitmapCanvas && _density != (_canvas as BitmapCanvas)._density); // The picture is the same. Attempt to avoid repaint. - if (cullRectChangeRequiresRepaint || densityChanged) { + if (_requiresRepaint || densityChanged) { // Cull rect changed such that a repaint is still necessary. _applyPaint(oldSurface); } else { @@ -581,9 +580,10 @@ class PersistedPicture extends PersistedLeafSurface { @override void retain() { super.retain(); - final bool cullRectChangeRequiresRepaint = _computeOptimalCullRect(this); - if (cullRectChangeRequiresRepaint) { + _computeOptimalCullRect(this); + if (_requiresRepaint) { _applyPaint(this); + _requiresRepaint = false; } } From 4ba4642824cf01e724f39a24b0b4bc962152b6c0 Mon Sep 17 00:00:00 2001 From: ferhatb Date: Tue, 3 Nov 2020 10:43:19 -0800 Subject: [PATCH 2/3] clear requiresRepaint in applyPaint --- lib/web_ui/lib/src/engine/html/picture.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/web_ui/lib/src/engine/html/picture.dart b/lib/web_ui/lib/src/engine/html/picture.dart index 504524d65dd2a..a0c130a2520e7 100644 --- a/lib/web_ui/lib/src/engine/html/picture.dart +++ b/lib/web_ui/lib/src/engine/html/picture.dart @@ -308,6 +308,7 @@ class PersistedPicture extends PersistedLeafSurface { void _applyPaint(PersistedPicture? oldSurface) { final EngineCanvas? oldCanvas = oldSurface?._canvas; + _requiresRepaint = false; if (!picture.recordingCanvas!.didDraw || _optimalLocalCullRect!.isEmpty) { // The picture is empty, or it has been completely clipped out. Skip // painting. This removes all the setup work and scaffolding objects @@ -583,7 +584,6 @@ class PersistedPicture extends PersistedLeafSurface { _computeOptimalCullRect(this); if (_requiresRepaint) { _applyPaint(this); - _requiresRepaint = false; } } From 1d5e902beb15e88f9323026bf3c90e81565b6e97 Mon Sep 17 00:00:00 2001 From: ferhatb Date: Tue, 3 Nov 2020 10:45:32 -0800 Subject: [PATCH 3/3] address reviewer comments --- lib/web_ui/lib/src/engine/html/picture.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/web_ui/lib/src/engine/html/picture.dart b/lib/web_ui/lib/src/engine/html/picture.dart index a0c130a2520e7..691501044f1fb 100644 --- a/lib/web_ui/lib/src/engine/html/picture.dart +++ b/lib/web_ui/lib/src/engine/html/picture.dart @@ -271,7 +271,6 @@ class PersistedPicture extends PersistedLeafSurface { _requiresRepaint = _optimalLocalCullRect != newLocalCullRect; _optimalLocalCullRect = newLocalCullRect; - return; } /// Predicts the delta a particular side of a clip rect will move given the