Skip to content

Preserve mask definition nodes during painting - #457

Open
AfonsoZhang wants to merge 1 commit into
Kozea:mainfrom
AfonsoZhang:fix-mask-definition-mutation
Open

AfonsoZhang wants to merge 1 commit into
Kozea:mainfrom
AfonsoZhang:fix-mask-definition-mutation

Conversation

@AfonsoZhang

Copy link
Copy Markdown

A <mask> declared after the element that references it can be painted as ordinary content, covering the intended image. For example, this SVG should be red but currently renders white:

<svg xmlns="http://www.w3.org/2000/svg" width="10" height="10">
  <rect width="10" height="10" fill="red" mask="url(#m)"/>
  <mask id="m" maskUnits="userSpaceOnUse" x="0" y="0" width="10" height="10">
    <rect width="10" height="10" fill="white"/>
  </mask>
</svg>

parse_all_defs stores the original node. paint_mask then changes that shared node's tag to g and replaces its geometry while preparing the mask surface. If normal document traversal reaches the definition afterwards, it no longer recognizes an invisible mask and draws its children. This was encountered when converting D2-generated SVG, but the reproducer does not depend on D2.

Copy the mask node before changing its tag and attributes. Add pixel-level regression tests for a mask before the reference, after the reference, and inside defs. The all-white mask avoids conflating this issue with alpha-versus-luminance support (#149).

Validation against main 3b7a250c9e955db9c3cdf525d5b5027a9a396201:

  • New test before the fix: 1 failed (after), 2 passed.
  • Full test suite after the fix, including the reference submodule: 293 passed (Python 3.10, Linux).
  • flake8 and isort --check passed for both changed files.

SVG 1.1 specifies that mask definitions do not render directly: https://www.w3.org/TR/SVG11/masking.html#MaskElement

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant