diff --git a/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/surface.dart b/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/surface.dart index 69041aee75..e058b5f5fc 100644 --- a/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/surface.dart +++ b/engine/src/flutter/lib/web_ui/lib/src/engine/canvaskit/surface.dart @@ -237,7 +237,8 @@ class Surface extends DisplayCanvas { } // TODO(jonahwilliams): this is somewhat wasteful. We should probably // eagerly setup this surface instead of delaying until the first frame? - // Or at least cache the estimated window sizeThis is the first frame we have rendered with this canvas. + // Or at least cache the estimated window size. + // This is the first frame we have rendered with this canvas. createOrUpdateSurface(size); } @@ -261,16 +262,13 @@ class Surface extends DisplayCanvas { return _surface!; } - final BitmapSize? previousCanvasSize = _currentCanvasPhysicalSize; - // Initialize a new, larger, canvas. If the size is growing, then make the - // new canvas larger than required to avoid many canvas creations. - if (previousCanvasSize != null && - (size.width > previousCanvasSize.width || size.height > previousCanvasSize.height)) { - final BitmapSize newSize = BitmapSize.fromSize(size.toSize() * 1.4); + if (_currentCanvasPhysicalSize != null && + (size.width != _currentCanvasPhysicalSize!.width || + size.height != _currentCanvasPhysicalSize!.height)) { _surface?.dispose(); _surface = null; - _pixelWidth = newSize.width; - _pixelHeight = newSize.height; + _pixelWidth = size.width; + _pixelHeight = size.height; if (useOffscreenCanvas) { _offscreenCanvas!.width = _pixelWidth.toDouble(); _offscreenCanvas!.height = _pixelHeight.toDouble(); @@ -285,10 +283,14 @@ class Surface extends DisplayCanvas { } } + // If we reached here, then either we are forcing a new context, or + // the size of the surface has changed so we need to make a new one. + + _surface?.dispose(); + _surface = null; + // Either a new context is being forced or we've never had one. if (_forceNewContext || _currentCanvasPhysicalSize == null) { - _surface?.dispose(); - _surface = null; _grContext?.releaseResourcesAndAbandonContext(); _grContext?.delete(); _grContext = null; @@ -297,7 +299,6 @@ class Surface extends DisplayCanvas { _currentCanvasPhysicalSize = size; } - _surface?.dispose(); return _surface = _createNewSurface(size); } diff --git a/engine/src/flutter/lib/web_ui/test/canvaskit/surface_test.dart b/engine/src/flutter/lib/web_ui/test/canvaskit/surface_test.dart index 8861d8a1fe..a7875bfa68 100644 --- a/engine/src/flutter/lib/web_ui/test/canvaskit/surface_test.dart +++ b/engine/src/flutter/lib/web_ui/test/canvaskit/surface_test.dart @@ -34,8 +34,8 @@ void testMain() { expect(originalSurface.width(), 9); expect(originalSurface.height(), 19); - // Shrinking reuses the existing canvas but translates it so - // Skia renders into the visible area. + // Shrinking causes the surface to create a new canvas with the exact + // size requested. final CkSurface shrunkSurface = surface.acquireFrame(const ui.Size(5, 15)).skiaSurface; final DomOffscreenCanvas shrunk = surface.debugOffscreenCanvas!; expect(shrunk, same(original)); @@ -43,21 +43,21 @@ void testMain() { expect(shrunkSurface.width(), 5); expect(shrunkSurface.height(), 15); - // The first increase will allocate a new surface, but will overallocate - // by 40% to accommodate future increases. + // The first increase will allocate a new surface to exactly the + // requested size. final CkSurface firstIncreaseSurface = surface.acquireFrame(const ui.Size(10, 20)).skiaSurface; final DomOffscreenCanvas firstIncrease = surface.debugOffscreenCanvas!; expect(firstIncrease, same(original)); expect(firstIncreaseSurface, isNot(same(shrunkSurface))); - // Expect overallocated dimensions - expect(firstIncrease.width, 14); - expect(firstIncrease.height, 28); + // Expect exact dimensions + expect(firstIncrease.width, 10); + expect(firstIncrease.height, 20); expect(firstIncreaseSurface.width(), 10); expect(firstIncreaseSurface.height(), 20); - // Subsequent increases within 40% reuse the old canvas. + // Subsequent increases within 40% will still allocate a new canvas. final CkSurface secondIncreaseSurface = surface.acquireFrame(const ui.Size(11, 22)).skiaSurface; final DomOffscreenCanvas secondIncrease = surface.debugOffscreenCanvas!; @@ -72,13 +72,13 @@ void testMain() { expect(huge, same(secondIncrease)); expect(hugeSurface, isNot(same(secondIncreaseSurface))); - // Also over-allocated - expect(huge.width, 28); - expect(huge.height, 56); + // Also exactly-allocated + expect(huge.width, 20); + expect(huge.height, 40); expect(hugeSurface.width(), 20); expect(hugeSurface.height(), 40); - // Shrink again. Reuse the last allocated surface. + // Shrink again. Create a new surface. final CkSurface shrunkSurface2 = surface.acquireFrame(const ui.Size(5, 15)).skiaSurface; final DomOffscreenCanvas shrunk2 = surface.debugOffscreenCanvas!; expect(shrunk2, same(huge)); @@ -116,64 +116,62 @@ void testMain() { expect(canvasSize.width, 9); expect(canvasSize.height, 19); - // Shrinking reuses the existing canvas but translates it so - // Skia renders into the visible area. + // Shrinking causes us to resize the canvas. surface.createOrUpdateSurface(const BitmapSize(5, 15)); final DomCanvasElement shrunk = getDisplayCanvas(surface); canvasSize = getCssSize(surface); - expect(shrunk.width, 9); - expect(shrunk.height, 19); - expect(canvasSize.width, 9); - expect(canvasSize.height, 19); + expect(shrunk.width, 5); + expect(shrunk.height, 15); + expect(canvasSize.width, 5); + expect(canvasSize.height, 15); - // The first increase will allocate a new surface, but will overallocate - // by 40% to accommodate future increases. + // Increasing the size causes us to resize the canvas. surface.createOrUpdateSurface(const BitmapSize(10, 20)); final DomCanvasElement firstIncrease = getDisplayCanvas(surface); canvasSize = getCssSize(surface); expect(firstIncrease, same(original)); - // Expect overallocated dimensions - expect(firstIncrease.width, 14); - expect(firstIncrease.height, 28); - expect(canvasSize.width, 14); - expect(canvasSize.height, 28); + // Expect exact dimensions + expect(firstIncrease.width, 10); + expect(firstIncrease.height, 20); + expect(canvasSize.width, 10); + expect(canvasSize.height, 20); - // Subsequent increases within 40% reuse the old canvas. + // Subsequent increases also cause canvas resizing. surface.createOrUpdateSurface(const BitmapSize(11, 22)); final DomCanvasElement secondIncrease = getDisplayCanvas(surface); canvasSize = getCssSize(surface); expect(secondIncrease, same(firstIncrease)); - expect(secondIncrease.width, 14); - expect(secondIncrease.height, 28); - expect(canvasSize.width, 14); - expect(canvasSize.height, 28); + expect(secondIncrease.width, 11); + expect(secondIncrease.height, 22); + expect(canvasSize.width, 11); + expect(canvasSize.height, 22); - // Increases beyond the 40% limit will cause a new allocation. + // Increases beyond the 40% limit will cause a canvas resize. surface.createOrUpdateSurface(const BitmapSize(20, 40)); final DomCanvasElement huge = getDisplayCanvas(surface); canvasSize = getCssSize(surface); expect(huge, same(secondIncrease)); - // Also over-allocated - expect(huge.width, 28); - expect(huge.height, 56); - expect(canvasSize.width, 28); - expect(canvasSize.height, 56); + // Also exact + expect(huge.width, 20); + expect(huge.height, 40); + expect(canvasSize.width, 20); + expect(canvasSize.height, 40); - // Shrink again. Reuse the last allocated surface. + // Shrink again. Resize the canvas. surface.createOrUpdateSurface(const BitmapSize(5, 15)); final DomCanvasElement shrunk2 = getDisplayCanvas(surface); canvasSize = getCssSize(surface); expect(shrunk2, same(huge)); - expect(shrunk2.width, 28); - expect(shrunk2.height, 56); - expect(canvasSize.width, 28); - expect(canvasSize.height, 56); + expect(shrunk2.width, 5); + expect(shrunk2.height, 15); + expect(canvasSize.width, 5); + expect(canvasSize.height, 15); // Doubling the DPR should halve the CSS width, height, and translation of the canvas. // This tests https://github.com/flutter/flutter/issues/77084 @@ -183,12 +181,12 @@ void testMain() { canvasSize = getCssSize(surface); expect(dpr2Canvas, same(huge)); - expect(dpr2Canvas.width, 28); - expect(dpr2Canvas.height, 56); + expect(dpr2Canvas.width, 5); + expect(dpr2Canvas.height, 15); // Canvas is half the size in logical pixels because device pixel ratio is // 2.0. - expect(canvasSize.width, 14); - expect(canvasSize.height, 28); + expect(canvasSize.width, 2.5); + expect(canvasSize.height, 7.5); // Skip on wasm since same() doesn't work for JSValues. }, skip: isWasm);