diff --git a/packages/vector_graphics/CHANGELOG.md b/packages/vector_graphics/CHANGELOG.md index a839b37646d..089cfabb562 100644 --- a/packages/vector_graphics/CHANGELOG.md +++ b/packages/vector_graphics/CHANGELOG.md @@ -1,3 +1,7 @@ +## 1.1.19 + +* Fix incorrect scaling under Transform in raster mode and reduce extra rendering cost from ColorFilter/Opacity in raster mode. + ## 1.1.18 * Allow transition between placeholder and loaded image to have an animation. diff --git a/packages/vector_graphics/lib/src/render_vector_graphic.dart b/packages/vector_graphics/lib/src/render_vector_graphic.dart index a6b530469e5..4c4f7818922 100644 --- a/packages/vector_graphics/lib/src/render_vector_graphic.dart +++ b/packages/vector_graphics/lib/src/render_vector_graphic.dart @@ -2,6 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'dart:math' as math; import 'dart:ui' as ui; import 'package:flutter/animation.dart'; @@ -15,7 +16,7 @@ import 'listener.dart'; @immutable class RasterKey { /// Create a new [RasterKey]. - const RasterKey(this.assetKey, this.width, this.height); + const RasterKey(this.assetKey, this.width, this.height, this.paint); /// An object that is used to identify the raster data this key will store. /// @@ -28,16 +29,22 @@ class RasterKey { /// The width of this vector graphic raster, in physical pixels. final int height; + /// The paint of this vector graphic raster. + final Paint paint; + @override bool operator ==(Object other) { return other is RasterKey && other.assetKey == assetKey && other.width == width && - other.height == height; + other.height == height && + other.paint.color == paint.color && + other.paint.colorFilter == paint.colorFilter; } @override - int get hashCode => Object.hash(assetKey, width, height); + int get hashCode => + Object.hash(assetKey, width, height, paint.color, paint.colorFilter); } /// The cache entry for a rasterized vector graphic. @@ -81,7 +88,6 @@ class RenderVectorGraphic extends RenderBox { this._colorFilter, this._devicePixelRatio, this._opacity, - this._scale, ) { _opacity?.addListener(_updateOpacity); _updateOpacity(); @@ -116,6 +122,8 @@ class RenderVectorGraphic extends RenderBox { /// An optional [ColorFilter] to apply to the rasterized vector graphic. ColorFilter? get colorFilter => _colorFilter; ColorFilter? _colorFilter; + double _rasterScaleFactor = 1.0; + final Paint _colorPaint = Paint(); set colorFilter(ColorFilter? value) { if (colorFilter == value) { return; @@ -173,16 +181,6 @@ class RenderVectorGraphic extends RenderBox { /// the vector graphic itself has a size of 50x50, and [BoxFit.fill] /// is used. This will compute a scale of 2.0, which will result in a /// raster that is 100x100. - double get scale => _scale; - double _scale; - set scale(double value) { - assert(value != 0); - if (value == scale) { - return; - } - _scale = value; - markNeedsPaint(); - } @override bool hitTestSelf(Offset position) => true; @@ -196,7 +194,7 @@ class RenderVectorGraphic extends RenderBox { } static RasterData _createRaster( - RasterKey key, double scaleFactor, PictureInfo info) { + RasterKey key, double scaleFactor, PictureInfo info, Paint colorPaint) { final int scaledWidth = key.width; final int scaledHeight = key.height; // In order to scale a picture, it must be placed in a new picture @@ -204,11 +202,19 @@ class RenderVectorGraphic extends RenderBox { // arguments of Picture.toImage do not control the resolution that the // picture is rendered at, instead it controls how much of the picture to // capture in a raster. + // Note: Transform applies a transformation to the canvas matrix. final ui.PictureRecorder recorder = ui.PictureRecorder(); final ui.Canvas canvas = ui.Canvas(recorder); - + final Rect drawSize = + ui.Rect.fromLTWH(0, 0, scaledWidth.toDouble(), scaledHeight.toDouble()); + canvas.clipRect(drawSize); + final int saveCount = canvas.getSaveCount(); + if (colorPaint.color.opacity != 1.0 || colorPaint.colorFilter != null) { + canvas.saveLayer(drawSize, colorPaint); + } canvas.scale(scaleFactor); canvas.drawPicture(info.picture); + canvas.restoreToCount(saveCount); final ui.Picture rasterPicture = recorder.endRecording(); final ui.Image pending = @@ -230,12 +236,14 @@ class RenderVectorGraphic extends RenderBox { // Re-create the raster for a given vector graphic if the target size // is sufficiently different. Returns `null` if rasterData has been // updated immediately. - void _maybeUpdateRaster() { + void _maybeUpdateRaster(double drawScaleFactor) { + _rasterScaleFactor = devicePixelRatio * drawScaleFactor; final int scaledWidth = - (pictureInfo.size.width * devicePixelRatio / scale).round(); + (pictureInfo.size.width * _rasterScaleFactor).round(); final int scaledHeight = - (pictureInfo.size.height * devicePixelRatio / scale).round(); - final RasterKey key = RasterKey(assetKey, scaledWidth, scaledHeight); + (pictureInfo.size.height * _rasterScaleFactor).round(); + final RasterKey key = + RasterKey(assetKey, scaledWidth, scaledHeight, _colorPaint); // First check if the raster is available synchronously. This also handles // a no-op change that would resolve to an identical picture. @@ -249,7 +257,7 @@ class RenderVectorGraphic extends RenderBox { return; } final RasterData data = - _createRaster(key, devicePixelRatio / scale, pictureInfo); + _createRaster(key, _rasterScaleFactor, pictureInfo, _colorPaint); data.count += 1; assert(!_liveRasterCache.containsKey(key)); @@ -296,18 +304,23 @@ class RenderVectorGraphic extends RenderBox { return; } - _maybeUpdateRaster(); + if (colorFilter != null) { + _colorPaint.colorFilter = colorFilter; + } + _colorPaint.color = Color.fromRGBO(0, 0, 0, _opacityValue); + + // Use this transform to get real x/y scale facotr. + final Float64List transformList = context.canvas.getTransform(); + final double drawScaleFactor = math.max( + transformList[0 + 0 * 4], + transformList[1 + 1 * 4], + ); + _maybeUpdateRaster(drawScaleFactor); + final ui.Image image = _rasterData!.image; final int width = _rasterData!.key.width; final int height = _rasterData!.key.height; - // Use `FilterQuality.low` to scale the image, which corresponds to - // bilinear interpolation. - final Paint colorPaint = Paint()..filterQuality = ui.FilterQuality.low; - if (colorFilter != null) { - colorPaint.colorFilter = colorFilter; - } - colorPaint.color = Color.fromRGBO(0, 0, 0, _opacityValue); final Rect src = ui.Rect.fromLTWH( 0, 0, @@ -325,7 +338,7 @@ class RenderVectorGraphic extends RenderBox { image, src, dst, - colorPaint, + Paint(), ); } } diff --git a/packages/vector_graphics/lib/src/vector_graphics.dart b/packages/vector_graphics/lib/src/vector_graphics.dart index f43990fdab8..f34a388d502 100644 --- a/packages/vector_graphics/lib/src/vector_graphics.dart +++ b/packages/vector_graphics/lib/src/vector_graphics.dart @@ -3,7 +3,6 @@ // found in the LICENSE file. import 'dart:async'; -import 'dart:math' as math; import 'dart:ui' as ui; import 'package:flutter/foundation.dart'; @@ -455,12 +454,6 @@ class _VectorGraphicWidgetState extends State { assert(width != null && height != null); - double scale = 1.0; - scale = math.min( - pictureInfo.size.width / width!, - pictureInfo.size.height / height!, - ); - if (_webRenderObject) { child = _RawWebVectorGraphicWidget( pictureInfo: pictureInfo, @@ -474,7 +467,6 @@ class _VectorGraphicWidgetState extends State { assetKey: _pictureInfo!.key, colorFilter: widget.colorFilter, opacity: widget.opacity, - scale: scale, ); } else { child = _RawPictureVectorGraphicWidget( @@ -554,13 +546,11 @@ class _RawVectorGraphicWidget extends SingleChildRenderObjectWidget { required this.pictureInfo, required this.colorFilter, required this.opacity, - required this.scale, required this.assetKey, }); final PictureInfo pictureInfo; final ColorFilter? colorFilter; - final double scale; final Animation? opacity; final Object assetKey; @@ -572,7 +562,6 @@ class _RawVectorGraphicWidget extends SingleChildRenderObjectWidget { colorFilter, MediaQuery.maybeOf(context)?.devicePixelRatio ?? 1.0, opacity, - scale, ); } @@ -586,8 +575,7 @@ class _RawVectorGraphicWidget extends SingleChildRenderObjectWidget { ..assetKey = assetKey ..colorFilter = colorFilter ..devicePixelRatio = MediaQuery.maybeOf(context)?.devicePixelRatio ?? 1.0 - ..opacity = opacity - ..scale = scale; + ..opacity = opacity; } } diff --git a/packages/vector_graphics/pubspec.yaml b/packages/vector_graphics/pubspec.yaml index 67d3f0ed3d3..d4112b5c657 100644 --- a/packages/vector_graphics/pubspec.yaml +++ b/packages/vector_graphics/pubspec.yaml @@ -2,7 +2,7 @@ name: vector_graphics description: A vector graphics rendering package for Flutter using a binary encoding. repository: https://github.com/flutter/packages/tree/main/packages/vector_graphics issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+vector_graphics%22 -version: 1.1.18 +version: 1.1.19 environment: sdk: ^3.4.0 diff --git a/packages/vector_graphics/test/render_vector_graphics_test.dart b/packages/vector_graphics/test/render_vector_graphics_test.dart index bf5f12889ca..79a9d87c79c 100644 --- a/packages/vector_graphics/test/render_vector_graphics_test.dart +++ b/packages/vector_graphics/test/render_vector_graphics_test.dart @@ -46,7 +46,6 @@ void main() { null, 1.0, null, - 1.0, ); renderVectorGraphic.layout(BoxConstraints.tight(const Size(50, 50))); final FakePaintingContext context = FakePaintingContext(); @@ -67,7 +66,6 @@ void main() { null, 1.0, null, - 1.0, ); final RenderVectorGraphic renderVectorGraphicB = RenderVectorGraphic( pictureInfo, @@ -75,7 +73,6 @@ void main() { null, 1.0, null, - 1.0, ); renderVectorGraphicA.layout(BoxConstraints.tight(const Size(50, 50))); renderVectorGraphicB.layout(BoxConstraints.tight(const Size(50, 50))); @@ -96,7 +93,6 @@ void main() { null, 1.0, null, - 1.0, ); final RenderVectorGraphic renderVectorGraphicB = RenderVectorGraphic( pictureInfo, @@ -104,7 +100,6 @@ void main() { null, 1.0, null, - 1.0, ); renderVectorGraphicA.layout(BoxConstraints.tight(const Size(50, 50))); final FakeHistoryPaintingContext context = FakeHistoryPaintingContext(); @@ -131,7 +126,6 @@ void main() { null, 1.0, null, - 1.0, ); final RenderVectorGraphic renderVectorGraphicB = RenderVectorGraphic( pictureInfo, @@ -139,7 +133,6 @@ void main() { null, 1.0, null, - 1.0, ); renderVectorGraphicA.layout(BoxConstraints.tight(const Size(50, 50))); final FakeHistoryPaintingContext context = FakeHistoryPaintingContext(); @@ -156,14 +149,13 @@ void main() { expect(identical(context.canvas.images[0], context.canvas.images[1]), true); }); - test('Changing color filter does not re-rasterize', () async { + test('Changing color filter does re-rasterize', () async { final RenderVectorGraphic renderVectorGraphic = RenderVectorGraphic( pictureInfo, 'test', null, 1.0, null, - 1.0, ); renderVectorGraphic.layout(BoxConstraints.tight(const Size(50, 50))); final FakePaintingContext context = FakePaintingContext(); @@ -175,11 +167,11 @@ void main() { const ui.ColorFilter.mode(Colors.red, ui.BlendMode.colorBurn); renderVectorGraphic.paint(context, Offset.zero); - expect(firstImage.debugDisposed, false); + expect(firstImage.debugDisposed, true); renderVectorGraphic.paint(context, Offset.zero); - expect(context.canvas.lastImage, equals(firstImage)); + expect(context.canvas.lastImage, isNot(firstImage)); }); test('Changing device pixel ratio does re-rasterize and dispose old raster', @@ -190,7 +182,6 @@ void main() { null, 1.0, null, - 1.0, ); renderVectorGraphic.layout(BoxConstraints.tight(const Size(50, 50))); final FakePaintingContext context = FakePaintingContext(); @@ -215,7 +206,6 @@ void main() { null, 1.0, null, - 1.0, ); renderVectorGraphic.layout(BoxConstraints.tight(const Size(50, 50))); final FakePaintingContext context = FakePaintingContext(); @@ -223,7 +213,7 @@ void main() { final ui.Image firstImage = context.canvas.lastImage!; - renderVectorGraphic.scale = 2.0; + context.canvas.scale(2.0, 2.0); renderVectorGraphic.paint(context, Offset.zero); expect(firstImage.debugDisposed, true); @@ -233,23 +223,23 @@ void main() { expect(context.canvas.lastImage!.debugDisposed, false); }); - test('The raster size is increased by the inverse picture scale', () async { + test('The raster size is increased by the canvas scale', () async { final RenderVectorGraphic renderVectorGraphic = RenderVectorGraphic( pictureInfo, 'test', null, 1.0, null, - 0.5, // twice as many pixels ); renderVectorGraphic.layout(BoxConstraints.tight(const Size(50, 50))); final FakePaintingContext context = FakePaintingContext(); + context.canvas.scale(2.0, 2.0); renderVectorGraphic.paint(context, Offset.zero); // Dst rect is always size of RO. expect(context.canvas.lastDst, const Rect.fromLTWH(0, 0, 50, 50)); expect( - context.canvas.lastSrc, const Rect.fromLTWH(0, 0, 50 / 0.5, 50 / 0.5)); + context.canvas.lastSrc, const Rect.fromLTWH(0, 0, 50 * 2.0, 50 * 2.0)); }); test('The raster size is increased by the device pixel ratio', () async { @@ -259,7 +249,6 @@ void main() { null, 2.0, null, - 1.0, ); renderVectorGraphic.layout(BoxConstraints.tight(const Size(50, 50))); final FakePaintingContext context = FakePaintingContext(); @@ -278,10 +267,10 @@ void main() { null, 2.0, null, - 0.5, ); renderVectorGraphic.layout(BoxConstraints.tight(const Size(50, 50))); final FakePaintingContext context = FakePaintingContext(); + context.canvas.scale(2.0, 2.0); renderVectorGraphic.paint(context, Offset.zero); // Dst rect is always size of RO. @@ -297,7 +286,6 @@ void main() { null, 1.0, null, - 1.0, ); renderVectorGraphic.layout(BoxConstraints.tight(const Size(50, 50))); final FakePaintingContext context = FakePaintingContext(); @@ -318,7 +306,6 @@ void main() { null, 1.0, opacity, - 1.0, ); renderVectorGraphic.layout(BoxConstraints.tight(const Size(50, 50))); final FakePaintingContext context = FakePaintingContext(); @@ -344,13 +331,13 @@ void main() { null, 1.0, opacity, - 1.0, ); renderVectorGraphic.layout(BoxConstraints.tight(const Size(50, 50))); final FakePaintingContext context = FakePaintingContext(); renderVectorGraphic.paint(context, Offset.zero); - expect(context.canvas.lastPaint?.color, const Color.fromRGBO(0, 0, 0, 0.5)); + // opaque is used to generate raster cache. + expect(context.canvas.lastPaint?.color, const Color.fromRGBO(0, 0, 0, 1.0)); }); test('Disposing render object disposes picture', () async { @@ -360,7 +347,6 @@ void main() { null, 1.0, null, - 1.0, ); renderVectorGraphic.layout(BoxConstraints.tight(const Size(50, 50))); final FakePaintingContext context = FakePaintingContext(); @@ -381,7 +367,6 @@ void main() { null, 1.0, opacity, - 1.0, ); final PipelineOwner pipelineOwner = PipelineOwner(); expect(opacity._listeners, hasLength(1)); @@ -403,7 +388,9 @@ void main() { final ui.PictureRecorder recorder = ui.PictureRecorder(); ui.Canvas(recorder); final ui.Image image = await recorder.endRecording().toImage(1, 1); - final RasterData data = RasterData(image, 1, const RasterKey('test', 1, 1)); + final Paint paint = Paint(); + final RasterData data = + RasterData(image, 1, RasterKey('test', 1, 1, paint)); data.dispose(); @@ -426,6 +413,101 @@ void main() { expect(context.canvas.totalSaves, 1); expect(context.canvas.totalSaveLayers, 1); }); + + testWidgets('Changing offset does not re-rasterize in auto strategy', + (WidgetTester tester) async { + final RenderVectorGraphic renderAutoVectorGraphic = RenderVectorGraphic( + pictureInfo, + 'testOffset', + null, + 1.0, + null, + ); + renderAutoVectorGraphic.layout(BoxConstraints.tight(const Size(50, 50))); + final FakePaintingContext context = FakePaintingContext(); + + renderAutoVectorGraphic.paint(context, Offset.zero); + expect(context.canvas.lastImage, isNotNull); + + final ui.Image? oldImage = context.canvas.lastImage; + + renderAutoVectorGraphic.paint(context, const Offset(20, 30)); + expect(context.canvas.lastImage, isNotNull); + expect(context.canvas.lastImage, equals(oldImage)); + + renderAutoVectorGraphic.dispose(); + }); + + testWidgets('RenderAutoVectorGraphic re-rasterizes when opacity changes', + (WidgetTester tester) async { + final FixedOpacityAnimation opacity = FixedOpacityAnimation(0.2); + final RenderVectorGraphic renderAutoVectorGraphic = RenderVectorGraphic( + pictureInfo, + 'testOpacity', + null, + 1.0, + opacity, + ); + + renderAutoVectorGraphic.layout(BoxConstraints.tight(const Size(50, 50))); + final FakePaintingContext context = FakePaintingContext(); + renderAutoVectorGraphic.paint(context, Offset.zero); + + final ui.Image? oldImage = context.canvas.lastImage; + + opacity.value = 0.5; + opacity.notifyListeners(); + + // Changing opacity requires painting. + expect(renderAutoVectorGraphic.debugNeedsPaint, true); + + // Changing opacity need create new raster cache. + renderAutoVectorGraphic.paint(context, Offset.zero); + expect(context.canvas.lastImage, isNotNull); + + expect(context.canvas.lastImage, isNot(oldImage)); + + renderAutoVectorGraphic.dispose(); + }); + + testWidgets( + 'Identical widgets reuse raster cache when available in auto startegy', + (WidgetTester tester) async { + final RenderVectorGraphic renderAutoVectorGraphic1 = RenderVectorGraphic( + pictureInfo, + 'testOffset', + null, + 1.0, + null, + ); + final RenderVectorGraphic renderAutoVectorGraphic2 = RenderVectorGraphic( + pictureInfo, + 'testOffset', + null, + 1.0, + null, + ); + renderAutoVectorGraphic1.layout(BoxConstraints.tight(const Size(50, 50))); + renderAutoVectorGraphic2.layout(BoxConstraints.tight(const Size(50, 50))); + + final FakePaintingContext context = FakePaintingContext(); + + renderAutoVectorGraphic1.paint(context, Offset.zero); + + final ui.Image? image1 = context.canvas.lastImage; + + renderAutoVectorGraphic2.paint(context, Offset.zero); + + final ui.Image? image2 = context.canvas.lastImage; + + expect(image1, isNotNull); + expect(image2, isNotNull); + + expect(image1, equals(image2)); + + renderAutoVectorGraphic1.dispose(); + renderAutoVectorGraphic2.dispose(); + }); } class FakeCanvas extends Fake implements Canvas { @@ -437,6 +519,8 @@ class FakeCanvas extends Fake implements Canvas { int saveCount = 0; int totalSaves = 0; int totalSaveLayers = 0; + double scaleX = 1.0; + double scaleY = 1.0; @override void drawImageRect(ui.Image image, Rect src, Rect dst, Paint paint) { @@ -481,6 +565,24 @@ class FakeCanvas extends Fake implements Canvas { {ui.ClipOp clipOp = ui.ClipOp.intersect, bool doAntiAlias = true}) { lastClipRect = rect; } + + @override + void scale(double sx, [double? sy]) { + scaleX = sx; + scaleY = sx; + if (sy != null) { + scaleY = sy; + } + } + + @override + void translate(double dx, double dy) {} + + @override + Float64List getTransform() { + return Float64List.fromList( + [scaleX, 0, 0, 0, 0, scaleY, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1]); + } } class FakeHistoryCanvas extends Fake implements Canvas { @@ -490,6 +592,12 @@ class FakeHistoryCanvas extends Fake implements Canvas { void drawImageRect(ui.Image image, Rect src, Rect dst, Paint paint) { images.add(image); } + + @override + Float64List getTransform() { + return Float64List.fromList( + [1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1]); + } } class FakePaintingContext extends Fake implements PaintingContext {