[web] Reuse ImageElement(s) across frames #18437
Conversation
| // Maps src url to image element to reuse image instances across frames. | ||
| // Optimization for some browsers that keep refreshing images causing | ||
| // flicker. | ||
| Map<String, List<html.ImageElement>> _reusableImages; |
There was a problem hiding this comment.
Can we make this global and clean up in commitScene instead of endOfPaint? This way images can be reused across canvases (we frequently create a new canvas because of a failed match, but it will contain the same image from the previous frame), we will allocate fewer maps and lists, and we can remove several null checks (we can keep the global map always non-null).
There was a problem hiding this comment.
Moved cache to picture level, so it survives across picture updates.
| @@ -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) { | |||
There was a problem hiding this comment.
If the image is clipped it won't be a direct child of the root element but a descendant of a clipping element.
| if (child is html.ImageElement) { | ||
| _reusableImages ??= {}; | ||
| final String src = child.src; | ||
| _reusableImages[src] ??= []..add(child); |
There was a problem hiding this comment.
Instead of adding the image during deconstruction of the canvas, I would add the image at the time it is painted. This way we don't have to read back from the DOM, and we can still reuse the image even if the canvas is not reused but a new one is painted.
| class TestItem { | ||
| final String label; | ||
| TestItem(this.label); | ||
| } |
There was a problem hiding this comment.
nit: newline at end of file
| @@ -116,6 +117,11 @@ class BitmapCanvas extends EngineCanvas { | |||
| _setupInitialTransform(); | |||
| } | |||
|
|
|||
| /// Setup cache for reusing DOM elements across frames. | |||
| set elementCache(CrossFrameCache<html.HtmlElement> cache) { | |||
There was a problem hiding this comment.
| /// A monotonically increasing frame number being rendered. | ||
| /// | ||
| /// Used for debugging only. | ||
| int _debugFrameNumber = 1; |
| (_cache[key] ??= [])..add(item); | ||
| } | ||
|
|
||
| /// Given a key, consumes an item that has been cached in a prior frame. |
There was a problem hiding this comment.
This should also talk about what it returns, and in particular when does it return null.
| Map<String, List<_CrossFrameCacheItem<T>>> _cache; | ||
|
|
||
| // Cached items that have been committed, ready for reuse on next frame. | ||
| Map<String, List<_CrossFrameCacheItem<T>>> _reusablePool; |
There was a problem hiding this comment.
I would call this field _cache, and I would rename the above _cache to _itemUsedThisFrame. After all, we want to cache items across frames, but confusingly the current _cache variable travels to the next frame as null.
* reuse images across frames * Change implementation to CrossFrameCache at picture level * Update licenses_flutter
Related issue: flutter/flutter#48373