From cbf8134faf5deef9e2b1f2eaab62655ef75a7f57 Mon Sep 17 00:00:00 2001 From: ferhatb Date: Fri, 23 Oct 2020 12:40:23 -0700 Subject: [PATCH 01/10] Protect against canvas constructor NNBD optimizing out reduceCanvasMemoryUsage --- lib/web_ui/lib/src/engine/canvas_pool.dart | 37 +++++++++++++++------ lib/web_ui/lib/src/engine/html/surface.dart | 22 ++++++++++++ 2 files changed, 48 insertions(+), 11 deletions(-) diff --git a/lib/web_ui/lib/src/engine/canvas_pool.dart b/lib/web_ui/lib/src/engine/canvas_pool.dart index d39aee1171e91..a8c56bbfe4184 100644 --- a/lib/web_ui/lib/src/engine/canvas_pool.dart +++ b/lib/web_ui/lib/src/engine/canvas_pool.dart @@ -83,7 +83,7 @@ class _CanvasPool extends _SaveStackTracking { void _createCanvas() { bool requiresClearRect = false; bool reused = false; - html.CanvasElement canvas; + html.CanvasElement? canvas; if (_reusablePool != null && _reusablePool!.isNotEmpty) { canvas = _canvas = _reusablePool!.removeAt(0); requiresClearRect = true; @@ -99,10 +99,7 @@ class _CanvasPool extends _SaveStackTracking { _widthInBitmapPixels / EnginePlatformDispatcher.browserDevicePixelRatio; final double cssHeight = _heightInBitmapPixels / EnginePlatformDispatcher.browserDevicePixelRatio; - canvas = html.CanvasElement( - width: _widthInBitmapPixels, - height: _heightInBitmapPixels, - ); + canvas = _allocCanvas(_widthInBitmapPixels, _heightInBitmapPixels); _canvas = canvas; // Why is this null check here, even though we just allocated a canvas element above? @@ -113,12 +110,9 @@ class _CanvasPool extends _SaveStackTracking { if (_canvas == null) { // Evict BitmapCanvas(s) and retry. _reduceCanvasMemoryUsage(); - canvas = html.CanvasElement( - width: _widthInBitmapPixels, - height: _heightInBitmapPixels, - ); + canvas = _allocCanvas(_widthInBitmapPixels, _heightInBitmapPixels); } - canvas.style + canvas!.style ..position = 'absolute' ..width = '${cssWidth}px' ..height = '${cssHeight}px'; @@ -128,7 +122,7 @@ class _CanvasPool extends _SaveStackTracking { // optimization prevents DOM .append call when a PersistentSurface is // reused. Reading lastChild is faster than append call. if (_rootElement!.lastChild != canvas) { - _rootElement!.append(canvas); + _rootElement!.append(canvas!); } if (reused) { @@ -144,6 +138,27 @@ class _CanvasPool extends _SaveStackTracking { _replayClipStack(); } + html.CanvasElement? _allocCanvas(int width, int height) { + final dynamic canvas = + js_util.callMethod(html.document, 'createElement', ['CANVAS']); + if (canvas != null) { + try { + canvas.width = width; + canvas.height = height; + } catch (e) { + return null; + } + return canvas as html.CanvasElement; + } + return null; + // !!! We don't use the code below since NNBD assumes it can never return + // null and optimizes out code. + // return canvas = html.CanvasElement( + // width: _widthInBitmapPixels, + // height: _heightInBitmapPixels, + // ); + } + @override void clear() { super.clear(); diff --git a/lib/web_ui/lib/src/engine/html/surface.dart b/lib/web_ui/lib/src/engine/html/surface.dart index 68efb10a92556..e3d9cd105876b 100644 --- a/lib/web_ui/lib/src/engine/html/surface.dart +++ b/lib/web_ui/lib/src/engine/html/surface.dart @@ -356,6 +356,7 @@ abstract class PersistedSurface implements ui.EngineLayer { assert(rootElement == null); assert(debugAssertSurfaceState(this, PersistedSurfaceState.created)); rootElement = createElement(); + assert(rootElement != null); applyWebkitClipFix(rootElement); if (_debugExplainSurfaceStats) { _surfaceStatsFor(this).allocatedDomNodeCount++; @@ -650,6 +651,7 @@ abstract class PersistedContainerSurface extends PersistedSurface { @override void build() { + log.add('build'); super.build(); // Memoize length for efficiency. final int len = _children.length; @@ -815,6 +817,13 @@ abstract class PersistedContainerSurface extends PersistedSurface { // Move the HTML node if necessary. if (newChild.rootElement!.parent != childContainer) { + if (childContainer == null) { + html.Element? elm = childContainer; + print(elm); + } + if (newChild.rootElement == null) { + print(newChild); + } childContainer!.append(newChild.rootElement!); } @@ -838,6 +847,14 @@ abstract class PersistedContainerSurface extends PersistedSurface { // Move the HTML node if necessary. if (oldLayer.rootElement!.parent != childContainer) { + assert(oldLayer != null); + if (childContainer == null) { + html.Element? elm = childContainer; + print(elm); + } + assert(childContainer != null); + + childContainer!.append(oldLayer.rootElement!); } @@ -1121,8 +1138,11 @@ abstract class PersistedContainerSurface extends PersistedSurface { return result; } + List log = []; + @override void retain() { + log.add('retain'); super.retain(); final int len = _children.length; for (int i = 0; i < len; i++) { @@ -1132,6 +1152,7 @@ abstract class PersistedContainerSurface extends PersistedSurface { @override void revive() { + log.add('revive'); super.revive(); final int len = _children.length; for (int i = 0; i < len; i++) { @@ -1141,6 +1162,7 @@ abstract class PersistedContainerSurface extends PersistedSurface { @override void discard() { + log.add('discard'); super.discard(); _discardActiveChildren(this); } From c0583640aca98fcd1bf55bf42a66d4e739141d01 Mon Sep 17 00:00:00 2001 From: ferhatb Date: Fri, 23 Oct 2020 16:06:15 -0700 Subject: [PATCH 02/10] Reduce canvas memory usage when allocation fails --- lib/web_ui/lib/src/engine/canvas_pool.dart | 31 +++++++++++++++------ lib/web_ui/lib/src/engine/html/surface.dart | 29 ++++++++++--------- 2 files changed, 39 insertions(+), 21 deletions(-) diff --git a/lib/web_ui/lib/src/engine/canvas_pool.dart b/lib/web_ui/lib/src/engine/canvas_pool.dart index a8c56bbfe4184..56e8d2fcebb34 100644 --- a/lib/web_ui/lib/src/engine/canvas_pool.dart +++ b/lib/web_ui/lib/src/engine/canvas_pool.dart @@ -125,15 +125,30 @@ class _CanvasPool extends _SaveStackTracking { _rootElement!.append(canvas!); } - if (reused) { - // If a canvas is the first element we set z-index = -1 in [BitmapCanvas] - // endOfPaint to workaround blink compositing bug. To make sure this - // does not leak when reused reset z-index. - canvas.style.removeProperty('z-index'); + try { + if (reused) { + // If a canvas is the first element we set z-index = -1 in [BitmapCanvas] + // endOfPaint to workaround blink compositing bug. To make sure this + // does not leak when reused reset z-index. + canvas.style.removeProperty('z-index'); + } + _context = canvas.context2D; + } catch (e) { + // Handle OOM. } - - final html.CanvasRenderingContext2D context = _context = canvas.context2D; - _contextHandle = ContextStateHandle(this, context); + if (_context == null) { + _reduceCanvasMemoryUsage(); + _context = canvas.context2D; + } + if (_context == null) { + /// Browser ran out of memory, try to recover current allocation + /// and bail. + _canvas?.width = 0; + _canvas?.height = 0; + _canvas = null; + return; + } + _contextHandle = ContextStateHandle(this, _context!); _initializeViewport(requiresClearRect); _replayClipStack(); } diff --git a/lib/web_ui/lib/src/engine/html/surface.dart b/lib/web_ui/lib/src/engine/html/surface.dart index e3d9cd105876b..b2dfc65626183 100644 --- a/lib/web_ui/lib/src/engine/html/surface.dart +++ b/lib/web_ui/lib/src/engine/html/surface.dart @@ -33,21 +33,24 @@ const double _kScreenPixelRatioWarningThreshold = 6.0; /// Performs any outstanding painting work enqueued by [PersistedPicture]s. void commitScene(PersistedScene scene) { if (_paintQueue.isNotEmpty) { - if (_paintQueue.length > 1) { - // Sort paint requests in decreasing canvas size order. Paint requests - // attempt to reuse canvases. For efficiency we want the biggest pictures - // to find canvases before the smaller ones claim them. - _paintQueue.sort((_PaintRequest a, _PaintRequest b) { - final double aSize = a.canvasSize.height * a.canvasSize.width; - final double bSize = b.canvasSize.height * b.canvasSize.width; - return bSize.compareTo(aSize); - }); - } + try { + if (_paintQueue.length > 1) { + // Sort paint requests in decreasing canvas size order. Paint requests + // attempt to reuse canvases. For efficiency we want the biggest pictures + // to find canvases before the smaller ones claim them. + _paintQueue.sort((_PaintRequest a, _PaintRequest b) { + final double aSize = a.canvasSize.height * a.canvasSize.width; + final double bSize = b.canvasSize.height * b.canvasSize.width; + return bSize.compareTo(aSize); + }); + } - for (_PaintRequest request in _paintQueue) { - request.paintCallback(); + for (_PaintRequest request in _paintQueue) { + request.paintCallback(); + } + } finally { + _paintQueue = <_PaintRequest>[]; } - _paintQueue = <_PaintRequest>[]; } // After the update the retained surfaces are back to active. From 01e48436cb0a2d4ccc1735091ba6a26a224ebdec Mon Sep 17 00:00:00 2001 From: ferhatb Date: Tue, 27 Oct 2020 20:09:47 -0700 Subject: [PATCH 03/10] Implement canvas density based on transform stack --- lib/web_ui/lib/src/engine/bitmap_canvas.dart | 14 +++- lib/web_ui/lib/src/engine/canvas_pool.dart | 37 ++++++--- lib/web_ui/lib/src/engine/html/picture.dart | 80 +++++++++++++++++-- .../lib/src/engine/html/shaders/shader.dart | 10 +-- .../engine/surface/scene_builder_test.dart | 35 +++++++- 5 files changed, 148 insertions(+), 28 deletions(-) diff --git a/lib/web_ui/lib/src/engine/bitmap_canvas.dart b/lib/web_ui/lib/src/engine/bitmap_canvas.dart index 719737c8aa692..b67576f8a4023 100644 --- a/lib/web_ui/lib/src/engine/bitmap_canvas.dart +++ b/lib/web_ui/lib/src/engine/bitmap_canvas.dart @@ -104,18 +104,23 @@ class BitmapCanvas extends EngineCanvas { /// can be constructed from contents. bool _preserveImageData = false; + /// Canvas pixel to screen pixel ratio. Similar to dpi but + /// uses global transform of canvas to compute ratio. + final double _density; + /// Allocates a canvas with enough memory to paint a picture within the given /// [bounds]. /// /// This canvas can be reused by pictures with different paint bounds as long /// as the [Rect.size] of the bounds fully fit within the size used to /// initialize this canvas. - BitmapCanvas(this._bounds) + BitmapCanvas(this._bounds, {double density = 1.0}) : assert(_bounds != null), // ignore: unnecessary_null_comparison + _density = density, _widthInBitmapPixels = _widthToPhysical(_bounds.width), _heightInBitmapPixels = _heightToPhysical(_bounds.height), _canvasPool = _CanvasPool(_widthToPhysical(_bounds.width), - _heightToPhysical(_bounds.height)) { + _heightToPhysical(_bounds.height), density) { rootElement.style.position = 'absolute'; // Adds one extra pixel to the requested size. This is to compensate for // _initializeViewport() snapping canvas position to 1 pixel, causing @@ -179,10 +184,11 @@ class BitmapCanvas extends EngineCanvas { } // Used by picture to assess if canvas is large enough to reuse as is. - bool doesFitBounds(ui.Rect newBounds) { + bool doesFitBounds(ui.Rect newBounds, double newDensity) { assert(newBounds != null); // ignore: unnecessary_null_comparison return _widthInBitmapPixels >= _widthToPhysical(newBounds.width) && - _heightInBitmapPixels >= _heightToPhysical(newBounds.height); + _heightInBitmapPixels >= _heightToPhysical(newBounds.height) && + _density == newDensity; } @override diff --git a/lib/web_ui/lib/src/engine/canvas_pool.dart b/lib/web_ui/lib/src/engine/canvas_pool.dart index 56e8d2fcebb34..efe49dfe48925 100644 --- a/lib/web_ui/lib/src/engine/canvas_pool.dart +++ b/lib/web_ui/lib/src/engine/canvas_pool.dart @@ -33,8 +33,10 @@ class _CanvasPool extends _SaveStackTracking { html.HtmlElement? _rootElement; int _saveContextCount = 0; + final double _density; - _CanvasPool(this._widthInBitmapPixels, this._heightInBitmapPixels); + _CanvasPool(this._widthInBitmapPixels, this._heightInBitmapPixels, + this._density); html.CanvasRenderingContext2D get context { html.CanvasRenderingContext2D? ctx = _context; @@ -84,6 +86,11 @@ class _CanvasPool extends _SaveStackTracking { bool requiresClearRect = false; bool reused = false; html.CanvasElement? canvas; + if (_canvas != null) { + _canvas!.width = 0; + _canvas!.height = 0; + _canvas = null; + } if (_reusablePool != null && _reusablePool!.isNotEmpty) { canvas = _canvas = _reusablePool!.removeAt(0); requiresClearRect = true; @@ -122,7 +129,7 @@ class _CanvasPool extends _SaveStackTracking { // optimization prevents DOM .append call when a PersistentSurface is // reused. Reading lastChild is faster than append call. if (_rootElement!.lastChild != canvas) { - _rootElement!.append(canvas!); + _rootElement!.append(canvas); } try { @@ -148,7 +155,7 @@ class _CanvasPool extends _SaveStackTracking { _canvas = null; return; } - _contextHandle = ContextStateHandle(this, _context!); + _contextHandle = ContextStateHandle(this, _context!, this._density); _initializeViewport(requiresClearRect); _replayClipStack(); } @@ -158,8 +165,8 @@ class _CanvasPool extends _SaveStackTracking { js_util.callMethod(html.document, 'createElement', ['CANVAS']); if (canvas != null) { try { - canvas.width = width; - canvas.height = height; + canvas.width = (width * _density).ceil(); + canvas.height = (height * _density).ceil(); } catch (e) { return null; } @@ -218,7 +225,7 @@ class _CanvasPool extends _SaveStackTracking { clipTimeTransform[5] != prevTransform[5] || clipTimeTransform[12] != prevTransform[12] || clipTimeTransform[13] != prevTransform[13]) { - final double ratio = EnginePlatformDispatcher.browserDevicePixelRatio; + final double ratio = dpi; ctx.setTransform(ratio, 0, 0, ratio, 0, 0); ctx.transform( clipTimeTransform[0], @@ -252,7 +259,7 @@ class _CanvasPool extends _SaveStackTracking { transform[5] != prevTransform[5] || transform[12] != prevTransform[12] || transform[13] != prevTransform[13]) { - final double ratio = EnginePlatformDispatcher.browserDevicePixelRatio; + final double ratio = dpi; ctx.setTransform(ratio, 0, 0, ratio, 0, 0); ctx.transform(transform[0], transform[1], transform[4], transform[5], transform[12], transform[13]); @@ -330,15 +337,19 @@ class _CanvasPool extends _SaveStackTracking { // is applied on the DOM elements. ctx.setTransform(1, 0, 0, 1, 0, 0); if (clearCanvas) { - ctx.clearRect(0, 0, _widthInBitmapPixels, _heightInBitmapPixels); + ctx.clearRect(0, 0, _widthInBitmapPixels * _density, + _heightInBitmapPixels * _density); } // This scale makes sure that 1 CSS pixel is translated to the correct // number of bitmap pixels. - ctx.scale(EnginePlatformDispatcher.browserDevicePixelRatio, - EnginePlatformDispatcher.browserDevicePixelRatio); + ctx.scale(dpi, dpi); } + /// Returns effective dpi (browser DPI and pixel density due to transform). + double get dpi => + EnginePlatformDispatcher.browserDevicePixelRatio * _density; + void resetTransform() { final html.CanvasElement? canvas = _canvas; if (canvas != null) { @@ -718,8 +729,9 @@ class _CanvasPool extends _SaveStackTracking { class ContextStateHandle { final html.CanvasRenderingContext2D context; final _CanvasPool _canvasPool; + final double density; - ContextStateHandle(this._canvasPool, this.context); + ContextStateHandle(this._canvasPool, this.context, this.density); ui.BlendMode? _currentBlendMode = ui.BlendMode.srcOver; ui.StrokeCap? _currentStrokeCap = ui.StrokeCap.butt; ui.StrokeJoin? _currentStrokeJoin = ui.StrokeJoin.miter; @@ -808,7 +820,8 @@ class ContextStateHandle { if (paint.shader != null) { final EngineGradient engineShader = paint.shader as EngineGradient; final Object paintStyle = - engineShader.createPaintStyle(_canvasPool.context, shaderBounds); + engineShader.createPaintStyle(_canvasPool.context, shaderBounds, + density); fillStyle = paintStyle; strokeStyle = paintStyle; } else if (paint.color != null) { diff --git a/lib/web_ui/lib/src/engine/html/picture.dart b/lib/web_ui/lib/src/engine/html/picture.dart index f81395f7e4a78..cdda1ca8e11a8 100644 --- a/lib/web_ui/lib/src/engine/html/picture.dart +++ b/lib/web_ui/lib/src/engine/html/picture.dart @@ -90,6 +90,7 @@ class PersistedPicture extends PersistedLeafSurface { final EnginePicture picture; final ui.Rect? localPaintBounds; final int hints; + double _density = 1.0; /// Cache for reusing elements such as images across picture updates. CrossFrameCache? _elementCache = @@ -107,6 +108,23 @@ class PersistedPicture extends PersistedLeafSurface { _transform = _transform!.clone(); _transform!.translate(dx, dy); } + final double paintWidth = localPaintBounds!.width; + final double paintHeight = localPaintBounds!.height; + final double newDensity = localPaintBounds == null || paintWidth == 0 || paintHeight == 0 + ? 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); + } + } _computeExactCullRects(); } @@ -296,7 +314,12 @@ class PersistedPicture extends PersistedLeafSurface { // painting. This removes all the setup work and scaffolding objects // that won't be useful for anything anyway. _recycleCanvas(oldCanvas); - domRenderer.clearDom(rootElement!); + if (rootElement != null) { + domRenderer.clearDom(rootElement!); + } + if (_canvas != null) { + _recycleCanvas(_canvas); + } _canvas = null; return; } @@ -339,7 +362,7 @@ class PersistedPicture extends PersistedLeafSurface { // We did not allocate a canvas last time. This can happen when the // picture is completely clipped out of the view. return 1.0; - } else if (!oldCanvas.doesFitBounds(_exactLocalCullRect!)) { + } else if (!oldCanvas.doesFitBounds(_exactLocalCullRect!, _density)) { // The canvas needs to be resized before painting. return 1.0; } else { @@ -382,7 +405,7 @@ class PersistedPicture extends PersistedLeafSurface { void _applyBitmapPaint(EngineCanvas? oldCanvas) { if (oldCanvas is BitmapCanvas && - oldCanvas.doesFitBounds(_optimalLocalCullRect!) && + oldCanvas.doesFitBounds(_optimalLocalCullRect!, _density) && oldCanvas.isReusable()) { if (_debugShowCanvasReuseStats) { DebugCanvasReuseOverlay.instance.keptCount++; @@ -451,7 +474,7 @@ class PersistedPicture extends PersistedLeafSurface { final double candidatePixelCount = candidateSize.width * candidateSize.height; - final bool fits = candidate.doesFitBounds(bounds); + final bool fits = candidate.doesFitBounds(bounds, _density); final bool isSmaller = candidatePixelCount < lastPixelCount; if (fits && isSmaller) { // [isTooSmall] is used to make sure that a small picture doesn't @@ -493,7 +516,7 @@ class PersistedPicture extends PersistedLeafSurface { if (_debugShowCanvasReuseStats) { DebugCanvasReuseOverlay.instance.createdCount++; } - final BitmapCanvas canvas = BitmapCanvas(bounds); + final BitmapCanvas canvas = BitmapCanvas(bounds, density: _density); canvas.setElementCache(_elementCache); if (_debugExplainSurfaceStats) { _surfaceStatsFor(this) @@ -536,8 +559,12 @@ class PersistedPicture extends PersistedLeafSurface { final bool cullRectChangeRequiresRepaint = _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) { + if (cullRectChangeRequiresRepaint || densityChanged) { // Cull rect changed such that a repaint is still necessary. _applyPaint(oldSurface); } else { @@ -603,3 +630,44 @@ class PersistedPicture extends PersistedLeafSurface { } } } + +/// Given size of a rectangle and transform, computes pixel density +/// (scale factor). +double _computePixelDensity(Matrix4? transform, double width, double height) { + if (transform == null || transform.isIdentity()) { + return 1.0; + } + final Float32List m = transform.storage; + // Apply perspective transform. + final double topLeftX = m[12] * m[15]; + final double topLeftY = m[13] * m[15]; + double x = width; + double y = height; + final double xp = (m[0] * x) + (m[4] * y) + m[12]; + final double yp = (m[1] * x) + (m[5] * y) + m[13]; + final double wp = 1.0 / ((m[3] * x) + (m[7] * y) + m[15]); + double scaleX = ((xp * wp) - topLeftX).abs() / width; + double scaleY = ((yp * wp) - topLeftY).abs() / height; + double scale = math.min(scaleX, scaleY); + if (scale < kEpsilon || scale == 1) { + // Handle local paint bounds scaled to 0, typical when using + // transform animations. + return 1.0; + } + if (scale > 1) { + // Normalize scale to multiples of 2: 1x, 2x, 4x, 6x, 8x. + // This is to prevent frequent rescaling of canvas during animations. + // + // On a fullscreen high dpi device dpi*density*resolution will demand + // too much memory, so clamp at 4. + scale = math.min(4.0, ((scale / 2.0).ceil() * 2.0)); + } else { + scale = math.max(2.0 / (2.0 / scale).floor(), 0.0001); + } + // Guard against webkit absolute limit. + const double kPixelLimit = 1024 * 1024 * 4; + if ((width * height * scale * scale) > kPixelLimit) { + scale = (kPixelLimit * 0.8) / (width * height); + } + return scale; +} diff --git a/lib/web_ui/lib/src/engine/html/shaders/shader.dart b/lib/web_ui/lib/src/engine/html/shaders/shader.dart index 6ca1b812deed4..380dbfec8c9e1 100644 --- a/lib/web_ui/lib/src/engine/html/shaders/shader.dart +++ b/lib/web_ui/lib/src/engine/html/shaders/shader.dart @@ -11,7 +11,7 @@ abstract class EngineGradient implements ui.Gradient { /// Creates a fill style to be used in painting. Object createPaintStyle(html.CanvasRenderingContext2D? ctx, - ui.Rect? shaderBounds); + ui.Rect? shaderBounds, double density); } class GradientSweep extends EngineGradient { @@ -29,7 +29,7 @@ class GradientSweep extends EngineGradient { @override Object createPaintStyle(html.CanvasRenderingContext2D? ctx, - ui.Rect? shaderBounds) { + ui.Rect? shaderBounds, double density) { assert(shaderBounds != null); int widthInPixels = shaderBounds!.right.ceil(); int heightInPixels = shaderBounds.bottom.ceil(); @@ -167,7 +167,7 @@ class GradientLinear extends EngineGradient { @override html.CanvasGradient createPaintStyle(html.CanvasRenderingContext2D? ctx, - ui.Rect? shaderBounds) { + ui.Rect? shaderBounds, double density) { _FastMatrix64? matrix4 = this.matrix4; html.CanvasGradient gradient; if (matrix4 != null) { @@ -215,7 +215,7 @@ class GradientRadial extends EngineGradient { @override Object createPaintStyle(html.CanvasRenderingContext2D? ctx, - ui.Rect? shaderBounds) { + ui.Rect? shaderBounds, double density) { if (!useCanvasKit) { if (tileMode != ui.TileMode.clamp) { throw UnimplementedError( @@ -255,7 +255,7 @@ class GradientConical extends EngineGradient { @override Object createPaintStyle(html.CanvasRenderingContext2D? ctx, - ui.Rect? shaderBounds) { + ui.Rect? shaderBounds, double density) { throw UnimplementedError(); } } diff --git a/lib/web_ui/test/engine/surface/scene_builder_test.dart b/lib/web_ui/test/engine/surface/scene_builder_test.dart index fca7829288da2..6e330a10de1e7 100644 --- a/lib/web_ui/test/engine/surface/scene_builder_test.dart +++ b/lib/web_ui/test/engine/surface/scene_builder_test.dart @@ -27,7 +27,7 @@ void testMain() { await webOnlyInitializeEngine(); }); - group('SceneBuilder', () { + group('SceneBuilder', () { test('pushOffset implements surface lifecycle', () { testLayerLifeCycle((SceneBuilder sceneBuilder, EngineLayer oldLayer) { return sceneBuilder.pushOffset(10, 20, oldLayer: oldLayer); @@ -525,6 +525,39 @@ void testMain() { await testCase('be', 'remove in the middle', deletions: 2); await testCase('', 'remove all', deletions: 2); }); + + test('Canvas should allocate fewer pixels if scaled', () async { + final SurfaceSceneBuilder builder = SurfaceSceneBuilder(); + final Picture picture1 = _drawPicture(); + EngineLayer oldLayer = builder.pushClipRect( + const Rect.fromLTRB(10, 10, 300, 300), + ); + builder.addPicture(Offset.zero, picture1); + builder.pop(); + + html.HtmlElement content = builder.build().webOnlyRootElement; + html.CanvasElement canvas = content.querySelector('canvas'); + final int unscaledWidth = canvas.width; + final int unscaledHeight = canvas.height; + + // Force update to scene which will utilize reuse code path. + final SurfaceSceneBuilder builder2 = SurfaceSceneBuilder(); + builder2.pushOffset(0, 0); + builder2.pushTransform(Matrix4.identity().scaled(0.5, 0.5).toFloat64()); + builder2.pushClipRect( + const Rect.fromLTRB(10, 10, 300, 300), + ); + builder2.addPicture(Offset.zero, picture1); + builder2.pop(); + builder2.pop(); + builder2.pop(); + + html.HtmlElement contentAfterScale = builder2.build().webOnlyRootElement; + html.CanvasElement canvas2 = contentAfterScale.querySelector('canvas'); + // Although we are drawing same picture, due to scaling the new canvas + // should have fewer pixels. + expect(canvas2.width < unscaledWidth, true); + }); } typedef TestLayerBuilder = EngineLayer Function( From 3a0f7565a83dfeaa611f42ed560689ab960a87d4 Mon Sep 17 00:00:00 2001 From: ferhatb Date: Wed, 28 Oct 2020 10:05:45 -0700 Subject: [PATCH 04/10] remove logging code, fix analyzer warnings --- lib/web_ui/lib/src/engine/html/surface.dart | 14 -------------- .../test/engine/surface/scene_builder_test.dart | 5 ++--- 2 files changed, 2 insertions(+), 17 deletions(-) diff --git a/lib/web_ui/lib/src/engine/html/surface.dart b/lib/web_ui/lib/src/engine/html/surface.dart index b2dfc65626183..11c3d2ce0e2e3 100644 --- a/lib/web_ui/lib/src/engine/html/surface.dart +++ b/lib/web_ui/lib/src/engine/html/surface.dart @@ -654,7 +654,6 @@ abstract class PersistedContainerSurface extends PersistedSurface { @override void build() { - log.add('build'); super.build(); // Memoize length for efficiency. final int len = _children.length; @@ -850,14 +849,6 @@ abstract class PersistedContainerSurface extends PersistedSurface { // Move the HTML node if necessary. if (oldLayer.rootElement!.parent != childContainer) { - assert(oldLayer != null); - if (childContainer == null) { - html.Element? elm = childContainer; - print(elm); - } - assert(childContainer != null); - - childContainer!.append(oldLayer.rootElement!); } @@ -1141,11 +1132,8 @@ abstract class PersistedContainerSurface extends PersistedSurface { return result; } - List log = []; - @override void retain() { - log.add('retain'); super.retain(); final int len = _children.length; for (int i = 0; i < len; i++) { @@ -1155,7 +1143,6 @@ abstract class PersistedContainerSurface extends PersistedSurface { @override void revive() { - log.add('revive'); super.revive(); final int len = _children.length; for (int i = 0; i < len; i++) { @@ -1165,7 +1152,6 @@ abstract class PersistedContainerSurface extends PersistedSurface { @override void discard() { - log.add('discard'); super.discard(); _discardActiveChildren(this); } diff --git a/lib/web_ui/test/engine/surface/scene_builder_test.dart b/lib/web_ui/test/engine/surface/scene_builder_test.dart index 6e330a10de1e7..94f63e27ed5d8 100644 --- a/lib/web_ui/test/engine/surface/scene_builder_test.dart +++ b/lib/web_ui/test/engine/surface/scene_builder_test.dart @@ -529,9 +529,7 @@ void testMain() { test('Canvas should allocate fewer pixels if scaled', () async { final SurfaceSceneBuilder builder = SurfaceSceneBuilder(); final Picture picture1 = _drawPicture(); - EngineLayer oldLayer = builder.pushClipRect( - const Rect.fromLTRB(10, 10, 300, 300), - ); + builder.pushClipRect(const Rect.fromLTRB(10, 10, 300, 300),); builder.addPicture(Offset.zero, picture1); builder.pop(); @@ -557,6 +555,7 @@ void testMain() { // Although we are drawing same picture, due to scaling the new canvas // should have fewer pixels. expect(canvas2.width < unscaledWidth, true); + expect(canvas2.height < unscaledHeight, true); }); } From 8d9fe571d7fc5b01a38fc696fdd0057c94f339b2 Mon Sep 17 00:00:00 2001 From: ferhatb Date: Thu, 29 Oct 2020 12:58:49 -0700 Subject: [PATCH 05/10] Address review comments --- lib/web_ui/lib/src/engine/html/picture.dart | 3 +- lib/web_ui/lib/src/engine/html/surface.dart | 7 ---- .../engine/surface/scene_builder_test.dart | 38 +++++++++++++++++-- 3 files changed, 37 insertions(+), 11 deletions(-) diff --git a/lib/web_ui/lib/src/engine/html/picture.dart b/lib/web_ui/lib/src/engine/html/picture.dart index cdda1ca8e11a8..b8c68b495bdaf 100644 --- a/lib/web_ui/lib/src/engine/html/picture.dart +++ b/lib/web_ui/lib/src/engine/html/picture.dart @@ -649,9 +649,10 @@ double _computePixelDensity(Matrix4? transform, double width, double height) { double scaleX = ((xp * wp) - topLeftX).abs() / width; double scaleY = ((yp * wp) - topLeftY).abs() / height; double scale = math.min(scaleX, scaleY); + // kEpsilon guards against divide by zero below. if (scale < kEpsilon || scale == 1) { // Handle local paint bounds scaled to 0, typical when using - // transform animations. + // transform animations and nothing is drawn. return 1.0; } if (scale > 1) { diff --git a/lib/web_ui/lib/src/engine/html/surface.dart b/lib/web_ui/lib/src/engine/html/surface.dart index 11c3d2ce0e2e3..546e69384745f 100644 --- a/lib/web_ui/lib/src/engine/html/surface.dart +++ b/lib/web_ui/lib/src/engine/html/surface.dart @@ -819,13 +819,6 @@ abstract class PersistedContainerSurface extends PersistedSurface { // Move the HTML node if necessary. if (newChild.rootElement!.parent != childContainer) { - if (childContainer == null) { - html.Element? elm = childContainer; - print(elm); - } - if (newChild.rootElement == null) { - print(newChild); - } childContainer!.append(newChild.rootElement!); } diff --git a/lib/web_ui/test/engine/surface/scene_builder_test.dart b/lib/web_ui/test/engine/surface/scene_builder_test.dart index 94f63e27ed5d8..c85881123fec7 100644 --- a/lib/web_ui/test/engine/surface/scene_builder_test.dart +++ b/lib/web_ui/test/engine/surface/scene_builder_test.dart @@ -27,7 +27,7 @@ void testMain() { await webOnlyInitializeEngine(); }); - group('SceneBuilder', () { + group('SceneBuilder', () { test('pushOffset implements surface lifecycle', () { testLayerLifeCycle((SceneBuilder sceneBuilder, EngineLayer oldLayer) { return sceneBuilder.pushOffset(10, 20, oldLayer: oldLayer); @@ -526,10 +526,10 @@ void testMain() { await testCase('', 'remove all', deletions: 2); }); - test('Canvas should allocate fewer pixels if scaled', () async { + test('Canvas should allocate fewer pixels when zoomed out', () async { final SurfaceSceneBuilder builder = SurfaceSceneBuilder(); final Picture picture1 = _drawPicture(); - builder.pushClipRect(const Rect.fromLTRB(10, 10, 300, 300),); + builder.pushClipRect(const Rect.fromLTRB(10, 10, 300, 300)); builder.addPicture(Offset.zero, picture1); builder.pop(); @@ -557,6 +557,38 @@ void testMain() { expect(canvas2.width < unscaledWidth, true); expect(canvas2.height < unscaledHeight, true); }); + + test('Canvas should allocate more pixels when zoomed in', () async { + final SurfaceSceneBuilder builder = SurfaceSceneBuilder(); + final Picture picture1 = _drawPicture(); + builder.pushClipRect(const Rect.fromLTRB(10, 10, 300, 300)); + builder.addPicture(Offset.zero, picture1); + builder.pop(); + + html.HtmlElement content = builder.build().webOnlyRootElement; + html.CanvasElement canvas = content.querySelector('canvas'); + final int unscaledWidth = canvas.width; + final int unscaledHeight = canvas.height; + + // Force update to scene which will utilize reuse code path. + final SurfaceSceneBuilder builder2 = SurfaceSceneBuilder(); + builder2.pushOffset(0, 0); + builder2.pushTransform(Matrix4.identity().scaled(2, 2).toFloat64()); + builder2.pushClipRect( + const Rect.fromLTRB(10, 10, 300, 300), + ); + builder2.addPicture(Offset.zero, picture1); + builder2.pop(); + builder2.pop(); + builder2.pop(); + + html.HtmlElement contentAfterScale = builder2.build().webOnlyRootElement; + html.CanvasElement canvas2 = contentAfterScale.querySelector('canvas'); + // Although we are drawing same picture, due to scaling the new canvas + // should have more pixels. + expect(canvas2.width > unscaledWidth, true); + expect(canvas2.height > unscaledHeight, true); + }); } typedef TestLayerBuilder = EngineLayer Function( From 541e321b7cfe180e9f6927f2fd1909aee0a7ff00 Mon Sep 17 00:00:00 2001 From: ferhatb Date: Thu, 29 Oct 2020 13:22:37 -0700 Subject: [PATCH 06/10] fix computePixelDensity to use 4 corners --- lib/web_ui/lib/src/engine/html/picture.dart | 34 ++++++++++++++++----- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/lib/web_ui/lib/src/engine/html/picture.dart b/lib/web_ui/lib/src/engine/html/picture.dart index b8c68b495bdaf..3e2abc6cd9b92 100644 --- a/lib/web_ui/lib/src/engine/html/picture.dart +++ b/lib/web_ui/lib/src/engine/html/picture.dart @@ -638,16 +638,34 @@ double _computePixelDensity(Matrix4? transform, double width, double height) { return 1.0; } final Float32List m = transform.storage; - // Apply perspective transform. - final double topLeftX = m[12] * m[15]; - final double topLeftY = m[13] * m[15]; + // Apply perspective transform to all 4 corners. Can't use left,top, bottom, + // right since for example rotating 45 degrees would yield inaccurate size. + double minX = m[12] * m[15]; + double minY = m[13] * m[15]; + double maxX = minX; + double maxY = minY; double x = width; double y = height; - final double xp = (m[0] * x) + (m[4] * y) + m[12]; - final double yp = (m[1] * x) + (m[5] * y) + m[13]; - final double wp = 1.0 / ((m[3] * x) + (m[7] * y) + m[15]); - double scaleX = ((xp * wp) - topLeftX).abs() / width; - double scaleY = ((yp * wp) - topLeftY).abs() / height; + double wp = 1.0 / ((m[3] * x) + (m[7] * y) + m[15]); + double xp = ((m[0] * x) + (m[4] * y) + m[12]) * wp; + double yp = ((m[1] * x) + (m[5] * y) + m[13]) * wp; + minX = math.min(minX, xp); + maxX = math.max(maxX, xp); + x = 0; + wp = 1.0 / ((m[3] * x) + (m[7] * y) + m[15]); + xp = ((m[0] * x) + (m[4] * y) + m[12]) * wp; + yp = ((m[1] * x) + (m[5] * y) + m[13]) * wp; + minX = math.min(minX, xp); + maxX = math.max(maxX, xp); + x = width; + y = 0; + wp = 1.0 / ((m[3] * x) + (m[7] * y) + m[15]); + xp = ((m[0] * x) + (m[4] * y) + m[12]) * wp; + yp = ((m[1] * x) + (m[5] * y) + m[13]) * wp; + minX = math.min(minX, xp); + maxX = math.max(maxX, xp); + double scaleX = (maxX - minX) / width; + double scaleY = (maxY - minY) / height; double scale = math.min(scaleX, scaleY); // kEpsilon guards against divide by zero below. if (scale < kEpsilon || scale == 1) { From b44cb996498e6f381725a40fc9d84498518cf913 Mon Sep 17 00:00:00 2001 From: ferhatb Date: Thu, 29 Oct 2020 13:37:41 -0700 Subject: [PATCH 07/10] fix computeDensity --- lib/web_ui/lib/src/engine/html/picture.dart | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/web_ui/lib/src/engine/html/picture.dart b/lib/web_ui/lib/src/engine/html/picture.dart index 3e2abc6cd9b92..d2a42d02dc6c5 100644 --- a/lib/web_ui/lib/src/engine/html/picture.dart +++ b/lib/web_ui/lib/src/engine/html/picture.dart @@ -651,12 +651,16 @@ double _computePixelDensity(Matrix4? transform, double width, double height) { double yp = ((m[1] * x) + (m[5] * y) + m[13]) * wp; minX = math.min(minX, xp); maxX = math.max(maxX, xp); + minY = math.min(minY, xp); + maxY = math.max(maxY, xp); x = 0; wp = 1.0 / ((m[3] * x) + (m[7] * y) + m[15]); xp = ((m[0] * x) + (m[4] * y) + m[12]) * wp; yp = ((m[1] * x) + (m[5] * y) + m[13]) * wp; minX = math.min(minX, xp); maxX = math.max(maxX, xp); + minY = math.min(minY, xp); + maxY = math.max(maxY, xp); x = width; y = 0; wp = 1.0 / ((m[3] * x) + (m[7] * y) + m[15]); @@ -664,6 +668,8 @@ double _computePixelDensity(Matrix4? transform, double width, double height) { yp = ((m[1] * x) + (m[5] * y) + m[13]) * wp; minX = math.min(minX, xp); maxX = math.max(maxX, xp); + minY = math.min(minY, xp); + maxY = math.max(maxY, xp); double scaleX = (maxX - minX) / width; double scaleY = (maxY - minY) / height; double scale = math.min(scaleX, scaleY); From 971b3ec4549183f9a7b16549113f4e3c7099e1e3 Mon Sep 17 00:00:00 2001 From: ferhatb Date: Thu, 29 Oct 2020 14:32:20 -0700 Subject: [PATCH 08/10] fix transform y max/min --- lib/web_ui/lib/src/engine/html/picture.dart | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/web_ui/lib/src/engine/html/picture.dart b/lib/web_ui/lib/src/engine/html/picture.dart index d2a42d02dc6c5..6c981774b65cf 100644 --- a/lib/web_ui/lib/src/engine/html/picture.dart +++ b/lib/web_ui/lib/src/engine/html/picture.dart @@ -651,16 +651,16 @@ double _computePixelDensity(Matrix4? transform, double width, double height) { double yp = ((m[1] * x) + (m[5] * y) + m[13]) * wp; minX = math.min(minX, xp); maxX = math.max(maxX, xp); - minY = math.min(minY, xp); - maxY = math.max(maxY, xp); + minY = math.min(minY, yp); + maxY = math.max(maxY, yp); x = 0; wp = 1.0 / ((m[3] * x) + (m[7] * y) + m[15]); xp = ((m[0] * x) + (m[4] * y) + m[12]) * wp; yp = ((m[1] * x) + (m[5] * y) + m[13]) * wp; minX = math.min(minX, xp); maxX = math.max(maxX, xp); - minY = math.min(minY, xp); - maxY = math.max(maxY, xp); + minY = math.min(minY, yp); + maxY = math.max(maxY, yp); x = width; y = 0; wp = 1.0 / ((m[3] * x) + (m[7] * y) + m[15]); @@ -668,8 +668,8 @@ double _computePixelDensity(Matrix4? transform, double width, double height) { yp = ((m[1] * x) + (m[5] * y) + m[13]) * wp; minX = math.min(minX, xp); maxX = math.max(maxX, xp); - minY = math.min(minY, xp); - maxY = math.max(maxY, xp); + minY = math.min(minY, yp); + maxY = math.max(maxY, yp); double scaleX = (maxX - minX) / width; double scaleY = (maxY - minY) / height; double scale = math.min(scaleX, scaleY); From ae30c961aaf81fe7e5836a9ebc8061a079f19955 Mon Sep 17 00:00:00 2001 From: ferhatb Date: Thu, 29 Oct 2020 15:47:59 -0700 Subject: [PATCH 09/10] Only check against limit if scaling up --- lib/web_ui/lib/src/engine/html/picture.dart | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/lib/web_ui/lib/src/engine/html/picture.dart b/lib/web_ui/lib/src/engine/html/picture.dart index 6c981774b65cf..379b72ae87b9d 100644 --- a/lib/web_ui/lib/src/engine/html/picture.dart +++ b/lib/web_ui/lib/src/engine/html/picture.dart @@ -649,6 +649,7 @@ double _computePixelDensity(Matrix4? transform, double width, double height) { double wp = 1.0 / ((m[3] * x) + (m[7] * y) + m[15]); double xp = ((m[0] * x) + (m[4] * y) + m[12]) * wp; double yp = ((m[1] * x) + (m[5] * y) + m[13]) * wp; + print('$xp,$yp'); minX = math.min(minX, xp); maxX = math.max(maxX, xp); minY = math.min(minY, yp); @@ -657,6 +658,7 @@ double _computePixelDensity(Matrix4? transform, double width, double height) { wp = 1.0 / ((m[3] * x) + (m[7] * y) + m[15]); xp = ((m[0] * x) + (m[4] * y) + m[12]) * wp; yp = ((m[1] * x) + (m[5] * y) + m[13]) * wp; + print('$xp,$yp'); minX = math.min(minX, xp); maxX = math.max(maxX, xp); minY = math.min(minY, yp); @@ -666,6 +668,7 @@ double _computePixelDensity(Matrix4? transform, double width, double height) { wp = 1.0 / ((m[3] * x) + (m[7] * y) + m[15]); xp = ((m[0] * x) + (m[4] * y) + m[12]) * wp; yp = ((m[1] * x) + (m[5] * y) + m[13]) * wp; + print('$xp,$yp'); minX = math.min(minX, xp); maxX = math.max(maxX, xp); minY = math.min(minY, yp); @@ -686,13 +689,13 @@ double _computePixelDensity(Matrix4? transform, double width, double height) { // On a fullscreen high dpi device dpi*density*resolution will demand // too much memory, so clamp at 4. scale = math.min(4.0, ((scale / 2.0).ceil() * 2.0)); + // Guard against webkit absolute limit. + const double kPixelLimit = 1024 * 1024 * 4; + if ((width * height * scale * scale) > kPixelLimit && scale > 2) { + scale = (kPixelLimit * 0.8) / (width * height); + } } else { scale = math.max(2.0 / (2.0 / scale).floor(), 0.0001); } - // Guard against webkit absolute limit. - const double kPixelLimit = 1024 * 1024 * 4; - if ((width * height * scale * scale) > kPixelLimit) { - scale = (kPixelLimit * 0.8) / (width * height); - } return scale; } From 86be729d47b210a9a4b344ea212a13a42cb41048 Mon Sep 17 00:00:00 2001 From: ferhatb Date: Thu, 29 Oct 2020 16:37:12 -0700 Subject: [PATCH 10/10] Update shadow maxdiff due to rendering change --- lib/web_ui/test/golden_tests/engine/shadow_golden_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/web_ui/test/golden_tests/engine/shadow_golden_test.dart b/lib/web_ui/test/golden_tests/engine/shadow_golden_test.dart index 1822bc1c3e985..557280e2aecd1 100644 --- a/lib/web_ui/test/golden_tests/engine/shadow_golden_test.dart +++ b/lib/web_ui/test/golden_tests/engine/shadow_golden_test.dart @@ -163,7 +163,7 @@ void testMain() async { await matchGoldenFile( 'shadows.png', region: region, - maxDiffRatePercent: 0.0, + maxDiffRatePercent: 0.23, pixelComparison: PixelComparison.precise, ); },