[various] Support SVG filters (blur, offset, feMerge) and alpha masks#11909
[various] Support SVG filters (blur, offset, feMerge) and alpha masks#11909edwin-hollen wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces support for SVG filters, specifically blur and offset, and alpha masks across the vector_graphics, vector_graphics_codec, and vector_graphics_compiler packages. It updates the parser to handle , feGaussianBlur, feOffset, and feMerge elements, adds encoding/decoding for paint blur commands, and implements rendering support via ImageFilter.blur. The review feedback highlights several areas for improvement: handling comma-separated values in stdDeviation, defaulting omitted in attributes on feMergeNode, preserving the original element's opacity when applying isSourceAlpha filters, and implementing cycle detection during recursive filter tracing to prevent stack overflows.
0453cc0 to
5955e54
Compare
5955e54 to
5b44d76
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request adds support for SVG filters (specifically Gaussian blur) and alpha masks (mask-type=alpha) across the vector graphics packages, including updates to the codec, compiler, and renderer, along with an interactive gallery example. Feedback on the changes highlights a redundant check in the filter parser, a limitation in parsing filters where feMerge is not the final operation, and incomplete support for multi-layer filters on group nodes (ParentNode).
5b44d76 to
a6f0287
Compare
|
/gemini review |
a6f0287 to
ce01413
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for SVG filters (specifically Gaussian blur) and alpha masks across the vector graphics packages. Key changes include adding a paint blur command to the codec, parsing filter primitives (feGaussianBlur, feOffset, feMerge) in the compiler, resolving filter layers in the resolver visitor, and applying image filters to paints in the renderer. Additionally, the example app is updated with a filter and mask gallery. The review feedback highlights three areas for improvement: using the ui. prefix for ImageFilter to prevent compilation errors, resolving a transformation mismatch in visitTextPositionNode that could cause layout misalignment, and using ByteData.sublistView instead of asByteData() for safer byte buffer handling.
There was a problem hiding this comment.
Code Review
This pull request adds support for SVG filters (specifically Gaussian blur, offset, and merge) and alpha masks across the vector graphics packages, including the compiler, codec, and runtime renderer. It also updates the example application with a gallery demonstrating these features. The reviewer feedback suggests handling trailing delimiters in stdDeviation parsing to prevent invalidating the filter, and combining chained Gaussian blurs using the square root of the sum of squares rather than linear addition.
|
/gemini review |
ce01413 to
53a75e8
Compare
There was a problem hiding this comment.
Code Review
This pull request introduces support for SVG filters, specifically blur and multi-layer filters, and alpha masks (mask-type=alpha) across the vector_graphics package, compiler, and codec. It also adds a filter and mask gallery to the example application along with corresponding tests. Review feedback points out three areas for improvement: handling single-layer filters with offsets at the TextPositionNode level to avoid text positioning bugs, ensuring single-layer filters with isSourceAlpha enabled are not ignored during parsing, and using the ui. prefix for ImageFilter.blur in the listener for import consistency.
f4e0eff to
98a6f03
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request adds support for SVG filters (such as Gaussian blur and offset) and alpha masks across the vector graphics packages. It updates the compiler to parse and resolve these filters, adds encoding and decoding support in the codec, and implements paint blur rendering in the listener. An issue was identified in the resolver where multi-layer filters on text nodes only process the first layer, ignoring subsequent layers, and a fix was suggested to resolve all layers into a parent node.
98a6f03 to
d02df90
Compare
|
/gemini review |
d02df90 to
f51ca1b
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for SVG filters (specifically Gaussian blur, offsets, and multi-layer merging) and alpha masks across the vector graphics packages. Key changes include codec updates to handle paint blur commands, XML parsing for filter and mask attributes, and resolving these filters into paint properties or layers during compilation. Feedback on these changes points out a missing dart:math import in the parser, opportunities to optimize performance by caching a regular expression and avoiding redundant node recreation when the current filter layer has no visual effect, and a suggestion to refactor duplicated paint-creation logic into a helper method.
There was a problem hiding this comment.
Code Review
This pull request introduces support for SVG filters (such as blur and offset) and alpha masks across the vector_graphics, vector_graphics_codec, and vector_graphics_compiler packages. It adds the capability to parse, encode, decode, and render these filters and masks, along with corresponding tests and gallery examples. Feedback on the changes suggests defaulting parsedX and parsedY to 0.0 when stdDeviation is omitted on a <feGaussianBlur> element to comply with SVG specifications and avoid breaking the filter reference chain.
f51ca1b to
c525511
Compare
|
/gemini review |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request adds support for SVG filters (including Gaussian blur, offset, and merge) and alpha masks across the vector_graphics, vector_graphics_codec, and vector_graphics_compiler packages. It implements parsing, resolving, encoding, decoding, and rendering for these features, accompanied by extensive tests and example updates. The feedback suggests simplifying a redundant null-aware operator when accessing the non-nullable gradient.colors property in the resolver.
4baf2c4 to
1a21c97
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request adds support for SVG filters (such as blur, offset, and feMerge) and alpha masks across the vector_graphics, vector_graphics_codec, and vector_graphics_compiler packages. The changes implement SVG filter parsing, layer resolution, and alpha mask blending using BlendMode.dstIn, as well as updating the binary codec to encode and decode paint blur commands. Comprehensive unit and end-to-end tests have been added, and the example application has been updated to demonstrate these features. There are no review comments, and I have no feedback to provide.
|
Thanks Stuart, love your work and love using Flutter! I'm honored to contribute :) |
1a21c97 to
ff88e14
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for SVG filters (such as blur, offset, and feMerge) and alpha masks across the vector_graphics, vector_graphics_codec, and vector_graphics_compiler packages. It updates the binary codec to handle paint blur commands, parses filter elements and attributes in the compiler, and resolves filter layers and alpha masks during SVG resolution. Feedback on the changes suggests checking for null colors in _blackenGradient to prevent potential runtime errors when reconstructing gradients.
ff88e14 to
310aecc
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for SVG filters (blur, offset, feMerge) and alpha masks across the vector graphics packages. It adds paint blur decoding in the listener, binary encoding/decoding in the codec, and filter parsing and resolution in the compiler. Feedback on the changes suggests optimizing path translation by translating the already-transformed path directly, and scaling the blur standard deviations by the accumulated transform's scale factors at both the group and path/text levels to ensure correct rendering when scaled.
310aecc to
b6a50dc
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request adds support for SVG filters (blur, offset, feMerge) and alpha masks across the vector_graphics, vector_graphics_codec, and vector_graphics_compiler packages. Key changes include parsing filter elements, calculating and scaling blur sigmas, encoding/decoding paint blur commands, and applying alpha masks. The review feedback suggests caching blackened gradients in _blackenGradient to prevent redundant allocations and wrapping parent node resolution in a try-finally block to guarantee robust restoration of the _currentLayer state.
b6a50dc to
aa7d90d
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for SVG filters (such as blur, offset, and merge) and alpha masks across the vector graphics packages, including parsing, resolving, encoding, and rendering. A review comment suggests guarding against setting imageFilter when both standard deviations are zero to avoid unnecessary rendering overhead in Flutter.
aa7d90d to
4ebe390
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request adds support for SVG filters (including blur, offset, and feMerge) and alpha masks across the vector_graphics, vector_graphics_codec, and vector_graphics_compiler packages. The changes update the binary codec to encode and decode paint blur commands, extend the compiler's parser and resolver to process filter elements and alpha masks, and update the rendering listener to apply blur image filters to paints. As there are no review comments, no feedback is provided.
4ebe390 to
84c91e7
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for SVG filters (blur, offset, feMerge) and alpha masks across the vector graphics packages, updating the compiler, codec, and rendering listener to handle these features. Feedback on the changes suggests translating paths in their local coordinate space before applying accumulated transforms to ensure correct filter offsets, and defensively clamping standard deviation values to non-negative numbers in the rendering listener to prevent crashes from malformed files.
This PR introduces support for basic SVG filters and alpha masking to the vector_graphics ecosystem. Key Features: * Added parsing, resolution, and encoding of SVG filters including `feGaussianBlur`, `feOffset`, and `feMerge` in `vector_graphics_compiler`. * Added support for alpha masks (`mask-type=alpha`). * Updated the binary format and `vector_graphics_codec` to support encoding and decoding paint blurs. * Updated `vector_graphics` to render paint blurs using Flutter's `ui.ImageFilter.blur` and alpha masks via `BlendMode.dstIn`. * Enhanced `SourceAlpha` filters to preserve opacity variations in gradient stops by mapping gradient colors to black while maintaining their alpha values. Implementation Details: * **Filter Chaining**: Filters are parsed into a directed acyclic graph (DAG) of primitives. The resolver traces these primitives from the filter outputs back to `SourceGraphic`/`SourceAlpha` to determine the necessary rendering layers. * **Multi-layer Filter Resolution**: Handles multi-layer filters (e.g. from `feMerge`) by duplicating the target rendering nodes for each layer in the AST. * **Text Filter Resolution**: Resolves filters at the `TextPositionNode` level (rather than leaf `TextNode`s) to maintain correct text chunk alignment and cursor positioning. * **Nested Filter Propagation**: Enhanced nested parent nodes (groups), inner path filters, and text position nodes to correctly inherit and propagate the outer `SourceAlpha` state down the tree. * **Blur Scaling**: Scales the blur standard deviations by the accumulated transform's scale factors at both the group and path/text levels to ensure correct rendering when scaled (since the compiler bakes geometry transforms at compile time, the blur sigmas must also be scaled to match). * **Spec Compliance**: * Supports both comma and space-separated `stdDeviation` values. * Defaults omitted `stdDeviation` attributes to `0.0` (as per SVG spec). * Automatically resolves default `in` attributes for `feMergeNode`s (falling back to the previous sibling primitive's result or `SourceGraphic`). * Preserves original fill/stroke opacity when applying `SourceAlpha` filters. * Handles self-closing `<feMerge />` elements. * **Robustness, Performance & Optimizations**: * Added cycle detection during filter tracing to prevent stack overflow on circular filter references. * Skips rendering node recreation for filter layers that do not affect the paint (e.g. plain `SourceGraphic` branches in a merge). * Wrapped current layer state restoration in `try-finally` blocks inside parent node and text position node visitors to guarantee robust recovery in case of exceptions. * Caches blackened gradients in `ResolvingVisitor` to prevent redundant object allocations for shared SVG gradients. * Guarded against setting paint `imageFilter` when blur standard deviations are `0.0` to avoid unnecessary rendering overhead in Flutter. * Defensively clamped blur standard deviations to non-negative values in the rendering listener to prevent crashes when processing malformed SVG files. ## Pre-Review Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [x] I read the [Tree Hygiene] page, which explains my responsibilities. - [x] I read and followed the [relevant style guides] and ran [the auto-formatter]. - [x] I signed the [CLA]. - [x] The title of the PR starts with the name of the package surrounded by square brackets, e.g. `[shared_preferences]` - [x] I linked to at least one issue that this PR fixes in the description above. * *Note: No existing issue, this is a new feature contribution.* - [x] I followed [the version and CHANGELOG instructions], using [semantic versioning] and the [repository CHANGELOG style], or I have commented below to indicate which documented exception this PR falls under[^1]. - [x] I updated/added any relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or I have commented below to indicate which [test exemption] this PR falls under[^1]. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. [^1]: Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. <!-- Links --> [Contributor Guide]: https://github.com/flutter/packages/blob/main/CONTRIBUTING.md [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines [Tree Hygiene]: https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md [relevant style guides]: https://github.com/flutter/packages/blob/main/CONTRIBUTING.md#style [the auto-formatter]: https://github.com/flutter/packages/blob/main/script/tool/README.md#format-code [CLA]: https://cla.developers.google.com/ [Discord]: https://github.com/flutter/flutter/blob/master/docs/contributing/Chat.md [linked to at least one issue that this PR fixes]: https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md#overview [the version and CHANGELOG instructions]: https://github.com/flutter/flutter/blob/master/docs/ecosystem/contributing/README.md#version-and-changelog-updates [semantic versioning]: https://dart.dev/tools/pub/versioning#semantic-versions [repository CHANGELOG style]: https://github.com/flutter/flutter/blob/master/docs/ecosystem/contributing/README.md#changelog-style [test exemption]: https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md#tests TAG=agy CONV=27cb188a-9ad9-4434-8918-8ef71bdba17c
84c91e7 to
3d710a2
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request adds support for SVG filters (including blur, offset, and feMerge) and alpha masks (mask-type=alpha) across the vector_graphics, vector_graphics_codec, and vector_graphics_compiler packages. The changes implement paint blur encoding and decoding in the codec, update the compiler's parser and resolver to process SVG filter elements and resolve filter layers, and update the listener to apply image filters to paints. Comprehensive unit, parser, and end-to-end tests have been added to verify these new features. There are no review comments, so I have no feedback to provide.
There was a problem hiding this comment.
Code Review
This pull request adds support for SVG filters (including blur, offset, and merge) and alpha masks across the vector_graphics, vector_graphics_codec, and vector_graphics_compiler packages. Key changes include updating the binary codec to support paint blur tags, parsing filter elements and attributes in the compiler, resolving filter layers and alpha masks during scene resolution, and implementing rendering support in the listener. Extensive unit and end-to-end tests have been added to verify these features. There are no review comments to assess, and I have no additional feedback to provide.
[various] Support SVG filters (blur, offset, feMerge) and alpha masks
This PR introduces support for basic SVG filters and alpha masking to the vector_graphics ecosystem.
Key Features:
feGaussianBlur,feOffset, andfeMergeinvector_graphics_compiler.mask-type=alpha).vector_graphics_codecto support encoding and decoding paint blurs.vector_graphicsto render paint blurs using Flutter'sui.ImageFilter.blurand alpha masks viaBlendMode.dstIn.SourceAlphafilters to preserve opacity variations in gradient stops by mapping gradient colors to black while maintaining their alpha values.Implementation Details:
SourceGraphic/SourceAlphato determine the necessary rendering layers.feMerge) by duplicating the target rendering nodes for each layer in the AST.TextPositionNodelevel (rather than leafTextNodes) to maintain correct text chunk alignment and cursor positioning.SourceAlphastate down the tree.stdDeviationvalues.stdDeviationattributes to0.0(as per SVG spec).inattributes forfeMergeNodes (falling back to the previous sibling primitive's result orSourceGraphic).SourceAlphafilters.<feMerge />elements.SourceGraphicbranches in a merge).try-finallyblocks inside parent node and text position node visitors to guarantee robust recovery in case of exceptions.ResolvingVisitorto prevent redundant object allocations for shared SVG gradients.imageFilterwhen blur standard deviations are0.0to avoid unnecessary rendering overhead in Flutter.Pre-Review Checklist
[shared_preferences]///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.TAG=agy
CONV=27cb188a-9ad9-4434-8918-8ef71bdba17c
Footnotes
Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩2