From ccd6302e2d781e311b387c2198f9ac073abb42b5 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Thu, 29 Dec 2022 20:00:59 -0800 Subject: [PATCH 01/10] [web] dont dispose of context on window resize --- .../lib/src/engine/canvaskit/surface.dart | 29 +++++++++++++++---- 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/lib/web_ui/lib/src/engine/canvaskit/surface.dart b/lib/web_ui/lib/src/engine/canvaskit/surface.dart index ff9d829f7aea6..37d3a29bbfcdd 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/surface.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/surface.dart @@ -161,14 +161,31 @@ class Surface { return _surface!; } - // If the current canvas size is smaller than the requested size then create - // a new, larger, canvas. Then update the GR context so we can create a new - // SkSurface. + final ui.Size? previousCanvasSize = _currentCanvasPhysicalSize; + + if (!_forceNewContext && previousCanvasSize != null + && (size.width > previousCanvasSize.width || + size.height > previousCanvasSize.height)) { + htmlCanvas!.width = size.width; + htmlCanvas!.height = size.height; + + _surface?.getCanvas().clear(const ui.Color(0x00000000)); + _surface?.dispose(); + _surface = null; + _addedToScene = false; + + _currentCanvasPhysicalSize = size; + _pixelWidth = size.width.ceil(); + _pixelHeight = size.height.ceil(); + _updateLogicalHtmlCanvasSize(); + + return _surface = _createNewSurface(size); + } + if (_forceNewContext || - previousCanvasSize == null || - size.width > previousCanvasSize.width || - size.height > previousCanvasSize.height) { + previousCanvasSize == null) { + // Initialize a new, larger, canvas. If the size is growing, then make the // new canvas larger than required to avoid many canvas creations. final ui.Size newSize = previousCanvasSize == null ? size : size * 1.4; From 2be10330a42a596d699be8239434dd5656062052 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 3 Jan 2023 10:21:12 -0800 Subject: [PATCH 02/10] ++ --- .../lib/src/engine/canvaskit/surface.dart | 4 +- lib/web_ui/test/canvaskit/surface_test.dart | 42 +++++++++---------- 2 files changed, 22 insertions(+), 24 deletions(-) diff --git a/lib/web_ui/lib/src/engine/canvaskit/surface.dart b/lib/web_ui/lib/src/engine/canvaskit/surface.dart index 37d3a29bbfcdd..ebeaba27f08ee 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/surface.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/surface.dart @@ -165,8 +165,8 @@ class Surface { final ui.Size? previousCanvasSize = _currentCanvasPhysicalSize; if (!_forceNewContext && previousCanvasSize != null - && (size.width > previousCanvasSize.width || - size.height > previousCanvasSize.height)) { + && (size.width != previousCanvasSize.width || + size.height != previousCanvasSize.height)) { htmlCanvas!.width = size.width; htmlCanvas!.height = size.height; diff --git a/lib/web_ui/test/canvaskit/surface_test.dart b/lib/web_ui/test/canvaskit/surface_test.dart index f3d7e26ddb044..ae4f724fe10a3 100644 --- a/lib/web_ui/test/canvaskit/surface_test.dart +++ b/lib/web_ui/test/canvaskit/surface_test.dart @@ -37,32 +37,30 @@ 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. final CkSurface shrunkSurface = surface.acquireFrame(const ui.Size(5, 15)).skiaSurface; final DomCanvasElement shrunk = surface.htmlCanvas!; expect(shrunk, same(original)); - expect(shrunk.style.width, '9px'); - expect(shrunk.style.height, '19px'); - expect(shrunk.style.transform, _isTranslate(0, -4)); + expect(shrunk.style.width, '5px'); + expect(shrunk.style.height, '15px'); + expect(shrunk.style.transform, _isTranslate(0, 0)); expect(shrunkSurface, isNot(same(original))); expect(shrunkSurface.width(), 5); expect(shrunkSurface.height(), 15); - // The first increase will allocate a new canvas, but will overallocate - // by 40% to accommodate future increases. + // The first increase use the same canvas element. final CkSurface firstIncreaseSurface = surface.acquireFrame(const ui.Size(10, 20)).skiaSurface; final DomCanvasElement firstIncrease = surface.htmlCanvas!; - expect(firstIncrease, isNot(same(original))); + expect(firstIncrease, same(original)); expect(firstIncreaseSurface, isNot(same(shrunkSurface))); - // Expect overallocated dimensions - expect(firstIncrease.width, 14); - expect(firstIncrease.height, 28); - expect(firstIncrease.style.width, '14px'); - expect(firstIncrease.style.height, '28px'); - expect(firstIncrease.style.transform, _isTranslate(0, -8)); + // Expect exact dimensions + expect(firstIncrease.width, 10); + expect(firstIncrease.height, 20); + expect(firstIncrease.style.width, '10px'); + expect(firstIncrease.style.height, '20px'); + expect(firstIncrease.style.transform,_isTranslate(0, 0)); expect(firstIncreaseSurface.width(), 10); expect(firstIncreaseSurface.height(), 20); @@ -71,23 +69,23 @@ void testMain() { surface.acquireFrame(const ui.Size(11, 22)).skiaSurface; final DomCanvasElement secondIncrease = surface.htmlCanvas!; expect(secondIncrease, same(firstIncrease)); - expect(secondIncrease.style.transform, _isTranslate(0, -6)); + expect(secondIncrease.style.transform, _isTranslate(0, 0)); expect(secondIncreaseSurface, isNot(same(firstIncreaseSurface))); expect(secondIncreaseSurface.width(), 11); expect(secondIncreaseSurface.height(), 22); - // Increases beyond the 40% limit will cause a new allocation. + // Increases beyond the 40% limit will still not cause a new allocation. final CkSurface hugeSurface = surface.acquireFrame(const ui.Size(20, 40)).skiaSurface; final DomCanvasElement huge = surface.htmlCanvas!; - expect(huge, isNot(same(secondIncrease))); + expect(huge, same(secondIncrease)); expect(hugeSurface, isNot(same(secondIncreaseSurface))); - // Also over-allocated - expect(huge.width, 28); - expect(huge.height, 56); - expect(huge.style.width, '28px'); - expect(huge.style.height, '56px'); - expect(huge.style.transform, _isTranslate(0, -16)); + // Exactly allocated + expect(huge.width, 20); + expect(huge.height, 40); + expect(huge.style.width, '20px'); + expect(huge.style.height, '40px'); + expect(huge.style.transform, _isTranslate(0, 0)); expect(hugeSurface.width(), 20); expect(hugeSurface.height(), 40); From 69bf91369b07a9f23efc7919fcebebe5205c536a Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 3 Jan 2023 11:45:23 -0800 Subject: [PATCH 03/10] ++ --- .../lib/src/engine/canvaskit/surface.dart | 76 ++++++++----------- lib/web_ui/test/canvaskit/surface_test.dart | 42 +++++----- 2 files changed, 54 insertions(+), 64 deletions(-) diff --git a/lib/web_ui/lib/src/engine/canvaskit/surface.dart b/lib/web_ui/lib/src/engine/canvaskit/surface.dart index ebeaba27f08ee..66757f8c7786d 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/surface.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/surface.dart @@ -146,53 +146,40 @@ class Surface { throw CanvasKitError('Cannot create surfaces of empty size.'); } - // Check if the window is the same size as before, and if so, don't allocate - // a new canvas as the previous canvas is big enough to fit everything. - final ui.Size? previousSurfaceSize = _currentSurfaceSize; - if (!_forceNewContext && - previousSurfaceSize != null && - size.width == previousSurfaceSize.width && - size.height == previousSurfaceSize.height) { - // The existing surface is still reusable. - if (window.devicePixelRatio != _currentDevicePixelRatio) { - _updateLogicalHtmlCanvasSize(); - _translateCanvas(); + if (!_forceNewContext) { + // Check if the window is the same size as before, and if so, don't allocate + // a new canvas as the previous canvas is big enough to fit everything. + final ui.Size? previousSurfaceSize = _currentSurfaceSize; + if (previousSurfaceSize != null && + size.width == previousSurfaceSize.width && + size.height == previousSurfaceSize.height) { + // The existing surface is still reusable. + if (window.devicePixelRatio != _currentDevicePixelRatio) { + _updateLogicalHtmlCanvasSize(); + _translateCanvas(); + } + return _surface!; } - return _surface!; - } - - - final ui.Size? previousCanvasSize = _currentCanvasPhysicalSize; - - if (!_forceNewContext && previousCanvasSize != null - && (size.width != previousCanvasSize.width || - size.height != previousCanvasSize.height)) { - htmlCanvas!.width = size.width; - htmlCanvas!.height = size.height; - - _surface?.getCanvas().clear(const ui.Color(0x00000000)); - _surface?.dispose(); - _surface = null; - _addedToScene = false; - - _currentCanvasPhysicalSize = size; - _pixelWidth = size.width.ceil(); - _pixelHeight = size.height.ceil(); - _updateLogicalHtmlCanvasSize(); - - return _surface = _createNewSurface(size); - } - - if (_forceNewContext || - previousCanvasSize == null) { + final ui.Size? 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. - final ui.Size newSize = previousCanvasSize == null ? size : size * 1.4; + if (previousCanvasSize != null && (size.width > previousCanvasSize.width || size.height > previousCanvasSize.height)) { + final ui.Size newSize = previousCanvasSize == null ? size : size * 1.4; + _surface?.dispose(); + _surface = null; + htmlCanvas!.width = newSize.width; + htmlCanvas!.height = newSize.height; + _currentCanvasPhysicalSize = newSize; + _pixelWidth = newSize.width.ceil(); + _pixelHeight = newSize.height.ceil(); + _updateLogicalHtmlCanvasSize(); + } + } - // If we have a surface, send a dummy command to its canvas to make its context - // current or else disposing the context could fail below. - _surface?.getCanvas().clear(const ui.Color(0x00000000)); + // Either a new context is being forced or we've never had one. + if (_forceNewContext || + _currentCanvasPhysicalSize == null) { _surface?.dispose(); _surface = null; _addedToScene = false; @@ -200,8 +187,9 @@ class Surface { _grContext?.delete(); _grContext = null; - _createNewCanvas(newSize); - _currentCanvasPhysicalSize = newSize; + _createNewCanvas(size); + _currentCanvasPhysicalSize = size; + _surface = _createNewSurface(size); } else if (window.devicePixelRatio != _currentDevicePixelRatio) { _updateLogicalHtmlCanvasSize(); } diff --git a/lib/web_ui/test/canvaskit/surface_test.dart b/lib/web_ui/test/canvaskit/surface_test.dart index ae4f724fe10a3..f3d7e26ddb044 100644 --- a/lib/web_ui/test/canvaskit/surface_test.dart +++ b/lib/web_ui/test/canvaskit/surface_test.dart @@ -37,30 +37,32 @@ 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. final CkSurface shrunkSurface = surface.acquireFrame(const ui.Size(5, 15)).skiaSurface; final DomCanvasElement shrunk = surface.htmlCanvas!; expect(shrunk, same(original)); - expect(shrunk.style.width, '5px'); - expect(shrunk.style.height, '15px'); - expect(shrunk.style.transform, _isTranslate(0, 0)); + expect(shrunk.style.width, '9px'); + expect(shrunk.style.height, '19px'); + expect(shrunk.style.transform, _isTranslate(0, -4)); expect(shrunkSurface, isNot(same(original))); expect(shrunkSurface.width(), 5); expect(shrunkSurface.height(), 15); - // The first increase use the same canvas element. + // The first increase will allocate a new canvas, but will overallocate + // by 40% to accommodate future increases. final CkSurface firstIncreaseSurface = surface.acquireFrame(const ui.Size(10, 20)).skiaSurface; final DomCanvasElement firstIncrease = surface.htmlCanvas!; - expect(firstIncrease, same(original)); + expect(firstIncrease, isNot(same(original))); expect(firstIncreaseSurface, isNot(same(shrunkSurface))); - // Expect exact dimensions - expect(firstIncrease.width, 10); - expect(firstIncrease.height, 20); - expect(firstIncrease.style.width, '10px'); - expect(firstIncrease.style.height, '20px'); - expect(firstIncrease.style.transform,_isTranslate(0, 0)); + // Expect overallocated dimensions + expect(firstIncrease.width, 14); + expect(firstIncrease.height, 28); + expect(firstIncrease.style.width, '14px'); + expect(firstIncrease.style.height, '28px'); + expect(firstIncrease.style.transform, _isTranslate(0, -8)); expect(firstIncreaseSurface.width(), 10); expect(firstIncreaseSurface.height(), 20); @@ -69,23 +71,23 @@ void testMain() { surface.acquireFrame(const ui.Size(11, 22)).skiaSurface; final DomCanvasElement secondIncrease = surface.htmlCanvas!; expect(secondIncrease, same(firstIncrease)); - expect(secondIncrease.style.transform, _isTranslate(0, 0)); + expect(secondIncrease.style.transform, _isTranslate(0, -6)); expect(secondIncreaseSurface, isNot(same(firstIncreaseSurface))); expect(secondIncreaseSurface.width(), 11); expect(secondIncreaseSurface.height(), 22); - // Increases beyond the 40% limit will still not cause a new allocation. + // Increases beyond the 40% limit will cause a new allocation. final CkSurface hugeSurface = surface.acquireFrame(const ui.Size(20, 40)).skiaSurface; final DomCanvasElement huge = surface.htmlCanvas!; - expect(huge, same(secondIncrease)); + expect(huge, isNot(same(secondIncrease))); expect(hugeSurface, isNot(same(secondIncreaseSurface))); - // Exactly allocated - expect(huge.width, 20); - expect(huge.height, 40); - expect(huge.style.width, '20px'); - expect(huge.style.height, '40px'); - expect(huge.style.transform, _isTranslate(0, 0)); + // Also over-allocated + expect(huge.width, 28); + expect(huge.height, 56); + expect(huge.style.width, '28px'); + expect(huge.style.height, '56px'); + expect(huge.style.transform, _isTranslate(0, -16)); expect(hugeSurface.width(), 20); expect(hugeSurface.height(), 40); From 54c80debe65756d597842ca0d95212e9b1d3b456 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 3 Jan 2023 12:03:16 -0800 Subject: [PATCH 04/10] ++ --- lib/web_ui/lib/src/engine/canvaskit/surface.dart | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/lib/web_ui/lib/src/engine/canvaskit/surface.dart b/lib/web_ui/lib/src/engine/canvaskit/surface.dart index 66757f8c7786d..817f677906e13 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/surface.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/surface.dart @@ -19,8 +19,7 @@ import 'util.dart'; // Only supported in profile/release mode. Allows Flutter to use MSAA but // removes the ability for disabling AA on Paint objects. -const bool _kUsingMSAA = - bool.fromEnvironment('flutter.canvaskit.msaa'); +const bool _kUsingMSAA = bool.fromEnvironment('flutter.canvaskit.msaa'); typedef SubmitCallback = bool Function(SurfaceFrame, CkCanvas); @@ -164,7 +163,9 @@ class Surface { final ui.Size? 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)) { + if (previousCanvasSize != null && + (size.width > previousCanvasSize.width || + size.height > previousCanvasSize.height)) { final ui.Size newSize = previousCanvasSize == null ? size : size * 1.4; _surface?.dispose(); _surface = null; @@ -178,8 +179,7 @@ class Surface { } // Either a new context is being forced or we've never had one. - if (_forceNewContext || - _currentCanvasPhysicalSize == null) { + if (_forceNewContext || _currentCanvasPhysicalSize == null) { _surface?.dispose(); _surface = null; _addedToScene = false; @@ -189,7 +189,6 @@ class Surface { _createNewCanvas(size); _currentCanvasPhysicalSize = size; - _surface = _createNewSurface(size); } else if (window.devicePixelRatio != _currentDevicePixelRatio) { _updateLogicalHtmlCanvasSize(); } @@ -197,7 +196,10 @@ class Surface { _currentDevicePixelRatio = window.devicePixelRatio; _currentSurfaceSize = size; _translateCanvas(); - return _surface = _createNewSurface(size); + // If the physical size of the canvas didn't change, do not create a new + // surface. + _surface ??= _createNewSurface(size); + return _surface!; } /// Sets the CSS size of the canvas so that canvas pixels are 1:1 with device From 85f5afd8e8a379dbec8a2810c3e6e74368b49f16 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 3 Jan 2023 12:31:52 -0800 Subject: [PATCH 05/10] ++ --- .../lib/src/engine/canvaskit/surface.dart | 2 +- lib/web_ui/test/canvaskit/surface_test.dart | 24 ++++++++++--------- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/lib/web_ui/lib/src/engine/canvaskit/surface.dart b/lib/web_ui/lib/src/engine/canvaskit/surface.dart index 817f677906e13..a462a7777b42f 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/surface.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/surface.dart @@ -166,7 +166,7 @@ class Surface { if (previousCanvasSize != null && (size.width > previousCanvasSize.width || size.height > previousCanvasSize.height)) { - final ui.Size newSize = previousCanvasSize == null ? size : size * 1.4; + final ui.Size newSize = size * 1.4; _surface?.dispose(); _surface = null; htmlCanvas!.width = newSize.width; diff --git a/lib/web_ui/test/canvaskit/surface_test.dart b/lib/web_ui/test/canvaskit/surface_test.dart index f3d7e26ddb044..e9a4ceb538bdb 100644 --- a/lib/web_ui/test/canvaskit/surface_test.dart +++ b/lib/web_ui/test/canvaskit/surface_test.dart @@ -37,7 +37,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 reuses the existing canvas and surface but translates it so + //// Skia renders into the visible area. final CkSurface shrunkSurface = surface.acquireFrame(const ui.Size(5, 15)).skiaSurface; final DomCanvasElement shrunk = surface.htmlCanvas!; @@ -45,11 +46,12 @@ void testMain() { expect(shrunk.style.width, '9px'); expect(shrunk.style.height, '19px'); expect(shrunk.style.transform, _isTranslate(0, -4)); - expect(shrunkSurface, isNot(same(original))); - expect(shrunkSurface.width(), 5); - expect(shrunkSurface.height(), 15); + expect(shrunkSurface, same(original)); + // original size is retained. + expect(shrunkSurface.width(), 9); + expect(shrunkSurface.height(), 19); - // The first increase will allocate a new canvas, but will overallocate + // The first increase will allocate a new surface, but will overallocate // by 40% to accommodate future increases. final CkSurface firstIncreaseSurface = surface.acquireFrame(const ui.Size(10, 20)).skiaSurface; @@ -72,9 +74,9 @@ void testMain() { final DomCanvasElement secondIncrease = surface.htmlCanvas!; expect(secondIncrease, same(firstIncrease)); expect(secondIncrease.style.transform, _isTranslate(0, -6)); - expect(secondIncreaseSurface, isNot(same(firstIncreaseSurface))); - expect(secondIncreaseSurface.width(), 11); - expect(secondIncreaseSurface.height(), 22); + expect(secondIncreaseSurface, same(firstIncreaseSurface)); + expect(secondIncreaseSurface.width(), 14); + expect(secondIncreaseSurface.height(), 28); // Increases beyond the 40% limit will cause a new allocation. final CkSurface hugeSurface = surface.acquireFrame(const ui.Size(20, 40)).skiaSurface; @@ -99,9 +101,9 @@ void testMain() { expect(shrunk2.style.width, '28px'); expect(shrunk2.style.height, '56px'); expect(shrunk2.style.transform, _isTranslate(0, -41)); - expect(shrunkSurface2, isNot(same(hugeSurface))); - expect(shrunkSurface2.width(), 5); - expect(shrunkSurface2.height(), 15); + expect(shrunkSurface2, same(hugeSurface)); + expect(shrunkSurface2.width(), 20); + expect(shrunkSurface2.height(), 40); // Doubling the DPR should halve the CSS width, height, and translation of the canvas. // This tests https://github.com/flutter/flutter/issues/77084 From 4d470e81de7da624350b60829b5ffa73c62652e2 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 3 Jan 2023 12:46:04 -0800 Subject: [PATCH 06/10] ++ --- lib/web_ui/test/canvaskit/surface_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/web_ui/test/canvaskit/surface_test.dart b/lib/web_ui/test/canvaskit/surface_test.dart index e9a4ceb538bdb..f9c5c4ea04d15 100644 --- a/lib/web_ui/test/canvaskit/surface_test.dart +++ b/lib/web_ui/test/canvaskit/surface_test.dart @@ -46,7 +46,7 @@ void testMain() { expect(shrunk.style.width, '9px'); expect(shrunk.style.height, '19px'); expect(shrunk.style.transform, _isTranslate(0, -4)); - expect(shrunkSurface, same(original)); + expect(shrunkSurface, same(originalSurface)); // original size is retained. expect(shrunkSurface.width(), 9); expect(shrunkSurface.height(), 19); From 42fe1762f441b4c182b6d6c7b1c3e2314b02bf49 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 3 Jan 2023 13:02:26 -0800 Subject: [PATCH 07/10] ++ --- lib/web_ui/test/canvaskit/surface_test.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/web_ui/test/canvaskit/surface_test.dart b/lib/web_ui/test/canvaskit/surface_test.dart index f9c5c4ea04d15..0316692be60ec 100644 --- a/lib/web_ui/test/canvaskit/surface_test.dart +++ b/lib/web_ui/test/canvaskit/surface_test.dart @@ -56,7 +56,7 @@ void testMain() { final CkSurface firstIncreaseSurface = surface.acquireFrame(const ui.Size(10, 20)).skiaSurface; final DomCanvasElement firstIncrease = surface.htmlCanvas!; - expect(firstIncrease, isNot(same(original))); + expect(firstIncrease, same(original)); expect(firstIncreaseSurface, isNot(same(shrunkSurface))); // Expect overallocated dimensions @@ -81,7 +81,7 @@ void testMain() { // Increases beyond the 40% limit will cause a new allocation. final CkSurface hugeSurface = surface.acquireFrame(const ui.Size(20, 40)).skiaSurface; final DomCanvasElement huge = surface.htmlCanvas!; - expect(huge, isNot(same(secondIncrease))); + expect(huge, same(secondIncrease)); expect(hugeSurface, isNot(same(secondIncreaseSurface))); // Also over-allocated From f944c289c7b50dde9e73e4381795302af92d64aa Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 3 Jan 2023 14:33:48 -0800 Subject: [PATCH 08/10] always re-create surface --- lib/web_ui/lib/src/engine/canvaskit/surface.dart | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/web_ui/lib/src/engine/canvaskit/surface.dart b/lib/web_ui/lib/src/engine/canvaskit/surface.dart index a462a7777b42f..cd9b910ee694b 100644 --- a/lib/web_ui/lib/src/engine/canvaskit/surface.dart +++ b/lib/web_ui/lib/src/engine/canvaskit/surface.dart @@ -196,9 +196,8 @@ class Surface { _currentDevicePixelRatio = window.devicePixelRatio; _currentSurfaceSize = size; _translateCanvas(); - // If the physical size of the canvas didn't change, do not create a new - // surface. - _surface ??= _createNewSurface(size); + _surface?.dispose(); + _surface = _createNewSurface(size); return _surface!; } From 1c1e9d03413417999d71e9536da14bd825553a61 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 3 Jan 2023 14:37:44 -0800 Subject: [PATCH 09/10] ++ --- lib/web_ui/test/canvaskit/surface_test.dart | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/lib/web_ui/test/canvaskit/surface_test.dart b/lib/web_ui/test/canvaskit/surface_test.dart index 0316692be60ec..fe6cecd95d0a5 100644 --- a/lib/web_ui/test/canvaskit/surface_test.dart +++ b/lib/web_ui/test/canvaskit/surface_test.dart @@ -37,8 +37,8 @@ void testMain() { expect(originalSurface.width(), 9); expect(originalSurface.height(), 19); - // Shrinking reuses the existing canvas and surface but translates it so - //// Skia renders into the visible area. + // Shrinking reuses the existing canvas but translates it so + // Skia renders into the visible area. final CkSurface shrunkSurface = surface.acquireFrame(const ui.Size(5, 15)).skiaSurface; final DomCanvasElement shrunk = surface.htmlCanvas!; @@ -46,10 +46,9 @@ void testMain() { expect(shrunk.style.width, '9px'); expect(shrunk.style.height, '19px'); expect(shrunk.style.transform, _isTranslate(0, -4)); - expect(shrunkSurface, same(originalSurface)); - // original size is retained. - expect(shrunkSurface.width(), 9); - expect(shrunkSurface.height(), 19); + expect(shrunkSurface, isNot(same(originalSurface))); + 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. @@ -75,8 +74,8 @@ void testMain() { expect(secondIncrease, same(firstIncrease)); expect(secondIncrease.style.transform, _isTranslate(0, -6)); expect(secondIncreaseSurface, same(firstIncreaseSurface)); - expect(secondIncreaseSurface.width(), 14); - expect(secondIncreaseSurface.height(), 28); + expect(secondIncreaseSurface.width(), 11); + expect(secondIncreaseSurface.height(), 22); // Increases beyond the 40% limit will cause a new allocation. final CkSurface hugeSurface = surface.acquireFrame(const ui.Size(20, 40)).skiaSurface; @@ -101,9 +100,9 @@ void testMain() { expect(shrunk2.style.width, '28px'); expect(shrunk2.style.height, '56px'); expect(shrunk2.style.transform, _isTranslate(0, -41)); - expect(shrunkSurface2, same(hugeSurface)); - expect(shrunkSurface2.width(), 20); - expect(shrunkSurface2.height(), 40); + expect(shrunkSurface2, isNot(same(hugeSurface))); + expect(shrunkSurface2.width(), 5); + expect(shrunkSurface2.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 From 718d2e8c10129579f6d1f3088bc04e5d1fed1502 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 3 Jan 2023 15:00:49 -0800 Subject: [PATCH 10/10] ++ --- lib/web_ui/test/canvaskit/surface_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/web_ui/test/canvaskit/surface_test.dart b/lib/web_ui/test/canvaskit/surface_test.dart index fe6cecd95d0a5..f593c37e20a29 100644 --- a/lib/web_ui/test/canvaskit/surface_test.dart +++ b/lib/web_ui/test/canvaskit/surface_test.dart @@ -73,7 +73,7 @@ void testMain() { final DomCanvasElement secondIncrease = surface.htmlCanvas!; expect(secondIncrease, same(firstIncrease)); expect(secondIncrease.style.transform, _isTranslate(0, -6)); - expect(secondIncreaseSurface, same(firstIncreaseSurface)); + expect(secondIncreaseSurface, isNot(same(firstIncreaseSurface))); expect(secondIncreaseSurface.width(), 11); expect(secondIncreaseSurface.height(), 22);