Skip to content

fix(import): carry Figma winding rule so multi-subpath icon holes render#6416

Merged
shipyard-local[bot] merged 17 commits into
mainfrom
fix/svg-fill-rule
Jul 23, 2026
Merged

fix(import): carry Figma winding rule so multi-subpath icon holes render#6416
shipyard-local[bot] merged 17 commits into
mainfrom
fix/svg-fill-rule

Conversation

@shipyard-local

@shipyard-local shipyard-local Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Item #1 of the Figma-import hardening plan — the worst + silent gap from the sweep.

Multi-subpath ODD-winding icons (e.g. designers-pick "Sub" speaker cabinets with a hollow woofer) rendered as solid slabs because the decoder never carried Figma's per-path winding rule (fill-rule), so everything filled nonzero.

Root cause + fix

Figma's fillGeometry is a list of per-region entries, each with its own windingRule. The Sub icon concatenates [NONZERO dot, ODD ring, ODD box-with-hole] into one path of same-direction contours. Correct merge is evenodd-wins (Figma direction-corrects its nonzero regions, so they fill identically either way, while ODD same-direction holes fill solid under nonzero). Wired end to end: paths.mjs reads per-entry winding → scene.mjs emits fillRuledesign_ir_json.cppsetSvgFillRule in web codegen + ->set_fill_rule in cpp codegen + native materializer. New vector-fill-rule-approximated diagnostic (warning) fires on mixed-rule or multi-subpath-no-rule vectors so the class can never go silent again.

Verification

  • Both Sub icons now show hollow woofers (before/after montage confirmed).
  • Apples-to-apples thumb_parity on designers-pick (same-build baseline): 77→73 bad blocks, both Sub blocks eliminated (worst dropped from dE 25.7 to 12.65). channel-fx 3→3 (13 evenodd emissions); sequencer 0 (no evenodd vectors, untouched).
  • Tests (all pass): fig-lane rule-pick + evenodd-wins + diagnostic; IR round-trip + exactly-one emission; materializer both spellings; a bridge raster donut proof (evenodd opens a hole, nonzero fills it). Full ctest -R "svg|path|import|design|native" 1056/1056 — the one exclusion is a pre-existing, unrelated PulpSampler SEGFAULT on main (heritage-kit merges feat: complete sampler heritage kit #6396/Harden sampler streaming, interpolation, and heritage pipeline #6332; none of this PR's files in its closure — flagged for a separate issue).

🤖 Generated with Claude Code


🔎 Provenance

Agent claude
Machine m5
Workspace w1
Directory ~/Code/pulp-harden1
Session fd0e374f-d5dc-4e2a-bb33-6fa8b66124a0

Resume

claude --resume fd0e374f-d5dc-4e2a-bb33-6fa8b66124a0

Jump to this tab

cmux surface focus B1B434D3-85F6-4937-8806-96D59886F574

Relaunch (any agent)

cmux surface resume get --surface B1B434D3-85F6-4937-8806-96D59886F574

Restore URLhttps://claude.ai/code/session_014e8kbMn8Sf4mF8ozsy1Jot

stamped 2026-07-20 23:05 UTC

danielraffel and others added 2 commits July 20, 2026 02:10
…entric

Importing "A Channel FX" (.fig) rendered every knob's value-ring arc
visibly off-center of its body — up-and-left, worst on the medium knobs
(~1.2px diagonal on a 3.6px groove). The source is concentric: the body
ELLIPSE at (7.51, 7.51, 22.53²) and the ring arc's circle center at
(18.78, 18.78) share one center only at fractional precision.

Two independent layers were rounding that precision away:

1. The .fig decoder's styleFor (scene.mjs) emitted Math.round()ed
   left/top/width/height for box-model nodes while the VECTOR_LIKE lane
   kept round2 (2-decimal) placement — the ellipse center drifted
   +0.72px/axis relative to its baked-path sibling. Both lanes now emit
   round2.

2. Yoga's default pointScaleFactor of 1.0 re-rounded every laid-out box
   to the whole-pixel grid, destroying the decoder's precision again.
   New opt-in: View::set_subpixel_layout(true) — discovered pass-wide in
   yoga_layout.cpp (one pass has one YGConfig) — sets pointScaleFactor 0
   for that pass. Imported designs opt in via build_native_view_tree
   (native lane) and a typeof-guarded setSubpixelLayout('', true) the JS
   codegen emits (bridge lane); stock flowed UIs keep pixel-grid
   rounding, pinned by test.

Verified against the .fig's own solved geometry: post-fix laid-out boxes
match Figma to 0.01px, ring/body cross-section asymmetry drops from
0.8px to 0.05px, all 27 ring-knob offsets collapse to the measurement
floor, and thumb_parity improves designers-pick from 77 to 39 bad
blocks (A Channel FX stays at its 3 marginal color blocks).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014e8kbMn8Sf4mF8ozsy1Jot
Hotspot-Grow: core/view/src/design_codegen.cpp reason="emit typeof-guarded setSubpixelLayout opt-in so imported designs keep sub-pixel geometry (knob-ring concentricity fix)"
Hotspot-Grow: core/view/include/pulp/view/view.hpp reason="add set_subpixel_layout opt-in API for import-time sub-pixel layout"
Hotspot-Grow: test/test_design_import_codegen.cpp reason="new test pinning the emitted setSubpixelLayout opt-in line"
Hotspot-Grow: core/view/src/design_import_native_common.cpp reason="set subpixel_layout on the materialized import root so ring geometry stays concentric"
A .fig vector's fillGeometry is a LIST of per-region entries, each with
its own windingRule, and the decoder dropped the field entirely — every
imported path filled under SvgPathWidget's nonzero default. Figma does
not direction-correct the contours of its ODD-declared regions, so a
subtracted icon (designers-pick's "Sub" speaker cabinet: five
same-direction subpaths declared [NONZERO, ODD, ODD]) rendered as a
solid slab instead of a hollow woofer — the fidelity sweep's worst
dE2000 (~25), and fully silent.

Wire the rule end to end:
- paths.mjs: read windingRule per geometry entry; when entries disagree,
  evenodd wins (Figma direction-corrects NONZERO regions, which fill
  identically under either rule; ODD regions require evenodd). Expose
  fillRule / mixedWinding / subpathCount from geometryToPath.
- scene.mjs: emit fillRule: 'evenodd' on the envelope vector (nonzero
  stays implicit); new warning diagnostic
  vector-fill-rule-approximated for mixed rules and for multi-subpath
  vectors with no declared rule, so this class can never be silent.
- design_ir_json: capture fillRule/fill_rule/fill-rule into
  attributes["svg_fill_rule"] (evenodd|nonzero only).
- design_codegen: emit setSvgFillRule(id,'evenodd') (bridge + widget
  already supported it; nothing emitted it).
- design_cpp_codegen: emit ->set_fill_rule(FillRule::evenodd) at the
  svg_path call site (SvgRect/SvgLine share emit_svg_paint but have no
  fill rule).
- design_import_native_common: apply_svg_paint(SvgPathWidget&) reads
  svg_fill_rule plus raw fill-rule/fillRule.

Verified on designers-pick 1:486: both Sub icons regain their hollow
woofers; thumb_parity's two worst blocks (dE 25.7/24.4 at design
482,461/419,461) collapse below threshold, 77 -> 73 visible-diff blocks
against a same-build baseline, and Tom/HiHat/WoodBlock icons pick up
their holes too. Tests: fig-lane node tests for the rule pick and the
envelope contract, IR round-trip + JS codegen emission, baked-C++
emission, native materializer application, and a raster donut proof
that evenodd opens a hole nonzero fills solid (renders compared by
difference — the screenshot backend composites onto an opaque
background at 2x, so absolute-pixel hole checks always read opaque).

Hotspot-Grow: core/view/src/design_ir_json.cpp reason="fillRule capture in the svg attribute block (+14)"
Hotspot-Grow: core/view/src/design_codegen.cpp reason="setSvgFillRule emission (+6)"
Hotspot-Grow: core/view/src/design_cpp_codegen.cpp reason="set_fill_rule emission at svg_path call site (+12)"
Hotspot-Grow: core/view/src/design_import_native_common.cpp reason="fill-rule application in apply_svg_paint (+12)"
Hotspot-Grow: test/test_design_import_ir.cpp reason="fill-rule IR round-trip + codegen emission test (+56)"
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014e8kbMn8Sf4mF8ozsy1Jot
…g vectors keep Figma's flex slot

A .fig transform's m02/m12 is where the local ORIGIN lands, and a mirror
(m00 = -1) puts the box on the other side of it: the visual box spans
[m02 - w, m02]. Reading m02 as "left" made the geometry sidecar report
the Triaz Env/Reverse chip's icon (Frame 274) one full icon-width right
of where Figma draws it, so layout_parity cried dx=-10.4 in all three
Triaz files at a node the flex pass had placed exactly right. Three
fixes, one rule (mirrorAwareOrigin):

- The geometry sidecar (node-box and vector-ink entries) and styleFor's
  plain absolute placement report the mirror-aware min corner. Scoped to
  TRUE mirrors (exactly one negative axis): both-negative is a 180deg
  rotation whose raw-m02 placement the slider-fill lesson pins.
- The emitted tree drops a container's mirror, so a vector under a
  net-mirrored ancestor chain bakes that mirror into its normalized path
  (geometryToPath's new mirror arg). The icon frame's flip cancels its
  child Union's own baked flip in Figma; without the re-bake our
  "reverse" arrow rendered pointing forwards.
- A flowing vector's flex slot is its NODE box, not its stroke-inflated
  ink: the env chip's 12px arrow carries a 2px stroke, and its 14.14px
  ink pushed the "env" label 2.14px right. The decoder reconciles ink to
  node box with negative layout margins (gated to overhangs within
  strokeWeight + 1 so instance-scale gaps are never "corrected"), and
  the JS codegen now lowers layout.margin_* — it silently dropped them.

layout_parity on the three Triaz files: Frame 274 dx=-10.4 -> 0.00 (six
instances across A Channel FX / designers-pick / Sequencer), env text
dx=+2.14 -> 0.00, Sequencer now fully clean; matched-node counts
unchanged (1248/1283/1497) and no new findings.

Hotspot-Grow: core/view/src/design_codegen.cpp reason="margin_* lowering in emit_js_layout_constraints; the IR carried margins the JS lane dropped"
Hotspot-Grow: test/test_design_import_codegen.cpp reason="regression test pinning setFlex margin lowering"
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014e8kbMn8Sf4mF8ozsy1Jot
@shipyard-local
shipyard-local Bot enabled auto-merge (squash) July 21, 2026 11:08
@shipyard-local
shipyard-local Bot added this pull request to the merge queue Jul 21, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 21, 2026
@shipyard-local
shipyard-local Bot enabled auto-merge July 21, 2026 18:05
shipyard-local Bot and others added 4 commits July 21, 2026 17:37
@danielraffel
danielraffel self-requested a review as a code owner July 23, 2026 07:00
@shipyard-local shipyard-local Bot closed this Jul 23, 2026
auto-merge was automatically disabled July 23, 2026 08:03

Pull request was closed

@shipyard-local shipyard-local Bot reopened this Jul 23, 2026
@shipyard-local
shipyard-local Bot added this pull request to the merge queue Jul 23, 2026
@shipyard-local
shipyard-local Bot removed this pull request from the merge queue due to a manual request Jul 23, 2026
@shipyard-local
shipyard-local Bot added this pull request to the merge queue Jul 23, 2026
@shipyard-local
shipyard-local Bot removed this pull request from the merge queue due to a manual request Jul 23, 2026
@shipyard-local
shipyard-local Bot added this pull request to the merge queue Jul 23, 2026
@shipyard-local
shipyard-local Bot removed this pull request from the merge queue due to a manual request Jul 23, 2026
@shipyard-local
shipyard-local Bot added this pull request to the merge queue Jul 23, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to invalid changes in the merge commit Jul 23, 2026
@shipyard-local
shipyard-local Bot added this pull request to the merge queue Jul 23, 2026
@shipyard-local
shipyard-local Bot removed this pull request from the merge queue due to a manual request Jul 23, 2026
@shipyard-local
shipyard-local Bot added this pull request to the merge queue Jul 23, 2026
@shipyard-local
shipyard-local Bot removed this pull request from the merge queue due to a manual request Jul 23, 2026
@shipyard-local
shipyard-local Bot added this pull request to the merge queue Jul 23, 2026
@shipyard-local
shipyard-local Bot removed this pull request from the merge queue due to a manual request Jul 23, 2026
@shipyard-local
shipyard-local Bot added this pull request to the merge queue Jul 23, 2026
@shipyard-local
shipyard-local Bot removed this pull request from the merge queue due to a manual request Jul 23, 2026
@shipyard-local
shipyard-local Bot added this pull request to the merge queue Jul 23, 2026
@shipyard-local
shipyard-local Bot removed this pull request from the merge queue due to a manual request Jul 23, 2026
@shipyard-local
shipyard-local Bot added this pull request to the merge queue Jul 23, 2026
@shipyard-local
shipyard-local Bot removed this pull request from the merge queue due to a manual request Jul 23, 2026
@shipyard-local
shipyard-local Bot added this pull request to the merge queue Jul 23, 2026
Merged via the queue into main with commit 8e6183b Jul 23, 2026
32 of 40 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant