From 4bad5451e2230d4c8618137054fef08d7adc238b Mon Sep 17 00:00:00 2001 From: ferhatb Date: Thu, 25 Feb 2021 22:53:23 -0800 Subject: [PATCH 1/9] Fix ClipOp.none handling. Fix exactCullRect computation --- lib/web_ui/lib/src/engine/html/clip.dart | 52 +++++++++++++------ lib/web_ui/lib/src/engine/html/offset.dart | 4 +- lib/web_ui/lib/src/engine/html/opacity.dart | 4 +- lib/web_ui/lib/src/engine/html/picture.dart | 19 ++++--- lib/web_ui/lib/src/engine/html/scene.dart | 4 +- lib/web_ui/lib/src/engine/html/surface.dart | 14 ++--- lib/web_ui/lib/src/engine/html/transform.dart | 9 ++-- 7 files changed, 67 insertions(+), 39 deletions(-) diff --git a/lib/web_ui/lib/src/engine/html/clip.dart b/lib/web_ui/lib/src/engine/html/clip.dart index 0b18137acff0d..4d5b6385127ef 100644 --- a/lib/web_ui/lib/src/engine/html/clip.dart +++ b/lib/web_ui/lib/src/engine/html/clip.dart @@ -76,8 +76,12 @@ class PersistedClipRect extends PersistedContainerSurface @override void recomputeTransformAndClip() { _transform = parent!._transform; - _localClipBounds = rect; - _localTransformInverse = null; + if (clipBehavior != ui.Clip.none) { + _localClipBounds = rect; + } else { + _localClipBounds = null; + } + _localTransform = null; _projectedClip = null; } @@ -107,6 +111,7 @@ class PersistedClipRect extends PersistedContainerSurface void update(PersistedClipRect oldSurface) { super.update(oldSurface); if (rect != oldSurface.rect || clipBehavior != oldSurface.clipBehavior) { + _localClipBounds = null; apply(); } } @@ -129,8 +134,12 @@ class PersistedClipRRect extends PersistedContainerSurface @override void recomputeTransformAndClip() { _transform = parent!._transform; - _localClipBounds = rrect.outerRect; - _localTransformInverse = null; + if (clipBehavior != ui.Clip.none) { + _localClipBounds = rrect.outerRect; + } else { + _localClipBounds = null; + } + _localTransform = null; _projectedClip = null; } @@ -165,6 +174,7 @@ class PersistedClipRRect extends PersistedContainerSurface void update(PersistedClipRRect oldSurface) { super.update(oldSurface); if (rrect != oldSurface.rrect || clipBehavior != oldSurface.clipBehavior) { + _localClipBounds = null; apply(); } } @@ -196,18 +206,22 @@ class PersistedPhysicalShape extends PersistedContainerSurface void recomputeTransformAndClip() { _transform = parent!._transform; - final ui.RRect? roundRect = path.toRoundedRect(); - if (roundRect != null) { - _localClipBounds = roundRect.outerRect; - } else { - final ui.Rect? rect = path.toRect(); - if (rect != null) { - _localClipBounds = rect; + if (clipBehavior != ui.Clip.none) { + final ui.RRect? roundRect = path.toRoundedRect(); + if (roundRect != null) { + _localClipBounds = roundRect.outerRect; } else { - _localClipBounds = null; + final ui.Rect? rect = path.toRect(); + if (rect != null) { + _localClipBounds = rect; + } else { + _localClipBounds = null; + } } + } else { + _localClipBounds = null; } - _localTransformInverse = null; + _localTransform = null; _projectedClip = null; } @@ -385,7 +399,11 @@ class PersistedPhysicalShape extends PersistedContainerSurface @override void update(PersistedPhysicalShape oldSurface) { super.update(oldSurface); - if (oldSurface.path != path || oldSurface.elevation != elevation || + bool pathChanged = oldSurface.path != path; + if (pathChanged) { + _localClipBounds = null; + } + if (pathChanged || oldSurface.elevation != elevation || oldSurface.shadowColor != shadowColor || oldSurface.color != color) { oldSurface._clipElement?.remove(); oldSurface._clipElement = null; @@ -433,7 +451,11 @@ class PersistedClipPath extends PersistedContainerSurface @override void recomputeTransformAndClip() { super.recomputeTransformAndClip(); - _localClipBounds ??= clipPath.getBounds(); + if (clipBehavior != ui.Clip.none) { + _localClipBounds ??= clipPath.getBounds(); + } else { + _localClipBounds = null; + } } @override diff --git a/lib/web_ui/lib/src/engine/html/offset.dart b/lib/web_ui/lib/src/engine/html/offset.dart index e1cb4226cf75f..8bde0a6218f17 100644 --- a/lib/web_ui/lib/src/engine/html/offset.dart +++ b/lib/web_ui/lib/src/engine/html/offset.dart @@ -24,12 +24,12 @@ class PersistedOffset extends PersistedContainerSurface _transform!.translate(dx, dy); } _projectedClip = null; - _localTransformInverse = null; + _localTransform = null; } @override Matrix4 get localTransformInverse => - _localTransformInverse ??= Matrix4.translationValues(-dx, -dy, 0); + _localTransform ??= Matrix4.translationValues(dx, dy, 0); @override html.Element createElement() { diff --git a/lib/web_ui/lib/src/engine/html/opacity.dart b/lib/web_ui/lib/src/engine/html/opacity.dart index 126bc5b7fec4a..ce106d6e244e9 100644 --- a/lib/web_ui/lib/src/engine/html/opacity.dart +++ b/lib/web_ui/lib/src/engine/html/opacity.dart @@ -25,12 +25,12 @@ class PersistedOpacity extends PersistedContainerSurface _transform = _transform!.clone(); _transform!.translate(dx, dy); } - _localTransformInverse = null; + _localTransform = null; _projectedClip = null; } @override - Matrix4 get localTransformInverse => _localTransformInverse ??= + Matrix4 get localTransformInverse => _localTransform ??= Matrix4.translationValues(-offset.dx, -offset.dy, 0); @override diff --git a/lib/web_ui/lib/src/engine/html/picture.dart b/lib/web_ui/lib/src/engine/html/picture.dart index 6fcdec940ecc8..2e7505ead3719 100644 --- a/lib/web_ui/lib/src/engine/html/picture.dart +++ b/lib/web_ui/lib/src/engine/html/picture.dart @@ -174,7 +174,13 @@ class PersistedPicture extends PersistedLeafSurface { ui.Rect? bounds; PersistedSurface? parentSurface = parent; final Matrix4 clipTransform = Matrix4.identity(); - while (parentSurface != null) { + final List surfaceList = []; + while(parentSurface != null) { + surfaceList.add(parentSurface); + parentSurface = parentSurface.parent; + } + for (int i = surfaceList.length - 1; i >= 0; i--) { + parentSurface = surfaceList[i]; final ui.Rect? localClipBounds = parentSurface._localClipBounds; if (localClipBounds != null) { if (bounds == null) { @@ -184,11 +190,10 @@ class PersistedPicture extends PersistedLeafSurface { bounds.intersect(transformRect(clipTransform, localClipBounds)); } } - final Matrix4? localInverse = parentSurface.localTransformInverse; - if (localInverse != null && !localInverse.isIdentity()) { - clipTransform.multiply(localInverse); + final Matrix4? localTransform = parentSurface.localTransform; + if (localTransform != null && !localTransform.isIdentity()) { + clipTransform.multiply(localTransform); } - parentSurface = parentSurface.parent; } if (bounds != null && (bounds.width <= 0 || bounds.height <= 0)) { bounds = ui.Rect.zero; @@ -201,8 +206,10 @@ class PersistedPicture extends PersistedLeafSurface { if (parent!._projectedClip == null) { _exactLocalCullRect = localPaintBounds; } else { + Matrix4? invertedTransform = Matrix4.tryInvert(parent!.transform!); _exactLocalCullRect = - localPaintBounds!.intersect(parent!._projectedClip!); + localPaintBounds!.intersect(transformRect(invertedTransform!, parent!._projectedClip!)); + _exactLocalCullRect = localPaintBounds; } if (_exactLocalCullRect!.width <= 0 || _exactLocalCullRect!.height <= 0) { _exactLocalCullRect = ui.Rect.zero; diff --git a/lib/web_ui/lib/src/engine/html/scene.dart b/lib/web_ui/lib/src/engine/html/scene.dart index 9e8a9e1159010..5a1fb73a57ebe 100644 --- a/lib/web_ui/lib/src/engine/html/scene.dart +++ b/lib/web_ui/lib/src/engine/html/scene.dart @@ -41,12 +41,12 @@ class PersistedScene extends PersistedContainerSurface { final double screenWidth = html.window.innerWidth!.toDouble(); final double screenHeight = html.window.innerHeight!.toDouble(); _localClipBounds = ui.Rect.fromLTRB(0, 0, screenWidth, screenHeight); - _localTransformInverse = Matrix4.identity(); + _localTransform = Matrix4.identity(); _projectedClip = null; } @override - Matrix4? get localTransformInverse => _localTransformInverse; + Matrix4? get localTransform => _localTransform; @override html.Element createElement() { diff --git a/lib/web_ui/lib/src/engine/html/surface.dart b/lib/web_ui/lib/src/engine/html/surface.dart index d1d816d29c4d8..1d13727062643 100644 --- a/lib/web_ui/lib/src/engine/html/surface.dart +++ b/lib/web_ui/lib/src/engine/html/surface.dart @@ -521,16 +521,16 @@ abstract class PersistedSurface implements ui.EngineLayer { /// Bounds of clipping performed by this layer. ui.Rect? _localClipBounds; - // Cached inverse of transform on this node. Unlike transform, this + // Cached transform on this node. Unlike transform, this // Matrix only contains local transform (not chain multiplied since root). - Matrix4? _localTransformInverse; + Matrix4? _localTransform; - /// The inverse of the local transform that this surface applies to its children. + /// The local transform that this surface applies to its children. /// /// The default implementation is identity transform. Concrete /// implementations may override this getter to supply a different transform. - Matrix4? get localTransformInverse => - _localTransformInverse ??= Matrix4.identity(); + Matrix4? get localTransform => + _localTransform ??= Matrix4.identity(); /// Recomputes [transform] and [globalClip] fields. /// @@ -542,7 +542,7 @@ abstract class PersistedSurface implements ui.EngineLayer { void recomputeTransformAndClip() { _transform = parent!._transform; _localClipBounds = null; - _localTransformInverse = null; + _localTransform = null; _projectedClip = null; } @@ -648,7 +648,7 @@ abstract class PersistedContainerSurface extends PersistedSurface { void recomputeTransformAndClip() { _transform = parent!._transform; _localClipBounds = null; - _localTransformInverse = null; + _localTransform = null; _projectedClip = null; } diff --git a/lib/web_ui/lib/src/engine/html/transform.dart b/lib/web_ui/lib/src/engine/html/transform.dart index ea540dd655b23..0d29b28de8b87 100644 --- a/lib/web_ui/lib/src/engine/html/transform.dart +++ b/lib/web_ui/lib/src/engine/html/transform.dart @@ -16,15 +16,14 @@ class PersistedTransform extends PersistedContainerSurface @override void recomputeTransformAndClip() { _transform = parent!._transform!.multiplied(Matrix4.fromFloat32List(matrix4)); - _localTransformInverse = null; + _localTransform = null; _projectedClip = null; } @override - Matrix4? get localTransformInverse { - _localTransformInverse ??= - Matrix4.tryInvert(Matrix4.fromFloat32List(matrix4)); - return _localTransformInverse; + Matrix4? get localTransform { + _localTransform ??= Matrix4.fromFloat32List(matrix4); + return _localTransform; } @override From 63bb29b9cab60b3943757dbcbe28a2e169a4c37f Mon Sep 17 00:00:00 2001 From: ferhatb Date: Mon, 1 Mar 2021 15:25:28 -0800 Subject: [PATCH 2/9] Change exact cull rect computation. Fix non-homogenous matrix failure. --- lib/web_ui/lib/src/engine/html/clip.dart | 6 +-- lib/web_ui/lib/src/engine/html/offset.dart | 4 +- lib/web_ui/lib/src/engine/html/opacity.dart | 2 +- lib/web_ui/lib/src/engine/html/picture.dart | 16 ++++--- lib/web_ui/lib/src/engine/html/scene.dart | 2 +- lib/web_ui/lib/src/engine/html/surface.dart | 9 ++-- lib/web_ui/lib/src/engine/html/transform.dart | 2 +- lib/web_ui/lib/src/engine/util.dart | 11 +++-- .../engine/surface/scene_builder_test.dart | 46 +++++++++++++++++-- 9 files changed, 74 insertions(+), 24 deletions(-) diff --git a/lib/web_ui/lib/src/engine/html/clip.dart b/lib/web_ui/lib/src/engine/html/clip.dart index 4d5b6385127ef..c5ea4b823830d 100644 --- a/lib/web_ui/lib/src/engine/html/clip.dart +++ b/lib/web_ui/lib/src/engine/html/clip.dart @@ -82,7 +82,7 @@ class PersistedClipRect extends PersistedContainerSurface _localClipBounds = null; } _localTransform = null; - _projectedClip = null; + _globalProjectedClip = null; } @override @@ -140,7 +140,7 @@ class PersistedClipRRect extends PersistedContainerSurface _localClipBounds = null; } _localTransform = null; - _projectedClip = null; + _globalProjectedClip = null; } @override @@ -222,7 +222,7 @@ class PersistedPhysicalShape extends PersistedContainerSurface _localClipBounds = null; } _localTransform = null; - _projectedClip = null; + _globalProjectedClip = null; } void _applyColor() { diff --git a/lib/web_ui/lib/src/engine/html/offset.dart b/lib/web_ui/lib/src/engine/html/offset.dart index 8bde0a6218f17..2d94354ee0ea6 100644 --- a/lib/web_ui/lib/src/engine/html/offset.dart +++ b/lib/web_ui/lib/src/engine/html/offset.dart @@ -23,12 +23,12 @@ class PersistedOffset extends PersistedContainerSurface _transform = _transform!.clone(); _transform!.translate(dx, dy); } - _projectedClip = null; + _globalProjectedClip = null; _localTransform = null; } @override - Matrix4 get localTransformInverse => + Matrix4 get localTransform => _localTransform ??= Matrix4.translationValues(dx, dy, 0); @override diff --git a/lib/web_ui/lib/src/engine/html/opacity.dart b/lib/web_ui/lib/src/engine/html/opacity.dart index ce106d6e244e9..417d79e44784b 100644 --- a/lib/web_ui/lib/src/engine/html/opacity.dart +++ b/lib/web_ui/lib/src/engine/html/opacity.dart @@ -26,7 +26,7 @@ class PersistedOpacity extends PersistedContainerSurface _transform!.translate(dx, dy); } _localTransform = null; - _projectedClip = null; + _globalProjectedClip = null; } @override diff --git a/lib/web_ui/lib/src/engine/html/picture.dart b/lib/web_ui/lib/src/engine/html/picture.dart index 2e7505ead3719..40d363dcab91a 100644 --- a/lib/web_ui/lib/src/engine/html/picture.dart +++ b/lib/web_ui/lib/src/engine/html/picture.dart @@ -167,7 +167,7 @@ class PersistedPicture extends PersistedLeafSurface { assert(transform != null); assert(localPaintBounds != null); - if (parent!._projectedClip == null) { + if (parent!._globalProjectedClip == null) { // Compute and cache chain of clipping bounds on parent of picture since // parent may include multiple pictures so it can be reused by all // child pictures. @@ -199,17 +199,21 @@ class PersistedPicture extends PersistedLeafSurface { bounds = ui.Rect.zero; } // Cache projected clip on parent. - parent!._projectedClip = bounds; + parent!._globalProjectedClip = bounds; } // Intersect localPaintBounds with parent projected clip to calculate // and cache [_exactLocalCullRect]. - if (parent!._projectedClip == null) { + if (parent!._globalProjectedClip == null) { _exactLocalCullRect = localPaintBounds; } else { Matrix4? invertedTransform = Matrix4.tryInvert(parent!.transform!); - _exactLocalCullRect = - localPaintBounds!.intersect(transformRect(invertedTransform!, parent!._projectedClip!)); - _exactLocalCullRect = localPaintBounds; + if (invertedTransform == null) { + _exactLocalCullRect = localPaintBounds; + } else { + _exactLocalCullRect = + localPaintBounds!.translate(dx, dy).intersect(transformRect( + invertedTransform, parent!._globalProjectedClip!)); + } } if (_exactLocalCullRect!.width <= 0 || _exactLocalCullRect!.height <= 0) { _exactLocalCullRect = ui.Rect.zero; diff --git a/lib/web_ui/lib/src/engine/html/scene.dart b/lib/web_ui/lib/src/engine/html/scene.dart index 5a1fb73a57ebe..07770187fda1d 100644 --- a/lib/web_ui/lib/src/engine/html/scene.dart +++ b/lib/web_ui/lib/src/engine/html/scene.dart @@ -42,7 +42,7 @@ class PersistedScene extends PersistedContainerSurface { final double screenHeight = html.window.innerHeight!.toDouble(); _localClipBounds = ui.Rect.fromLTRB(0, 0, screenWidth, screenHeight); _localTransform = Matrix4.identity(); - _projectedClip = null; + _globalProjectedClip = null; } @override diff --git a/lib/web_ui/lib/src/engine/html/surface.dart b/lib/web_ui/lib/src/engine/html/surface.dart index 1d13727062643..a75c92ef0a5df 100644 --- a/lib/web_ui/lib/src/engine/html/surface.dart +++ b/lib/web_ui/lib/src/engine/html/surface.dart @@ -516,8 +516,11 @@ abstract class PersistedSurface implements ui.EngineLayer { /// This value is the intersection of clips in the ancestor chain, including /// the clip added by this layer (if any). /// + /// This rectangle is in global coordinates obtained by transforming + /// stack of clip rects to obtain optimal cull rectangle. + /// /// The value is update by [recomputeTransformAndClip]. - ui.Rect? _projectedClip; + ui.Rect? _globalProjectedClip; /// Bounds of clipping performed by this layer. ui.Rect? _localClipBounds; @@ -543,7 +546,7 @@ abstract class PersistedSurface implements ui.EngineLayer { _transform = parent!._transform; _localClipBounds = null; _localTransform = null; - _projectedClip = null; + _globalProjectedClip = null; } /// Performs computations before [build], [update], or [retain] are called. @@ -649,7 +652,7 @@ abstract class PersistedContainerSurface extends PersistedSurface { _transform = parent!._transform; _localClipBounds = null; _localTransform = null; - _projectedClip = null; + _globalProjectedClip = null; } @override diff --git a/lib/web_ui/lib/src/engine/html/transform.dart b/lib/web_ui/lib/src/engine/html/transform.dart index 0d29b28de8b87..176b6afff43a5 100644 --- a/lib/web_ui/lib/src/engine/html/transform.dart +++ b/lib/web_ui/lib/src/engine/html/transform.dart @@ -17,7 +17,7 @@ class PersistedTransform extends PersistedContainerSurface void recomputeTransformAndClip() { _transform = parent!._transform!.multiplied(Matrix4.fromFloat32List(matrix4)); _localTransform = null; - _projectedClip = null; + _globalProjectedClip = null; } @override diff --git a/lib/web_ui/lib/src/engine/util.dart b/lib/web_ui/lib/src/engine/util.dart index e422d6661018b..15aea62de6f39 100644 --- a/lib/web_ui/lib/src/engine/util.dart +++ b/lib/web_ui/lib/src/engine/util.dart @@ -278,22 +278,25 @@ void transformLTRB(Matrix4 transform, Float32List ltrb) { _tempPointMatrix.multiplyTranspose(transform); + // Handle non-homogenous matrices. + double w = transform[15]; + ltrb[0] = math.min( math.min( math.min(_tempPointData[0], _tempPointData[1]), _tempPointData[2]), - _tempPointData[3]); + _tempPointData[3]) / w; ltrb[1] = math.min( math.min( math.min(_tempPointData[4], _tempPointData[5]), _tempPointData[6]), - _tempPointData[7]); + _tempPointData[7]) / w; ltrb[2] = math.max( math.max( math.max(_tempPointData[0], _tempPointData[1]), _tempPointData[2]), - _tempPointData[3]); + _tempPointData[3]) / w; ltrb[3] = math.max( math.max( math.max(_tempPointData[4], _tempPointData[5]), _tempPointData[6]), - _tempPointData[7]); + _tempPointData[7]) / w; } /// Returns true if [rect] contains every point that is also contained by the 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 24f5e7545389a..cc8e700868b89 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,8 @@ 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); @@ -328,6 +329,44 @@ void testMain() { } }); + test('does not skip painting picture when picture is inside transform', () async { + final Picture picture = _drawPicture(); + // Picture should not be clipped out since transform will offset it to 500,500 + final SurfaceSceneBuilder builder = SurfaceSceneBuilder(); + builder.pushOffset(0, 0); + final PersistedContainerSurface clip = builder.pushClipRect(const Rect.fromLTRB(0, 0, 1000, 1000)) as PersistedContainerSurface; + builder.pushTransform((Matrix4.identity()..scale(0.5, 0.5)).toFloat64()); + builder.addPicture(Offset(1000, 1000), picture); + builder.pop(); + builder.pop(); + builder.pop(); + html.HtmlElement content = builder.build().webOnlyRootElement; + expect(content.querySelectorAll('flt-picture').single.children, isNotEmpty); + }); + + test( + 'skips painting picture when picture fully clipped out with transform and offset', () async { + final Picture picture = _drawPicture(); + // Picture should not be clipped out since transform will offset it to 500,500 + final SurfaceSceneBuilder builder = SurfaceSceneBuilder(); + builder.pushOffset(50, 50); + final PersistedContainerSurface clip = builder.pushClipRect( + const Rect.fromLTRB(0, 0, 1000, 1000)) as PersistedContainerSurface; + builder.pushTransform((Matrix4.identity() + ..scale(2, 2)).toFloat64()); + builder.addPicture(Offset(500, 500), picture); + builder.pop(); + builder.pop(); + builder.pop(); + html.HtmlElement content = builder + .build() + .webOnlyRootElement; + expect(content + .querySelectorAll('flt-picture') + .single + .children, isEmpty); + }); + test('releases old canvas when picture is fully clipped out after addRetained', () async { final Picture picture = _drawPicture(); @@ -416,8 +455,8 @@ void testMain() { paragraph.layout(ParagraphConstraints(width: 1000)); canvas.drawParagraph(paragraph, Offset.zero); final EngineLayer newLayer = useOffset - ? builder.pushOffset(0, 0, oldLayer: oldLayer) - : builder.pushOpacity(100, oldLayer: oldLayer); + ? builder.pushOffset(0, 0, oldLayer: oldLayer) + : builder.pushOpacity(100, oldLayer: oldLayer); builder.addPicture(Offset.zero, recorder.endRecording()); builder.pop(); return newLayer; @@ -758,6 +797,7 @@ class MockPersistedPicture extends PersistedPicture { int get bitmapPixelCount => 0; } +/// Draw 4 circles within 50, 50, 120, 120 bounds Picture _drawPicture() { const double offsetX = 50; const double offsetY = 50; From ffb462f6e1d881411638ce472ddff77404fb51f3 Mon Sep 17 00:00:00 2001 From: ferhatb Date: Mon, 1 Mar 2021 16:36:51 -0800 Subject: [PATCH 3/9] Fix more localTransformInverse overrides --- lib/web_ui/lib/src/engine/html/opacity.dart | 4 ++-- lib/web_ui/lib/src/engine/html/picture.dart | 2 +- lib/web_ui/lib/src/engine/html/platform_view.dart | 2 +- lib/web_ui/test/engine/surface/scene_builder_test.dart | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/web_ui/lib/src/engine/html/opacity.dart b/lib/web_ui/lib/src/engine/html/opacity.dart index 417d79e44784b..8a6544be853b4 100644 --- a/lib/web_ui/lib/src/engine/html/opacity.dart +++ b/lib/web_ui/lib/src/engine/html/opacity.dart @@ -30,8 +30,8 @@ class PersistedOpacity extends PersistedContainerSurface } @override - Matrix4 get localTransformInverse => _localTransform ??= - Matrix4.translationValues(-offset.dx, -offset.dy, 0); + Matrix4 get localTransform => _localTransform ??= + Matrix4.translationValues(offset.dx, offset.dy, 0); @override html.Element createElement() { diff --git a/lib/web_ui/lib/src/engine/html/picture.dart b/lib/web_ui/lib/src/engine/html/picture.dart index 40d363dcab91a..92f3cc6dc7dbc 100644 --- a/lib/web_ui/lib/src/engine/html/picture.dart +++ b/lib/web_ui/lib/src/engine/html/picture.dart @@ -416,7 +416,7 @@ class PersistedPicture extends PersistedLeafSurface { } @override - Matrix4? get localTransformInverse => null; + Matrix4? get localTransform => null; void applyPaint(EngineCanvas? oldCanvas) { if (picture.recordingCanvas!.renderStrategy.hasArbitraryPaint) { diff --git a/lib/web_ui/lib/src/engine/html/platform_view.dart b/lib/web_ui/lib/src/engine/html/platform_view.dart index 5987fe2f840d1..da10d216e82a4 100644 --- a/lib/web_ui/lib/src/engine/html/platform_view.dart +++ b/lib/web_ui/lib/src/engine/html/platform_view.dart @@ -58,7 +58,7 @@ class PersistedPlatformView extends PersistedLeafSurface { } @override - Matrix4? get localTransformInverse => null; + Matrix4? get localTransform => null; @override void apply() { 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 cc8e700868b89..d3bf3f635530d 100644 --- a/lib/web_ui/test/engine/surface/scene_builder_test.dart +++ b/lib/web_ui/test/engine/surface/scene_builder_test.dart @@ -334,7 +334,7 @@ void testMain() { // Picture should not be clipped out since transform will offset it to 500,500 final SurfaceSceneBuilder builder = SurfaceSceneBuilder(); builder.pushOffset(0, 0); - final PersistedContainerSurface clip = builder.pushClipRect(const Rect.fromLTRB(0, 0, 1000, 1000)) as PersistedContainerSurface; + builder.pushClipRect(const Rect.fromLTRB(0, 0, 1000, 1000)) as PersistedContainerSurface; builder.pushTransform((Matrix4.identity()..scale(0.5, 0.5)).toFloat64()); builder.addPicture(Offset(1000, 1000), picture); builder.pop(); @@ -350,7 +350,7 @@ void testMain() { // Picture should not be clipped out since transform will offset it to 500,500 final SurfaceSceneBuilder builder = SurfaceSceneBuilder(); builder.pushOffset(50, 50); - final PersistedContainerSurface clip = builder.pushClipRect( + builder.pushClipRect( const Rect.fromLTRB(0, 0, 1000, 1000)) as PersistedContainerSurface; builder.pushTransform((Matrix4.identity() ..scale(2, 2)).toFloat64()); From d4acab5695ecde0b21d6a1b27296e474d60ee793 Mon Sep 17 00:00:00 2001 From: ferhatb Date: Mon, 1 Mar 2021 17:05:18 -0800 Subject: [PATCH 4/9] Fix test override --- lib/web_ui/test/engine/surface/scene_builder_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 d3bf3f635530d..44c22724405bf 100644 --- a/lib/web_ui/test/engine/surface/scene_builder_test.dart +++ b/lib/web_ui/test/engine/surface/scene_builder_test.dart @@ -768,7 +768,7 @@ class MockPersistedPicture extends PersistedPicture { } @override - Matrix4 get localTransformInverse => null; + Matrix4 get localTransform => null; @override void build() { From 0de1eb029651f8bc14f7f535d27cd4cf3d192fa1 Mon Sep 17 00:00:00 2001 From: ferhatb Date: Tue, 2 Mar 2021 11:15:45 -0800 Subject: [PATCH 5/9] Update canvaskit test since we handle perspective transform correctly now --- lib/web_ui/test/canvaskit/layer_test.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/web_ui/test/canvaskit/layer_test.dart b/lib/web_ui/test/canvaskit/layer_test.dart index 81b86c086b78e..05011521ad834 100644 --- a/lib/web_ui/test/canvaskit/layer_test.dart +++ b/lib/web_ui/test/canvaskit/layer_test.dart @@ -26,8 +26,8 @@ void testMain() { ui.window.platformDispatcher as EnginePlatformDispatcher; final CkPicture picture = - paintPicture(ui.Rect.fromLTRB(0, 0, 30, 30), (CkCanvas canvas) { - canvas.drawRect(ui.Rect.fromLTRB(0, 0, 30, 30), + paintPicture(ui.Rect.fromLTRB(0, 0, 60, 60), (CkCanvas canvas) { + canvas.drawRect(ui.Rect.fromLTRB(0, 0, 60, 60), CkPaint()..style = ui.PaintingStyle.fill); }); From 2a0d9df99edc76ccc237404258178a0e34d1c113 Mon Sep 17 00:00:00 2001 From: ferhatb Date: Thu, 18 Mar 2021 16:38:53 -0700 Subject: [PATCH 6/9] revert persist transform changed --- lib/web_ui/lib/src/engine/html/clip.dart | 36 +++++++++++-------- lib/web_ui/lib/src/engine/html/offset.dart | 8 ++--- lib/web_ui/lib/src/engine/html/opacity.dart | 8 ++--- lib/web_ui/lib/src/engine/html/picture.dart | 33 ++++++----------- .../lib/src/engine/html/platform_view.dart | 2 +- lib/web_ui/lib/src/engine/html/scene.dart | 6 ++-- lib/web_ui/lib/src/engine/html/surface.dart | 23 ++++++------ lib/web_ui/lib/src/engine/html/transform.dart | 11 +++--- 8 files changed, 60 insertions(+), 67 deletions(-) diff --git a/lib/web_ui/lib/src/engine/html/clip.dart b/lib/web_ui/lib/src/engine/html/clip.dart index c5ea4b823830d..ca0254b8e63be 100644 --- a/lib/web_ui/lib/src/engine/html/clip.dart +++ b/lib/web_ui/lib/src/engine/html/clip.dart @@ -81,8 +81,8 @@ class PersistedClipRect extends PersistedContainerSurface } else { _localClipBounds = null; } - _localTransform = null; - _globalProjectedClip = null; + _localTransformInverse = null; + _projectedClip = null; } @override @@ -139,8 +139,8 @@ class PersistedClipRRect extends PersistedContainerSurface } else { _localClipBounds = null; } - _localTransform = null; - _globalProjectedClip = null; + _localTransformInverse = null; + _projectedClip = null; } @override @@ -221,8 +221,8 @@ class PersistedPhysicalShape extends PersistedContainerSurface } else { _localClipBounds = null; } - _localTransform = null; - _globalProjectedClip = null; + _localTransformInverse = null; + _projectedClip = null; } void _applyColor() { @@ -337,6 +337,7 @@ class PersistedPhysicalShape extends PersistedContainerSurface offsetY: 0.0, scaleX: 1.0 / pathBounds.right, scaleY: 1.0 / pathBounds.bottom); + /// If apply is called multiple times (without update), remove prior /// svg clip and render elements. _clipElement?.remove(); @@ -377,20 +378,23 @@ class PersistedPhysicalShape extends PersistedContainerSurface final ui.Rect pathBounds2 = path.getBounds(); _svgElement = _pathToSvgElement( - path, SurfacePaintData() + path, + SurfacePaintData() ..style = ui.PaintingStyle.fill - ..color = color, '${pathBounds2.right}', '${pathBounds2.bottom}'); + ..color = color, + '${pathBounds2.right}', + '${pathBounds2.bottom}'); + /// Render element behind the clipped content. rootElement!.insertBefore(_svgElement!, childContainer); final SurfaceShadowData shadow = computeShadow(pathBounds, elevation)!; final ui.Color boxShadowColor = toShadowColor(shadowColor); _svgElement!.style - ..filter = - 'drop-shadow(${shadow.offset.dx}px ${shadow.offset.dy}px ' - '${shadow.blurWidth}px ' - 'rgba(${boxShadowColor.red}, ${boxShadowColor.green}, ' - '${boxShadowColor.blue}, ${boxShadowColor.alpha / 255}))' + ..filter = 'drop-shadow(${shadow.offset.dx}px ${shadow.offset.dy}px ' + '${shadow.blurWidth}px ' + 'rgba(${boxShadowColor.red}, ${boxShadowColor.green}, ' + '${boxShadowColor.blue}, ${boxShadowColor.alpha / 255}))' ..transform = 'translate(-${pathBounds2.left}px, -${pathBounds2.top}px)'; rootElement!.style.backgroundColor = ''; @@ -403,8 +407,10 @@ class PersistedPhysicalShape extends PersistedContainerSurface if (pathChanged) { _localClipBounds = null; } - if (pathChanged || oldSurface.elevation != elevation || - oldSurface.shadowColor != shadowColor || oldSurface.color != color) { + if (pathChanged || + oldSurface.elevation != elevation || + oldSurface.shadowColor != shadowColor || + oldSurface.color != color) { oldSurface._clipElement?.remove(); oldSurface._clipElement = null; oldSurface._svgElement?.remove(); diff --git a/lib/web_ui/lib/src/engine/html/offset.dart b/lib/web_ui/lib/src/engine/html/offset.dart index 2d94354ee0ea6..e1cb4226cf75f 100644 --- a/lib/web_ui/lib/src/engine/html/offset.dart +++ b/lib/web_ui/lib/src/engine/html/offset.dart @@ -23,13 +23,13 @@ class PersistedOffset extends PersistedContainerSurface _transform = _transform!.clone(); _transform!.translate(dx, dy); } - _globalProjectedClip = null; - _localTransform = null; + _projectedClip = null; + _localTransformInverse = null; } @override - Matrix4 get localTransform => - _localTransform ??= Matrix4.translationValues(dx, dy, 0); + Matrix4 get localTransformInverse => + _localTransformInverse ??= Matrix4.translationValues(-dx, -dy, 0); @override html.Element createElement() { diff --git a/lib/web_ui/lib/src/engine/html/opacity.dart b/lib/web_ui/lib/src/engine/html/opacity.dart index 8a6544be853b4..126bc5b7fec4a 100644 --- a/lib/web_ui/lib/src/engine/html/opacity.dart +++ b/lib/web_ui/lib/src/engine/html/opacity.dart @@ -25,13 +25,13 @@ class PersistedOpacity extends PersistedContainerSurface _transform = _transform!.clone(); _transform!.translate(dx, dy); } - _localTransform = null; - _globalProjectedClip = null; + _localTransformInverse = null; + _projectedClip = null; } @override - Matrix4 get localTransform => _localTransform ??= - Matrix4.translationValues(offset.dx, offset.dy, 0); + Matrix4 get localTransformInverse => _localTransformInverse ??= + Matrix4.translationValues(-offset.dx, -offset.dy, 0); @override html.Element createElement() { diff --git a/lib/web_ui/lib/src/engine/html/picture.dart b/lib/web_ui/lib/src/engine/html/picture.dart index 92f3cc6dc7dbc..6fcdec940ecc8 100644 --- a/lib/web_ui/lib/src/engine/html/picture.dart +++ b/lib/web_ui/lib/src/engine/html/picture.dart @@ -167,20 +167,14 @@ class PersistedPicture extends PersistedLeafSurface { assert(transform != null); assert(localPaintBounds != null); - if (parent!._globalProjectedClip == null) { + if (parent!._projectedClip == null) { // Compute and cache chain of clipping bounds on parent of picture since // parent may include multiple pictures so it can be reused by all // child pictures. ui.Rect? bounds; PersistedSurface? parentSurface = parent; final Matrix4 clipTransform = Matrix4.identity(); - final List surfaceList = []; - while(parentSurface != null) { - surfaceList.add(parentSurface); - parentSurface = parentSurface.parent; - } - for (int i = surfaceList.length - 1; i >= 0; i--) { - parentSurface = surfaceList[i]; + while (parentSurface != null) { final ui.Rect? localClipBounds = parentSurface._localClipBounds; if (localClipBounds != null) { if (bounds == null) { @@ -190,30 +184,25 @@ class PersistedPicture extends PersistedLeafSurface { bounds.intersect(transformRect(clipTransform, localClipBounds)); } } - final Matrix4? localTransform = parentSurface.localTransform; - if (localTransform != null && !localTransform.isIdentity()) { - clipTransform.multiply(localTransform); + final Matrix4? localInverse = parentSurface.localTransformInverse; + if (localInverse != null && !localInverse.isIdentity()) { + clipTransform.multiply(localInverse); } + parentSurface = parentSurface.parent; } if (bounds != null && (bounds.width <= 0 || bounds.height <= 0)) { bounds = ui.Rect.zero; } // Cache projected clip on parent. - parent!._globalProjectedClip = bounds; + parent!._projectedClip = bounds; } // Intersect localPaintBounds with parent projected clip to calculate // and cache [_exactLocalCullRect]. - if (parent!._globalProjectedClip == null) { + if (parent!._projectedClip == null) { _exactLocalCullRect = localPaintBounds; } else { - Matrix4? invertedTransform = Matrix4.tryInvert(parent!.transform!); - if (invertedTransform == null) { - _exactLocalCullRect = localPaintBounds; - } else { - _exactLocalCullRect = - localPaintBounds!.translate(dx, dy).intersect(transformRect( - invertedTransform, parent!._globalProjectedClip!)); - } + _exactLocalCullRect = + localPaintBounds!.intersect(parent!._projectedClip!); } if (_exactLocalCullRect!.width <= 0 || _exactLocalCullRect!.height <= 0) { _exactLocalCullRect = ui.Rect.zero; @@ -416,7 +405,7 @@ class PersistedPicture extends PersistedLeafSurface { } @override - Matrix4? get localTransform => null; + Matrix4? get localTransformInverse => null; void applyPaint(EngineCanvas? oldCanvas) { if (picture.recordingCanvas!.renderStrategy.hasArbitraryPaint) { diff --git a/lib/web_ui/lib/src/engine/html/platform_view.dart b/lib/web_ui/lib/src/engine/html/platform_view.dart index f2d52ccb8154a..d990c6b011905 100644 --- a/lib/web_ui/lib/src/engine/html/platform_view.dart +++ b/lib/web_ui/lib/src/engine/html/platform_view.dart @@ -58,7 +58,7 @@ class PersistedPlatformView extends PersistedLeafSurface { } @override - Matrix4? get localTransform => null; + Matrix4? get localTransformInverse => null; @override void apply() { diff --git a/lib/web_ui/lib/src/engine/html/scene.dart b/lib/web_ui/lib/src/engine/html/scene.dart index 07770187fda1d..9e8a9e1159010 100644 --- a/lib/web_ui/lib/src/engine/html/scene.dart +++ b/lib/web_ui/lib/src/engine/html/scene.dart @@ -41,12 +41,12 @@ class PersistedScene extends PersistedContainerSurface { final double screenWidth = html.window.innerWidth!.toDouble(); final double screenHeight = html.window.innerHeight!.toDouble(); _localClipBounds = ui.Rect.fromLTRB(0, 0, screenWidth, screenHeight); - _localTransform = Matrix4.identity(); - _globalProjectedClip = null; + _localTransformInverse = Matrix4.identity(); + _projectedClip = null; } @override - Matrix4? get localTransform => _localTransform; + Matrix4? get localTransformInverse => _localTransformInverse; @override html.Element createElement() { diff --git a/lib/web_ui/lib/src/engine/html/surface.dart b/lib/web_ui/lib/src/engine/html/surface.dart index a75c92ef0a5df..d1d816d29c4d8 100644 --- a/lib/web_ui/lib/src/engine/html/surface.dart +++ b/lib/web_ui/lib/src/engine/html/surface.dart @@ -516,24 +516,21 @@ abstract class PersistedSurface implements ui.EngineLayer { /// This value is the intersection of clips in the ancestor chain, including /// the clip added by this layer (if any). /// - /// This rectangle is in global coordinates obtained by transforming - /// stack of clip rects to obtain optimal cull rectangle. - /// /// The value is update by [recomputeTransformAndClip]. - ui.Rect? _globalProjectedClip; + ui.Rect? _projectedClip; /// Bounds of clipping performed by this layer. ui.Rect? _localClipBounds; - // Cached transform on this node. Unlike transform, this + // Cached inverse of transform on this node. Unlike transform, this // Matrix only contains local transform (not chain multiplied since root). - Matrix4? _localTransform; + Matrix4? _localTransformInverse; - /// The local transform that this surface applies to its children. + /// The inverse of the local transform that this surface applies to its children. /// /// The default implementation is identity transform. Concrete /// implementations may override this getter to supply a different transform. - Matrix4? get localTransform => - _localTransform ??= Matrix4.identity(); + Matrix4? get localTransformInverse => + _localTransformInverse ??= Matrix4.identity(); /// Recomputes [transform] and [globalClip] fields. /// @@ -545,8 +542,8 @@ abstract class PersistedSurface implements ui.EngineLayer { void recomputeTransformAndClip() { _transform = parent!._transform; _localClipBounds = null; - _localTransform = null; - _globalProjectedClip = null; + _localTransformInverse = null; + _projectedClip = null; } /// Performs computations before [build], [update], or [retain] are called. @@ -651,8 +648,8 @@ abstract class PersistedContainerSurface extends PersistedSurface { void recomputeTransformAndClip() { _transform = parent!._transform; _localClipBounds = null; - _localTransform = null; - _globalProjectedClip = null; + _localTransformInverse = null; + _projectedClip = null; } @override diff --git a/lib/web_ui/lib/src/engine/html/transform.dart b/lib/web_ui/lib/src/engine/html/transform.dart index 176b6afff43a5..ea540dd655b23 100644 --- a/lib/web_ui/lib/src/engine/html/transform.dart +++ b/lib/web_ui/lib/src/engine/html/transform.dart @@ -16,14 +16,15 @@ class PersistedTransform extends PersistedContainerSurface @override void recomputeTransformAndClip() { _transform = parent!._transform!.multiplied(Matrix4.fromFloat32List(matrix4)); - _localTransform = null; - _globalProjectedClip = null; + _localTransformInverse = null; + _projectedClip = null; } @override - Matrix4? get localTransform { - _localTransform ??= Matrix4.fromFloat32List(matrix4); - return _localTransform; + Matrix4? get localTransformInverse { + _localTransformInverse ??= + Matrix4.tryInvert(Matrix4.fromFloat32List(matrix4)); + return _localTransformInverse; } @override From e829a20071c376bbc170e4b3e0fe61325206d51b Mon Sep 17 00:00:00 2001 From: ferhatb Date: Thu, 18 Mar 2021 16:58:40 -0700 Subject: [PATCH 7/9] update test to use pushOffset --- lib/web_ui/test/engine/surface/scene_builder_test.dart | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) 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 44c22724405bf..4a6b297567fdc 100644 --- a/lib/web_ui/test/engine/surface/scene_builder_test.dart +++ b/lib/web_ui/test/engine/surface/scene_builder_test.dart @@ -347,14 +347,16 @@ void testMain() { test( 'skips painting picture when picture fully clipped out with transform and offset', () async { final Picture picture = _drawPicture(); - // Picture should not be clipped out since transform will offset it to 500,500 + // Picture should be clipped out since transform will offset it to 500,500 final SurfaceSceneBuilder builder = SurfaceSceneBuilder(); builder.pushOffset(50, 50); builder.pushClipRect( const Rect.fromLTRB(0, 0, 1000, 1000)) as PersistedContainerSurface; builder.pushTransform((Matrix4.identity() ..scale(2, 2)).toFloat64()); - builder.addPicture(Offset(500, 500), picture); + builder.pushOffset(500, 500); + builder.addPicture(Offset.zero, picture); + builder.pop(); builder.pop(); builder.pop(); builder.pop(); @@ -768,7 +770,7 @@ class MockPersistedPicture extends PersistedPicture { } @override - Matrix4 get localTransform => null; + Matrix4 get localTransformInverse => null; @override void build() { From 375a7e8d841afb9ece11ae4915bfe3c12c8c1463 Mon Sep 17 00:00:00 2001 From: ferhatb Date: Thu, 18 Mar 2021 17:01:01 -0700 Subject: [PATCH 8/9] Add test --- .../engine/surface/scene_builder_test.dart | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) 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 4a6b297567fdc..c964d4b22d627 100644 --- a/lib/web_ui/test/engine/surface/scene_builder_test.dart +++ b/lib/web_ui/test/engine/surface/scene_builder_test.dart @@ -329,7 +329,8 @@ void testMain() { } }); - test('does not skip painting picture when picture is inside transform', () async { + test('does not skip painting picture when picture is ' + 'inside transform with offset', () async { final Picture picture = _drawPicture(); // Picture should not be clipped out since transform will offset it to 500,500 final SurfaceSceneBuilder builder = SurfaceSceneBuilder(); @@ -344,8 +345,26 @@ void testMain() { expect(content.querySelectorAll('flt-picture').single.children, isNotEmpty); }); + test('does not skip painting picture when picture is ' + 'inside transform', () async { + final Picture picture = _drawPicture(); + // Picture should not be clipped out since transform will offset it to 500,500 + final SurfaceSceneBuilder builder = SurfaceSceneBuilder(); + builder.pushOffset(0, 0); + builder.pushClipRect(const Rect.fromLTRB(0, 0, 1000, 1000)) as PersistedContainerSurface; + builder.pushTransform((Matrix4.identity()..scale(0.5, 0.5)).toFloat64()); + builder.pushOffset(1000, 1000); + builder.addPicture(Offset.zero, picture); + builder.pop(); + builder.pop(); + builder.pop(); + html.HtmlElement content = builder.build().webOnlyRootElement; + expect(content.querySelectorAll('flt-picture').single.children, isNotEmpty); + }); + test( - 'skips painting picture when picture fully clipped out with transform and offset', () async { + 'skips painting picture when picture fully clipped out with' + ' transform and offset', () async { final Picture picture = _drawPicture(); // Picture should be clipped out since transform will offset it to 500,500 final SurfaceSceneBuilder builder = SurfaceSceneBuilder(); From 8e1b6343f125ee6ed559df59dd2fed6dd9f40afe Mon Sep 17 00:00:00 2001 From: ferhatb Date: Fri, 19 Mar 2021 15:33:23 -0700 Subject: [PATCH 9/9] protect against malformed matrix --- lib/web_ui/lib/src/engine/util.dart | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/web_ui/lib/src/engine/util.dart b/lib/web_ui/lib/src/engine/util.dart index 925e4f0552ed0..14dc2d9d175fc 100644 --- a/lib/web_ui/lib/src/engine/util.dart +++ b/lib/web_ui/lib/src/engine/util.dart @@ -280,6 +280,9 @@ void transformLTRB(Matrix4 transform, Float32List ltrb) { // Handle non-homogenous matrices. double w = transform[15]; + if (w == 0.0) { + w = 1.0; + } ltrb[0] = math.min( math.min(