Skip to content

fix SVG group clipPath importing#4338

Closed
NicTanghe wants to merge 2 commits into
GraphiteEditor:masterfrom
NicTanghe:clipPath
Closed

fix SVG group clipPath importing#4338
NicTanghe wants to merge 2 commits into
GraphiteEditor:masterfrom
NicTanghe:clipPath

Conversation

@NicTanghe

Copy link
Copy Markdown

Title

Fix SVG group clipPath importing

Description

Preserves clipPath clipping when importing SVG groups by connecting the importer to
Graphite’s existing clipping-mask system.

  • Imports clip-path geometry as a hidden mask layer
  • Marks group children as clipped using the existing Clipping Mask node
  • Preserves group and clip-path transforms
  • Supports nested clip paths without duplicating clipping logic
  • Adds regression coverage through the existing SVG renderer

Fixes #4331

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request implements support for importing SVG group clip paths as hidden clipping masks in Graphite. It introduces helper functions to traverse and import clip paths, updates the import logic to return an ImportedNode struct containing both the layer identifier and its vertical extent, and adds a comprehensive test suite. The feedback suggests ensuring that strokes defined on <clipPath> elements or their children are ignored or disabled during import, as they should not be rendered according to the SVG specification.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +729 to +731
// Fill opacity hides the clip geometry during normal rendering but is intentionally ignored
// by the renderer's mask pass, leaving the geometry fully opaque as a stencil.
modify_inputs.opacity_fill_set(0.);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

In the SVG specification, any stroke defined on a <clipPath> or its child geometries is ignored for clipping purposes and must not be rendered. Currently, import_usvg_clip_path only sets the fill opacity to 0 via modify_inputs.opacity_fill_set(0.). If any of the geometries inside the clip path have a stroke defined, those strokes will still be imported and rendered during normal rendering, creating a visual discrepancy. Consider passing a flag or context during the clip path children import traversal to ignore or disable strokes on any imported clip path geometries.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 2 files

Confidence score: 5/5

  • Safe to merge after the addressed issues were fixed.

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

@NicTanghe NicTanghe closed this Jul 15, 2026
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.

SVG importer ignores clipPath applied to a group

1 participant