From 71641a44b680530b285e2858e813dd6579476f7e Mon Sep 17 00:00:00 2001 From: ferhatb Date: Fri, 15 May 2020 14:00:08 -0700 Subject: [PATCH 1/5] reuse images across frames --- lib/web_ui/lib/src/engine/bitmap_canvas.dart | 38 ++++++++++++++++++-- 1 file changed, 35 insertions(+), 3 deletions(-) diff --git a/lib/web_ui/lib/src/engine/bitmap_canvas.dart b/lib/web_ui/lib/src/engine/bitmap_canvas.dart index ea1441a4bfeb1..c3d2c34ece57b 100644 --- a/lib/web_ui/lib/src/engine/bitmap_canvas.dart +++ b/lib/web_ui/lib/src/engine/bitmap_canvas.dart @@ -93,6 +93,11 @@ class BitmapCanvas extends EngineCanvas { _childOverdraw = value; } + // Maps src url to image element to reuse image instances across frames. + // Optimization for some browsers that keep refreshing images causing + // flicker. + Map> _reusableImages; + /// Allocates a canvas with enough memory to paint a picture within the given /// [bounds]. /// @@ -172,7 +177,14 @@ class BitmapCanvas extends EngineCanvas { _canvasPool.clear(); final int len = _children.length; for (int i = 0; i < len; i++) { - _children[i].remove(); + html.Element child = _children[i]; + if (child is html.ImageElement) { + _reusableImages ??= {}; + final String src = child.src; + _reusableImages[src] ??= []..add(child); + } else { + _children[i].remove(); + } } _children.clear(); _cachedLastStyle = null; @@ -363,6 +375,18 @@ class BitmapCanvas extends EngineCanvas { _cachedLastStyle = null; } + html.ImageElement _reuseOrCreateImage(HtmlImage htmlImage) { + if (_reusableImages != null) { + List cachedImages = + _reusableImages[htmlImage.imgElement.src]; + if (cachedImages != null && cachedImages.isNotEmpty) { + return cachedImages.removeAt(0); + } + } + // Can't reuse, create new instance. + return htmlImage.cloneImageElement(); + } + html.HtmlElement _drawImage( ui.Image image, ui.Offset p, SurfacePaintData paint) { final HtmlImage htmlImage = image; @@ -372,7 +396,7 @@ class BitmapCanvas extends EngineCanvas { html.HtmlElement imgElement; if (colorFilterBlendMode == null) { // No Blending, create an image by cloning original loaded image. - imgElement = htmlImage.cloneImageElement(); + imgElement = _reuseOrCreateImage(htmlImage); } else { switch (colorFilterBlendMode) { case ui.BlendMode.colorBurn: @@ -607,7 +631,7 @@ class BitmapCanvas extends EngineCanvas { html.Element.html(svgFilter, treeSanitizer: _NullTreeSanitizer()); rootElement.append(filterElement); _children.add(filterElement); - final html.HtmlElement imgElement = image.cloneImageElement(); + final html.HtmlElement imgElement = _reuseOrCreateImage(image); imgElement.style.filter = 'url(#_fcf${_filterIdCounter})'; if (colorFilterBlendMode == ui.BlendMode.saturation) { imgElement.style.backgroundColor = colorToCssString(filterColor); @@ -792,6 +816,14 @@ class BitmapCanvas extends EngineCanvas { void endOfPaint() { assert(_saveCount == 0); _canvasPool.endOfPaint(); + if (_reusableImages != null) { + for (List images in _reusableImages.values) { + for (int i = 0, len = images.length; i < len; i++) { + images[i].remove(); + } + } + } + _reusableImages = null; } } From af1ab48553a34211aa8de4a0c17dc88750995c70 Mon Sep 17 00:00:00 2001 From: ferhatb Date: Wed, 20 May 2020 09:53:05 -0700 Subject: [PATCH 2/5] Change implementation to CrossFrameCache at picture level --- lib/web_ui/lib/src/engine.dart | 1 + lib/web_ui/lib/src/engine/bitmap_canvas.dart | 49 ++++----- .../lib/src/engine/frame_reference.dart | 102 ++++++++++++++++++ .../lib/src/engine/surface/picture.dart | 11 +- .../lib/src/engine/surface/surface.dart | 24 ----- .../test/engine/frame_reference_test.dart | 76 +++++++++++++ 6 files changed, 209 insertions(+), 54 deletions(-) create mode 100644 lib/web_ui/lib/src/engine/frame_reference.dart create mode 100644 lib/web_ui/test/engine/frame_reference_test.dart diff --git a/lib/web_ui/lib/src/engine.dart b/lib/web_ui/lib/src/engine.dart index 4f6116195a7e9..9baff4398c533 100644 --- a/lib/web_ui/lib/src/engine.dart +++ b/lib/web_ui/lib/src/engine.dart @@ -56,6 +56,7 @@ part 'engine/conic.dart'; part 'engine/dom_canvas.dart'; part 'engine/dom_renderer.dart'; part 'engine/engine_canvas.dart'; +part 'engine/frame_reference.dart'; part 'engine/history.dart'; part 'engine/houdini_canvas.dart'; part 'engine/html_image_codec.dart'; diff --git a/lib/web_ui/lib/src/engine/bitmap_canvas.dart b/lib/web_ui/lib/src/engine/bitmap_canvas.dart index efbc7e926e0b8..18cb130cb963e 100644 --- a/lib/web_ui/lib/src/engine/bitmap_canvas.dart +++ b/lib/web_ui/lib/src/engine/bitmap_canvas.dart @@ -26,6 +26,7 @@ class BitmapCanvas extends EngineCanvas { } ui.Rect _bounds; + CrossFrameCache _elementCache; /// The amount of padding to add around the edges of this canvas to /// ensure that anti-aliased arcs are not clipped. @@ -93,11 +94,6 @@ class BitmapCanvas extends EngineCanvas { _childOverdraw = value; } - // Maps src url to image element to reuse image instances across frames. - // Optimization for some browsers that keep refreshing images causing - // flicker. - Map> _reusableImages; - /// Allocates a canvas with enough memory to paint a picture within the given /// [bounds]. /// @@ -121,6 +117,11 @@ class BitmapCanvas extends EngineCanvas { _setupInitialTransform(); } + /// Setup cache for reusing DOM elements across frames. + set elementCache(CrossFrameCache cache) { + _elementCache = cache; + } + void _updateRootElementTransform() { // Flutter emits paint operations positioned relative to the parent layer's // coordinate system. However, canvas' coordinate system's origin is always @@ -177,14 +178,7 @@ class BitmapCanvas extends EngineCanvas { _canvasPool.clear(); final int len = _children.length; for (int i = 0; i < len; i++) { - html.Element child = _children[i]; - if (child is html.ImageElement) { - _reusableImages ??= {}; - final String src = child.src; - _reusableImages[src] ??= []..add(child); - } else { - _children[i].remove(); - } + _children[i].remove(); } _children.clear(); _cachedLastStyle = null; @@ -367,15 +361,23 @@ class BitmapCanvas extends EngineCanvas { } html.ImageElement _reuseOrCreateImage(HtmlImage htmlImage) { - if (_reusableImages != null) { - List cachedImages = - _reusableImages[htmlImage.imgElement.src]; - if (cachedImages != null && cachedImages.isNotEmpty) { - return cachedImages.removeAt(0); + final String cacheKey = htmlImage.imgElement.src; + if (_elementCache != null) { + html.ImageElement imageElement = _elementCache.reuse(cacheKey); + if (imageElement != null) { + return imageElement; } } // Can't reuse, create new instance. - return htmlImage.cloneImageElement(); + html.ImageElement newImageElement = htmlImage.cloneImageElement(); + if (_elementCache != null) { + _elementCache.cache(cacheKey, newImageElement, _onEvictElement); + } + return newImageElement; + } + + static void _onEvictElement(html.HtmlElement element) { + element.remove(); } html.HtmlElement _drawImage( @@ -811,14 +813,7 @@ class BitmapCanvas extends EngineCanvas { void endOfPaint() { assert(_saveCount == 0); _canvasPool.endOfPaint(); - if (_reusableImages != null) { - for (List images in _reusableImages.values) { - for (int i = 0, len = images.length; i < len; i++) { - images[i].remove(); - } - } - } - _reusableImages = null; + _elementCache?.commitFrame(); } } diff --git a/lib/web_ui/lib/src/engine/frame_reference.dart b/lib/web_ui/lib/src/engine/frame_reference.dart new file mode 100644 index 0000000000000..9274d67633dce --- /dev/null +++ b/lib/web_ui/lib/src/engine/frame_reference.dart @@ -0,0 +1,102 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// @dart = 2.6 +part of engine; + +/// A monotonically increasing frame number being rendered. +/// +/// Used for debugging only. +int _debugFrameNumber = 1; + +List> _frameReferences = >[]; + +/// A temporary reference to a value of type [V]. +/// +/// The value automatically gets set to null after the current frame is +/// rendered. +/// +/// It is useful to think of this as a weak reference that's scoped to a +/// single frame. +class FrameReference { + /// Creates a frame reference to a value. + FrameReference([this.value]) { + _frameReferences.add(this); + } + + /// The current value of this reference. + V value; +} + +/// Cache where items cached before frame(N) is committed, can be reused in +/// frame(N+1) and are evicted if not. +/// +/// A typical use case is image elements. As images are created and added to +/// DOM when painting a scene they are cached and if possible reused on next +/// update. If the next update does not reuse the element, it is evicted. +/// +/// Maps are lazily allocated since many pictures don't contain cachable images +/// at all. +class CrossFrameCache { + // Cached items in a scene. + Map>> _cache; + + // Cached items that have been committed, ready for reuse on next frame. + Map>> _reusablePool; + + // Called when a scene or picture update is committed. + void commitFrame() { + // Evict unused items from prior frame. + if (_reusablePool != null) { + for (List<_CrossFrameCacheItem> items in _reusablePool.values) { + for (_CrossFrameCacheItem item in items) { + item.evict(); + } + } + } + // Move cached items to reusable pool. + _reusablePool = _cache; + _cache = null; + } + + /// Caches an item for reuse on next update frame. + /// + /// Duplicate keys are allowed. For example the same image url may be used + /// to create multiple instances of [ImageElement] to be reused in the future. + void cache(String key, T value, [CrossFrameCacheEvictCallback callback]) { + _addToCache(key, _CrossFrameCacheItem(value, callback)); + } + + void _addToCache(String key, _CrossFrameCacheItem item) { + _cache ??= {}; + (_cache[key] ??= [])..add(item); + } + + /// Given a key, consumes an item that has been cached in a prior frame. + T reuse(String key) { + if (_reusablePool == null) { + return null; + } + List<_CrossFrameCacheItem> items = _reusablePool[key]; + if (items == null || items.isEmpty) { + return null; + } + _CrossFrameCacheItem item = items.removeAt(0); + _addToCache(key, item); + return item.value; + } +} + +class _CrossFrameCacheItem { + final T value; + final CrossFrameCacheEvictCallback evictCallback; + _CrossFrameCacheItem(this.value, this.evictCallback); + void evict() { + if (evictCallback != null) { + evictCallback(value); + } + } +} + +typedef CrossFrameCacheEvictCallback = void Function(T value); diff --git a/lib/web_ui/lib/src/engine/surface/picture.dart b/lib/web_ui/lib/src/engine/surface/picture.dart index 6e37f57f0c0d7..0dd702f8edc6c 100644 --- a/lib/web_ui/lib/src/engine/surface/picture.dart +++ b/lib/web_ui/lib/src/engine/surface/picture.dart @@ -218,8 +218,6 @@ class PersistedStandardPicture extends PersistedPicture { return bitmapCanvas.bitmapPixelCount; } - FrameReference _didApplyPaint = FrameReference(false); - @override void applyPaint(EngineCanvas oldCanvas) { if (picture.recordingCanvas.hasArbitraryPaint) { @@ -227,7 +225,6 @@ class PersistedStandardPicture extends PersistedPicture { } else { _applyDomPaint(oldCanvas); } - _didApplyPaint.value = true; } void _applyDomPaint(EngineCanvas oldCanvas) { @@ -335,6 +332,7 @@ class PersistedStandardPicture extends PersistedPicture { DebugCanvasReuseOverlay.instance.reusedCount++; } bestRecycledCanvas.bounds = bounds; + bestRecycledCanvas.elementCache = _elementCache; return bestRecycledCanvas; } @@ -342,6 +340,7 @@ class PersistedStandardPicture extends PersistedPicture { DebugCanvasReuseOverlay.instance.createdCount++; } final BitmapCanvas canvas = BitmapCanvas(bounds); + canvas.elementCache = _elementCache; if (_debugExplainSurfaceStats) { _surfaceStatsFor(this) ..allocateBitmapCanvasCount += 1 @@ -371,6 +370,10 @@ abstract class PersistedPicture extends PersistedLeafSurface { final ui.Rect localPaintBounds; final int hints; + /// Cache for reusing elements such as images across picture updates. + CrossFrameCache _elementCache = + CrossFrameCache(); + @override html.Element createElement() { return defaultCreateElement('flt-picture'); @@ -591,6 +594,8 @@ abstract class PersistedPicture extends PersistedLeafSurface { @override void update(PersistedPicture oldSurface) { super.update(oldSurface); + // Transfer element cache over. + _elementCache = oldSurface._elementCache; if (dx != oldSurface.dx || dy != oldSurface.dy) { _applyTranslate(); diff --git a/lib/web_ui/lib/src/engine/surface/surface.dart b/lib/web_ui/lib/src/engine/surface/surface.dart index 28c0134b1594b..963c5b37c0473 100644 --- a/lib/web_ui/lib/src/engine/surface/surface.dart +++ b/lib/web_ui/lib/src/engine/surface/surface.dart @@ -30,30 +30,6 @@ bool debugShowClipLayers = false; /// reasonable. const double _kScreenPixelRatioWarningThreshold = 6.0; -/// A monotonically increasing frame number being rendered. -/// -/// Used for debugging only. -int _debugFrameNumber = 1; - -List> _frameReferences = >[]; - -/// A temporary reference to a value of type [V]. -/// -/// The value automatically gets set to null after the current frame is -/// rendered. -/// -/// It is useful to think of this as a weak reference that's scoped to a -/// single frame. -class FrameReference { - /// Creates a frame reference to a value. - FrameReference([this.value]) { - _frameReferences.add(this); - } - - /// The current value of this reference. - V value; -} - /// Performs any outstanding painting work enqueued by [PersistedPicture]s. void commitScene(PersistedScene scene) { if (_paintQueue.isNotEmpty) { diff --git a/lib/web_ui/test/engine/frame_reference_test.dart b/lib/web_ui/test/engine/frame_reference_test.dart new file mode 100644 index 0000000000000..749639a8fa86c --- /dev/null +++ b/lib/web_ui/test/engine/frame_reference_test.dart @@ -0,0 +1,76 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// @dart = 2.6 +import 'package:ui/src/engine.dart'; +import 'package:test/test.dart'; + +void main() { + group('CrossFrameCache', () { + test('Reuse returns no object when cache empty', () { + final CrossFrameCache cache = CrossFrameCache(); + cache.commitFrame(); + TestItem requestedItem = cache.reuse('item1'); + expect(requestedItem, null); + }); + + test('Reuses object across frames', () { + final CrossFrameCache cache = CrossFrameCache(); + final TestItem testItem1 = TestItem('item1'); + cache.cache(testItem1.label, testItem1); + cache.commitFrame(); + TestItem requestedItem = cache.reuse('item1'); + expect(requestedItem, testItem1); + requestedItem = cache.reuse('item1'); + expect(requestedItem, null); + }); + + test('Reuses objects that have same key across frames', () { + final CrossFrameCache cache = CrossFrameCache(); + final TestItem testItem1 = TestItem('sameLabel'); + final TestItem testItem2 = TestItem('sameLabel'); + final TestItem testItemX = TestItem('X'); + cache.cache(testItem1.label, testItem1); + cache.cache(testItemX.label, testItemX); + cache.cache(testItem2.label, testItem2); + cache.commitFrame(); + TestItem requestedItem = cache.reuse('sameLabel'); + expect(requestedItem, testItem1); + requestedItem = cache.reuse('sameLabel'); + expect(requestedItem, testItem2); + requestedItem = cache.reuse('sameLabel'); + expect(requestedItem, null); + }); + + test('Values don\'t survive beyond next frame', () { + final CrossFrameCache cache = CrossFrameCache(); + final TestItem testItem1 = TestItem('item1'); + cache.cache(testItem1.label, testItem1); + cache.commitFrame(); + cache.commitFrame(); + TestItem requestedItem = cache.reuse('item1'); + expect(requestedItem, null); + }); + + test('Values are evicted when not reused', () { + final Set _evictedItems = {}; + final CrossFrameCache cache = CrossFrameCache(); + final TestItem testItem1 = TestItem('item1'); + final TestItem testItem2 = TestItem('item2'); + cache.cache(testItem1.label, testItem1, (TestItem item) {_evictedItems.add(item);}); + cache.cache(testItem2.label, testItem2, (TestItem item) {_evictedItems.add(item);}); + cache.commitFrame(); + expect(_evictedItems.length, 0); + TestItem requestedItem = cache.reuse('item2'); + cache.commitFrame(); + expect(_evictedItems.contains(testItem1), true); + expect(_evictedItems.contains(testItem2), false); + }); + }); +} + +class TestItem { + final String label; + TestItem(this.label); +} \ No newline at end of file From 9406125d2f72a450e3810c84000164364c27e97b Mon Sep 17 00:00:00 2001 From: ferhatb Date: Wed, 20 May 2020 10:09:12 -0700 Subject: [PATCH 3/5] Add newline at end of test --- lib/web_ui/test/engine/frame_reference_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/web_ui/test/engine/frame_reference_test.dart b/lib/web_ui/test/engine/frame_reference_test.dart index 749639a8fa86c..507e57bb2c483 100644 --- a/lib/web_ui/test/engine/frame_reference_test.dart +++ b/lib/web_ui/test/engine/frame_reference_test.dart @@ -73,4 +73,4 @@ void main() { class TestItem { final String label; TestItem(this.label); -} \ No newline at end of file +} From beebb44ba3b6ead7a531eb7a4aa25ded157a10ac Mon Sep 17 00:00:00 2001 From: ferhatb Date: Wed, 20 May 2020 11:07:00 -0700 Subject: [PATCH 4/5] Update licenses_flutter --- ci/licenses_golden/licenses_flutter | 1 + 1 file changed, 1 insertion(+) diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index a9daedd5960ed..fb970c8716052 100755 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -438,6 +438,7 @@ FILE: ../../../flutter/lib/web_ui/lib/src/engine/conic.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/dom_canvas.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/dom_renderer.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/engine_canvas.dart +FILE: ../../../flutter/lib/web_ui/lib/src/engine/frame_reference.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/history.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/houdini_canvas.dart FILE: ../../../flutter/lib/web_ui/lib/src/engine/html_image_codec.dart From 8b37c34eb2aa5c350617d779d4f981dfdb133c66 Mon Sep 17 00:00:00 2001 From: ferhatb Date: Wed, 20 May 2020 11:40:01 -0700 Subject: [PATCH 5/5] remove unused local var --- lib/web_ui/test/engine/frame_reference_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/web_ui/test/engine/frame_reference_test.dart b/lib/web_ui/test/engine/frame_reference_test.dart index 507e57bb2c483..6ccd91c57b207 100644 --- a/lib/web_ui/test/engine/frame_reference_test.dart +++ b/lib/web_ui/test/engine/frame_reference_test.dart @@ -62,7 +62,7 @@ void main() { cache.cache(testItem2.label, testItem2, (TestItem item) {_evictedItems.add(item);}); cache.commitFrame(); expect(_evictedItems.length, 0); - TestItem requestedItem = cache.reuse('item2'); + cache.reuse('item2'); cache.commitFrame(); expect(_evictedItems.contains(testItem1), true); expect(_evictedItems.contains(testItem2), false);