diff --git a/packages/vector_graphics/CHANGELOG.md b/packages/vector_graphics/CHANGELOG.md index 962000513c6d..bf794cefff31 100644 --- a/packages/vector_graphics/CHANGELOG.md +++ b/packages/vector_graphics/CHANGELOG.md @@ -1,3 +1,9 @@ +## 1.2.1 + +* Fixes `text-anchor` on `` with multiple `` children. The + anchor now applies to the entire anchored chunk as required by the SVG + spec, instead of independently to each tspan. + ## 1.2.0 * Adds `imageBuilder` property to `VectorGraphic` for wrapping the loaded vector graphic widget. diff --git a/packages/vector_graphics/lib/src/listener.dart b/packages/vector_graphics/lib/src/listener.dart index cc9e96af8376..765aa100a783 100644 --- a/packages/vector_graphics/lib/src/listener.dart +++ b/packages/vector_graphics/lib/src/listener.dart @@ -274,6 +274,20 @@ class FlutterVectorGraphicsListener extends VectorGraphicsCodecListener { double _textPositionY = 0; Float64List? _textTransform; + // Pending text draws within the current SVG anchored chunk. Per the SVG + // spec, `text-anchor` applies to the chunk as a whole, so we cannot + // commit a paragraph to the canvas until we know the full chunk width. + final List<_PendingTextDraw> _pendingChunk = <_PendingTextDraw>[]; + // The user-space x at which the current chunk begins (i.e. the value of + // `_accumulatedTextPositionX` at the time the first paragraph in the + // chunk was queued). Null when no chunk is open. + double? _chunkOriginX; + // The text-anchor multiplier of the first paragraph in the chunk; used + // to position the chunk as a whole. + double _chunkAnchorMultiplier = 0; + // Cumulative pen-advance within the current chunk so far. + double _chunkAdvance = 0; + _PatternConfig? _currentPattern; static final Paint _emptyPaint = Paint(); @@ -294,6 +308,7 @@ class FlutterVectorGraphicsListener extends VectorGraphicsCodecListener { PictureInfo toPicture() { assert(!_done); _done = true; + _flushPendingTextChunk(); try { return PictureInfo._(_recorder.endRecording(), _size); } finally { @@ -652,6 +667,15 @@ class FlutterVectorGraphicsListener extends VectorGraphicsCodecListener { @override void onUpdateTextPosition(int textPositionId) { final _TextPosition position = _textPositions[textPositionId]; + // Per the SVG spec, a new anchored chunk begins only when the element + // establishes an explicit absolute position (i.e. an `x` or `y` on a + // or ). Relative `dx`/`dy` move the pen but do NOT + // start a new chunk; neither does the bare per-tspan TextPosition the + // parser emits when the tspan has no x/y of its own. `reset` (set on + // elements) likewise starts a fresh chunk. + if (position.reset || position.x != null || position.y != null) { + _flushPendingTextChunk(); + } if (position.reset) { _accumulatedTextPositionX = 0; _textPositionY = 0; @@ -685,9 +709,28 @@ class FlutterVectorGraphicsListener extends VectorGraphicsCodecListener { final _TextConfig textConfig = _textConfig[textId]; final double dx = _accumulatedTextPositionX ?? 0; final double dy = _textPositionY; - double paragraphWidth = 0; - void draw(int paintId) { + // A change in text-anchor on a continuing chunk also starts a new + // anchored chunk per the SVG spec. + if (_pendingChunk.isNotEmpty && + textConfig.xAnchorMultiplier != _chunkAnchorMultiplier) { + _flushPendingTextChunk(); + } + + if (_pendingChunk.isEmpty) { + _chunkOriginX = dx; + _chunkAnchorMultiplier = textConfig.xAnchorMultiplier; + _chunkAdvance = 0; + } else { + // Continuing the chunk: take the live pen position so any in-chunk + // relative `dx="..."` movements applied via onUpdateTextPosition + // since the last segment are accounted for in the segment's offset + // within the chunk. + _chunkAdvance = dx - _chunkOriginX!; + } + final double offsetWithinChunk = _chunkAdvance; + + Paragraph buildParagraph(int paintId) { final Paint paint = _paints[paintId]; if (patternId != null) { paint.shader = _patterns[patternId]!.shader; @@ -707,37 +750,60 @@ class FlutterVectorGraphicsListener extends VectorGraphicsCodecListener { decorationColor: textConfig.decorationColor, ), ); - builder.addText(textConfig.text); - final Paragraph paragraph = builder.build(); paragraph.layout(const ParagraphConstraints(width: double.infinity)); - paragraphWidth = paragraph.maxIntrinsicWidth; + return paragraph; + } + + double paragraphWidth = 0; + if (fillId != null) { + final Paragraph p = buildParagraph(fillId); + paragraphWidth = p.maxIntrinsicWidth; + _pendingChunk.add( + _PendingTextDraw(p, offsetWithinChunk, dy, _textTransform), + ); + } + if (strokeId != null) { + final Paragraph p = buildParagraph(strokeId); + paragraphWidth = p.maxIntrinsicWidth; + _pendingChunk.add( + _PendingTextDraw(p, offsetWithinChunk, dy, _textTransform), + ); + } - if (_textTransform != null) { + _chunkAdvance += paragraphWidth; + _accumulatedTextPositionX = dx + paragraphWidth; + } + + void _flushPendingTextChunk() { + if (_pendingChunk.isEmpty) { + return; + } + final double originX = _chunkOriginX ?? 0; + final double anchorOffset = _chunkAdvance * _chunkAnchorMultiplier; + for (final _PendingTextDraw draw in _pendingChunk) { + final Paragraph paragraph = draw.paragraph; + if (draw.transform != null) { _canvas.save(); - _canvas.transform(_textTransform!); + _canvas.transform(draw.transform!); } _canvas.drawParagraph( paragraph, Offset( - dx - paragraph.maxIntrinsicWidth * textConfig.xAnchorMultiplier, - dy - paragraph.alphabeticBaseline, + originX + draw.offsetWithinChunk - anchorOffset, + draw.dy - paragraph.alphabeticBaseline, ), ); paragraph.dispose(); - if (_textTransform != null) { + if (draw.transform != null) { _canvas.restore(); } } - - if (fillId != null) { - draw(fillId); - } - if (strokeId != null) { - draw(strokeId); - } - _accumulatedTextPositionX = dx + paragraphWidth; + _pendingChunk.clear(); + _chunkOriginX = null; + _chunkAnchorMultiplier = 0; + _chunkAdvance = 0; } int _createImageKey(int imageId, int format) { @@ -885,6 +951,20 @@ class _TextConfig { final Color decorationColor; } +class _PendingTextDraw { + _PendingTextDraw( + this.paragraph, + this.offsetWithinChunk, + this.dy, + this.transform, + ); + + final Paragraph paragraph; + final double offsetWithinChunk; + final double dy; + final Float64List? transform; +} + /// An exception thrown if decoding fails. /// /// The [originalException] is a detailed exception about what failed in diff --git a/packages/vector_graphics/pubspec.yaml b/packages/vector_graphics/pubspec.yaml index e4699cba72ed..d45dd90f0e46 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.2.0 +version: 1.2.1 environment: sdk: ^3.9.0 diff --git a/packages/vector_graphics/test/listener_test.dart b/packages/vector_graphics/test/listener_test.dart index 72330c7dd002..28dea35919a3 100644 --- a/packages/vector_graphics/test/listener_test.dart +++ b/packages/vector_graphics/test/listener_test.dart @@ -120,6 +120,9 @@ void main() { listener.onTextConfig('foo', null, 0, 0, 16, 0, 0, 0, 0); await listener.onDrawText(0, 0, null, null); await listener.onDrawText(0, 0, null, null); + // Force flush of the pending anchored chunk by starting a new one. + listener.onTextPosition(1, 0, 0, null, null, true, null); + listener.onUpdateTextPosition(1); final Invocation drawParagraph0 = factory.fakeCanvases.last.invocations[0]; final Invocation drawParagraph1 = factory.fakeCanvases.last.invocations[1]; @@ -133,6 +136,62 @@ void main() { expect((drawParagraph1.positionalArguments[1] as Offset).dx, 58); }); + test('Text anchor middle centers the entire chunk across tspans', () async { + // SVG: + // ABCDEFGABCDEFG + // + // Per SVG spec, the concatenation of both tspans forms a single + // anchored chunk that should be centered around x=100. + final factory = TestPictureFactory(); + final listener = FlutterVectorGraphicsListener(pictureFactory: factory); + listener.onPaintObject( + color: const ui.Color(0xffff0000).toARGB32(), + strokeCap: null, + strokeJoin: null, + blendMode: BlendMode.srcIn.index, + strokeMiterLimit: null, + strokeWidth: null, + paintStyle: ui.PaintingStyle.fill.index, + id: 0, + shaderId: null, + ); + listener.onTextPosition(0, 100, 50, null, null, true, null); + listener.onUpdateTextPosition(0); + // xAnchorMultiplier = 0.5 corresponds to text-anchor="middle". + listener.onTextConfig('ABCDEFG', null, 0.5, 0, 16, 0, 0, 0, 0); + await listener.onDrawText(0, 0, null, null); + // The parser emits a TextPosition for every , including those + // with no x/y. That must NOT break the current anchored chunk. + listener.onTextPosition(1, null, null, null, null, false, null); + listener.onUpdateTextPosition(1); + listener.onTextConfig('ABCDEFG', null, 0.5, 0, 16, 0, 0, 0, 1); + await listener.onDrawText(1, 0, null, null); + // Force flush of the pending anchored chunk by starting a new one. + listener.onTextPosition(2, 0, 0, null, null, true, null); + listener.onUpdateTextPosition(2); + + final Invocation drawParagraph0 = factory.fakeCanvases.last.invocations[0]; + final Invocation drawParagraph1 = factory.fakeCanvases.last.invocations[1]; + expect(drawParagraph0.memberName, #drawParagraph); + expect(drawParagraph1.memberName, #drawParagraph); + + final double dx0 = (drawParagraph0.positionalArguments[1] as Offset).dx; + final double dx1 = (drawParagraph1.positionalArguments[1] as Offset).dx; + + // The chunk is two equal tspans of width w. text-anchor="middle" centers + // the whole chunk (total width 2w) around x=100, so: + // dx0 = 100 - w (left tspan) + // dx1 = 100 (right tspan) + // Therefore the second tspan should start exactly at the original x=100. + expect(dx1, 100, reason: 'second tspan should start at the original x'); + final double w = 100 - dx0; + expect( + dx1 - dx0, + w, + reason: 'tspans should be contiguous within the chunk', + ); + }); + test('should assert when imageId is invalid', () async { final factory = TestPictureFactory(); final listener = FlutterVectorGraphicsListener(pictureFactory: factory); diff --git a/packages/vector_graphics_compiler/CHANGELOG.md b/packages/vector_graphics_compiler/CHANGELOG.md index 13682cba547d..03bbc1517c5f 100644 --- a/packages/vector_graphics_compiler/CHANGELOG.md +++ b/packages/vector_graphics_compiler/CHANGELOG.md @@ -1,6 +1,11 @@ ## 1.2.1 * Fixes HSL/HSLA color parsing for decimal percentage components (e.g. `hsl(270, 100%, 76.27%)`). +* Fixes the SVG parser injecting a spurious space between adjacent + `` elements that have no whitespace between them in the source. + Previously `AB` was emitted as `"A"` + + `" B"`, producing a visible gap; it now emits `"A"` + `"B"` to match + every browser. ## 1.2.0 diff --git a/packages/vector_graphics_compiler/lib/src/svg/parser.dart b/packages/vector_graphics_compiler/lib/src/svg/parser.dart index 52eb38612a48..f5bcdaf90db6 100644 --- a/packages/vector_graphics_compiler/lib/src/svg/parser.dart +++ b/packages/vector_graphics_compiler/lib/src/svg/parser.dart @@ -758,16 +758,23 @@ class SvgParser { final textHasNonWhitespace = text.trim() != ''; // Not from the spec, but seems like how Chrome behaves. - // - If `x` is specified, don't prepend whitespace. - // - If the last element was a tspan and we're dealing with some - // non-whitespace data, prepend a space. - // - If the last text wasn't whitespace and ended with whitespace, prepend - // a space. + // - If `x` is specified on the current element, don't prepend whitespace. + // - Otherwise prepend a space if either: + // * the previous text emission ended on a space character, or + // * we are following a `` and the source actually contains + // whitespace at the boundary (either as a leading-whitespace prefix + // on this text or as an earlier whitespace-only text event that + // was trimmed). + // The "tspan" gate is what prevents `AB` + // from rendering as "A B" — without it the parser would always inject + // a space between adjacent tspans even when no whitespace exists in + // the source. + final bool textHasLeadingWhitespace = + text.isNotEmpty && _whitespacePattern.matchAsPrefix(text) != null; + final followsTspan = _lastEndElementEvent?.localName == 'tspan'; final bool prependSpace = _currentAttributes.x == null && - (_lastEndElementEvent?.localName == 'tspan' && - textHasNonWhitespace) || - _lastTextEndedWithSpace; + (_lastTextEndedWithSpace || (followsTspan && textHasLeadingWhitespace)); _lastTextEndedWithSpace = textHasNonWhitespace && @@ -785,6 +792,12 @@ class SvgParser { .replaceAll(_contiguousSpaceMatcher, ' '); if (text.isEmpty) { + // A pure-whitespace text event sitting between two sibling tspans + // still needs to flag that whitespace existed, so the next + // non-empty text can prepend a space. + if (textHasLeadingWhitespace && followsTspan) { + _lastTextEndedWithSpace = true; + } return; } diff --git a/packages/vector_graphics_compiler/test/parser_test.dart b/packages/vector_graphics_compiler/test/parser_test.dart index b99fd0085745..df096e713567 100644 --- a/packages/vector_graphics_compiler/test/parser_test.dart +++ b/packages/vector_graphics_compiler/test/parser_test.dart @@ -277,6 +277,46 @@ void main() { ]); }); + test('adjacent tspans without whitespace are not separated by a space', () { + // Regression test: previously the parser unconditionally injected a + // space between the text of any two consecutive tspans, even when the + // source XML contained no whitespace between and . + // That caused `AB` to render as "A B" + // (a visible gap), instead of "AB" as every browser does. + const svg = + '' + 'ABCDEFGHIJKLMN' + // ignore: missing_whitespace_between_adjacent_strings + ''; + + final VectorInstructions instructions = parseWithoutOptimizers(svg); + + expect(instructions.text.map((TextConfig t) => t.text), [ + 'ABCDEFG', + 'HIJKLMN', + ]); + }); + + test('adjacent tspans with whitespace between still get a space', () { + // Sibling case to the regression test above: when there *is* source + // whitespace between and , that whitespace must be + // preserved as a single space prepended to the second tspan. + const svg = + // ignore: missing_whitespace_between_adjacent_strings + '' + // ignore: missing_whitespace_between_adjacent_strings + 'A B'; + + final VectorInstructions instructions = parseWithoutOptimizers(svg); + + expect(instructions.text.map((TextConfig t) => t.text), [ + 'A', + ' B', + ]); + }); + test('stroke-opacity', () { const strokeOpacitySvg = '''