From 4b54ac383ffc9b6e437b72af4f54e164df47312e Mon Sep 17 00:00:00 2001 From: Universe Date: Tue, 28 Apr 2026 23:20:25 +0900 Subject: [PATCH 01/59] docs(research): add Blink SVG research docs and tighten research skill Adds upstream surveys covering Blink's SVG rendering (clip-path, feImage, feTile, textPath, module structure) plus external-CSS lifecycle, and a companion feat-2d doc for htmlcss-svg. Rewrites the research skill to enforce pure-survey docs: forbid Grida content in research docs, add a pre-save review checklist, drop plan-doc coupling. --- .agents/skills/research/SKILL.md | 242 ++---- docs/wg/feat-2d/htmlcss-svg.md | 460 ++++++++++ docs/wg/research/chromium/external-css.md | 313 +++++++ docs/wg/research/chromium/index.md | 45 +- docs/wg/research/chromium/svg/clip-path.md | 786 ++++++++++++++++++ docs/wg/research/chromium/svg/fe-image.md | 487 +++++++++++ docs/wg/research/chromium/svg/fe-tile.md | 492 +++++++++++ docs/wg/research/chromium/svg/index.md | 5 + .../research/chromium/svg/module-structure.md | 235 ++++++ docs/wg/research/chromium/svg/text-on-path.md | 725 ++++++++++++++++ 10 files changed, 3602 insertions(+), 188 deletions(-) create mode 100644 docs/wg/feat-2d/htmlcss-svg.md create mode 100644 docs/wg/research/chromium/external-css.md create mode 100644 docs/wg/research/chromium/svg/clip-path.md create mode 100644 docs/wg/research/chromium/svg/fe-image.md create mode 100644 docs/wg/research/chromium/svg/fe-tile.md create mode 100644 docs/wg/research/chromium/svg/module-structure.md create mode 100644 docs/wg/research/chromium/svg/text-on-path.md diff --git a/.agents/skills/research/SKILL.md b/.agents/skills/research/SKILL.md index 46f70d200..406b2f103 100644 --- a/.agents/skills/research/SKILL.md +++ b/.agents/skills/research/SKILL.md @@ -3,16 +3,15 @@ name: research description: > Research upstream and peer projects to inform Grida's design and implementation. Use when investigating how Chromium, Skia, Servo, - Taffy, or peer canvas editors solve a problem before writing code. Covers - source-code exploration, research document authoring, and the - study-adapt-differ pattern used in .plan.md files. Relevant dirs: - docs/wg/research/, docs/wg/feat-2d/, crates/grida/. + Taffy, or peer canvas editors solve a problem before writing code. + Covers source-code exploration and research document authoring under + docs/wg/research/. --- # Code Research Skill -Workflow for going from "how does X work?" to a documented, actionable -plan grounded in prior art from upstream and peer projects. +Workflow for going from "how does X work?" to a documented survey +grounded in prior art from upstream and peer projects. ## When to Use This Skill @@ -20,7 +19,7 @@ plan grounded in prior art from upstream and peer projects. - Looking up undocumented Skia API behavior - Understanding CSS feature semantics before adding support - Comparing canvas editor architectures (excalidraw, tldraw) -- Writing or extending `docs/wg/research/` documents or `.plan.md` files +- Writing or extending `docs/wg/research/` documents --- @@ -28,220 +27,131 @@ plan grounded in prior art from upstream and peer projects. Before touching any external repo, check what Grida already knows. -### 1. Check existing research - -```text -docs/wg/research/chromium/ # 15 docs covering the full compositor pipeline -├── index.md # START HERE — topic map -├── glossary.md ├── compositor-architecture.md -├── property-trees.md ├── render-surfaces.md -├── damage-tracking.md ├── paint-recording.md -├── tiling-and-rasterization.md ├── tiling-deep-dive.md -├── memory-and-priority.md ├── scheduler.md -├── interaction-and-quality.md -├── resolution-scaling-during-interaction.md -├── pinch-zoom-deep-dive.md └── effect-optimizations.md -``` - -### 2. Check plan documents - -Key plan docs with distilled research: - -- `docs/wg/feat-2d/zoom-compositor-strategy.plan.md` — Chromium pinch-zoom adaptation -- `docs/wg/feat-2d/renderer-rewrite.plan.md` — Chromium compositor mapping -- `docs/wg/feat-2d/optimization.md` — master optimization catalog - -### 3. Check code cross-references - -```sh -grep "chromium\|servo\|adapted from\|ported from\|based on" --include="*.rs" -``` - -Known citations: - -- `crates/grida/src/runtime/effect_tree.rs` — Chromium EffectTree/EffectNode -- `crates/csscascade/src/rcdom/mod.rs` — Servo html5ever rcdom -- `crates/csscascade/` — Servo style system (`style::servo::*`) -- `third_party/usvg/src/text/layout.rs` — Chromium font metrics -- `packages/grida-cmath/index.ts` — Snap.svg arc-to-cubic - -### Discovery queries - | What you need | How to find it | | ---------------------------- | ------------------------------------------------------------- | -| Existing research on a topic | `docs/wg/research/chromium/index.md` | -| Plan docs with external refs | `grep "Chromium\|servo\|upstream" docs/wg/**/*.plan.md` | +| Existing research on a topic | `docs/wg/research/chromium/index.md` (topic map) | | Code that cites sources | `grep "based on\|adapted from\|ported from" --include="*.rs"` | | Vendored third-party code | `ls third_party/` | | Feature docs for a subsystem | `ls docs/wg/feat-*/` | +If already documented, cite it and move on. + --- ## Reference Repositories **Local clones (optional):** If `~/Documents/GitHub/` exists, it may contain default-style clones (sibling dirs named by repo, e.g. `skia`). Prefer searching there before cloning or using only the web. -### Graphics & Rendering - -| Repo | Lang | When to reference | Key paths | -| --------------------------------------------------- | ---- | ----------------------------------------------------------------------------- | ----------------------------------------------------- | -| [chromium](https://github.com/chromium/chromium) | C++ | Skia usage, compositing, layer trees, paint scheduling, tiling, GPU resources | `cc/` `third_party/blink/renderer/` `components/viz/` | -| [skia](https://github.com/google/skia) | C++ | Undocumented API behavior, GPU internals, filter details | `src/gpu/` `src/core/` `src/effects/` | -| [rust-skia](https://github.com/rust-skia/rust-skia) | Rust | Rust binding ergonomics — our direct `skia-safe` dependency | `skia-safe/src/` | -| [resvg](https://github.com/linebender/resvg) | Rust | SVG rendering, path conversion, filter effects | `crates/resvg/src/` `crates/usvg/src/` | - -### Web Standards & CSS - -| Repo | Lang | When to reference | Key paths | -| -------------------------------------------- | ---- | ------------------------------------------------------------------------- | ---------------------------------------- | -| [servo](https://github.com/servo/servo) | Rust | CSS layout, DOM, Rust browser-engine patterns. We vendor its style system | `components/style/` `components/layout/` | -| [stylo](https://github.com/servo/stylo) | Rust | CSS parsing and style resolution | `style/` | -| [taffy](https://github.com/DioxusLabs/taffy) | Rust | Flexbox/Grid layout algorithms and Rust-native layout engine internals | `src/tree/` `src/compute/` `src/style/` | - -### Canvas Editor Peers +| Repo | Lang | When to reference | Key paths | +| ------------------------------------------------------ | ---- | ----------------------------------------------------------------------------- | ----------------------------------------------------- | +| [chromium](https://github.com/chromium/chromium) | C++ | Skia usage, compositing, layer trees, paint scheduling, tiling, GPU resources | `cc/` `third_party/blink/renderer/` `components/viz/` | +| [skia](https://github.com/google/skia) | C++ | Undocumented API behavior, GPU internals, filter details | `src/gpu/` `src/core/` `src/effects/` | +| [rust-skia](https://github.com/rust-skia/rust-skia) | Rust | Rust binding ergonomics — our direct `skia-safe` dependency | `skia-safe/src/` | +| [resvg](https://github.com/linebender/resvg) | Rust | SVG rendering, path conversion, filter effects | `crates/resvg/src/` `crates/usvg/src/` | +| [servo](https://github.com/servo/servo) | Rust | CSS layout, DOM, Rust browser-engine patterns. We vendor its style system | `components/style/` `components/layout/` | +| [stylo](https://github.com/servo/stylo) | Rust | CSS parsing and style resolution | `style/` | +| [taffy](https://github.com/DioxusLabs/taffy) | Rust | Flexbox/Grid layout algorithms | `src/tree/` `src/compute/` `src/style/` | +| [excalidraw](https://github.com/excalidraw/excalidraw) | TS | Canvas API optimization, rendering heuristics, interaction | `packages/excalidraw/renderer/` | +| [tldraw](https://github.com/tldraw/tldraw) | TS | CRDT state model, modular SDK, DOM/SVG canvas | `packages/editor/` `packages/store/` | -| Repo | Lang | When to reference | Key paths | -| ------------------------------------------------------ | ---- | ---------------------------------------------------------- | -------------------------------------------------------------- | -| [excalidraw](https://github.com/excalidraw/excalidraw) | TS | Canvas API optimization, rendering heuristics, interaction | `packages/excalidraw/renderer/` `packages/excalidraw/element/` | -| [tldraw](https://github.com/tldraw/tldraw) | TS | CRDT state model, modular SDK, DOM/SVG canvas | `packages/editor/` `packages/store/` | - -### Searching large repos - -**Chromium** — use https://source.chromium.org/ for fast browsing. Narrow to: `cc/layers/` `cc/tiles/` `cc/trees/` (compositing), `cc/paint/` (recording), `cc/scheduler/` (frame scheduling). - -**Servo** — `components/style/properties/` (CSS props), `components/style/stylist.rs` (resolution), `components/layout/` (layout). - -**Skia** — headers in `include/core/` `include/effects/`, GPU in `src/gpu/ganesh/` (GL) or `src/gpu/graphite/` (Metal/Vulkan). +**Searching large repos:** use https://source.chromium.org/ for Chromium. Narrow to `cc/layers/` `cc/tiles/` `cc/trees/` (compositing), `cc/paint/` (recording), `cc/scheduler/` (frame scheduling). Servo: `components/style/properties/`, `components/style/stylist.rs`, `components/layout/`. Skia: `include/core/` `include/effects/`, GPU in `src/gpu/ganesh/` (GL) or `src/gpu/graphite/` (Metal/Vulkan). --- ## The Research Workflow -### Step 1: Frame the question - -Write a specific, bounded question. Bad: "how does Chromium handle rendering?" Good: "how does Chromium decide which layers get their own composited surface?" - -### Step 2: Check existing knowledge - -1. Read `docs/wg/research/chromium/index.md` -2. `grep "" docs/wg/**/*.plan.md` -3. `grep "" --include="*.rs" crates/` -4. Read `docs/wg/feat-2d/optimization.md` - -If already documented, cite it and move on. - -### Step 3: Explore the source - -Use targeted searches — do not read entire codebases: - -```sh -grep "struct LayerTreeHost" --include="*.h" -r cc/ # find the owning type -grep "ShouldCreateRenderSurface" --include="*.cc" -r cc/ # find decision points -grep "kDefault.*Tile\|kMax.*Memory" --include="*.h" -r cc/ # find design constants -``` - -### Step 4: Extract findings - -For each finding, record: **what** (mechanism + file path), **why** (rationale), **constants** (thresholds/heuristics), **applicability** (maps to our architecture?). - -### Step 5: Document or apply +1. **Frame the question.** Specific and bounded. Bad: "how does Chromium handle rendering?" Good: "how does Chromium decide which layers get their own composited surface?" +2. **Check existing knowledge** in `docs/wg/research/` and code comments before going upstream. +3. **Explore the source** with targeted searches — never read entire codebases: + ```sh + grep "struct LayerTreeHost" --include="*.h" -r cc/ # owning type + grep "ShouldCreateRenderSurface" --include="*.cc" -r cc/ # decision points + grep "kDefault.*Tile\|kMax.*Memory" --include="*.h" -r cc/ # design constants + ``` +4. **Extract findings:** **what** (mechanism + file path), **why** (rationale), **constants** (thresholds/heuristics). +5. **Document:** | Scope | Action | | ---------------------------- | --------------------------------------------- | | Quick answer | Code comment citing the source | | Reusable subsystem knowledge | Research doc in `docs/wg/research//` | -| New feature design | "Reference Approach" section in `.plan.md` | | Confirming existing approach | Update the relevant research doc | --- ## Writing Research Documents -Docs live in `docs/wg/research//`. Create new subdirectories as needed (`servo/`, `skia/`). +> **A research document is a pure survey of how upstream solves a problem.** It describes +> the upstream system on its own terms, in enough depth that a reader could reimplement +> the design from the doc alone. It is **not** a plan, a proposal, or a gap analysis — +> Grida should be essentially absent from these pages. -Every research document must contain: - -1. **Title and scope** — what subsystem, what questions it answers -2. **Source references** — upstream file paths (with commit hash if volatile) -3. **Architecture description** — how the subsystem works, with diagrams for pipelines -4. **Key data structures** — important types and relationships (use upstream names) -5. **Constants and heuristics** — magic numbers and their reasoning -6. **Relevance to Grida** — how this maps or doesn't map to our architecture +Docs live in `docs/wg/research//`. Create new subdirectories as needed (`servo/`, `skia/`). -**Conventions:** Use upstream terminology. Include short code excerpts (5-15 lines) with file path citations. Organize by concept, not by file. Update `index.md` when adding new docs. File names: lowercase, hyphenated, topic-descriptive. +### Stay in survey mode ---- +Write as if Grida did not exist. The reader is someone trying to understand the upstream project — Chromium, Skia, Servo, etc. — not someone planning a Grida change. Concretely: -## Applying Research to Plan Documents +- **Frame in upstream terms.** "Blink resolves `clip-path` by..." (✅) — not "Blink does X, which is what we'd need..." (❌). +- **Use upstream names** for types, files, functions, constants. +- **Quote the spec, the source, the upstream commit.** Anchor every claim to a file path or spec section. +- **Compare upstream to upstream.** Chromium vs. resvg vs. Servo belongs here; Chromium vs. Grida does not. +- **Spec gotchas, magic numbers, edge cases** are on-topic — they explain the upstream design. -Grida uses a **study-adapt-differ** pattern. Every `.plan.md` referencing external work needs three sections: +### Keep Grida out of the body -### 1. "Reference Approach" (study) +Forbidden in research docs: -Describe how upstream solves it — cite the research doc, name types/algorithms, include key constants: +- "Relevance to Grida" / "What we borrow" / "What we differ on" sections. +- Citations of `crates/grida/...` or any in-repo file path. +- "Plan for our implementation" / "Implementation checklist" sections. +- Sentences using "we", "our renderer", "our codebase", or "our fix" for Grida-side work. +- "This is the gap blocking N fixtures" intros, "TODO" lists, "where the fix lands" footers. -```markdown -## Chromium's Approach (Reference) +A neutral "the Rust binding for this Skia API is `skia_safe::FooBar`" is fine — it surveys the primitive. Becomes off-limits the moment the sentence ties it to Grida ("we use this at filename.rs:42"). -Chromium uses a CoverageIterator that walks tilings from highest to -lowest resolution. During pinch-zoom, TreePriority is set to -SMOOTHNESS_TAKES_PRIORITY to show stretched tiles rather than checkerboard. -See: `docs/wg/research/chromium/pinch-zoom-deep-dive.md` -``` +### Required structure -### 2. "What We Borrow" (adapt) +1. **Title and scope** — what subsystem, what questions it answers (in upstream terms). +2. **Source references** — upstream file paths (with commit hash if volatile). +3. **Architecture description** — how the upstream subsystem works, with diagrams for pipelines. +4. **Key data structures** — important types and relationships (use upstream names). +5. **Constants and heuristics** — magic numbers and their reasoning. +6. **Cross-project comparison** (when relevant) — Chromium vs. resvg vs. Servo vs. Skia, each on its own terms. -Mapping table from upstream concepts to our types: +**Conventions:** Upstream terminology. Short code excerpts (5–15 lines) with file path citations. Organize by concept, not by file. Update `index.md` when adding new docs. File names: lowercase, hyphenated, topic-descriptive. -```markdown -| Chromium | Grida | Notes | -| ----------------------------- | ------------------------------- | ---------------------------- | -| Stale-tile GPU stretching | `LayerImage` reuse at old scale | Per-node instead of per-tile | -| Power-of-2 raster scale steps | `snap_to_power_of_two()` | Reduces re-rasterization | -``` +### Review your draft before saving -### 3. "What We Do Differently" (differ) +Treat this as a required step — past drafts have repeatedly slipped Grida-side content into research, and it's cheapest to catch right before save: -What we're NOT adopting and why. This prevents future contributors from "fixing" intentional divergences: +- [ ] Search for `Grida`, `grida`, `crates/grida`, `our `, `we `, `we'd`, `we have`, `we use`. Every match must justify itself — usually by being removed. +- [ ] Skim for headings like "Relevance to ...", "What we borrow", "What we differ on", "Plan for ...", "Implementation checklist", "Where our code is wrong". Delete them. +- [ ] Check the intro and conclusion. Framed around an upstream question, or around a Grida gap? Reframe to upstream. +- [ ] Check "See also" / "References". Internal Grida paths there are a smell. +- [ ] If the doc would be useless to a reader who didn't know Grida exists, it isn't a research doc yet. Move the Grida-side content out. -```markdown -- **No multiple concurrent tilings.** We cache per-node, not per-tile. - WASM memory constraints make multi-tiling impractical. -- **No worker-thread rasterization.** Single-threaded WASM constraint. - We compensate with time-budgeted incremental re-raster. -``` +If you find yourself wanting to write "and here's how this maps to our renderer", stop — that's not what a research doc is for. --- ## Pitfalls -**Researching what's already documented.** Check `docs/wg/research/`, plan docs, and code comments first. The Chromium research alone is 15 documents. - -**Cargo-culting without understanding constraints.** Always filter upstream approaches through our constraints: - -- Single thread (WASM) -- Per-node cache (not spatial tiles) -- Infinite canvas (viewport culling is primary) -- Stable scene graph (no CSS reflow on zoom) - -**Reading too broadly.** Chromium is 30M+ lines. Arrive with a specific question, find the code path, extract the answer, leave. Use source.chromium.org. - -**Skipping the "differ" section.** Most dangerous omission. Without it, future contributors will "fix" intentional divergences from Chromium. - -**Confusing Skia docs with Skia behavior.** Skia's documentation is minimal and sometimes wrong. Read the implementation for performance characteristics and edge cases. - -**Stale source references.** Reference stable concepts (struct names, enum variants) over line numbers. Include enough context to relocate if files move. - -**Mixing terminologies.** Research docs: upstream terms. Plan docs: mapping tables. Code comments: our terms with parenthetical upstream reference. +- **Writing a Grida-flavored survey.** Most common failure mode — the brief is "research X" and the agent writes a survey ending in "here's how we should do it". Stay in survey mode. +- **Researching what's already documented.** Check `docs/wg/research/` and code comments first. The Chromium research alone is 15 documents. +- **Reading too broadly.** Arrive with a specific question, find the code path, extract the answer, leave. Use source.chromium.org. +- **Confusing Skia docs with Skia behavior.** Skia's documentation is minimal and sometimes wrong. Read the implementation. +- **Stale source references.** Reference stable concepts (struct names, enum variants) over line numbers. +- **Mixing terminologies.** Research docs: upstream terms. Code comments: Grida terms with parenthetical upstream reference. --- ## Checklist - [ ] Framed a specific, bounded question -- [ ] Checked existing research, plan docs, and code comments +- [ ] Checked existing research and code comments - [ ] Identified the right repo and narrowed to specific directories - [ ] Extracted findings with file paths, rationale, and constants -- [ ] Assessed applicability against our constraints -- [ ] Documented findings (research doc, plan section, or code comment) +- [ ] Wrote the research doc as a pure upstream survey — no Grida content +- [ ] **Reviewed the draft** against the [review checklist](#review-your-draft-before-saving) — searched for `Grida`, `our`, `we`; deleted any planning sections - [ ] Updated `index.md` if a new research doc was created diff --git a/docs/wg/feat-2d/htmlcss-svg.md b/docs/wg/feat-2d/htmlcss-svg.md new file mode 100644 index 000000000..e7c065b5d --- /dev/null +++ b/docs/wg/feat-2d/htmlcss-svg.md @@ -0,0 +1,460 @@ +--- +title: "htmlcss::svg — Design Study" +format: md +tags: + - internal + - wg + - feat-2d + - svg + - design + - study +--- + +# htmlcss::svg — Design Study + +The structure-and-semantics study that informs the Skia-backed SVG +renderer at `crates/grida/src/htmlcss/svg/`. Companion to the module's +`crates/grida/src/htmlcss/svg/README.md`. + +The renderer's lineage is **Blink** (Chromium). usvg and resvg are +secondary references — usvg as a parse-time normalization model, resvg +as a one-pass renderer model. Each section below ends with an explicit +**Adopt** / **Differ** line so a reader can audit our deviations from +Blink in one pass. + +## Why this study exists + +The htmlcss module already accepts SVG content in two places, but both +delegate to Skia's built-in `svg::Dom` (`SkSVGDOM`). Chromium +**deliberately does not use** `SkSVGDOM`; per +[`docs/wg/research/chromium/svg/index.md`](../research/chromium/svg/index.md) +lines 87–89, Skia's DOM is _"for embedders that need standalone SVG +rendering without DOM/CSS/JS integration"_. Grida is becoming such an +embedder, but with a sharper goal: parity with Chromium for static SVG, +validated against the resvg-test-suite (1,679 fixtures already on disk +at `fixtures/local/resvg-test-suite/`). + +This study captures the design decisions before code lands. Implementation +proceeds per-feature through +`.agents/skills/dev-render-htmlcss-feature/SKILL.md`. + +--- + +## S1. Chromium / Blink — the lead reference + +**Anchors**: +[`docs/wg/research/chromium/svg/{index, pipeline, coordinate-systems, paint-servers, resources-and-effects, path-geometry, text, use-and-foreign-object, svg-as-image}.md`](../research/chromium/svg/). + +### Pipeline shape + +Blink's SVG pipeline is **parsing → style → layout → paint → composite** +(`pipeline.md` §3, lines 88–124). The composite phase (cc property trees, +GPU raster) is Chromium's compositor; we don't replicate it. The first +four phases are what we mirror. + +The bridge between CSS layout and the SVG coordinate system is +`LayoutSVGRoot extends LayoutReplaced`. The outer `` element is a +CSS replaced element with a CSS box; that box is the SVG viewport. Inside, +geometry is in user units mapped via `viewBox` + `preserveAspectRatio` +to the viewport rect. + +### CSS-cascade integration + +`SVGComputedStyle` rides alongside `ComputedStyle`. SVG-specific +properties (`fill`, `stroke`, `stroke-dasharray`, `marker-*`, `stop-color`, +`paint-order`, `clip-rule`, `fill-rule`, `flood-color`, etc.) live in +`SVGComputedStyle` and are accessed via `style.SvgStyle()`. The cascade +itself is the same code path as for HTML. + +The crucial detail: SVG **presentation attributes** (`fill="red"`, +`stroke-width="2"`) are translated to CSS values and inserted into the +cascade at presentation-attribute specificity (lower than rules, higher +than UA defaults). The bridge is `SVGAnimatedLength::CssValue()` and the +`CSSPropertyID` argument wired into each `SVGAnimated*` constructor +(`pipeline.md` lines 52–69). + +This is _the_ Blink pattern we copy: it's why we reuse Stylo. Selector +matching, cascade order, custom-property resolution, `currentColor` +fallback — all come for free. + +### Layout tree + +``` +LayoutSVGRoot extends LayoutReplaced; CSS box ↔ SVG viewport +├── LayoutSVGContainer , , etc. +│ ├── LayoutSVGTransformableContainer +│ ├── LayoutSVGViewportContainer nested +│ ├── LayoutSVGHiddenContainer , , , , +│ │ └── LayoutSVGResourceContainer base for all resource containers +│ │ ├── LayoutSVGResourcePaintServer +│ │ │ ├── LayoutSVGResourcePattern +│ │ │ └── LayoutSVGResourceGradient +│ │ ├── LayoutSVGResourceClipper +│ │ ├── LayoutSVGResourceMasker +│ │ ├── LayoutSVGResourceFilter +│ │ └── LayoutSVGResourceMarker +├── LayoutSVGShape , , , , , , +├── LayoutSVGImage +└── LayoutSVGForeignObject +LayoutSVGText +└── LayoutSVGInline , + └── LayoutSVGInlineText +``` + +Layout produces **paths and bounding boxes in local coordinates plus a +`LocalToSVGParentTransform()` per node** — _not_ block fragments. Hidden +containers exist for bbox/resource resolution but are skipped during +paint. + +### Painter family + +One `*Painter` per `Layout*` class. Per-node setup: + +- `ScopedSVGTransformState` — concats `LocalSVGTransform()` onto the + `GraphicsContext` CTM (RAII). +- `ScopedSVGPaintState` — prepares fill/stroke `cc::PaintFlags`, applies + paint servers, clips, masks, filters; decides isolation + (`save_layer`). + +Paint order inside a shape respects the `paint-order` property (default +`fill stroke markers`). + +### Resource model + +`LayoutSVGResource{PaintServer, Clipper, Masker, Filter, Marker}` plus +`SVGResources::GetClient()` for `url(#id)` resolution. Per-client cache +because `objectBoundingBox` units make shaders bbox-dependent. + +### Filter graph + +`SVGFilterBuilder` walks the `` children and builds a +`FilterEffect` graph (a DAG with named inputs/outputs). The graph is then +realized as a single `SkImageFilter` and set on the painted layer's +`SkPaint`. Skia's filter compiler does the heavy lifting; we mirror that +flow. + +### Text + +Two-phase. LayoutNG inline layout produces glyph runs (the same +machinery HTML uses). Then `SvgTextLayoutAlgorithm` rewrites per-glyph +positions for `x` / `y` / `dx` / `dy` / `rotate` / `textPath`. Text +stays semantic to paint (`DrawTextBlob`), not flattened. + +### `` / `` + +`` is a runtime shadow-instance tree (`use-and-foreign-object.md`). +The target subtree is deep-cloned at the use site with the use element's +transform composed in. `` recurses back into the HTML +layout/paint pipeline with a sub-canvas — a true HTML-in-SVG bridge. + +### Adopt / Differ + +**Adopt**: All of the above — pipeline shape, type taxonomy, public +interface, presentation-attribute → CSS aliasing, `LayoutSVGRoot extends +LayoutReplaced` viewport bridge, painter family, resource model with +per-client caches, filter graph, two-phase text. + +**Differ**: No SMIL, no Web Animations, no scripting (no `ScriptWrappable`, +no tear-offs, no `baseVal`/`animVal` split — Grida is static, so each +attribute carries a single resolved value). No compositor property trees +(we emit a single `Picture`; Skia handles its own internal compositing). +No invalidation graph (single-shot renderer; nothing to invalidate). No +hit testing, no accessibility tree. + +--- + +## S2. usvg — a parse-time normalization model (study, not a dependency) + +**Anchors**: `resvg/crates/usvg/src/parser/`, `resvg/crates/usvg/src/tree/`, +summarized in [`comparison.md`](../research/chromium/svg/comparison.md) +lines 56–73. + +usvg's job is to take a raw SVG document and normalize away every +ambiguity, producing a frozen tree where the renderer doesn't have to +ask "what does this inherit?" or "what units are these in?". The list +of normalizations: + +| usvg normalization | Where in `htmlcss::svg` | Phase | +| --------------------------------------- | -------------------------------------------------------- | ------------------ | +| Inheritance flatten | `style/inherit.rs` (after Stylo cascade) | style | +| `` deep-clone | `layout/use_expand.rs` | layout | +| Basic-shape → path | `layout/shape.rs` | layout | +| Arc → cubic decomposition | `dom/path_d.rs` (during `d=` parse) | parse | +| `` resolution | `dom/parser.rs` | parse | +| `objectBoundingBox` → user space | `resources/{gradient,pattern,clipper,masker,filter}.rs` | paint (per-client) | +| Paint-server pool extraction | `resources/mod.rs` (`ResourceTable`) | layout | +| Pre-computed bboxes (`Group::abs_bbox`) | `layout/bbox.rs` | layout | +| Text shape + outline at parse | **NOT adopted** — we keep text semantic (Blink approach) | n/a | + +### What usvg gives us as an idea + +The principle of an **explicit normalized IR between parse and paint** is +what we copy from usvg. Resvg's renderer never sees ``, never sees +``, never sees percent units, never resolves inheritance — usvg +already did all that. Our pipeline does the same work, but spread across +parse / style / layout phases (not all at parse time, because Stylo +operates on the _unflattened_ DOM and we want to keep the cascade alive +through layout). + +### Why we don't link usvg + +usvg is a great library; using it in the _runtime_ would be wrong because +it pulls a parallel ecosystem: + +- **fontdb** vs our `FontRepository` — two font registries fighting for + the same fonts. +- **simplecss** vs Stylo — usvg's CSS support is intentionally minimal + (no custom properties, no `:hover`, no media queries); Stylo is the + full thing. +- **tiny-skia-path** vs `SkPath` — duplicate path representations + inside the same crate. + +We already have the more capable half of every pair. We re-implement +usvg's normalization ideas against our Blink-shaped IR. + +### Adopt / Differ + +**Adopt**: The normalization _list_ and the principle of a frozen IR. +Pre-computed absolute bboxes for cull-rect skipping. Arc-to-cubic at +parse time (no style dependence). `` at parse time (no style +dependence). Basic-shape → path at layout time. + +**Differ**: We do not consume `usvg::Tree`. We re-implement these +normalizations against our own IR using Stylo for the cascade and Skia's +`Font` / `Paragraph` machinery for text. Text is kept semantic (Blink's +strategy), not flattened to paths. + +--- + +## S3. resvg — a one-pass renderer model + +**Anchors**: `resvg/crates/resvg/src/{render.rs, path.rs, filter/mod.rs}`. + +resvg is "render a usvg::Tree to a tiny-skia Pixmap". Static, CPU-only, +single-pass, no caches beyond what the IR already provides. Its +simplicity is instructive — Blink's equivalents are heavier than they +need to be for a static renderer, and we steal the simpler shape where +correctness allows. + +### Patterns we adopt + +- **Isolate-on-effect group rule**: a Group needs a `save_layer` only + when it has opacity < 1, a non-default blend mode, a filter, a mask, + or a non-trivial clip. resvg encodes this as a single boolean per + group; Blink reaches the same outcome via `PaintLayer` stacking-context + rules. We use the resvg formulation — simpler, sufficient for static + rendering. +- **Named-result filter table**: filter primitives walked in document + order, reading from / writing to a `HashMap<&str, Image>` + (`SourceGraphic`, `SourceAlpha`, `BackgroundImage`, plus `result=` + outputs). resvg does this on tiny-skia pixmaps; we do it on + `skia_safe::ImageFilter` nodes — same control flow, different leaf + nodes. +- **Pattern-as-recorded-shader**: render the pattern definition once + into an `SkPicture`, wrap as `image_shader`. resvg renders to a + per-use bitmap; Skia gives us shader-from-picture for free, so we + record once and reuse across all clients. + +### Where resvg is wrong (and we follow Blink instead) + +- **`color-interpolation-filters`** default. SVG spec says `linearRGB` + for `` content; resvg gets this wrong on several tests. + Chromium handles it correctly by wrapping filter graphs in sRGB↔linear + conversion at graph boundaries. We mirror Chromium. +- **Text flattening**. resvg flattens text to paths at parse time. This + loses selectability (no editor copy/paste), inflates picture size, + and breaks animations on text content. Blink keeps text semantic. We + follow Blink. + +### Adopt / Differ + +**Adopt**: Isolate-on-effect group rule, named-result filter table, +pattern-as-recorded-shader. + +**Differ**: All draw ops to Skia (not tiny-skia). Filters compose into +`skia_safe::ImageFilter` graphs (Skia's own filter compiler), not a CPU +primitive loop. Shadow-tree `` (Blink) instead of parse-time deep +clone (resvg) — leaves the door open for live-DOM scenarios later. +Text stays semantic. + +--- + +## S4. Synthesis — the Grida pipeline + +``` +parse style layout paint +───── ───── ────── ───── +xml bytes ──▶ SvgDocument with ──▶ LayoutSvgRoot tree ──▶ Skia Canvas + resolved SvgComputedStyle (paths, transforms, + per element paint-server table, + pre-computed bboxes) + ▲ ▲ + │ │ + Stylo cascade applied; SkShaders + + presentation attrs aliased SkImageFilters + to CSS; / resolved per-client + resolved +``` + +### Phase responsibilities + +**Parse** (`dom/`): XML → typed `SvgDocument` via `roxmltree`. One Rust +type per Blink `SVG*Element`. Path `d=` → `SvgPathCommands` (no Skia +type). Arc-to-cubic during parse. `` resolved during parse. + +**Style** (`style/`): Reuse htmlcss's existing Stylo session. +Presentation attrs aliased to CSS. ` + + +
+
+
+ + htmlcss reftest + · + resvg-test-suite +
+
+
+ + categories ▾ + +
+
+
+
+
+ + +
+ + + + + + +
+ + — +
+
+
+ +
+
+
+ + + + + + + + diff --git a/crates/grida_dev/src/main.rs b/crates/grida_dev/src/main.rs index 19cc05b7b..b186862e0 100644 --- a/crates/grida_dev/src/main.rs +++ b/crates/grida_dev/src/main.rs @@ -337,6 +337,117 @@ fn scene_from_svg_path(path: &Path) -> Result { }) } +/// Best-effort intrinsic size from the root tag — width/height first, +/// then viewBox. Returns None if the file isn't a single rooted SVG with +/// parseable dimensions; caller should fall back to measure-based sizing. +fn sniff_svg_size(xml: &str) -> Option<(f32, f32)> { + let open = xml.find("')?; + let tag = &open[..tag_end]; + fn attr(tag: &str, name: &str) -> Option { + let needle = format!("{}=", name); + let start = tag.find(&needle)? + needle.len(); + let rest = &tag[start..]; + let (quote, rest) = rest.split_at(1); + let q = quote.chars().next()?; + if q != '"' && q != '\'' { + return None; + } + let end = rest.find(q)?; + let raw = &rest[..end]; + let numeric_end = raw + .find(|c: char| !(c.is_ascii_digit() || c == '.' || c == '-')) + .unwrap_or(raw.len()); + raw[..numeric_end].parse::().ok().map(|v| v.max(1.0)) + } + if let (Some(w), Some(h)) = (attr(tag, "width"), attr(tag, "height")) { + return Some((w, h)); + } + let vb = { + let needle = "viewBox="; + let start = tag.find(needle)? + needle.len(); + let rest = &tag[start..]; + let (quote, rest) = rest.split_at(1); + let q = quote.chars().next()?; + if q != '"' && q != '\'' { + return None; + } + let end = rest.find(q)?; + rest[..end].to_string() + }; + let parts: Vec = vb + .split(|c: char| c.is_whitespace() || c == ',') + .filter_map(|s| s.parse::().ok()) + .collect(); + if parts.len() >= 4 { + Some((parts[2].max(1.0), parts[3].max(1.0))) + } else { + None + } +} + +async fn scene_from_svg_embed_path(path: &Path) -> Result { + use grida::cg::prelude::CGColor; + use grida::node::factory::NodeFactory; + use grida::node::scene_graph::{Parent, SceneGraph}; + use grida::node::schema::Node; + + let svg_source = std::fs::read_to_string(path) + .with_context(|| format!("failed to read {}", path.display()))?; + + // htmlcss renders inline inside HTML via the same code path as standalone + // SVG, so wrapping is a thin envelope — no parsing/serialization roundtrip on the + // SVG body itself. + let html = format!( + "{}", + svg_source + ); + + let (width, height) = match sniff_svg_size(&svg_source) { + Some((w, h)) => (w, h), + None => { + let temp_fonts = { + use grida::resources::ByteStore; + use grida::runtime::font_repository::FontRepository; + let mut repo = FontRepository::new(std::sync::Arc::new(std::sync::Mutex::new( + ByteStore::new(), + ))); + repo.enable_system_fallback(); + repo + }; + let w = 800.0_f32; + let h = grida::htmlcss::measure_content_height( + &html, + w, + &temp_fonts, + &grida::htmlcss::NoImages, + ) + .unwrap_or(600.0); + (w, h) + } + }; + + let nf = NodeFactory::new(); + let mut node = nf.create_html_embed_node(); + node.html = html; + node.size = grida::node::schema::Size { width, height }; + + let mut graph = SceneGraph::new(); + graph.append_child(Node::HTMLEmbed(node), Parent::Root); + + Ok(HtmlEmbedScene { + scene: Scene { + name: path + .file_name() + .map(|n| n.to_string_lossy().into_owned()) + .unwrap_or_else(|| "SVG (Embed)".to_string()), + graph, + background_color: Some(CGColor::from_u32(0xFFFFFFFF)), + }, + images: Vec::new(), + }) +} + fn scene_from_html_path(path: &Path) -> Result { use grida::cg::prelude::CGColor; let html_source = std::fs::read_to_string(path) @@ -431,6 +542,26 @@ fn ask_html_import_mode() -> bool { } } +/// Show a dialog asking the user to choose between Embed and Convert for SVG files. +/// Returns true for Embed, false for Convert. +fn ask_svg_import_mode() -> bool { + use rfd::MessageButtons; + use rfd::MessageDialog; + use rfd::MessageDialogResult; + + let result = MessageDialog::new() + .set_title("SVG Import Mode") + .set_description("How would you like to import this SVG file?\n\n\u{2022} Embed \u{2014} Render as opaque picture (htmlcss-accurate, non-editable)\n\u{2022} Convert \u{2014} Convert to editable Grida vector nodes (lossy SVG)") + .set_buttons(MessageButtons::OkCancelCustom("Embed".to_string(), "Convert".to_string())) + .show(); + + match result { + MessageDialogResult::Ok => true, + MessageDialogResult::Custom(s) if s == "Embed" => true, + _ => false, + } +} + fn scene_from_markdown_embed_path(path: &Path) -> Result { use grida::cg::prelude::CGColor; use grida::node::factory::NodeFactory; @@ -538,10 +669,20 @@ async fn load_master_scenes_from_path(path: &Path) -> Result { images: Vec::new(), }) } - "svg" => scene_from_svg_path(path).map(|s| LoadedScenes { - scenes: vec![s], - images: Vec::new(), - }), + "svg" => { + if ask_svg_import_mode() { + let result = scene_from_svg_embed_path(path).await?; + Ok(LoadedScenes { + scenes: vec![result.scene], + images: result.images, + }) + } else { + scene_from_svg_path(path).map(|s| LoadedScenes { + scenes: vec![s], + images: Vec::new(), + }) + } + } "html" | "htm" => { if ask_html_import_mode() { let result = scene_from_html_embed_path(path).await?; diff --git a/crates/grida_dev/src/reftest/args.rs b/crates/grida_dev/src/reftest/args.rs index e2d80007f..3209d1fad 100644 --- a/crates/grida_dev/src/reftest/args.rs +++ b/crates/grida_dev/src/reftest/args.rs @@ -27,17 +27,12 @@ impl std::str::FromStr for BgColor { /// - `Htmlcss`: goes through `grida::htmlcss::render_svg`, which records /// into a Skia `Picture` via `PictureRecorder` before rasterizing. /// Exercises the exact code path that inline `` inside HTML -/// takes. -/// - `Sksvg`: **minimal** direct path — `skia_safe::svg::Dom::from_bytes` -/// → `surface.canvas()` → `dom.render()`. No htmlcss module, no -/// Picture recording, no Grida tree surgery. Used to isolate Skia's -/// native SVG module so that any failure is attributable to Skia -/// itself, not our wrapping / plumbing. +/// takes, end-to-end through the in-tree htmlcss::svg renderer (no +/// fallback to Skia's built-in `svg::Dom`). #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum SvgRenderer { Iosvg, Htmlcss, - Sksvg, } impl std::str::FromStr for SvgRenderer { @@ -46,11 +41,7 @@ impl std::str::FromStr for SvgRenderer { match s.to_ascii_lowercase().as_str() { "iosvg" | "grida" | "pack" => Ok(SvgRenderer::Iosvg), "htmlcss" => Ok(SvgRenderer::Htmlcss), - "sksvg" | "skia-svg" | "skia_svg" | "skiasvg" | "skia" => Ok(SvgRenderer::Sksvg), - other => Err(format!( - "invalid renderer: {} (use iosvg|htmlcss|sksvg)", - other - )), + other => Err(format!("invalid renderer: {} (use iosvg|htmlcss)", other)), } } } @@ -92,10 +83,9 @@ pub(crate) struct ReftestArgs { /// SVG renderer backend: /// - `iosvg` (default): grida scene graph via usvg → pack. - /// - `htmlcss`: grida::htmlcss::render_svg → PictureRecorder → surface. - /// - `sksvg`: direct Skia svg::Dom → surface (no htmlcss wrapping). - /// Use this to prove a failure is Skia's own SVG module, not our - /// plumbing. Aliases: `skia-svg`, `skia_svg`, `skia`. + /// - `htmlcss`: grida::htmlcss::render_svg → PictureRecorder → + /// surface. End-to-end through the in-tree htmlcss::svg + /// renderer; no fallback to Skia's built-in `svg::Dom`. #[arg(long = "renderer", default_value = "iosvg")] pub renderer: SvgRenderer, } diff --git a/crates/grida_dev/src/reftest/render.rs b/crates/grida_dev/src/reftest/render.rs index fc7a6d0a6..b239aee5f 100644 --- a/crates/grida_dev/src/reftest/render.rs +++ b/crates/grida_dev/src/reftest/render.rs @@ -150,15 +150,15 @@ pub(crate) fn find_test_pairs_in_dirs(svg_dir: &Path, png_dir: &Path) -> Result< Ok(pairs) } -/// Render an SVG file through the new htmlcss → `skia_safe::svg::Dom` -/// path and write a PNG. +/// Render an SVG file through `grida::htmlcss::render_svg` (the +/// in-tree htmlcss::svg renderer) and write a PNG. /// /// Unlike [`render_svg_to_png`], which round-trips through the Grida -/// scene graph via `grida::import::svg::pack`, this path delegates directly to -/// Skia's SVG module — matching Chromium's rendering for WPT-style -/// reftests. Target size is the reference PNG's pixel dimensions; -/// `viewBox` + `preserveAspectRatio` inside the SVG map user units to -/// that box. +/// scene graph via `grida::import::svg::pack`, this path uses our +/// own Skia-backed SVG renderer end-to-end — there is no fallback to +/// Skia's built-in `svg::Dom`. Target size is the reference PNG's +/// pixel dimensions; `viewBox` + `preserveAspectRatio` inside the SVG +/// map user units to that box. pub(crate) fn render_svg_to_png_via_htmlcss( svg_path: &Path, output_path: &Path, @@ -180,10 +180,34 @@ pub(crate) fn render_svg_to_png_via_htmlcss( } }; - // Record the SVG into a Skia Picture via the htmlcss module's public - // entry point. - let picture = grida::htmlcss::render_svg(&svg_source, width as f32, height as f32) - .map_err(|e| anyhow!("htmlcss::render_svg failed: {e}"))?; + // Pre-resolve any relative `xlink:href` / `href` references in the + // SVG against the fixture's directory so the painter's + // `ImageProvider` can find them. Without this, fixtures using + // `` render blank. + let mut images = grida::htmlcss::PreloadedImages::new(); + let mut css = grida::htmlcss::svg::PreloadedCss::new(); + if let Some(parent) = svg_path.parent() { + preload_referenced_images(&svg_source, parent, &mut images); + preload_referenced_css(&svg_source, parent, &mut css); + } + + // Build a font resolver. For the resvg-test-suite path, this loads + // the suite's bundled `fonts/` directory and applies the same + // generic-family map vdiff sets up via `--*-family` flags. Mirrors + // `--skip-system-fonts`: families not in the curated set return + // `None` rather than silently falling through to CoreText/fontconfig. + let fonts = resolve_test_suite_fonts(svg_path); + + // Record the SVG into a Skia Picture via the htmlcss module's + // resource-aware entry point. + let context = grida::htmlcss::svg::RenderContext::new(&images, &css, &fonts); + let picture = grida::htmlcss::svg::render_to_picture_with_context( + &svg_source, + width as f32, + height as f32, + context, + ) + .map_err(|e| anyhow!("htmlcss::svg::render_to_picture_with_context failed: {e}"))?; // Rasterize the Picture onto a CPU-backed surface. Transparent clear // lets the reftest's background masking (`bg = white|black`) composite @@ -236,70 +260,37 @@ fn sniff_svg_dimensions(svg: &str) -> Option<(u32, u32)> { .unwrap_or(raw.len()); raw[..numeric_end].parse::().ok().map(|v| v as u32) } - let w = attr(tag, "width")?; - let h = attr(tag, "height")?; - Some((w.max(1), h.max(1))) + if let (Some(w), Some(h)) = (attr(tag, "width"), attr(tag, "height")) { + return Some((w.max(1), h.max(1))); + } + // No explicit width/height — derive intrinsic dims from viewBox. + // Per SVG 2 §5.4 / CSS Images 3 §5, an SVG with only a viewBox has + // intrinsic dimensions equal to the viewBox extents. + let vb = sniff_view_box(tag)?; + Some((vb.0.max(1.0) as u32, vb.1.max(1.0) as u32)) } -/// Render an SVG file through **Skia's native SVG module** with the -/// thinnest possible wrapper. -/// -/// Pipeline: `fs::read` → `Data::new_copy` → `svg::Dom::from_bytes` → -/// `surface.canvas()` → `dom.render()` → PNG. No htmlcss module, no -/// `PictureRecorder`, no Grida tree surgery. -/// -/// Purpose: attribute reftest failures correctly. A test that fails -/// here is a limitation of Skia's own `svg::Dom` implementation, not -/// of Grida's wiring. If the same test fails identically under -/// `--renderer htmlcss`, then our htmlcss wrapping adds zero -/// divergence; any delta between the two backends attributes to the -/// `PictureRecorder` round-trip we add in htmlcss. -pub(crate) fn render_svg_to_png_via_sksvg( - svg_path: &Path, - output_path: &Path, - target_size: Option<(u32, u32)>, -) -> Result<()> { - use skia_safe::{surfaces, svg, Color as SkColor, Data, EncodedImageFormat, FontMgr, Size}; - - let svg_bytes = fs::read(svg_path) - .with_context(|| format!("failed to read SVG file {}", svg_path.display()))?; - - let (width, height) = match target_size { - Some((w, h)) => (w.max(1) as i32, h.max(1) as i32), - None => { - let svg_source = std::str::from_utf8(&svg_bytes) - .with_context(|| format!("SVG file {} is not UTF-8", svg_path.display()))?; - let (w, h) = sniff_svg_dimensions(svg_source).unwrap_or((512, 512)); - (w as i32, h as i32) - } - }; - - let data = Data::new_copy(&svg_bytes); - let mut dom = svg::Dom::from_bytes(&data, FontMgr::default()) - .map_err(|e| anyhow!("skia svg::Dom::from_bytes failed: {e}"))?; - dom.set_container_size(Size::new(width as f32, height as f32)); - - let mut surface = surfaces::raster_n32_premul((width, height)) - .ok_or_else(|| anyhow!("failed to create raster surface {}x{}", width, height))?; - { - let canvas = surface.canvas(); - canvas.clear(SkColor::TRANSPARENT); - dom.render(canvas); +fn sniff_view_box(tag: &str) -> Option<(f32, f32)> { + let needle = "viewBox="; + let start = tag.find(needle)? + needle.len(); + let rest = &tag[start..]; + let (quote, rest) = rest.split_at(1); + let quote = quote.chars().next()?; + if quote != '"' && quote != '\'' { + return None; } - - let image = surface.image_snapshot(); - let encoded = image - .encode(None, EncodedImageFormat::PNG, None) - .ok_or_else(|| anyhow!("Failed to encode PNG"))?; - - if let Some(parent) = output_path.parent() { - fs::create_dir_all(parent) - .with_context(|| format!("failed to create output directory {}", parent.display()))?; + let end = rest.find(quote)?; + let raw = &rest[..end]; + let parts: Vec = raw + .split(|c: char| c == ',' || c.is_ascii_whitespace()) + .filter(|s| !s.is_empty()) + .filter_map(|s| s.parse::().ok()) + .collect(); + if parts.len() == 4 { + Some((parts[2], parts[3])) + } else { + None } - fs::write(output_path, encoded.as_bytes()) - .with_context(|| format!("failed to write PNG to {}", output_path.display()))?; - - Ok(()) } pub(crate) fn render_svg_to_png( @@ -406,3 +397,257 @@ pub(crate) fn render_svg_to_png( Ok(()) } + +/// Scan an SVG document for `xlink:href="…"` and `href="…"` attribute +/// values that look like local file references, resolve each against +/// the SVG's directory, and pre-decode them into `images`. This makes +/// fixtures using `` (the common +/// resvg-test-suite shape) render correctly through the standalone +/// `render_to_picture_with_images` entry point. +/// +/// Excluded: empty values, fragment-only refs (`#id`), and `data:` +/// URIs (the painter decodes those itself). +fn preload_referenced_images( + svg: &str, + base_dir: &Path, + images: &mut grida::htmlcss::PreloadedImages, +) { + preload_referenced_images_inner(svg, base_dir, images, 0); +} + +/// Preload the bodies of any CSS files the SVG `@import`s into a +/// [`grida::htmlcss::svg::PreloadedCss`]. Mirrors `preload_referenced_images`: +/// the renderer's stylesheet collector queries the loader by path +/// during parsing, so the harness's job is just to read every +/// reachable `.css` file from disk and stuff it in. +/// +/// Recursive imports (CSS A imports CSS B) are handled by recursing +/// into each loaded file with the *file's own* parent directory as +/// the base, so chained relative paths resolve correctly. +fn preload_referenced_css(svg: &str, base_dir: &Path, css: &mut grida::htmlcss::svg::PreloadedCss) { + let mut visited: std::collections::HashSet = + std::collections::HashSet::new(); + preload_css_recursive(svg, base_dir, css, &mut visited, 0); +} + +/// Cap on the harness preload's recursion depth. Independent of the +/// renderer's `MAX_IMPORT_DEPTH` — the harness side is bounded by the +/// `visited` canonical-path set, this is just a belt against deep +/// chains taking too long to walk on disk. +const MAX_CSS_IMPORT_DEPTH: u32 = 8; + +/// Build the font resolver used when rendering a resvg-test-suite +/// fixture. Walks up from the fixture path looking for a sibling +/// `fonts/` directory; if found, registers every `.ttf` / `.otf` and +/// applies the generic-family bindings vdiff configures via `--*-family` +/// flags (see `fixtures/.../tools/vdiff/src/render.cpp`): +/// +/// - default + `sans-serif` → `Noto Sans` +/// - `serif` → `Noto Serif` +/// - `cursive` → `Yellowtail` +/// - `fantasy` → `Sedgwick Ave Display` +/// - `monospace` → `Noto Mono` +/// +/// When no `fonts/` directory is found, returns an empty resolver — +/// the painter then no-ops on `` rather than silently falling +/// through to system fonts. That matches the suite's +/// `--skip-system-fonts` invariant: any text scoring difference is +/// then attributable to the renderer, not to font availability. +fn resolve_test_suite_fonts(svg_path: &Path) -> grida::htmlcss::svg::PreloadedFonts { + let mut fonts = grida::htmlcss::svg::PreloadedFonts::new(); + let Some(fonts_dir) = find_test_suite_fonts_dir(svg_path) else { + return fonts; + }; + if let Ok(entries) = fs::read_dir(&fonts_dir) { + for entry in entries.flatten() { + let path = entry.path(); + let is_font = path + .extension() + .and_then(|s| s.to_str()) + .map(|ext| ext.eq_ignore_ascii_case("ttf") || ext.eq_ignore_ascii_case("otf")) + .unwrap_or(false); + if !is_font { + continue; + } + if let Ok(bytes) = fs::read(&path) { + fonts.register(&bytes); + } + } + } + // Generic-family bindings — verbatim from vdiff's CLI flags. + // + // Note: the suite's expected PNGs for `font-family/{cursive,fantasy, + // serif}.svg` appear to have been rendered against the host's + // system fallbacks rather than against these vdiff-config fonts. + // Our render uses the documented Noto Serif / Yellowtail / Sedgwick + // Ave Display (matching vdiff itself); divergence on those three + // generic fixtures is a property of the test data, not a wiring + // bug. `sans-serif` and `monospace` agree on metrics with most + // host defaults so they score high regardless. + fonts.set_generic("serif", "Noto Serif"); + fonts.set_generic("sans-serif", "Noto Sans"); + fonts.set_generic("cursive", "Yellowtail"); + fonts.set_generic("fantasy", "Sedgwick Ave Display"); + fonts.set_generic("monospace", "Noto Mono"); + + // One fixture (`tspan/style-override.svg`) references "Times New + // Roman" via inline CSS but the suite ships no matching .ttf. Map + // it to Noto Serif — the closest registered face — so the fixture + // renders something rather than blank text. Mirrors the per-host + // alias hosts get for free on macOS / Windows. + fonts.set_alias("Times New Roman", "Noto Serif"); + + fonts.set_default_family("Noto Sans"); + fonts +} + +/// Walk up from `svg_path` looking for a sibling `fonts/` directory +/// (the resvg-test-suite layout has `fonts/` at the suite root, with +/// fixtures nested under `tests//.svg`). Returns the +/// first hit, or `None` if no ancestor has one. +fn find_test_suite_fonts_dir(svg_path: &Path) -> Option { + let mut cursor = svg_path.parent()?; + loop { + let candidate = cursor.join("fonts"); + if candidate.is_dir() { + return Some(candidate); + } + cursor = cursor.parent()?; + } +} + +fn preload_css_recursive( + css_or_svg_text: &str, + base_dir: &Path, + css: &mut grida::htmlcss::svg::PreloadedCss, + visited: &mut std::collections::HashSet, + depth: u32, +) { + if depth >= MAX_CSS_IMPORT_DEPTH { + return; + } + for path in grida::htmlcss::svg::style::stylesheet::scan_imports(css_or_svg_text) { + if path.starts_with("http://") || path.starts_with("https://") || path.starts_with("data:") + { + continue; + } + let resolved = base_dir.join(&path); + let canonical = resolved.canonicalize().unwrap_or_else(|_| resolved.clone()); + if !visited.insert(canonical.clone()) { + continue; + } + let Ok(body) = fs::read_to_string(&resolved) else { + continue; + }; + // Register under the original (unresolved) path the SVG used + // so the renderer's `CssLoader::get(path)` lookup hits. + css.insert(path.clone(), body.clone()); + // Recurse — chained imports resolve relative to the imported + // file's own directory. + let nested_base = resolved.parent().unwrap_or(base_dir).to_path_buf(); + preload_css_recursive(&body, &nested_base, css, visited, depth + 1); + } +} + +const MAX_NESTED_SVG_DEPTH: u32 = 4; + +fn preload_referenced_images_inner( + svg: &str, + base_dir: &Path, + images: &mut grida::htmlcss::PreloadedImages, + depth: u32, +) { + use std::collections::HashSet; + let mut seen: HashSet = HashSet::new(); + for needle in ["xlink:href=\"", "xlink:href='", "href=\"", "href='"] { + let mut rest = svg; + while let Some(idx) = rest.find(needle) { + let after = &rest[idx + needle.len()..]; + let quote = needle.chars().last().unwrap(); + let Some(end) = after.find(quote) else { + break; + }; + let value = &after[..end]; + rest = &after[end + 1..]; + if value.is_empty() || value.starts_with('#') { + continue; + } + if value.starts_with("data:") || value.starts_with("DATA:") { + continue; + } + if value.starts_with("http://") + || value.starts_with("https://") + || value.starts_with("file://") + { + continue; + } + if !seen.insert(value.to_string()) { + continue; + } + let path = base_dir.join(value); + let Ok(bytes) = fs::read(&path) else { + continue; + }; + let lower = value.to_ascii_lowercase(); + if lower.ends_with(".svg") { + if depth >= MAX_NESTED_SVG_DEPTH { + continue; + } + if let Ok(svg_str) = std::str::from_utf8(&bytes) { + if let Some(image) = rasterize_external_svg(svg_str, base_dir, depth + 1) { + if let Some(png) = image.encode(None, EncodedImageFormat::PNG, None) { + images.insert_bytes(value.to_string(), png.as_bytes()); + } + } + } + } else { + images.insert_bytes(value.to_string(), &bytes); + } + } + } +} + +/// Render an external `.svg` file to an in-memory `skia_safe::Image` +/// at its sniffed intrinsic size (or 512×512 fallback). Recursive +/// `` references inside the embedded SVG are resolved against +/// the same `base_dir` as the outer SVG. +fn rasterize_external_svg(svg: &str, base_dir: &Path, depth: u32) -> Option { + use skia_safe::{surfaces, Color as SkColor}; + let (w, h) = sniff_svg_dimensions(svg).unwrap_or((512, 512)); + let mut nested = grida::htmlcss::PreloadedImages::new(); + preload_referenced_images_inner(svg, base_dir, &mut nested, depth); + let picture = + grida::htmlcss::svg::render_to_picture_with_images(svg, w as f32, h as f32, &nested) + .ok()?; + let mut surface = surfaces::raster_n32_premul((w as i32, h as i32))?; + { + let canvas = surface.canvas(); + canvas.clear(SkColor::TRANSPARENT); + canvas.draw_picture(&picture, None, None); + } + Some(surface.image_snapshot()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn preload_css_picks_up_external_import() { + let svg_path = std::path::Path::new( + "../../fixtures/local/resvg-test-suite/tests/structure/style/external-CSS.svg", + ); + let svg = std::fs::read_to_string(svg_path).expect("read fixture"); + let parent = svg_path.parent().unwrap(); + use grida::htmlcss::svg::CssLoader; + let mut css = grida::htmlcss::svg::PreloadedCss::new(); + preload_referenced_css(&svg, parent, &mut css); + let body = css + .get("../../../resources/green.css") + .expect("imported css preloaded under its original path"); + assert!( + body.contains("#rect1 { fill:green; }"), + "imported css body unexpected: {body}" + ); + } +} diff --git a/crates/grida_dev/src/reftest/runner.rs b/crates/grida_dev/src/reftest/runner.rs index d46620c8c..67defab98 100644 --- a/crates/grida_dev/src/reftest/runner.rs +++ b/crates/grida_dev/src/reftest/runner.rs @@ -3,7 +3,7 @@ use crate::reftest::compare::{compare_images, ScoringMask}; use crate::reftest::config::ReftestToml; use crate::reftest::render::{ find_test_pairs_from_glob, find_test_pairs_in_dirs, render_svg_to_png, - render_svg_to_png_via_htmlcss, render_svg_to_png_via_sksvg, TestPair, + render_svg_to_png_via_htmlcss, TestPair, }; use crate::reftest::report::{generate_json_report, ReftestReport, TestResult}; use anyhow::{Context, Result}; @@ -69,7 +69,6 @@ pub(crate) async fn run_reftest(args: &ReftestArgs) -> Result<()> { let suffix = match args.renderer { SvgRenderer::Iosvg => String::new(), SvgRenderer::Htmlcss => ".htmlcss".to_string(), - SvgRenderer::Sksvg => ".sksvg".to_string(), }; output_dir = output_dir.join(format!("{}{}", sanitize_dir_name(&name), suffix)); } @@ -222,9 +221,6 @@ pub(crate) async fn run_reftest(args: &ReftestArgs) -> Result<()> { SvgRenderer::Htmlcss => { render_svg_to_png_via_htmlcss(&pair.svg_path, &temp_output_png, target_size) } - SvgRenderer::Sksvg => { - render_svg_to_png_via_sksvg(&pair.svg_path, &temp_output_png, target_size) - } })); let render_result = match render_result { From de1265b8909cb99ca968a29abf5583553212ea79 Mon Sep 17 00:00:00 2001 From: Universe Date: Wed, 29 Apr 2026 00:38:22 +0900 Subject: [PATCH 04/59] refactor(htmlcss::svg): split dom/element.rs, extract text shaping, drop dead filter::apply MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Continues the Blink-aligned reorganization with the items previously flagged as regression risk. Reftest score unchanged at 89.50% (1679 fixtures, exact baseline match). dom/element.rs split into three: - dom/element.rs: keeps ElementKind, get_attr, element_kind (pure DOM) - paint/visibility.rs: is_painted, has_display_none, is_visible_self, is_visible_inherited (paint dispatch concerns) - style/cascade.rs: cascade_property, get_attr_or_style (single funnel for property resolution; future Stylo migration target) Updates the three callers (svg_root_painter, svg_container_painter, svg_shape_painter) to import from the new locations. paint/svg_text_painter.rs (2,468 LOC) split into a directory: - svg_text_painter/mod.rs: orchestrator, attribute flattening, layout - svg_text_painter/shaping.rs (~190 LOC): SkShaper integration, ShapedGlyph, compute_kerned_advances, shape_text Further text submodules (textPath, layout algorithm) deferred — their cross-dependencies risk regression for marginal gain. Removes resources/filter.rs::apply (dead code; the painter applies filters inline at svg_container_painter.rs). Eliminates the only resources/ Canvas violation, removing two allowlist entries from tests/htmlcss_svg_architecture.rs. README module map updated. --- .../grida/examples/tool_gen_bench_fixture.rs | 2 - crates/grida/src/htmlcss/svg/README.md | 81 +++---- crates/grida/src/htmlcss/svg/dom/element.rs | 189 ---------------- crates/grida/src/htmlcss/svg/paint/mod.rs | 1 + .../svg/paint/svg_container_painter.rs | 7 +- .../src/htmlcss/svg/paint/svg_root_painter.rs | 3 +- .../htmlcss/svg/paint/svg_shape_painter.rs | 3 +- .../mod.rs} | 187 +--------------- .../svg/paint/svg_text_painter/shaping.rs | 203 ++++++++++++++++++ .../grida/src/htmlcss/svg/paint/visibility.rs | 127 +++++++++++ .../src/htmlcss/svg/resources/clipper.rs | 2 +- .../grida/src/htmlcss/svg/resources/filter.rs | 32 +-- crates/grida/src/htmlcss/svg/style/cascade.rs | 106 +++++++++ crates/grida/src/htmlcss/svg/style/mod.rs | 1 + .../grida/tests/htmlcss_svg_architecture.rs | 8 - 15 files changed, 504 insertions(+), 448 deletions(-) rename crates/grida/src/htmlcss/svg/paint/{svg_text_painter.rs => svg_text_painter/mod.rs} (92%) create mode 100644 crates/grida/src/htmlcss/svg/paint/svg_text_painter/shaping.rs create mode 100644 crates/grida/src/htmlcss/svg/paint/visibility.rs create mode 100644 crates/grida/src/htmlcss/svg/style/cascade.rs diff --git a/crates/grida/examples/tool_gen_bench_fixture.rs b/crates/grida/examples/tool_gen_bench_fixture.rs index 1bb4c1290..06b50f62e 100644 --- a/crates/grida/examples/tool_gen_bench_fixture.rs +++ b/crates/grida/examples/tool_gen_bench_fixture.rs @@ -19,8 +19,6 @@ use fixture_helpers::*; use grida::cg::color::CGColor; use grida::cg::fe::*; use grida::cg::stroke_width::StrokeWidth; -use grida::cg::types::*; -use grida::node::schema::*; use std::collections::HashMap; /// 100×100 grid of rectangles (10 000 nodes). diff --git a/crates/grida/src/htmlcss/svg/README.md b/crates/grida/src/htmlcss/svg/README.md index 9f57453e1..9886d1ba6 100644 --- a/crates/grida/src/htmlcss/svg/README.md +++ b/crates/grida/src/htmlcss/svg/README.md @@ -104,7 +104,7 @@ The SVG static-rendering surface, modeled on Blink's coverage: - **Paint servers**: ``, ``, ``. - **Effects**: ``, ``, ``, ``. - **Filter primitives**: full set mapped to `skia_safe::ImageFilter` — - see the table in `resources/filter_effect.rs`. + see the table in `resources/svg_filter_builder.rs`. - **Coordinate systems**: `viewBox`, `preserveAspectRatio`, nested ``, `transform=` attribute. - **Style**: temporary SVG CSS subset. Full Stylo integration remains @@ -174,44 +174,47 @@ Chromium as the oracle). ## Module map (Grida ↔ Blink) -| Grida file (`crates/grida/src/htmlcss/svg/`) | Blink anchor (`third_party/blink/renderer/`) | -| -------------------------------------------- | ------------------------------------------------------------------------------- | -| `error.rs` | (Grida-only) renderer error type | -| `context.rs` | host hooks (image / font / css providers) | -| `dom/element.rs` | `core/svg/svg_*_element.{h,cc}` (one per tag) | -| `dom/attrs.rs` | `core/svg/svg_animated_*.{h,cc}` (typed; static, no animVal) | -| `dom/path_d.rs` | `core/svg/svg_path_*` parser family | -| `dom/parser.rs` | `core/svg/svg_parser_utilities.cc` + Blink XML parser frontend | -| `dom/href.rs` | `core/svg/svg_uri_reference.{h,cc}` | -| `style/stylesheet.rs` | `core/css/parser/css_parser_impl.cc` (a tiny in-tree subset; see note above) | -| `style/stylo_bridge.rs` | (placeholder) future Stylo cascade entry point | -| `geometry/basic_shape.rs` | `core/css/basic_shape_functions.{h,cc}` + `core/style/basic_shapes.{h,cc}` | -| `layout/bbox.rs` | scattered `core/layout/svg/*::ObjectBoundingBox` overrides | -| `layout/transform.rs` | `core/layout/svg/transform_helper.{h,cc}` | -| `layout/viewport.rs` | `core/layout/svg/layout_svg_viewport_container.{h,cc}` + viewBox math | -| `layout/layout_svg_element.rs` | (bridge type) thin wrapper paint/resources see for "an SVG element" | -| `paint/svg_painter.rs` | (trait) uniform painter contract (Blink: `*Painter::Paint(PaintInfo)`) | -| `paint/scoped_svg_paint_state.rs` | `core/paint/scoped_svg_paint_state.{h,cc}` | -| `paint/svg_object_painter.rs` | `core/paint/svg_object_painter.{h,cc}` | -| `paint/svg_root_painter.rs` | `core/paint/svg_root_painter.{h,cc}` | -| `paint/svg_container_painter.rs` | `core/paint/svg_container_painter.{h,cc}` | -| `paint/svg_shape_painter.rs` | `core/paint/svg_shape_painter.{h,cc}` | -| `paint/svg_text_painter.rs` | `core/layout/svg/svg_text_layout_algorithm.{h,cc}` + HTML inline-text painter | -| `paint/svg_image_painter.rs` | `core/paint/svg_image_painter.{h,cc}` | -| `paint/svg_use_painter.rs` | `core/svg/svg_use_element.{h,cc}` shadow-tree expansion | -| `paint/svg_marker_painter.rs` | `core/paint/marker_range_mapping_context.{h,cc}` + svg_shape_painter:256–323 | -| `paint/clip_path_clipper.rs` | `core/paint/clip_path_clipper.{h,cc}` | -| `paint/effects.rs` | (helpers) opacity, filter resolution, font-size resolution | -| `resources/svg_resources.rs` | `core/layout/svg/svg_resources.{h,cc}` + `layout_svg_resource_container.{h,cc}` | -| `resources/svg_resource_container.rs` | (trait) uniform resource-container contract | -| `resources/cache.rs` | (placeholder) per-client cache, mirrors `SVGElementResourceClient` | -| `resources/paint_server.rs` | `core/layout/svg/layout_svg_resource_paint_server.{h,cc}` | -| `resources/gradient.rs` | `core/layout/svg/layout_svg_resource_gradient.{h,cc}` (+ linear/radial) | -| `resources/pattern.rs` | `core/layout/svg/layout_svg_resource_pattern.{h,cc}` | -| `resources/clipper.rs` | `core/layout/svg/layout_svg_resource_clipper.{h,cc}` | -| `resources/masker.rs` | `core/layout/svg/layout_svg_resource_masker.{h,cc}` | -| `resources/filter.rs` | `core/layout/svg/layout_svg_resource_filter.{h,cc}` | -| `resources/svg_filter_builder.rs` | `core/svg/graphics/filters/svg_filter_builder.{h,cc}` + `FilterEffect` family | +| Grida file (`crates/grida/src/htmlcss/svg/`) | Blink anchor (`third_party/blink/renderer/`) | +| -------------------------------------------- | ------------------------------------------------------------------------------------- | +| `error.rs` | (Grida-only) renderer error type | +| `context.rs` | host hooks (image / font / css providers) | +| `dom/element.rs` | `core/svg/svg_*_element.{h,cc}` (one per tag) | +| `dom/attrs.rs` | `core/svg/svg_animated_*.{h,cc}` (typed; static, no animVal) | +| `dom/path_d.rs` | `core/svg/svg_path_*` parser family | +| `dom/parser.rs` | `core/svg/svg_parser_utilities.cc` + Blink XML parser frontend | +| `dom/href.rs` | `core/svg/svg_uri_reference.{h,cc}` | +| `style/cascade.rs` | `core/css/resolver/style_resolver.cc` (single funnel for property resolution) | +| `style/stylesheet.rs` | `core/css/parser/css_parser_impl.cc` (a tiny in-tree subset; see note above) | +| `style/stylo_bridge.rs` | (placeholder) future Stylo cascade entry point | +| `geometry/basic_shape.rs` | `core/css/basic_shape_functions.{h,cc}` + `core/style/basic_shapes.{h,cc}` | +| `layout/bbox.rs` | scattered `core/layout/svg/*::ObjectBoundingBox` overrides | +| `layout/transform.rs` | `core/layout/svg/transform_helper.{h,cc}` | +| `layout/viewport.rs` | `core/layout/svg/layout_svg_viewport_container.{h,cc}` + viewBox math | +| `layout/layout_svg_element.rs` | (bridge type) thin wrapper paint/resources see for "an SVG element" | +| `paint/svg_painter.rs` | (trait) uniform painter contract (Blink: `*Painter::Paint(PaintInfo)`) | +| `paint/scoped_svg_paint_state.rs` | `core/paint/scoped_svg_paint_state.{h,cc}` | +| `paint/svg_object_painter.rs` | `core/paint/svg_object_painter.{h,cc}` | +| `paint/svg_root_painter.rs` | `core/paint/svg_root_painter.{h,cc}` | +| `paint/svg_container_painter.rs` | `core/paint/svg_container_painter.{h,cc}` | +| `paint/svg_shape_painter.rs` | `core/paint/svg_shape_painter.{h,cc}` | +| `paint/svg_text_painter/mod.rs` | `core/paint/svg_text_painter.{h,cc}` + `core/layout/svg/svg_text_layout_algorithm.cc` | +| `paint/svg_text_painter/shaping.rs` | `platform/fonts/shaping/harfbuzz_shaper.{h,cc}` (the SkShaper bridge) | +| `paint/visibility.rs` | (paint helper) `display:none` / `visibility:hidden` predicates | +| `paint/svg_image_painter.rs` | `core/paint/svg_image_painter.{h,cc}` | +| `paint/svg_use_painter.rs` | `core/svg/svg_use_element.{h,cc}` shadow-tree expansion | +| `paint/svg_marker_painter.rs` | `core/paint/marker_range_mapping_context.{h,cc}` + svg_shape_painter:256–323 | +| `paint/clip_path_clipper.rs` | `core/paint/clip_path_clipper.{h,cc}` | +| `paint/effects.rs` | (helpers) opacity, filter resolution, font-size resolution | +| `resources/svg_resources.rs` | `core/layout/svg/svg_resources.{h,cc}` + `layout_svg_resource_container.{h,cc}` | +| `resources/svg_resource_container.rs` | (trait) uniform resource-container contract | +| `resources/cache.rs` | (placeholder) per-client cache, mirrors `SVGElementResourceClient` | +| `resources/paint_server.rs` | `core/layout/svg/layout_svg_resource_paint_server.{h,cc}` | +| `resources/gradient.rs` | `core/layout/svg/layout_svg_resource_gradient.{h,cc}` (+ linear/radial) | +| `resources/pattern.rs` | `core/layout/svg/layout_svg_resource_pattern.{h,cc}` | +| `resources/clipper.rs` | `core/layout/svg/layout_svg_resource_clipper.{h,cc}` | +| `resources/masker.rs` | `core/layout/svg/layout_svg_resource_masker.{h,cc}` | +| `resources/filter.rs` | `core/layout/svg/layout_svg_resource_filter.{h,cc}` | +| `resources/svg_filter_builder.rs` | `core/svg/graphics/filters/svg_filter_builder.{h,cc}` + `FilterEffect` family | ## Cross-references diff --git a/crates/grida/src/htmlcss/svg/dom/element.rs b/crates/grida/src/htmlcss/svg/dom/element.rs index 28649db22..ae3b91932 100644 --- a/crates/grida/src/htmlcss/svg/dom/element.rs +++ b/crates/grida/src/htmlcss/svg/dom/element.rs @@ -111,112 +111,6 @@ impl ElementKind { } } -/// Returns `true` when `display="none"` or `visibility="hidden"` (or the -/// equivalent in the inline `style` attribute) should suppress paint of -/// this element and its subtree. -pub fn is_painted(node: &csscascade::dom::DemoNode) -> bool { - if has_display_none(node) { - return false; - } - is_visible_self(node) -} - -/// `display: none` on this element only (no inheritance walk; SVG 2 §11.1 -/// — children of a `display:none` ancestor are themselves not painted, -/// but each level only needs to check its own value because the early -/// return at the ancestor stops descent). -pub fn has_display_none(node: &csscascade::dom::DemoNode) -> bool { - if matches_attr(node, "display", "none") { - return true; - } - if let Some(style) = get_attr(node, "style") { - if style_contains_pair(style, "display", "none") { - return true; - } - } - false -} - -/// `visibility: hidden`/`collapse` on this element only — no -/// inheritance walk. Use [`is_visible_inherited`] when you need the -/// effective value with parent-cascade applied. -pub fn is_visible_self(node: &csscascade::dom::DemoNode) -> bool { - if matches_attr(node, "visibility", "hidden") || matches_attr(node, "visibility", "collapse") { - return false; - } - if let Some(style) = get_attr(node, "style") { - if style_contains_pair(style, "visibility", "hidden") - || style_contains_pair(style, "visibility", "collapse") - { - return false; - } - } - true -} - -/// Effective `visibility` for a leaf element — walks the ancestor -/// chain, taking the first explicit `visibility` value. SVG 2 §11.4: -/// `visibility` is inherited; a descendant can override an ancestor's -/// `hidden` by setting `visibility: visible` on itself. -pub fn is_visible_inherited( - dom: &csscascade::dom::DemoDom, - start: csscascade::dom::NodeId, -) -> bool { - let mut current = Some(start); - while let Some(id) = current { - let n = dom.node(id); - if let Some(v) = read_visibility(n) { - return v; - } - current = n.parent; - } - true // CSS default `visible` -} - -fn read_visibility(node: &csscascade::dom::DemoNode) -> Option { - if matches_attr(node, "visibility", "visible") { - return Some(true); - } - if matches_attr(node, "visibility", "hidden") || matches_attr(node, "visibility", "collapse") { - return Some(false); - } - if let Some(style) = get_attr(node, "style") { - for decl in style.split(';') { - if let Some((k, v)) = decl.split_once(':') { - if k.trim().eq_ignore_ascii_case("visibility") { - let v = v.trim(); - if v.eq_ignore_ascii_case("visible") { - return Some(true); - } - if v.eq_ignore_ascii_case("hidden") || v.eq_ignore_ascii_case("collapse") { - return Some(false); - } - } - } - } - } - None -} - -fn matches_attr(node: &csscascade::dom::DemoNode, name: &str, value: &str) -> bool { - get_attr(node, name).map(str::trim) == Some(value) -} - -/// Crude `key: value` lookup inside a `style="..."` blob. Good enough for -/// `display:none` / `visibility:hidden` checks — proper CSS resolution -/// will go through Stylo once the SVG cascade hook is wired. -fn style_contains_pair(style: &str, key: &str, value: &str) -> bool { - for decl in style.split(';') { - let Some((k, v)) = decl.split_once(':') else { - continue; - }; - if k.trim().eq_ignore_ascii_case(key) && v.trim().eq_ignore_ascii_case(value) { - return true; - } - } - false -} - /// Local-name attribute lookup. Returns `None` for non-elements or /// missing attributes. SVG-namespaced and unprefixed attrs both match. pub fn get_attr<'a>(node: &'a DemoNode, name: &str) -> Option<&'a str> { @@ -231,89 +125,6 @@ pub fn get_attr<'a>(node: &'a DemoNode, name: &str) -> Option<&'a str> { None } -/// Read a presentation property cascading author `` placeholder; matches the documented "no fallback" intent. Tests: - font_weight_named_and_numeric: was asserting clamping for out-of-range numeric values; impl correctly returns the inherited value per CSS Fonts 4 §3.2.4 (Blink: `ConsumeFontWeightNumber` returns nullptr -> cascade fallback). Test now matches the spec-correct behavior. - length_px_handles_unit_suffix: was asserting `None` for `10em`; impl returns the documented default-context approximation (em/rem = 16px). Test updated to match the doc. Doc cleanups: - reftest/args.rs: `Htmlcss` backend description points at `htmlcss::svg::render_to_picture*` (the old `htmlcss::render_svg` is just a thin wrapper but the API surface in `htmlcss::svg` is the canonical entry). - htmlcss/mod.rs: `render_svg`'s doc no longer claims the function is an XML validator — the parser is permissive by design. Deferred from the review (not "deserved enough"): - RTL shaping (no fixtures, design decision needed) - Pattern href fallback / picture bounds (high regression risk on paint-servers fixtures) - Per-subpath markers, font shorthand, currentColor for stroke, textPath consumed counter (text/marker logic risk) - Root effects via CSS, nested-svg defaults, ellipse axes (moderate regression risk) - math2 migration (out of scope) - AST-based architecture test, doc frontmatter (style nits) See the aggregate in the PR conversation for full triage. --- crates/grida/src/htmlcss/mod.rs | 6 +- crates/grida/src/htmlcss/paint.rs | 26 ++++---- crates/grida/src/htmlcss/svg/dom/attrs.rs | 11 +++- .../htmlcss/svg/paint/clip_path_clipper.rs | 17 +++-- .../svg/paint/scoped_svg_paint_state.rs | 65 ++++++++++++++++--- .../svg/paint/svg_container_painter.rs | 12 +++- .../htmlcss/svg/paint/svg_shape_painter.rs | 14 ++-- .../htmlcss/svg/paint/svg_text_painter/mod.rs | 26 +++++--- .../svg/paint/svg_text_painter/shaping.rs | 6 +- .../src/htmlcss/svg/paint/svg_use_painter.rs | 20 +++++- .../grida/src/htmlcss/svg/paint/visibility.rs | 50 ++++++++------ .../src/htmlcss/svg/resources/gradient.rs | 15 +++++ crates/grida/src/htmlcss/svg/style/cascade.rs | 24 ++++++- .../grida/src/htmlcss/svg/style/stylesheet.rs | 26 +++++--- crates/grida_dev/src/reftest/args.rs | 19 +++--- 15 files changed, 243 insertions(+), 94 deletions(-) diff --git a/crates/grida/src/htmlcss/mod.rs b/crates/grida/src/htmlcss/mod.rs index a1efc5a7b..82fc391e9 100644 --- a/crates/grida/src/htmlcss/mod.rs +++ b/crates/grida/src/htmlcss/mod.rs @@ -216,7 +216,11 @@ pub fn render( /// `width` / `height` are CSS pixels; `viewBox` + `preserveAspectRatio` /// inside the SVG map user units to that box. /// -/// Returns `Err` only when the input is malformed XML. +/// Returns `Err` only when SVG structure is unrecoverable (e.g. no +/// `` element, picture-recording failure). The XML parser is +/// permissive — best-effort recoverable input may render as `Ok` +/// even if it isn't strictly well-formed. Use this as a render +/// entry point, not as an XML validator. pub fn render_svg(svg: &str, width: f32, height: f32) -> Result { crate::htmlcss::svg::render_to_picture(svg, width, height) .map_err(|e| format!("htmlcss::svg::render_to_picture: {e}")) diff --git a/crates/grida/src/htmlcss/paint.rs b/crates/grida/src/htmlcss/paint.rs index 5adff131a..ff0f61ece 100644 --- a/crates/grida/src/htmlcss/paint.rs +++ b/crates/grida/src/htmlcss/paint.rs @@ -797,23 +797,23 @@ fn paint_replaced( // than resvg+tiny-skia or Skia's built-in svg::Dom. There is no // fallback — features still under construction render as best-effort // gaps so we feel the motivation to implement them. - let svg_handled = if let Some(ref xml) = content.svg_xml { - paint_inline_svg(canvas, xml.as_bytes(), w, h, images) - } else { - false - }; - - if svg_handled { + // Inline SVG: routes through the in-tree htmlcss::svg renderer. + // Per the "no fallback" intent, an inline-SVG element terminates + // here whether the render succeeded or not — we do NOT fall back + // to the gray placeholder. The placeholder is for missing `` + // resources only. (Reviewer note: see PR #698 / coderabbit + // finding "Inline `` failure still falls back to placeholder + // rendering"; the previous flow let `paint_inline_svg`-returned- + // false fall through to the placeholder draw at the bottom of + // this function.) + if let Some(ref xml) = content.svg_xml { + let _ = paint_inline_svg(canvas, xml.as_bytes(), w, h, images); canvas.restore(); return; } - // Image path: only applies to -style replaced elements (not SVG). - let image_opt = if content.svg_xml.is_none() { - images.get(&content.src) - } else { - None - }; + // Image path: only applies to -style replaced elements. + let image_opt = images.get(&content.src); if let Some(image) = image_opt { let img_w = image.width() as f32; let img_h = image.height() as f32; diff --git a/crates/grida/src/htmlcss/svg/dom/attrs.rs b/crates/grida/src/htmlcss/svg/dom/attrs.rs index 297c5e036..0f326b106 100644 --- a/crates/grida/src/htmlcss/svg/dom/attrs.rs +++ b/crates/grida/src/htmlcss/svg/dom/attrs.rs @@ -669,7 +669,16 @@ mod tests { assert_eq!(parse_length_px("10"), Some(10.0)); assert_eq!(parse_length_px("10px"), Some(10.0)); assert_eq!(parse_length_px(" 10.5 "), Some(10.5)); - assert_eq!(parse_length_px("10em"), None); + // Relative units (`em`, `rem`, `ex`, `ch`) use the + // default-context approximation documented on + // `parse_length_px`: em/rem = 16px, ex/ch = 8px. We + // intentionally return a value rather than `None` so callers + // without a cascade context still produce visible geometry. + assert_eq!(parse_length_px("10em"), Some(160.0)); + assert_eq!(parse_length_px("1rem"), Some(16.0)); + assert_eq!(parse_length_px("1ex"), Some(8.0)); + // Percentages still fail — they need the containing viewport. + assert_eq!(parse_length_px("50%"), None); } #[test] diff --git a/crates/grida/src/htmlcss/svg/paint/clip_path_clipper.rs b/crates/grida/src/htmlcss/svg/paint/clip_path_clipper.rs index 1c91a3aa2..ac6c57bcc 100644 --- a/crates/grida/src/htmlcss/svg/paint/clip_path_clipper.rs +++ b/crates/grida/src/htmlcss/svg/paint/clip_path_clipper.rs @@ -1,13 +1,12 @@ //! `clip-path` application for the direct SVG painter. -use csscascade::dom::{DemoNode, DemoNodeData}; +use csscascade::dom::DemoNode; use skia_safe::{Canvas, ClipOp, Rect}; use super::super::geometry::basic_shape::{ build_basic_shape_path, parse_basic_shape, ReferenceBox, }; use super::scoped_svg_paint_state::PaintCtx; -use crate::htmlcss::svg::dom::element::ElementKind; use crate::htmlcss::svg::layout::bbox::element_object_bbox; use crate::htmlcss::svg::layout::viewport::nearest_svg_viewport; use crate::htmlcss::svg::resources::clipper; @@ -37,15 +36,15 @@ pub(super) fn apply_clip_path( let Some(target) = ctx.resources.lookup(id) else { return true; }; - let target_is_clip_path = match &ctx.dom.node(target).data { - DemoNodeData::Element(d) => { - ElementKind::from_local_name(d.name.local.as_ref()) == ElementKind::ClipPath - } - _ => false, - }; + // Resolution failure for a `` reference renders unclipped + // per htmlcss-svg design study §S-clip-path "Differ" — empty / + // unsupported / cyclic clip paths fall through rather than hide + // content. (Blink's spec-strict behavior is to clip-everything; + // resvg renders unclipped, and the design doc commits to resvg's + // formulation.) let bbox = element_object_bbox(ctx.dom, node); let Some(path) = clipper::resolve_to_path(ctx.dom, ctx.resources, target, bbox) else { - return !target_is_clip_path; + return true; }; canvas.clip_path(&path, ClipOp::Intersect, true); true diff --git a/crates/grida/src/htmlcss/svg/paint/scoped_svg_paint_state.rs b/crates/grida/src/htmlcss/svg/paint/scoped_svg_paint_state.rs index 3a81d32cb..79e50db53 100644 --- a/crates/grida/src/htmlcss/svg/paint/scoped_svg_paint_state.rs +++ b/crates/grida/src/htmlcss/svg/paint/scoped_svg_paint_state.rs @@ -79,13 +79,58 @@ pub struct PaintCtx<'a> { /// `vmin` / `vmax` (CSS Values 4 §8). For a standalone SVG /// rendered to a 500×500 PNG, this is `(500.0, 500.0)`. pub initial_viewport: (f32, f32), - /// `` element whose subtree we're currently painting, if any. - /// Inheritable property resolution (fill, stroke, font-size, …) - /// consults attributes here in addition to the DOM ancestor chain - /// so an SVG like `` propagates the - /// green fill into the cloned subtree. Mirrors Blink's "use shadow - /// tree" composition (`SVGUseElement::CreateInstanceTree`). - pub use_inherit: Option, + /// Stack of nested `` elements whose subtree we're currently + /// painting, innermost last. Inheritable property resolution + /// (fill, stroke, font-size, …) consults attributes here in + /// addition to the DOM ancestor chain so an SVG like + /// `` propagates the green fill into + /// the cloned subtree. Mirrors Blink's "use shadow tree" + /// composition (`SVGUseElement::CreateInstanceTree`). + /// + /// Modeled as a stack of references so multiple levels of `` + /// each contribute (innermost wins, then outer). Implemented as a + /// linked list of stack frames so `PaintCtx` stays `Copy` — each + /// recursion adds one frame on the caller's stack and points + /// `use_chain` at it. + pub use_chain: Option<&'a UseFrame<'a>>, +} + +/// One link in the `` recursion stack. Carries the `` +/// element id (for inheritance lookup) and the resolved target id +/// (for cycle detection — non-ancestor cycles like `#a → #b → #a` +/// are caught by walking this chain). +#[derive(Clone, Copy)] +pub struct UseFrame<'a> { + pub use_id: NodeId, + pub target_id: NodeId, + pub parent: Option<&'a UseFrame<'a>>, +} + +impl<'a> UseFrame<'a> { + /// Returns true if `target` already appears in the chain — used + /// by `paint_use` to abort recursion before painting a cyclic + /// reference. + pub fn contains_target(&self, target: NodeId) -> bool { + if self.target_id == target { + return true; + } + match self.parent { + Some(p) => p.contains_target(target), + None => false, + } + } +} + +/// Walk the `` chain innermost-first. Each `use_id` is yielded +/// in turn so callers can read inheritable attributes from the +/// closest `` first. +pub fn use_chain_iter<'a>(chain: Option<&'a UseFrame<'a>>) -> impl Iterator + 'a { + let mut cursor = chain; + std::iter::from_fn(move || { + let frame = cursor?; + cursor = frame.parent; + Some(frame.use_id) + }) } impl<'a> PaintCtx<'a> { @@ -107,13 +152,13 @@ impl<'a> PaintCtx<'a> { pattern_depth: 0, marker_depth: 0, initial_viewport, - use_inherit: None, + use_chain: None, } } - pub fn with_use_inherit(self, use_node: NodeId) -> Self { + pub fn with_use_chain(self, frame: &'a UseFrame<'a>) -> Self { Self { - use_inherit: Some(use_node), + use_chain: Some(frame), ..self } } diff --git a/crates/grida/src/htmlcss/svg/paint/svg_container_painter.rs b/crates/grida/src/htmlcss/svg/paint/svg_container_painter.rs index e4167d3d3..269f0f8c4 100644 --- a/crates/grida/src/htmlcss/svg/paint/svg_container_painter.rs +++ b/crates/grida/src/htmlcss/svg/paint/svg_container_painter.rs @@ -28,7 +28,7 @@ use super::svg_image_painter; use super::svg_shape_painter; use super::svg_text_painter; use super::svg_use_painter::paint_use; -use super::visibility::{has_display_none, is_painted, is_visible_inherited}; +use super::visibility::{has_display_none, is_visible_inherited}; /// Paint each SVG-namespace child of `parent_id`. pub fn paint_children(canvas: &Canvas, ctx: &PaintCtx<'_>, parent_id: NodeId) { @@ -68,9 +68,15 @@ fn paint_switch_child(canvas: &Canvas, ctx: &PaintCtx<'_>, parent_id: NodeId) { if !system_language_match(n) { continue; } - // `display:none`/`visibility:hidden` skip the candidate (still + // `display:none` / `visibility:hidden` skip the candidate (still // counts as visited — the next sibling becomes eligible). - if !is_painted(n) { + // Visibility check uses the *inherited* result so a + // `` (or any ancestor `hidden`) + // doesn't pin the first child as winner just because it lacks + // its own `visibility` declaration. Without this, a hidden + // ancestor would let the first eligible child swallow the + // selection while painting nothing. + if has_display_none(n) || !is_visible_inherited(ctx.dom, id) { continue; } paint_node(canvas, ctx, id); diff --git a/crates/grida/src/htmlcss/svg/paint/svg_shape_painter.rs b/crates/grida/src/htmlcss/svg/paint/svg_shape_painter.rs index 051d8efc6..f9a125a08 100644 --- a/crates/grida/src/htmlcss/svg/paint/svg_shape_painter.rs +++ b/crates/grida/src/htmlcss/svg/paint/svg_shape_painter.rs @@ -692,9 +692,11 @@ fn inherited_paint( current = n.parent; } // `` instance shadow tree — Blink's `SVGUseElement::CreateInstanceTree` - // makes the `` element act as an additional ancestor for - // inheritance purposes. - if let Some(use_id) = ctx.use_inherit { + // makes each `` element along the recursion chain act as an + // additional ancestor for inheritance purposes. Innermost `` + // wins; if it doesn't supply the property, walk outward. + for use_id in crate::htmlcss::svg::paint::scoped_svg_paint_state::use_chain_iter(ctx.use_chain) + { let mut current = Some(use_id); while let Some(id) = current { let n = ctx.dom.node(id); @@ -775,7 +777,8 @@ fn resolve_current_color(ctx: &PaintCtx<'_>, node: &DemoNode) -> Color { } current = n.parent; } - if let Some(use_id) = ctx.use_inherit { + for use_id in crate::htmlcss::svg::paint::scoped_svg_paint_state::use_chain_iter(ctx.use_chain) + { let mut current = Some(use_id); while let Some(id) = current { let n = ctx.dom.node(id); @@ -827,7 +830,8 @@ fn opacity_property(ctx: &PaintCtx<'_>, node: &DemoNode, name: &str) -> Option Option { /// When painting a `` instance, the use's own attributes also /// participate in inheritance (Blink's "use shadow tree"), even though /// the cloned subtree's DOM parent is wherever it lives in ``. -/// `ctx.use_inherit` is the use element to consult. +/// `ctx.use_chain` is the stack of `` elements to consult, +/// innermost first. fn read_inherited(ctx: &PaintCtx<'_>, node: &DemoNode, name: &str) -> Option { /// CSS-wide keywords that don't carry information at this level — /// the cascade should continue walking. `inherit` explicitly asks @@ -1912,7 +1913,8 @@ fn read_inherited(ctx: &PaintCtx<'_>, node: &DemoNode, name: &str) -> Option, node: &DemoNode) -> f32 { } cur = n.parent; } - if let Some(use_id) = ctx.use_inherit { + for use_id in crate::htmlcss::svg::paint::scoped_svg_paint_state::use_chain_iter(ctx.use_chain) + { let mut cur = Some(use_id); while let Some(id) = cur { let n = ctx.dom.node(id); @@ -2247,16 +2250,21 @@ mod tests { #[test] fn font_weight_named_and_numeric() { // CSS Fonts 4 §3.2.4: named keywords and the 1..=1000 numeric - // range. `inherited` is irrelevant for absolute values. + // range. In-range absolute values resolve to themselves. assert_eq!(resolve_font_weight_token("normal", 400), 400); assert_eq!(resolve_font_weight_token("bold", 400), 700); assert_eq!(resolve_font_weight_token("100", 400), 100); assert_eq!(resolve_font_weight_token("700", 400), 700); - // Out-of-range numeric values clamp to 1..=1000. - assert_eq!(resolve_font_weight_token("0", 400), 1); - assert_eq!(resolve_font_weight_token("1500", 400), 1000); - // Non-numeric, non-keyword falls back to 400 (the initial - // value per §3.2.4). + // Out-of-range numeric values are invalid declarations per + // CSS Fonts 4 §3.2.4 (Blink: + // `ConsumeFontWeightNumber` returns nullptr → cascade falls + // back to the inherited value). We mirror that — `inherited` + // is returned, not a clamp to 1/1000. + assert_eq!(resolve_font_weight_token("0", 400), 400); + assert_eq!(resolve_font_weight_token("1500", 400), 400); + assert_eq!(resolve_font_weight_token("0", 700), 700); + // Non-numeric, non-keyword falls back to the inherited value + // for the same reason. assert_eq!(resolve_font_weight_token("heavy", 400), 400); // Case-insensitive named keywords. assert_eq!(resolve_font_weight_token("BOLD", 400), 700); diff --git a/crates/grida/src/htmlcss/svg/paint/svg_text_painter/shaping.rs b/crates/grida/src/htmlcss/svg/paint/svg_text_painter/shaping.rs index 92375c726..5db71cdf7 100644 --- a/crates/grida/src/htmlcss/svg/paint/svg_text_painter/shaping.rs +++ b/crates/grida/src/htmlcss/svg/paint/svg_text_painter/shaping.rs @@ -154,7 +154,11 @@ pub(super) fn compute_kerned_advances( // shaped glyph at or after end_byte (or end of shaped list); // its origin marks the next character's start, so the advance // for this character is the difference. - let start_glyph = shaped.iter().position(|g| g.cluster >= start_byte); + // Use `==` (not `>=`) for `start_byte` so a character with no + // shaped glyph (combining mark, mark cluster) gets advance 0 + // instead of borrowing the next character's advance — preserving + // the zero-advance contract documented above. + let start_glyph = shaped.iter().position(|g| g.cluster == start_byte); let end_glyph = shaped.iter().position(|g| g.cluster >= end_byte); let advance = match start_glyph { None => 0.0, diff --git a/crates/grida/src/htmlcss/svg/paint/svg_use_painter.rs b/crates/grida/src/htmlcss/svg/paint/svg_use_painter.rs index 3a71a602f..a7d61b376 100644 --- a/crates/grida/src/htmlcss/svg/paint/svg_use_painter.rs +++ b/crates/grida/src/htmlcss/svg/paint/svg_use_painter.rs @@ -16,7 +16,7 @@ use crate::htmlcss::svg::dom::element::{get_attr, ElementKind}; use crate::htmlcss::svg::dom::href::{href_attr, same_document_fragment}; use crate::htmlcss::svg::layout::viewport::{ancestor_svg_viewport, compute_viewbox_matrix}; use crate::htmlcss::svg::paint::effects::group_opacity; -use crate::htmlcss::svg::paint::scoped_svg_paint_state::{PaintCtx, MAX_USE_DEPTH}; +use crate::htmlcss::svg::paint::scoped_svg_paint_state::{PaintCtx, UseFrame, MAX_USE_DEPTH}; use crate::htmlcss::svg::paint::svg_container_painter::{paint_children, paint_node}; pub(crate) fn paint_use(canvas: &Canvas, ctx: &PaintCtx<'_>, use_id: NodeId) { @@ -33,6 +33,7 @@ pub(crate) fn paint_use(canvas: &Canvas, ctx: &PaintCtx<'_>, use_id: NodeId) { if target_id == use_id { return; } + // Ancestor-chain cycle: target is a DOM ancestor of this ``. let mut anc = node.parent; while let Some(id) = anc { if id == target_id { @@ -40,6 +41,16 @@ pub(crate) fn paint_use(canvas: &Canvas, ctx: &PaintCtx<'_>, use_id: NodeId) { } anc = ctx.dom.node(id).parent; } + // Recursion-chain cycle: target was already painted as a `` + // host higher in the recursion stack (e.g. mutual references like + // `#a → #b → #a` that don't share a DOM ancestor). Without this + // check we'd recurse to MAX_USE_DEPTH painting intermediate + // content multiple times before the depth cap kicked in. + if let Some(chain) = ctx.use_chain { + if chain.contains_target(target_id) { + return; + } + } let restore = canvas.save(); let x = get_attr(node, "x").and_then(parse_length_px).unwrap_or(0.0); @@ -48,7 +59,12 @@ pub(crate) fn paint_use(canvas: &Canvas, ctx: &PaintCtx<'_>, use_id: NodeId) { canvas.translate((x, y)); } - let deeper = ctx.with_deeper_use().with_use_inherit(use_id); + let frame = UseFrame { + use_id, + target_id, + parent: ctx.use_chain, + }; + let deeper = ctx.with_deeper_use().with_use_chain(&frame); let target = ctx.dom.node(target_id); if let DemoNodeData::Element(d) = &target.data { let kind = ElementKind::from_local_name(d.name.local.as_ref()); diff --git a/crates/grida/src/htmlcss/svg/paint/visibility.rs b/crates/grida/src/htmlcss/svg/paint/visibility.rs index 2187ff8bf..b163d76ff 100644 --- a/crates/grida/src/htmlcss/svg/paint/visibility.rs +++ b/crates/grida/src/htmlcss/svg/paint/visibility.rs @@ -38,32 +38,31 @@ pub fn is_painted(node: &DemoNode) -> bool { /// but each level only needs to check its own value because the early /// return at the ancestor stops descent). pub fn has_display_none(node: &DemoNode) -> bool { - if matches_attr(node, "display", "none") { - return true; - } + // CSS specificity: inline `style="..."` beats the presentation + // attribute. Look up `style` first; if it sets `display`, that + // value wins and we ignore the attribute. Only fall through to + // the attribute when `style` doesn't mention `display`. if let Some(style) = get_attr(node, "style") { - if style_contains_pair(style, "display", "none") { - return true; + if let Some(v) = read_style_pair(style, "display") { + return v.eq_ignore_ascii_case("none"); } } - false + matches_attr(node, "display", "none") } /// `visibility: hidden`/`collapse` on this element only — no /// inheritance walk. Use [`is_visible_inherited`] when you need the /// effective value with parent-cascade applied. pub fn is_visible_self(node: &DemoNode) -> bool { - if matches_attr(node, "visibility", "hidden") || matches_attr(node, "visibility", "collapse") { - return false; - } + // Inline `style="..."` overrides the presentation attribute. Look + // up `style` first; only consult the attribute when `style` + // doesn't mention `visibility`. if let Some(style) = get_attr(node, "style") { - if style_contains_pair(style, "visibility", "hidden") - || style_contains_pair(style, "visibility", "collapse") - { - return false; + if let Some(v) = read_style_pair(style, "visibility") { + return !(v.eq_ignore_ascii_case("hidden") || v.eq_ignore_ascii_case("collapse")); } } - true + !(matches_attr(node, "visibility", "hidden") || matches_attr(node, "visibility", "collapse")) } /// Effective `visibility` for a leaf element — walks the ancestor @@ -111,17 +110,26 @@ fn matches_attr(node: &DemoNode, name: &str, value: &str) -> bool { get_attr(node, name).map(str::trim) == Some(value) } -/// Crude `key: value` lookup inside a `style="..."` blob. Good enough for -/// `display:none` / `visibility:hidden` checks — proper CSS resolution -/// will go through Stylo once the SVG cascade hook is wired. -fn style_contains_pair(style: &str, key: &str, value: &str) -> bool { +/// Crude `key: value` lookup inside a `style="..."` blob. Returns the +/// declared value (without `!important`) or `None` if the key is +/// absent. Good enough for `display` / `visibility` checks — proper CSS +/// resolution will go through Stylo once the SVG cascade hook is wired. +fn read_style_pair(style: &str, key: &str) -> Option { for decl in style.split(';') { let Some((k, v)) = decl.split_once(':') else { continue; }; - if k.trim().eq_ignore_ascii_case(key) && v.trim().eq_ignore_ascii_case(value) { - return true; + if k.trim().eq_ignore_ascii_case(key) { + // Strip an `!important` priority marker before returning; + // gating helpers compare against keyword values like `none` + // or `hidden`, not against priority annotations. + let v = v + .trim() + .trim_end_matches(|c: char| c.is_ascii_whitespace()) + .trim_end_matches("!important") + .trim(); + return Some(v.to_string()); } } - false + None } diff --git a/crates/grida/src/htmlcss/svg/resources/gradient.rs b/crates/grida/src/htmlcss/svg/resources/gradient.rs index 4c68021a2..19ccf07ee 100644 --- a/crates/grida/src/htmlcss/svg/resources/gradient.rs +++ b/crates/grida/src/htmlcss/svg/resources/gradient.rs @@ -320,6 +320,21 @@ fn resolve_common(dom: &DemoDom, chain: &[NodeId]) -> Option { return None; } + // Per SVG 1.1 §13.2.4 and SVG 2 §13.2.4: each stop's offset must be + // greater than or equal to the previous stop's. If not, the user + // agent must "use the previous offset value." Without this pass an + // out-of-order list like `0.8, 0.2, 1` would feed Skia a malformed + // distribution and produce the wrong gradient. Sorting would change + // the color order; spec says clamp upward instead. + let mut max_so_far = 0.0_f32; + for stop in stops.iter_mut() { + if stop.offset < max_so_far { + stop.offset = max_so_far; + } else { + max_so_far = stop.offset; + } + } + Some(GradientCommon { units, transform, diff --git a/crates/grida/src/htmlcss/svg/style/cascade.rs b/crates/grida/src/htmlcss/svg/style/cascade.rs index 03efc9452..6ee525fa4 100644 --- a/crates/grida/src/htmlcss/svg/style/cascade.rs +++ b/crates/grida/src/htmlcss/svg/style/cascade.rs @@ -63,7 +63,7 @@ fn get_inline_style(node: &DemoNode, name: &str) -> Option { continue; }; if k.trim().eq_ignore_ascii_case(name) { - return Some(v.trim().to_string()); + return Some(strip_priority(v.trim()).to_string()); } } None @@ -81,13 +81,33 @@ pub fn get_attr_or_style(node: &DemoNode, name: &str) -> Option { continue; }; if k.trim().eq_ignore_ascii_case(name) { - return Some(v.trim().to_string()); + return Some(strip_priority(v.trim()).to_string()); } } } get_attr(node, name).map(|s| s.to_string()) } +/// Strip a trailing `!important` priority marker from a declaration +/// value. CSS Cascading 5 §6.2: `!important` is part of the priority +/// (used by the cascade-order layer), not the value. Downstream parsers +/// expect `red`, not `red !important`. +fn strip_priority(value: &str) -> &str { + let lower = value.trim_end(); + // Case-insensitive trim of `!important` plus any whitespace that + // separates it from the value. We do the comparison on the lower + // 10 bytes ASCII; `!important` is ASCII-only. + let bytes = lower.as_bytes(); + if bytes.len() < 10 { + return lower; + } + let tail = &lower[lower.len() - 10..]; + if tail.eq_ignore_ascii_case("!important") { + return lower[..lower.len() - 10].trim_end(); + } + lower +} + /// Strip `/* ... */` C-style comments from a CSS declaration block. /// Nested comments are not part of CSS — first `*/` closes the run. fn strip_css_comments(s: &str) -> String { diff --git a/crates/grida/src/htmlcss/svg/style/stylesheet.rs b/crates/grida/src/htmlcss/svg/style/stylesheet.rs index 446b98d80..97ee77f6b 100644 --- a/crates/grida/src/htmlcss/svg/style/stylesheet.rs +++ b/crates/grida/src/htmlcss/svg/style/stylesheet.rs @@ -457,11 +457,16 @@ fn parse_at_import(s: &str) -> Option<(String, usize)> { /// portion after the keyword + whitespace). Returns `(url, bytes /// consumed)`. Shared between [`parse_at_import`] (which needs the /// length to advance) and the public [`scan_imports`] helper. +/// +/// `url(` is matched case-insensitively per CSS Syntax §4.3 (token +/// keywords are ASCII-case-insensitive). fn parse_import_url(body: &str) -> Option<(String, usize)> { - if let Some(rest) = body.strip_prefix("url(") { + let starts_with_url = body.len() >= 4 && body[..4].eq_ignore_ascii_case("url("); + if starts_with_url { + let rest = &body[4..]; let close = rest.find(')')?; let raw = rest[..close].trim().trim_matches(|c| c == '\'' || c == '"'); - Some((raw.to_string(), "url(".len() + close + 1)) + Some((raw.to_string(), 4 + close + 1)) } else if let Some(rest) = body.strip_prefix('"') { let close = rest.find('"')?; Some((rest[..close].to_string(), 1 + close + 1)) @@ -481,12 +486,15 @@ fn parse_import_url(body: &str) -> Option<(String, usize)> { pub fn scan_imports(text: &str) -> Vec { let stripped = strip_css_comments(text); let mut out = Vec::new(); - let mut rest = stripped.as_str(); - while let Some(idx) = rest.find("@import") { - let after = &rest[idx + "@import".len()..]; - // Same keyword-boundary check as `parse_at_import`: the byte - // after `@import` must be whitespace; otherwise this is a - // longer ident like `@importurl` and we move past it. + // `@import` matches case-insensitively per CSS Syntax §4.3. We + // scan a lowercase copy for keyword positions and slice the + // original-case `stripped` for the URL (positions match because + // ASCII case-folding preserves byte indices). + let lower = stripped.to_ascii_lowercase(); + let mut cursor = 0usize; + while let Some(rel) = lower[cursor..].find("@import") { + let idx = cursor + rel; + let after = &stripped[idx + "@import".len()..]; let boundary_ok = after .as_bytes() .first() @@ -497,7 +505,7 @@ pub fn scan_imports(text: &str) -> Vec { out.push(url); } } - rest = after; + cursor = idx + "@import".len(); } out } diff --git a/crates/grida_dev/src/reftest/args.rs b/crates/grida_dev/src/reftest/args.rs index 3209d1fad..3a7c2c1b0 100644 --- a/crates/grida_dev/src/reftest/args.rs +++ b/crates/grida_dev/src/reftest/args.rs @@ -24,11 +24,13 @@ impl std::str::FromStr for BgColor { /// convert to the Grida scene graph through `grida::import::svg::pack`, render /// via the canvas runtime. Lossy (editor-oriented tree surgery), but /// GPU-native and consistent with the in-editor experience. -/// - `Htmlcss`: goes through `grida::htmlcss::render_svg`, which records -/// into a Skia `Picture` via `PictureRecorder` before rasterizing. -/// Exercises the exact code path that inline `` inside HTML -/// takes, end-to-end through the in-tree htmlcss::svg renderer (no -/// fallback to Skia's built-in `svg::Dom`). +/// - `Htmlcss`: goes through `grida::htmlcss::svg::render_to_picture*` +/// (or its `_with_context` variant when host hooks are needed), +/// which records into a Skia `Picture` via `PictureRecorder` before +/// rasterizing. Exercises the exact code path that inline `` +/// inside HTML takes, end-to-end through the in-tree +/// `htmlcss::svg` renderer (no fallback to Skia's built-in +/// `svg::Dom`). #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub(crate) enum SvgRenderer { Iosvg, @@ -83,9 +85,10 @@ pub(crate) struct ReftestArgs { /// SVG renderer backend: /// - `iosvg` (default): grida scene graph via usvg → pack. - /// - `htmlcss`: grida::htmlcss::render_svg → PictureRecorder → - /// surface. End-to-end through the in-tree htmlcss::svg - /// renderer; no fallback to Skia's built-in `svg::Dom`. + /// - `htmlcss`: `grida::htmlcss::svg::render_to_picture*` → + /// PictureRecorder → surface. End-to-end through the in-tree + /// `htmlcss::svg` renderer; no fallback to Skia's built-in + /// `svg::Dom`. #[arg(long = "renderer", default_value = "iosvg")] pub renderer: SvgRenderer, } From de59d7c3ae70be2f6d2ea7c812828eee4d00f9b9 Mon Sep 17 00:00:00 2001 From: Universe Date: Wed, 29 Apr 2026 11:42:10 +0900 Subject: [PATCH 06/59] chore(grida_dev): silence dead_code on ComparisonResult.diff_percentage The field is still written by the comparator, but consumers now route through a separately-computed `diff_pct` value (in reftest::runner). Allow the dead_code warning so `cargo clippy --no-deps -- -D warnings` stays clean while the reftest tooling overhaul is in flight. --- crates/grida_dev/src/reftest/compare.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/grida_dev/src/reftest/compare.rs b/crates/grida_dev/src/reftest/compare.rs index 15140d7af..c99dcfd69 100644 --- a/crates/grida_dev/src/reftest/compare.rs +++ b/crates/grida_dev/src/reftest/compare.rs @@ -7,7 +7,8 @@ use crate::reftest::args::BgColor; pub(crate) struct ComparisonResult { pub similarity_score: f64, // 0.0 (completely different) to 1.0 (identical) - pub diff_percentage: f64, // percentage of pixels that differ + #[allow(dead_code)] // written by the comparator; consumers now use a separately-computed value + pub diff_percentage: f64, // percentage of pixels that differ pub error: Option, // if comparison failed } From e7a8ee5a0f26ef4e899e386bb905979e1caae74f Mon Sep 17 00:00:00 2001 From: Universe Date: Wed, 29 Apr 2026 11:42:37 +0900 Subject: [PATCH 07/59] fix(htmlcss::svg): drop spec-invalid clipPath `` targets instead of bailing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per CSS Masking 1 §6.2, a `` inside `` is only valid when its target is a basic shape, ``, or ``. Previously the path strategy bailed (returning `None` from `build_child_path`) when the target was ``, ``, or another non-shape, which propagated up to `resolve_to_path` and caused the clipped element to render unclipped. Chrome, Firefox, Safari, resvg and librsvg instead drop the invalid `` silently, leaving the clipPath empty (per SVG 2 §14.3.5 an empty clipPath clips everything, so the element is fully clipped). Mirror that by returning `Some(None)` for non-shape targets — the strategy stays intact and the clipPath becomes empty. Reftest impact (resvg-test-suite, htmlcss renderer): - consensus_pass_rate 0.7881 -> 0.7895 (1123/1425 -> 1125/1425) - masking_clipPath_symbol-via-use-is-not-a-valid-child: 0.024 -> 1.0 - masking_clipPath_with-invalid-child-via-use: 0.024 -> 1.0 --- .../src/htmlcss/svg/resources/clipper.rs | 34 ++++++++++++++----- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/crates/grida/src/htmlcss/svg/resources/clipper.rs b/crates/grida/src/htmlcss/svg/resources/clipper.rs index eaf2017d2..c45179758 100644 --- a/crates/grida/src/htmlcss/svg/resources/clipper.rs +++ b/crates/grida/src/htmlcss/svg/resources/clipper.rs @@ -345,19 +345,37 @@ fn build_child_path( parse_path(d) } ElementKind::Use => { - // Resolve the href target and recurse into its geometry. - // For the path strategy we only support a `` that - // references a single shape (path/rect/circle/etc) — a - // `` of a `` would need full subtree walking - // which we defer. - let target_id = deref_use_target(_dom, _resources, node)?; + // Per CSS Masking 1 §6.2 a `` in `` is only + // valid when it directly references a basic shape, `` + // or ``. Any other target (``, ``, nested + // ``, etc.) is a spec-invalid clipPath child: Chrome, + // Firefox, Safari, resvg and librsvg drop the contribution + // silently — the clipPath stays in the cascade and becomes + // empty if no other valid children remain (empty clipPath + // clips everything per SVG 2 §14.3.5). We mirror that: + // return `Some(None)` to drop the use's geometry without + // bailing the path strategy. An unresolved href falls into + // the same bucket. + let Some(target_id) = deref_use_target(_dom, _resources, node) else { + return Some(None); + }; let target = _dom.node(target_id); let DemoNodeData::Element(td) = &target.data else { return Some(None); }; let target_kind = ElementKind::from_local_name(td.name.local.as_ref()); - if !is_supported_clipper_child(target_kind) || target_kind == ElementKind::Use { - return None; + let target_is_shape = matches!( + target_kind, + ElementKind::Path + | ElementKind::Rect + | ElementKind::Circle + | ElementKind::Ellipse + | ElementKind::Line + | ElementKind::Polygon + | ElementKind::Polyline + ); + if !target_is_shape { + return Some(None); } let mut p = match build_child_path(_dom, _resources, target, target_kind)? { Some(p) => p, From 161feea9e6e5ccbce36514ba2246e55c0e8dfbd1 Mon Sep 17 00:00:00 2001 From: Universe Date: Thu, 30 Apr 2026 03:21:02 +0900 Subject: [PATCH 08/59] fix(htmlcss::svg): treat broken filter primitive references as transparent / identity - detect feImage self-reference cycle and emit transparent - treat invalid url() inside a filter list as identity --- crates/grida/src/htmlcss/svg/paint/effects.rs | 12 +++++++++- .../grida/src/htmlcss/svg/resources/filter.rs | 24 ++++++++++++++++++- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/crates/grida/src/htmlcss/svg/paint/effects.rs b/crates/grida/src/htmlcss/svg/paint/effects.rs index b089ba56f..b42fa6b7c 100644 --- a/crates/grida/src/htmlcss/svg/paint/effects.rs +++ b/crates/grida/src/htmlcss/svg/paint/effects.rs @@ -70,6 +70,15 @@ pub(crate) fn resolve_filter_chain( let cc = super::super::resources::svg_filter_builder::resolve_current_color(ctx.dom, node); let fs = resolve_font_size_px(ctx.dom, node); + // A solitary `url(#…)` whose target cannot be resolved is the + // "invalid funcIRI" case: the resvg-aligned consensus behavior is + // to *hide* the element entirely (see svg_container_painter early + // return on `filter_failed_invalid`). When the same `url(#missing)` + // appears alongside other entries, CSS Filter Effects 2 / SVG 2 + // §15.4 demand we treat it as the identity instead — the rest of + // the list still applies. Branch on list length to keep both shapes + // correct. + let single_url_invalid = items.len() == 1 && items[0].0.eq_ignore_ascii_case("url"); for (name, args) in &items { if name.eq_ignore_ascii_case("url") { let id = args @@ -82,10 +91,11 @@ pub(crate) fn resolve_filter_chain( }); match resolved { Some(inv) => out.push(inv), - None => { + None if single_url_invalid => { *failed_invalid = true; return Vec::new(); } + None => {} } } else { let segment = format!("{}({})", name, args); diff --git a/crates/grida/src/htmlcss/svg/resources/filter.rs b/crates/grida/src/htmlcss/svg/resources/filter.rs index fa3740b8e..e1cee90e0 100644 --- a/crates/grida/src/htmlcss/svg/resources/filter.rs +++ b/crates/grida/src/htmlcss/svg/resources/filter.rs @@ -43,7 +43,7 @@ use super::svg_filter_builder::{ FeInput, FilterEffect, LightSource, LightingKind, MorphOp, Primitive, TransferFn, TurbulenceKind, }; -use super::svg_resources::Resources; +use super::svg_resources::{parse_url_ref, Resources}; use crate::htmlcss::svg::dom::attrs::{compute_image_dst_rect, parse_length_px}; use crate::htmlcss::svg::dom::element::{get_attr, ElementKind}; use crate::htmlcss::svg::dom::href::{href_attr, same_document_fragment}; @@ -158,6 +158,7 @@ pub fn resolve<'a>( images, paint_ctx, user_viewport, + current_filter_id: filter_id, }; let image_filter = build_dag(&primitives, &context)?; Some(FilterInvocation { @@ -300,6 +301,12 @@ struct BuildContext<'a> { /// user units). Used to resolve `` values on filter /// primitive subregion attrs when `primitiveUnits=userSpaceOnUse`. user_viewport: (f32, f32), + /// The `` element being resolved. `feImage` uses this to + /// detect direct self-reference cycles — if its internal `href` + /// target carries `filter=url(#self)`, recording the subtree would + /// re-enter this same filter and Chrome treats the result as + /// transparent black per Filter Effects §15.21. + current_filter_id: NodeId, } #[derive(Clone)] @@ -783,6 +790,21 @@ fn build_primitive( let internal_filter = same_document_fragment(href).and_then(|id| { let target = ctx.paint_ctx.resources.lookup(id)?; let target_node = ctx.paint_ctx.dom.node(target); + // Direct self-reference: target carries `filter=url(#self)` + // pointing back at the filter we're inside. Recording its + // subtree would re-enter the same filter; Chrome treats + // the result as transparent black per Filter Effects + // §15.21 ("source unavailable"). Bail to the + // `transparent_flood` fallback below. + if let Some(raw) = get_attr(target_node, "filter") { + if let Some(fid_str) = parse_url_ref(raw.trim()) { + if let Some(fid) = ctx.paint_ctx.resources.lookup(fid_str) { + if fid == ctx.current_filter_id { + return None; + } + } + } + } let target_bbox = super::super::layout::bbox::element_object_bbox(ctx.paint_ctx.dom, target_node); // Compute internal-ref subregion. Explicit x/y/w/h From f9618f42871cc105e579722ec8ee9f066139afe8 Mon Sep 17 00:00:00 2001 From: Universe Date: Thu, 30 Apr 2026 03:21:10 +0900 Subject: [PATCH 09/59] fix(htmlcss::svg): apply requiredExtensions / systemLanguage to all rendered elements Conditional processing previously only ran inside ; honour it on every paintable element and stop skipping display:none in . --- .../svg/paint/svg_container_painter.rs | 45 +++++++++++++------ 1 file changed, 32 insertions(+), 13 deletions(-) diff --git a/crates/grida/src/htmlcss/svg/paint/svg_container_painter.rs b/crates/grida/src/htmlcss/svg/paint/svg_container_painter.rs index 269f0f8c4..afc32a0ea 100644 --- a/crates/grida/src/htmlcss/svg/paint/svg_container_painter.rs +++ b/crates/grida/src/htmlcss/svg/paint/svg_container_painter.rs @@ -62,21 +62,21 @@ fn paint_switch_child(canvas: &Canvas, ctx: &PaintCtx<'_>, parent_id: NodeId) { if matches!(kind, ElementKind::Unknown) { continue; } - // `requiredFeatures` / `requiredExtensions` are deprecated and - // accepted-when-empty (Blink returns true unconditionally). - // `systemLanguage` defaults to true unless explicitly mismatched. - if !system_language_match(n) { + // SVG 1.1 §5.8.2 selection criteria: + // - `requiredFeatures` (deprecated; Blink returns true unconditionally, + // so an empty-or-missing list is true and a non-empty list is also + // accepted as true today) + // - `requiredExtensions`: a list of extension URIs; we don't claim any + // UA extensions, so a present, non-empty list fails the test + // - `systemLanguage`: defaults to true unless explicitly mismatched + // `display:none` / `visibility:hidden` are *not* selection criteria — + // the selected child is the first whose system tests pass, and normal + // display/visibility rules then apply to its paint (so a selected + // `display:none` child silently bypasses the entire ``). + if !required_extensions_match(n) { continue; } - // `display:none` / `visibility:hidden` skip the candidate (still - // counts as visited — the next sibling becomes eligible). - // Visibility check uses the *inherited* result so a - // `` (or any ancestor `hidden`) - // doesn't pin the first child as winner just because it lacks - // its own `visibility` declaration. Without this, a hidden - // ancestor would let the first eligible child swallow the - // selection while painting nothing. - if has_display_none(n) || !is_visible_inherited(ctx.dom, id) { + if !system_language_match(n) { continue; } paint_node(canvas, ctx, id); @@ -84,6 +84,15 @@ fn paint_switch_child(canvas: &Canvas, ctx: &PaintCtx<'_>, parent_id: NodeId) { } } +fn required_extensions_match(node: &DemoNode) -> bool { + match get_attr(node, "requiredExtensions").map(str::trim) { + // Missing or empty list — selection passes per §5.8.2. + None | Some("") => true, + // We don't claim any extension URIs, so any non-empty list fails. + Some(_) => false, + } +} + fn system_language_match(node: &DemoNode) -> bool { match get_attr(node, "systemLanguage").map(str::trim) { None => true, @@ -104,6 +113,16 @@ pub fn paint_node(canvas: &Canvas, ctx: &PaintCtx<'_>, id: NodeId) { if kind.is_hidden_container() { return; } + // SVG 2 §5.10.1 conditional processing: `requiredExtensions` and + // `systemLanguage` act as render-time filters on any element, not + // just `` children. A failing test omits the element from + // the rendering tree entirely. (Resource-only containers such as + // `` / `` are already handled above by + // `is_hidden_container`; their conditional-processing semantics + // belong on the resource lookup path, not here.) + if !required_extensions_match(node) || !system_language_match(node) { + return; + } // SVG 2 §11.1: `display: none` skips the entire subtree. But // `visibility: hidden` on a container is *not* a subtree skip — // descendants can override with `visibility: visible`. So early- From 808ffa08cd20017ea4dd1af7814078cb21a59b55 Mon Sep 17 00:00:00 2001 From: Universe Date: Wed, 29 Apr 2026 12:12:55 +0900 Subject: [PATCH 10/59] fix(htmlcss::svg): keep first-in-document-order entry on duplicated ids `Resources::build` walked the DOM and `insert`ed into the id table, so on duplicate ids the LAST element in document order silently shadowed the first. The DOM Core / SVG 2 contract for `getElementById`-style lookups (and what every reference renderer does) is that the FIRST declaration wins; `` should resolve to that one. Switch to `entry().or_insert()` so later duplicates are ignored. Reftest impact (resvg-test-suite, htmlcss renderer): - consensus_pass_rate 0.7951 -> 0.7958 (1133/1425 -> 1134/1425) - structure_use_duplicated-IDs: 0.036 -> 1.0 --- crates/grida/src/htmlcss/svg/resources/svg_resources.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/grida/src/htmlcss/svg/resources/svg_resources.rs b/crates/grida/src/htmlcss/svg/resources/svg_resources.rs index 1bebb15d9..97c0aef6b 100644 --- a/crates/grida/src/htmlcss/svg/resources/svg_resources.rs +++ b/crates/grida/src/htmlcss/svg/resources/svg_resources.rs @@ -33,7 +33,11 @@ impl Resources { continue; } if let Some(s) = get_attr(node, "id") { - by_id.insert(s.to_string(), id); + // Per DOM Core / SVG 2: when multiple elements share the + // same `id`, the *first* in document order wins for + // `getElementById`-style lookups. Use `or_insert` so a + // later duplicate doesn't shadow the first declaration. + by_id.entry(s.to_string()).or_insert(id); } } let stylesheet = Stylesheet::collect(dom, css); From 950fa90477da654fda609751e7b8cf971ef98d90 Mon Sep 17 00:00:00 2001 From: Universe Date: Wed, 29 Apr 2026 12:23:33 +0900 Subject: [PATCH 11/59] fix(htmlcss::svg): bound the fill/stroke ancestor walk at the target MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per SVG 2 §5.6.4, `` clones the referenced subtree into a shadow tree rooted at the target — the target's source-DOM parent is NOT part of the cascade for the cloned content. We were walking the source-DOM ancestors all the way to the document root, so a `` inside `` referenced by `` inherited red from the original `` instead of green from the ``. In `inherited_paint`, bound the source-ancestor walk at the innermost `` frame's `target_id`: process up to (and including) the target, then jump straight to the `use_chain` walk. When `node` is itself the target, the source-DOM walk is skipped entirely (the cloned subtree's parent is the ``). Reftest impact (resvg-test-suite, htmlcss renderer): - consensus_pass_rate 0.7958 -> 0.7965 (1134/1425 -> 1135/1425) - structure_use_style-inheritance-2: 0.036 -> 1.0 --- .../htmlcss/svg/paint/svg_shape_painter.rs | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/crates/grida/src/htmlcss/svg/paint/svg_shape_painter.rs b/crates/grida/src/htmlcss/svg/paint/svg_shape_painter.rs index f9a125a08..536899153 100644 --- a/crates/grida/src/htmlcss/svg/paint/svg_shape_painter.rs +++ b/crates/grida/src/htmlcss/svg/paint/svg_shape_painter.rs @@ -681,7 +681,28 @@ fn inherited_paint( } None } + // SVG 2 §5.6.4: a ``'s shadow tree is rooted at the cloned + // target — the source-DOM parent of the target is NOT in the + // cascade chain. Bound the source-ancestor walk at the innermost + // ``'s `target_id`: process up to (and including) that node, + // then jump straight to the `use_chain`. When `node` itself is the + // use target, the source-DOM walk is skipped entirely. + let boundary = ctx.use_chain.map(|f| f.target_id); let mut current = node.parent; + if let Some(target) = boundary { + let mut p = current; + let mut descends = false; + while let Some(id) = p { + if id == target { + descends = true; + break; + } + p = ctx.dom.node(id).parent; + } + if !descends { + current = None; + } + } while let Some(id) = current { let n = ctx.dom.node(id); if let Some(v) = read(n, property) { @@ -689,6 +710,9 @@ fn inherited_paint( return resolve_paint_with_server(ctx, n, property, Some(&v), default_solid); } } + if Some(id) == boundary { + break; + } current = n.parent; } // `` instance shadow tree — Blink's `SVGUseElement::CreateInstanceTree` From d11371ec721e934a33377fab35205ecfd3291ff1 Mon Sep 17 00:00:00 2001 From: Universe Date: Wed, 29 Apr 2026 12:37:28 +0900 Subject: [PATCH 12/59] fix(htmlcss::svg): resolve stroke-width percentages against the viewport diagonal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `stroke-width` is a `` per SVG 2 §11.1, and per §7.10 percentages on axis-agnostic length properties resolve against the viewport diagonal normalized by sqrt(2). The shape painter routed the value through `num_attr` (= `parse_length_px`), which returns `None` for any `%` token, so a `stroke-width="10%"` silently fell back to the 1.0-px default instead of the spec'd 20-px stroke on a 200×200 viewBox. Switch to `len_attr(..., Axis::D)` — the same helper already used for ``, gradient radii, and other diagonal-normalized lengths. Reftest impact (resvg-test-suite, htmlcss renderer): - consensus_pass_rate 0.7965 -> 0.7972 (1135/1425 -> 1136/1425) - painting_stroke-width_percentage: 0.163 -> 1.0 --- crates/grida/src/htmlcss/svg/paint/svg_shape_painter.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/crates/grida/src/htmlcss/svg/paint/svg_shape_painter.rs b/crates/grida/src/htmlcss/svg/paint/svg_shape_painter.rs index 536899153..eb8d68820 100644 --- a/crates/grida/src/htmlcss/svg/paint/svg_shape_painter.rs +++ b/crates/grida/src/htmlcss/svg/paint/svg_shape_painter.rs @@ -431,7 +431,13 @@ fn paint_stroke( let Some(paint_choice) = resolution else { return; }; - let width = num_attr(node, "stroke-width").unwrap_or(1.0); + // SVG 2 §7.10: percentages on `stroke-width` (and other length + // properties without an explicit axis) resolve against the + // viewport diagonal normalized by sqrt(2). `len_attr` with + // `Axis::D` does exactly that, so a `` + // inside a 200×200 viewBox renders the 20-px stroke Chrome and + // resvg agree on instead of falling back to 1px. + let width = len_attr(ctx, node, "stroke-width", Axis::D).unwrap_or(1.0); if width <= 0.0 { return; } From 0b1bcf0085ab4f208c5bc996d2befbb331c6cdc0 Mon Sep 17 00:00:00 2001 From: Universe Date: Wed, 29 Apr 2026 12:48:36 +0900 Subject: [PATCH 13/59] fix(htmlcss::svg): honor transform-origin on and MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CSS Transforms 1 / SVG 2 §8.3 say `transform-origin` shifts the pivot of any `transform=` (or pattern's `patternTransform`): the effective matrix is `T(o) ∘ M ∘ T(-o)`. The container painter already applies this for rendered elements, but the resource-side resolvers ignored it, so a `` rotated about (0,0) instead of the viewport center, and the same for ``. `clipper::resolve_to_path` now takes the `PaintCtx` directly (the only caller already had it) so it can read the viewport via `transform_origin_for`. `pattern::build_shader` wraps the parsed `patternTransform` matrix once after `collect_inherited` returns, so the rest of the pipeline keeps treating it as an opaque user-space transform. The origin lives on the pattern element itself, not the href chain. Reftest impact (resvg-test-suite, htmlcss renderer): - consensus_pass_rate 0.7972 -> 0.7993 (1136/1425 -> 1139/1425) - structure_transform-origin_on-clippath: 0.140 -> 1.0 - structure_transform-origin_on-clippath-objectBoundingBox: 0.140 -> 1.0 - structure_transform-origin_on-pattern-user-space-on-use: 0.136 -> 1.0 --- .../htmlcss/svg/paint/clip_path_clipper.rs | 2 +- .../src/htmlcss/svg/resources/clipper.rs | 24 +++++++++++++------ .../src/htmlcss/svg/resources/pattern.rs | 18 +++++++++++++- 3 files changed, 35 insertions(+), 9 deletions(-) diff --git a/crates/grida/src/htmlcss/svg/paint/clip_path_clipper.rs b/crates/grida/src/htmlcss/svg/paint/clip_path_clipper.rs index ac6c57bcc..245b469ff 100644 --- a/crates/grida/src/htmlcss/svg/paint/clip_path_clipper.rs +++ b/crates/grida/src/htmlcss/svg/paint/clip_path_clipper.rs @@ -43,7 +43,7 @@ pub(super) fn apply_clip_path( // resvg renders unclipped, and the design doc commits to resvg's // formulation.) let bbox = element_object_bbox(ctx.dom, node); - let Some(path) = clipper::resolve_to_path(ctx.dom, ctx.resources, target, bbox) else { + let Some(path) = clipper::resolve_to_path(ctx, target, bbox) else { return true; }; canvas.clip_path(&path, ClipOp::Intersect, true); diff --git a/crates/grida/src/htmlcss/svg/resources/clipper.rs b/crates/grida/src/htmlcss/svg/resources/clipper.rs index c45179758..5f3653a7f 100644 --- a/crates/grida/src/htmlcss/svg/resources/clipper.rs +++ b/crates/grida/src/htmlcss/svg/resources/clipper.rs @@ -29,6 +29,8 @@ use crate::htmlcss::svg::dom::attrs::{ use crate::htmlcss::svg::dom::element::{get_attr, ElementKind}; use crate::htmlcss::svg::dom::href::{href_attr, same_document_fragment}; use crate::htmlcss::svg::dom::path_d::parse_path; +use crate::htmlcss::svg::layout::transform::transform_origin_for; +use crate::htmlcss::svg::paint::scoped_svg_paint_state::PaintCtx; use super::svg_resources::Resources; @@ -45,12 +47,9 @@ enum ClipPathUnits { /// element's local coordinate space (the same space its `transform=` was /// just concatenated into). `None` means "this clipper has features /// outside the path strategy — caller should fall back". -pub fn resolve_to_path( - dom: &DemoDom, - resources: &Resources, - clip_id: NodeId, - object_bbox: Rect, -) -> Option { +pub fn resolve_to_path(ctx: &PaintCtx<'_>, clip_id: NodeId, object_bbox: Rect) -> Option { + let dom = ctx.dom; + let resources = ctx.resources; let node = dom.node(clip_id); let DemoNodeData::Element(data) = &node.data else { return None; @@ -105,8 +104,19 @@ pub fn resolve_to_path( } // `transform=` on the clipper itself, applied in user space. + // SVG 2 §8.3 / CSS Transforms 1: `transform-origin` shifts the + // pivot; the effective transform is `T(o) ∘ M ∘ T(-o)`. Default + // origin is `(0, 0)` (the existing behavior). if let Some(t) = get_attr(node, "transform").and_then(parse_transform) { - path = path.with_transform(&t); + let origin = transform_origin_for(ctx, node); + if origin != (0.0, 0.0) { + let mut wrapped = Matrix::translate(origin); + wrapped.pre_concat(&t); + wrapped.pre_concat(&Matrix::translate((-origin.0, -origin.1))); + path = path.with_transform(&wrapped); + } else { + path = path.with_transform(&t); + } } Some(path) diff --git a/crates/grida/src/htmlcss/svg/resources/pattern.rs b/crates/grida/src/htmlcss/svg/resources/pattern.rs index 8c439e7bd..eeb69d39e 100644 --- a/crates/grida/src/htmlcss/svg/resources/pattern.rs +++ b/crates/grida/src/htmlcss/svg/resources/pattern.rs @@ -84,10 +84,26 @@ pub fn build_shader(ctx: &PaintCtx<'_>, node: NodeId, bbox: Rect) -> Option= MAX_PATTERN_DEPTH { return None; } - let (attrs, content_node) = collect_inherited(ctx.dom, ctx.resources, node, 0)?; + let (mut attrs, content_node) = collect_inherited(ctx.dom, ctx.resources, node, 0)?; if !attrs.has_children { return None; } + // SVG 2 §8.3 / CSS Transforms 1: `transform-origin` shifts the + // pivot of `patternTransform` (the value sits on the pattern + // element itself, not the href chain). Wrap the parsed matrix in + // `T(o) ∘ M ∘ T(-o)` once here so the rest of the builder treats + // it as an opaque user-space transform. + if let Some(t) = attrs.pattern_transform { + let pattern_node = ctx.dom.node(node); + let origin = + crate::htmlcss::svg::layout::transform::transform_origin_for(ctx, pattern_node); + if origin != (0.0, 0.0) { + let mut wrapped = Matrix::translate(origin); + wrapped.pre_concat(&t); + wrapped.pre_concat(&Matrix::translate((-origin.0, -origin.1))); + attrs.pattern_transform = Some(wrapped); + } + } // Defaults per SVG 1.1 §13.3 / SVG 2 §13.3: // - patternUnits: objectBoundingBox From e2bd180658f6f567ecd9f2439363569fa179d8af Mon Sep 17 00:00:00 2001 From: Universe Date: Wed, 29 Apr 2026 12:57:59 +0900 Subject: [PATCH 14/59] fix(htmlcss::svg): chain clip-path=url(#empty) clips everything MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `clip-path=` attribute that resolves to an empty `` is a spec-edge case: per SVG 2 §14.3.5 / CSS Masking 1 §6.4 an empty clipPath clips everything, so chaining one on either the clipPath itself or any of its children should fully clip the referencing element. Today we silently ignored the chained reference and let the path strategy build the union from the raw geometry, so a fixture where the chain *should* erase the clip output instead rendered the unmodified shape. Add a small helper that detects `url(#id)` references to an empty ``. Two consult sites: - `resolve_to_path` short-circuits to `Path::default()` when the clipPath element itself carries such a reference — the empty path produces the SVG 2 §14.3.5 "empty output" semantics under `ClipOp::Intersect`. - `walk_clipper_children` skips children with the same kind of reference; their geometry contributes nothing to the union. Generic chained-clipPath composition (where the chained target *isn't* empty) is still out of scope for the path strategy — that lands when we promote those fixtures next. Reftest impact (resvg-test-suite, htmlcss renderer): - consensus_pass_rate 0.7993 -> 0.8007 (1139/1425 -> 1141/1425) - masking_clipPath_invalid-clip-path-on-child: 0.186 -> 1.0 - masking_clipPath_invalid-clip-path-on-self: 0.186 -> 1.0 --- .../src/htmlcss/svg/resources/clipper.rs | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/crates/grida/src/htmlcss/svg/resources/clipper.rs b/crates/grida/src/htmlcss/svg/resources/clipper.rs index 5f3653a7f..943a20297 100644 --- a/crates/grida/src/htmlcss/svg/resources/clipper.rs +++ b/crates/grida/src/htmlcss/svg/resources/clipper.rs @@ -79,6 +79,17 @@ pub fn resolve_to_path(ctx: &PaintCtx<'_>, clip_id: NodeId, object_bbox: Rect) - // resolve the clipper's own inherited rule once and pass it as the // default into the child walk. Children's own `clip-rule` (or a // wrapping ``'s) overrides. + // SVG 2 §14.3.5 / CSS Masking 1 §6.4: a `clip-path=` chained on + // the `` element itself composes another clipping + // region. Generic composition is out of scope for the path + // strategy, but a chain to an *empty* (or unresolvable) clipPath + // is the common spec-edge case and clips everything — the rect + // ends up fully clipped. Detect that here so we return an empty + // path instead of silently ignoring the chained reference. + if references_empty_clip_path(dom, resources, get_attr(node, "clip-path")) { + return Some(Path::default()); + } + let initial_rule = inherited_clip_rule(dom, node); let mut acc: Option = None; walk_clipper_children(dom, resources, node, initial_rule, &mut acc)?; @@ -147,6 +158,14 @@ fn walk_clipper_children( continue; } + // A child carrying its own `clip-path=` that resolves to an + // empty `` is fully clipped — its geometry + // contributes nothing to the union. Drop the child instead of + // letting its raw shape leak through the path strategy. + if references_empty_clip_path(dom, resources, get_attr(child, "clip-path")) { + continue; + } + let mut child_path = match build_child_path(dom, resources, child, kind)? { Some(p) => p, None => continue, @@ -199,6 +218,36 @@ fn inherited_clip_rule(dom: &DemoDom, start: &DemoNode) -> Option None } +/// Returns true when `attr` is a `url(#id)` whose target is a +/// `` with no element children. An empty `` clips +/// everything per SVG 2 §14.3.5, so a chain pointing at one is the +/// "fully clipped" sentinel for the path strategy. Missing attribute, +/// `none`, malformed reference, or non-clipPath target return false — +/// we leave broader chained-clipPath handling to a future extension. +fn references_empty_clip_path(dom: &DemoDom, resources: &Resources, attr: Option<&str>) -> bool { + let raw = match attr.map(str::trim) { + Some(s) if !s.is_empty() && !s.eq_ignore_ascii_case("none") => s, + _ => return false, + }; + let Some(id) = super::svg_resources::parse_url_ref(raw) else { + return false; + }; + let Some(target_id) = resources.lookup(id) else { + return false; + }; + let target = dom.node(target_id); + let DemoNodeData::Element(d) = &target.data else { + return false; + }; + if ElementKind::from_local_name(d.name.local.as_ref()) != ElementKind::ClipPath { + return false; + } + !target + .children + .iter() + .any(|&cid| matches!(dom.node(cid).data, DemoNodeData::Element(_))) +} + fn is_supported_clipper_child(kind: ElementKind) -> bool { matches!( kind, From 12e3e7e4d3295bdd2d4f5aed021374dcb9a811fc Mon Sep 17 00:00:00 2001 From: Universe Date: Wed, 29 Apr 2026 13:13:02 +0900 Subject: [PATCH 15/59] fix(htmlcss::svg): resolve nested- percentage x/y/width/height MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SVG 2 §7.7 lets x, y, width, height on a nested `` be a ``; percentages resolve against the enclosing viewport (the parent ``'s viewBox or width/height). Today `paint_nested_svg` and `viewport_box_for` ran their attribute values through `parse_length_px`, which drops `%`, so a `` collapsed to (0,0,0,0) and the entire nested viewport was skipped (children rendered nothing). - Add a small `length_or_percent(s, axis)` helper in `layout::viewport` that resolves `%` against the supplied axis and falls back to `parse_length_px` otherwise. - Use it in `paint_nested_svg` for x/y/w/h. - Use it in `viewport_box_for` so a `` inside the nested svg measures against the parent viewport when the nested svg's own width is also a percentage. The shape painter had a duplicate copy of `viewport_box_for`; collapse it to a thin shim forwarding to the layout one so both paths share the percentage resolution. Reftest impact (resvg-test-suite, htmlcss renderer): - consensus_pass_rate 0.8007 -> 0.8014 (1141/1425 -> 1142/1425) - structure_svg_nested-svg-with-relative-width-and-height: 0.087 -> 1.0 --- .../grida/src/htmlcss/svg/layout/viewport.rs | 41 ++++++++++++++++--- .../htmlcss/svg/paint/svg_shape_painter.rs | 26 ++---------- 2 files changed, 38 insertions(+), 29 deletions(-) diff --git a/crates/grida/src/htmlcss/svg/layout/viewport.rs b/crates/grida/src/htmlcss/svg/layout/viewport.rs index 3aa25c0e8..7f89e84ed 100644 --- a/crates/grida/src/htmlcss/svg/layout/viewport.rs +++ b/crates/grida/src/htmlcss/svg/layout/viewport.rs @@ -12,10 +12,20 @@ use crate::htmlcss::svg::paint::scoped_svg_paint_state::PaintCtx; use crate::htmlcss::svg::paint::svg_container_painter::paint_children; pub(crate) fn paint_nested_svg(canvas: &Canvas, ctx: &PaintCtx<'_>, id: NodeId, node: &DemoNode) { - let x = get_attr(node, "x").and_then(parse_length_px).unwrap_or(0.0); - let y = get_attr(node, "y").and_then(parse_length_px).unwrap_or(0.0); - let w = get_attr(node, "width").and_then(parse_length_px); - let h = get_attr(node, "height").and_then(parse_length_px); + // SVG 2 §7.7: x, y, width, height on a nested `` accept + // ``. Percentages resolve against the parent + // viewport (the enclosing `` element's viewBox or + // width/height). `parse_length_px` returns None for `%`, so + // resolve those explicitly here. + let (_, _, parent_w, parent_h) = viewport_box_for(ctx, node); + let x = get_attr(node, "x") + .and_then(|s| length_or_percent(s, parent_w)) + .unwrap_or(0.0); + let y = get_attr(node, "y") + .and_then(|s| length_or_percent(s, parent_h)) + .unwrap_or(0.0); + let w = get_attr(node, "width").and_then(|s| length_or_percent(s, parent_w)); + let h = get_attr(node, "height").and_then(|s| length_or_percent(s, parent_h)); let viewport_w = w.unwrap_or(0.0); let viewport_h = h.unwrap_or(0.0); @@ -106,11 +116,17 @@ pub(crate) fn viewport_box_for(ctx: &PaintCtx<'_>, node: &DemoNode) -> (f32, f32 if let Some(vb) = get_attr(n, "viewBox").and_then(parse_viewbox) { return vb; } + // SVG 2 §7.7: a nested `` accepts percentage + // width/height that resolve against its own enclosing + // viewport. Recurse so a `` inside + // a nested `` ultimately measures + // against the outer viewBox / canvas. + let (_, _, parent_w, parent_h) = viewport_box_for(ctx, n); let w = get_attr(n, "width") - .and_then(parse_length_px) + .and_then(|s| length_or_percent(s, parent_w)) .unwrap_or(0.0); let h = get_attr(n, "height") - .and_then(parse_length_px) + .and_then(|s| length_or_percent(s, parent_h)) .unwrap_or(0.0); return (0.0, 0.0, w, h); } @@ -120,6 +136,19 @@ pub(crate) fn viewport_box_for(ctx: &PaintCtx<'_>, node: &DemoNode) -> (f32, f32 (0.0, 0.0, 0.0, 0.0) } +/// Parse an SVG `` token, resolving `%` against the +/// supplied axis extent. Bare numbers and absolute units pass through +/// `parse_length_px`. Used by callers that need a single hop of +/// percentage resolution against a known viewport axis. +fn length_or_percent(s: &str, axis: f32) -> Option { + let s = s.trim(); + if let Some(p) = s.strip_suffix('%') { + let n: f32 = p.trim().parse().ok()?; + return Some(n / 100.0 * axis); + } + parse_length_px(s) +} + pub(crate) fn compute_viewbox_matrix( viewport: (f32, f32, f32, f32), view_box: (f32, f32, f32, f32), diff --git a/crates/grida/src/htmlcss/svg/paint/svg_shape_painter.rs b/crates/grida/src/htmlcss/svg/paint/svg_shape_painter.rs index eb8d68820..4cf91b056 100644 --- a/crates/grida/src/htmlcss/svg/paint/svg_shape_painter.rs +++ b/crates/grida/src/htmlcss/svg/paint/svg_shape_painter.rs @@ -17,13 +17,12 @@ use skia_safe::{ PathEffect, PathFillType, Point, RRect, Rect, }; -use super::super::dom::attrs::{parse_length_px, parse_paint, parse_points, parse_viewbox, Paint}; -use super::super::dom::element::{get_attr, ElementKind}; +use super::super::dom::attrs::{parse_length_px, parse_paint, parse_points, Paint}; +use super::super::dom::element::get_attr; use super::super::dom::path_d::parse_path; use super::super::resources::paint_server::{self, Resolved}; use super::super::style::cascade::cascade_property; use super::scoped_svg_paint_state::PaintCtx; -use csscascade::dom::DemoNodeData; /// Axis used to resolve a `` against the SVG viewport /// (per SVG 2 §10.2 "Length values"). `X` resolves against viewport @@ -42,26 +41,7 @@ enum Axis { /// `width`/`height` attributes. Returns `(0,0,0,0)` if no viewport /// is reachable; that collapses any percentage to 0. fn viewport_box_for(ctx: &PaintCtx<'_>, node: &DemoNode) -> (f32, f32, f32, f32) { - let mut current = node.parent; - while let Some(id) = current { - let n = ctx.dom.node(id); - if let DemoNodeData::Element(d) = &n.data { - if ElementKind::from_local_name(d.name.local.as_ref()) == ElementKind::Svg { - if let Some(vb) = get_attr(n, "viewBox").and_then(parse_viewbox) { - return vb; - } - let w = get_attr(n, "width") - .and_then(parse_length_px) - .unwrap_or(0.0); - let h = get_attr(n, "height") - .and_then(parse_length_px) - .unwrap_or(0.0); - return (0.0, 0.0, w, h); - } - } - current = n.parent; - } - (0.0, 0.0, 0.0, 0.0) + super::super::layout::viewport::viewport_box_for(ctx, node) } fn axis_extent(viewport: (f32, f32, f32, f32), axis: Axis) -> f32 { From be327fb7fb96402121aa69219f71731eb15cb84e Mon Sep 17 00:00:00 2001 From: Universe Date: Wed, 29 Apr 2026 13:20:57 +0900 Subject: [PATCH 16/59] fix(htmlcss::svg): explicit feConvolveMatrix divisor=0 emits transparent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Filter Effects 1 §15.16 calls a `divisor` of exactly 0 an error; Blink (`svg_fe_convolve_matrix_element.cc:169-175`) drops the primitive's output to transparent black, and Chrome's reference render confirms. Today the parser collapsed "unspecified" and "explicit 0" into the same `Option::None` value, which the build path then expanded via the "use sum of kernel" fallback — silently amplifying the input instead of zeroing it. Distinguish the two before stripping the zero. When the attribute is present and parses to exactly 0, return `transparent_flood()`. When it is absent, fall through to the existing kernel-sum default. Reftest impact (resvg-test-suite, htmlcss renderer): - consensus_pass_rate 0.8014 -> 0.8021 (1142/1425 -> 1143/1425) - filters_feConvolveMatrix_divisor=0: 0.241 -> 1.0 --- .../svg/resources/svg_filter_builder.rs | 24 ++++++++++++++----- 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/crates/grida/src/htmlcss/svg/resources/svg_filter_builder.rs b/crates/grida/src/htmlcss/svg/resources/svg_filter_builder.rs index 4ca2850bf..bce58c1d5 100644 --- a/crates/grida/src/htmlcss/svg/resources/svg_filter_builder.rs +++ b/crates/grida/src/htmlcss/svg/resources/svg_filter_builder.rs @@ -1261,12 +1261,24 @@ fn parse_convolve_matrix(node: &DemoNode, is_first: bool) -> Option Date: Wed, 29 Apr 2026 13:39:28 +0900 Subject: [PATCH 17/59] fix(htmlcss::svg): resolve userSpaceOnUse pattern percentages against the SVG viewport MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `` should resolve the percentage against the enclosing `` viewport (its viewBox or width/height) per SVG 2 §13.3. We were resolving it against `ctx.initial_viewport` instead — which is the host pixel canvas, not the SVG user-space viewport — so a 200×200 viewBox rendered at a larger output canvas produced an over-large tile and the pattern appeared sparsely tiled instead of densely packed. Switch the userSpaceOnUse + Percent branch to `nearest_svg_viewport`, the same helper the shape painter uses for percent attributes. Reftest impact (resvg-test-suite, htmlcss renderer): - consensus_pass_rate 0.8021 -> 0.8028 (1143/1425 -> 1144/1425) - paint-servers_pattern_patternUnits=userSpaceOnUse-with-percent: 0.237 -> 1.0 --- crates/grida/src/htmlcss/svg/resources/pattern.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/crates/grida/src/htmlcss/svg/resources/pattern.rs b/crates/grida/src/htmlcss/svg/resources/pattern.rs index eeb69d39e..cbe96c286 100644 --- a/crates/grida/src/htmlcss/svg/resources/pattern.rs +++ b/crates/grida/src/htmlcss/svg/resources/pattern.rs @@ -116,7 +116,14 @@ pub fn build_shader(ctx: &PaintCtx<'_>, node: NodeId, bbox: Rect) -> Option`'s + // viewBox or width/height) per SVG 2 §13.3 — not the host pixel + // canvas in `ctx.initial_viewport`. Use `nearest_svg_viewport`, + // which walks up to the closest `` and reports its user-space + // extents. + let pattern_node = ctx.dom.node(node); + let (svg_vp_w, svg_vp_h) = + crate::htmlcss::svg::layout::viewport::nearest_svg_viewport(ctx, pattern_node); let resolve = |v: Option, axis: Axis| -> f32 { let Some(v) = v else { return 0.0 }; match (pattern_units, v) { @@ -132,8 +139,8 @@ pub fn build_shader(ctx: &PaintCtx<'_>, node: NodeId, bbox: Rect) -> Option n, (Units::UserSpaceOnUse, Length::Percent(p)) => match axis { - Axis::X => ctx.initial_viewport.0 * (p / 100.0), - Axis::Y => ctx.initial_viewport.1 * (p / 100.0), + Axis::X => svg_vp_w * (p / 100.0), + Axis::Y => svg_vp_h * (p / 100.0), }, } }; From eb3d730ee6cdb86a80c05231fa8208fd4d32c82b Mon Sep 17 00:00:00 2001 From: Universe Date: Thu, 30 Apr 2026 03:21:37 +0900 Subject: [PATCH 18/59] fix(htmlcss::svg): of behaves like the spec'd shadow root - establishes a viewport like - target's x/y/width/height applies to the use shadow - honour overflow=visible/auto on nested --- .../grida/src/htmlcss/svg/layout/viewport.rs | 12 ++++++- .../src/htmlcss/svg/paint/svg_use_painter.rs | 33 ++++++++++++++++++- 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/crates/grida/src/htmlcss/svg/layout/viewport.rs b/crates/grida/src/htmlcss/svg/layout/viewport.rs index 7f89e84ed..a1b042f85 100644 --- a/crates/grida/src/htmlcss/svg/layout/viewport.rs +++ b/crates/grida/src/htmlcss/svg/layout/viewport.rs @@ -36,7 +36,17 @@ pub(crate) fn paint_nested_svg(canvas: &Canvas, ctx: &PaintCtx<'_>, id: NodeId, let viewport_rect = Rect::from_xywh(x, y, viewport_w, viewport_h); canvas.save(); - canvas.clip_rect(viewport_rect, skia_safe::ClipOp::Intersect, true); + // SVG 2 §7.7: a nested `` clips to its viewport unless the + // `overflow` property says otherwise. The default is `hidden`, + // but `visible` / `auto` (Chrome treats `auto` like `visible` + // here) skip the clip so children can extend past the box. + let overflow = get_attr(node, "overflow") + .map(str::trim) + .unwrap_or("hidden"); + let clip_to_viewport = !matches!(overflow, "visible" | "auto"); + if clip_to_viewport { + canvas.clip_rect(viewport_rect, skia_safe::ClipOp::Intersect, true); + } if let Some(view_box) = get_attr(node, "viewBox").and_then(parse_viewbox) { let par = get_attr(node, "preserveAspectRatio") diff --git a/crates/grida/src/htmlcss/svg/paint/svg_use_painter.rs b/crates/grida/src/htmlcss/svg/paint/svg_use_painter.rs index a7d61b376..d5a558598 100644 --- a/crates/grida/src/htmlcss/svg/paint/svg_use_painter.rs +++ b/crates/grida/src/htmlcss/svg/paint/svg_use_painter.rs @@ -69,7 +69,17 @@ pub(crate) fn paint_use(canvas: &Canvas, ctx: &PaintCtx<'_>, use_id: NodeId) { if let DemoNodeData::Element(d) = &target.data { let kind = ElementKind::from_local_name(d.name.local.as_ref()); match kind { - ElementKind::Symbol => paint_symbol_use(canvas, ctx, &deeper, use_id, target_id), + // Per SVG 2 §5.6.5, `` to a `` *or* an + // `` establishes a new viewport: the ``'s + // `width` / `height` override the target's own size, and + // the target's `viewBox` / `preserveAspectRatio` / + // `overflow` apply within it. `paint_symbol_use` already + // implements that contract; route both kinds through it + // so a fixture like `` + // doesn't paint the referenced svg at full natural size. + ElementKind::Symbol | ElementKind::Svg => { + paint_symbol_use(canvas, ctx, &deeper, use_id, target_id) + } _ => paint_node(canvas, &deeper, target_id), } } @@ -103,6 +113,27 @@ fn paint_symbol_use( .or_else(|| get_attr(target, "height").and_then(parse_length_px)) .unwrap_or(vp_h_default); + // SVG 2 §5.6.5: when the target is an ``, the cloned svg + // keeps its own `x` / `y` translation (they're properties of the + // referenced element, not overridden by the use). `` + // doesn't have x/y attributes, so this no-ops for that path. + let target_kind = if let DemoNodeData::Element(d) = &target.data { + ElementKind::from_local_name(d.name.local.as_ref()) + } else { + ElementKind::Unknown + }; + if target_kind == ElementKind::Svg { + let tx = get_attr(target, "x") + .and_then(parse_length_px) + .unwrap_or(0.0); + let ty = get_attr(target, "y") + .and_then(parse_length_px) + .unwrap_or(0.0); + if tx != 0.0 || ty != 0.0 { + canvas.translate((tx, ty)); + } + } + let overflow = get_attr(target, "overflow") .map(str::trim) .unwrap_or("hidden"); From 0c9f4226e5f971a2d1cdac550f94eb22cb25689d Mon Sep 17 00:00:00 2001 From: Universe Date: Wed, 29 Apr 2026 14:14:42 +0900 Subject: [PATCH 19/59] fix(htmlcss::svg): resolve stroke-dashoffset percentages against the viewport diagonal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `stroke-dashoffset` is a `` per SVG 2 §11.5, and percentages resolve against the viewport diagonal normalized by sqrt(2) — the same axis-agnostic length resolution we already use for `stroke-width`. The shape painter routed the value through `num_attr` (= `parse_length_px`), which returns `None` for any `%` token, so a `stroke-dashoffset="20%"` silently fell back to 0 and the dash phase matched the no-offset case. Switch to `len_attr(..., Axis::D)`, mirroring the iter-8 fix for `stroke-width`. Reftest impact (resvg-test-suite, htmlcss renderer): - consensus_pass_rate 0.8077 -> 0.8091 (1151/1425 -> 1153/1425) - painting_stroke-dashoffset_percent-units: 0.347 -> 1.0 - one neighbouring dashoffset-percent fixture also flipped --- crates/grida/src/htmlcss/svg/paint/svg_shape_painter.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/crates/grida/src/htmlcss/svg/paint/svg_shape_painter.rs b/crates/grida/src/htmlcss/svg/paint/svg_shape_painter.rs index 4cf91b056..e72c3e099 100644 --- a/crates/grida/src/htmlcss/svg/paint/svg_shape_painter.rs +++ b/crates/grida/src/htmlcss/svg/paint/svg_shape_painter.rs @@ -465,7 +465,14 @@ fn paint_stroke( if let Some(dash_attr) = get_attr(node, "stroke-dasharray") { if let Some(intervals) = parse_dash_intervals(dash_attr) { - let phase = num_attr(node, "stroke-dashoffset").unwrap_or(0.0); + // SVG 2 §11.5: `stroke-dashoffset` is a `` + // and percentages resolve against the viewport diagonal + // normalized by sqrt(2) — same axis as `stroke-width`. Use + // `len_attr` with `Axis::D` so a value like + // `stroke-dashoffset="20%"` on a 200×200 viewBox produces + // the 40-px offset Chrome and resvg agree on, instead of + // silently falling back to 0 from `num_attr`'s `%` reject. + let phase = len_attr(ctx, node, "stroke-dashoffset", Axis::D).unwrap_or(0.0); if let Some(effect) = PathEffect::dash(&intervals, phase) { p.set_path_effect(effect); } From 1f7178a13c63cff991568dc3be23f9f619456deb Mon Sep 17 00:00:00 2001 From: Universe Date: Wed, 29 Apr 2026 14:25:31 +0900 Subject: [PATCH 20/59] fix(htmlcss::svg): detect cycles via an active-mask chain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `mask=` reference whose target is already on the in-flight mask stack should be treated as `mask: none` per CSS Masking 1 — invalid funcIRIs render the element unmasked. Today the only protection was a hard 4-level `MAX_MASK_DEPTH` cap, so a self-referential mask (`` inside ``) stacked four DstIn layers, multiplying the gradient alpha down to opaque black before the cap aborted; the masked rect rendered as nothing. Add a `MaskFrame` linked list mirroring the `UseFrame` shape and a `mask_chain` slot on `PaintCtx`. `apply_mask` pushes its own mask before descending into the mask's children, and both `paint_node`'s mask resolution and the chained-mask resolution skip any target already on the chain. The depth cap stays in place as a last-resort guard. Reftest impact (resvg-test-suite, htmlcss renderer): - consensus_pass_rate 0.8091 -> 0.8105 (1153/1425 -> 1155/1425) - masking_mask_self-recursive: 0.365 -> 1.0 - one neighbouring mask-cycle fixture also flipped to passing - masking_mask_recursive-on-self (mutual cycle) still failing — needs multi-step transitive cycle detection in a follow-up --- .../svg/paint/scoped_svg_paint_state.rs | 37 +++++++++++++++++++ .../svg/paint/svg_container_painter.rs | 18 ++++++++- 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/crates/grida/src/htmlcss/svg/paint/scoped_svg_paint_state.rs b/crates/grida/src/htmlcss/svg/paint/scoped_svg_paint_state.rs index 79e50db53..1446cc91f 100644 --- a/crates/grida/src/htmlcss/svg/paint/scoped_svg_paint_state.rs +++ b/crates/grida/src/htmlcss/svg/paint/scoped_svg_paint_state.rs @@ -93,6 +93,35 @@ pub struct PaintCtx<'a> { /// recursion adds one frame on the caller's stack and points /// `use_chain` at it. pub use_chain: Option<&'a UseFrame<'a>>, + /// Stack of `` elements currently being applied, innermost + /// last. Lets us detect a `mask=` reference that points back at a + /// mask already in flight (direct self-reference, mutual + /// `#a → #b → #a`, or any longer cycle) and treat it as + /// `mask: none` per CSS Masking 1 — invalid mask funcIRIs render + /// the element unmasked. Same linked-list shape as `use_chain` so + /// `PaintCtx` stays `Copy`. + pub mask_chain: Option<&'a MaskFrame<'a>>, +} + +/// One link in the active-mask stack. Holds the mask element's id so +/// the chain walk can detect cycles before reopening another save +/// layer. +#[derive(Clone, Copy)] +pub struct MaskFrame<'a> { + pub mask_id: NodeId, + pub parent: Option<&'a MaskFrame<'a>>, +} + +impl<'a> MaskFrame<'a> { + pub fn contains(&self, id: NodeId) -> bool { + if self.mask_id == id { + return true; + } + match self.parent { + Some(p) => p.contains(id), + None => false, + } + } } /// One link in the `` recursion stack. Carries the `` @@ -153,6 +182,7 @@ impl<'a> PaintCtx<'a> { marker_depth: 0, initial_viewport, use_chain: None, + mask_chain: None, } } @@ -163,6 +193,13 @@ impl<'a> PaintCtx<'a> { } } + pub fn with_mask_chain(self, frame: &'a MaskFrame<'a>) -> Self { + Self { + mask_chain: Some(frame), + ..self + } + } + pub fn with_deeper_use(self) -> Self { Self { use_depth: self.use_depth + 1, diff --git a/crates/grida/src/htmlcss/svg/paint/svg_container_painter.rs b/crates/grida/src/htmlcss/svg/paint/svg_container_painter.rs index afc32a0ea..09d54862c 100644 --- a/crates/grida/src/htmlcss/svg/paint/svg_container_painter.rs +++ b/crates/grida/src/htmlcss/svg/paint/svg_container_painter.rs @@ -258,6 +258,14 @@ pub fn paint_node(canvas: &Canvas, ctx: &PaintCtx<'_>, id: NodeId) { .filter(|v| !v.is_empty() && *v != "none") .and_then(parse_url_ref) .and_then(|id| ctx.resources.lookup(id)) + // CSS Masking 1: a `mask=` whose target is already on the + // active-mask stack would create a cycle. Treat the + // reference as `mask: none` so the element renders + // unmasked — matches Chrome / resvg. + .filter(|target| match ctx.mask_chain { + Some(chain) => !chain.contains(*target), + None => true, + }) .and_then(|target| { let bbox = element_object_bbox(ctx.dom, node); masker::resolve(ctx.dom, target, bbox) @@ -360,7 +368,14 @@ fn apply_mask(canvas: &Canvas, ctx: &PaintCtx<'_>, inv: &masker::MaskInvocation) canvas.concat(&inv.content_to_user_space); } - let deeper = ctx.with_deeper_mask(); + // Push this mask onto the active-mask chain so any descendant or + // chained reference back to it is detected as a cycle and treated + // as `mask: none` per CSS Masking 1. + let mask_frame = super::scoped_svg_paint_state::MaskFrame { + mask_id: inv.mask_id, + parent: ctx.mask_chain, + }; + let deeper = ctx.with_deeper_mask().with_mask_chain(&mask_frame); let mask_node = ctx.dom.node(inv.mask_id); let chained = if deeper.mask_depth >= MAX_MASK_DEPTH { None @@ -370,6 +385,7 @@ fn apply_mask(canvas: &Canvas, ctx: &PaintCtx<'_>, inv: &masker::MaskInvocation) .filter(|v| !v.is_empty() && *v != "none") .and_then(parse_url_ref) .and_then(|id| ctx.resources.lookup(id)) + .filter(|target| !mask_frame.contains(*target)) .and_then(|target| { let bbox = element_object_bbox(ctx.dom, mask_node); masker::resolve(ctx.dom, target, bbox) From 301e86fd44a89e6994fade4a220aa33c8d4df284 Mon Sep 17 00:00:00 2001 From: Universe Date: Wed, 29 Apr 2026 14:39:04 +0900 Subject: [PATCH 21/59] fix(htmlcss::svg): collapse non-overlapping primitive subregions to empty MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Skia's `Rect::intersect` leaves `self` untouched when the rects don't overlap (it just returns `false`). Our `subregion_for` was relying on the in-place intersect to bound each primitive's subregion to the filter region, so a `` declared entirely outside the filter region kept its original (0,0,10,10) crop instead of collapsing to empty. The downstream `feFlood` arm then bounded its red shader to that 10×10 rect, the chained `feOffset` translated it, and `feTile` happily tiled the red output across the filter region. After the intersect, explicitly substitute an empty rect when there was no overlap so the Flood arm's existing empty-crop branch fires and the chain produces transparent black, matching Chrome / resvg. Reftest impact (resvg-test-suite, htmlcss renderer): - consensus_pass_rate 0.8105 -> 0.8112 (1155/1425 -> 1156/1425) - filters_feTile_empty-region: 0.268 -> 1.0 --- .../grida/src/htmlcss/svg/resources/filter.rs | 26 ++++++++++++++++--- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/crates/grida/src/htmlcss/svg/resources/filter.rs b/crates/grida/src/htmlcss/svg/resources/filter.rs index e1cee90e0..521e1d3a4 100644 --- a/crates/grida/src/htmlcss/svg/resources/filter.rs +++ b/crates/grida/src/htmlcss/svg/resources/filter.rs @@ -481,8 +481,20 @@ fn build_primitive( // logic insert the correct sRGB→linear wrap at the boundary // into a linearRGB consumer (and the chain tail's linear→sRGB // wrap is then a no-op for solo-flood chains). - let shader = skia_safe::shaders::color(*color); - let filter = image_filters::shader(shader, crop); + // + // Filter Effects 1 §15.5: a primitive whose subregion has + // collapsed to empty (e.g. `feFlood x=0 y=0 w=10 h=10` + // entirely outside the filter region) produces no graphical + // output. Skia's `image_filters::shader` with an empty + // crop rect treats the crop as absent and floods the + // shader across the whole filter region instead, so we + // explicitly substitute a transparent flood here. + let filter = if crop.is_empty() { + transparent_flood(crop) + } else { + let shader = skia_safe::shaders::color(*color); + image_filters::shader(shader, crop) + }; Some(ResolvedResult { filter, subregion: crop, @@ -1248,10 +1260,16 @@ fn subregion_for(prim: &Primitive, filter_region: Rect, ctx: &BuildContext<'_>) .map(|v| resolve_extent(v, bbox.height(), viewport.1)) .unwrap_or(filter_region.height()); // Intersect with filter region — every primitive is bounded by the - // filter region per Blink (`filter_effect.cc:50-55`). + // filter region per Blink (`filter_effect.cc:50-55`). Skia's + // `Rect::intersect` leaves the rect untouched when the inputs + // don't overlap (it just reports `false`); we want an empty rect + // in that case so downstream primitives can detect "no graphical + // output" via `is_empty()`. let raw = Rect::from_xywh(x, y, w, h); let mut out = raw; - out.intersect(filter_region); + if !out.intersect(filter_region) { + out = Rect::new_empty(); + } out } From 2b7b165ed48b5e0a12a7b2020354cb9f0eb93ecc Mon Sep 17 00:00:00 2001 From: Universe Date: Wed, 29 Apr 2026 14:48:29 +0900 Subject: [PATCH 22/59] fix(htmlcss::svg): reject chained targets that close a transitive cycle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Iter 19 only caught direct self-reference (`mask` whose own `mask=` points back at itself, or at any ancestor on the active mask stack). A mutual cycle — `` ↔ `` — slipped through: when applying mask `b`, the chained reference to mask `a` looks safe because `a` isn't on the chain yet, but `a`'s `mask=` then references `b` which IS, and we'd happily nest another DstIn layer that the depth cap eventually aborted. Replace the immediate `chain.contains(target)` filter on the chained candidate with a transitive walk: follow `target.mask`, `target.mask.mask`, … until we either bottom out cleanly, revisit a node, or land on a chain entry. Bound at `MAX_MASK_DEPTH + 4` so a pathological / malformed mask chain still terminates. Reftest impact (resvg-test-suite, htmlcss renderer): - consensus_pass_rate 0.8112 -> 0.8119 (1156/1425 -> 1157/1425) - masking_mask_recursive-on-self: 0.365 -> 1.0 --- .../svg/paint/svg_container_painter.rs | 43 ++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/crates/grida/src/htmlcss/svg/paint/svg_container_painter.rs b/crates/grida/src/htmlcss/svg/paint/svg_container_painter.rs index 09d54862c..80ed64f5f 100644 --- a/crates/grida/src/htmlcss/svg/paint/svg_container_painter.rs +++ b/crates/grida/src/htmlcss/svg/paint/svg_container_painter.rs @@ -385,7 +385,11 @@ fn apply_mask(canvas: &Canvas, ctx: &PaintCtx<'_>, inv: &masker::MaskInvocation) .filter(|v| !v.is_empty() && *v != "none") .and_then(parse_url_ref) .and_then(|id| ctx.resources.lookup(id)) - .filter(|target| !mask_frame.contains(*target)) + // Reject the chained mask if applying it would close a + // mask-reference cycle. Direct self-reference is `target + // is in chain`; mutual / longer cycles need a transitive + // walk through `mask=` attrs on each mask element. + .filter(|target| !chained_mask_would_cycle(ctx, *target, &mask_frame)) .and_then(|target| { let bbox = element_object_bbox(ctx.dom, mask_node); masker::resolve(ctx.dom, target, bbox) @@ -405,3 +409,40 @@ fn apply_mask(canvas: &Canvas, ctx: &PaintCtx<'_>, inv: &masker::MaskInvocation) canvas.restore(); } + +/// Walk a candidate chained-mask target's `mask=` chain and return +/// true if any node along the way is already in `chain` (or appears +/// twice in the walk itself). Catches mutual `` / +/// `` cycles and longer rings that the immediate +/// `chain.contains(target)` check misses. +fn chained_mask_would_cycle( + ctx: &PaintCtx<'_>, + target: csscascade::dom::NodeId, + chain: &super::scoped_svg_paint_state::MaskFrame<'_>, +) -> bool { + let mut cur = target; + let mut visited: Vec = Vec::new(); + for _ in 0..super::scoped_svg_paint_state::MAX_MASK_DEPTH as usize + 4 { + if chain.contains(cur) || visited.contains(&cur) { + return true; + } + visited.push(cur); + let n = ctx.dom.node(cur); + let Some(raw) = get_attr(n, "mask") else { + return false; + }; + let raw = raw.trim(); + if raw.is_empty() || raw.eq_ignore_ascii_case("none") { + return false; + } + let Some(id) = parse_url_ref(raw) else { + return false; + }; + let Some(next) = ctx.resources.lookup(id) else { + return false; + }; + cur = next; + } + // Walk exceeded its bound — be conservative and treat as a cycle. + true +} From 2611959329898631d97cde9eadf34821a5e66340 Mon Sep 17 00:00:00 2001 From: Universe Date: Wed, 29 Apr 2026 15:00:54 +0900 Subject: [PATCH 23/59] fix(htmlcss::svg): reject negative arguments to CSS filter shorthands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CSS Filter Effects 1 §3.1 forbids negative values for `brightness`, `contrast`, `grayscale`, `invert`, `opacity`, `saturate`, and `sepia`. An invalid argument invalidates the entire `filter` property and the element renders as if no filter were applied. `parse_amount_strict` previously accepted any number; each function then clamped via `.max(0.0)` or `.clamp(0.0, 1.0)`. The result was a visibly-active filter (black for `brightness(-1)`, mid-gray for `contrast(-1)`, transparent for `opacity(-1)`, …) instead of the unfiltered seagreen rect Chrome / resvg agree on. Reject negatives in `parse_amount_strict` so the `?` in each function arm propagates `None`, and the caller falls through to "render unfiltered". The fix is a single check; the existing clamps stay as belt-and-braces for fractional rounding. Reftest impact (resvg-test-suite, htmlcss renderer): - consensus_pass_rate 0.8119 -> 0.8126 (1157/1425 -> 1158/1425) - filters_filter-functions_color-adjust-functions-negative: 0.50 -> 0.99 --- crates/grida/src/htmlcss/svg/resources/filter.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/crates/grida/src/htmlcss/svg/resources/filter.rs b/crates/grida/src/htmlcss/svg/resources/filter.rs index 521e1d3a4..2d4671d43 100644 --- a/crates/grida/src/htmlcss/svg/resources/filter.rs +++ b/crates/grida/src/htmlcss/svg/resources/filter.rs @@ -1688,7 +1688,19 @@ fn parse_amount_strict(s: &str) -> Option { if s.is_empty() { return Some(1.0); } - parse_amount(s) + // CSS Filter Effects 1 §3.1 explicitly forbids negative values for + // every shorthand that flows through here — `brightness`, + // `contrast`, `grayscale`, `invert`, `opacity`, `saturate`, + // `sepia`. Per the same section: an invalid argument invalidates + // the entire `filter` property, and the element renders as if + // there were no filter. Reject negatives here so the calling + // chain bails via the `?` and reverts to the unfiltered draw, + // instead of clamping to zero and producing a black/gray result. + let v = parse_amount(s)?; + if v < 0.0 { + return None; + } + Some(v) } /// Strict CSS `` parser used by `blur()`. Empty arg From 02cc61ac156c2c2db23332198ca85885ffbe0ba6 Mon Sep 17 00:00:00 2001 From: Universe Date: Wed, 29 Apr 2026 15:10:58 +0900 Subject: [PATCH 24/59] fix(htmlcss::svg): resolve stroke-dasharray percentage entries against the viewport diagonal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each entry in `stroke-dasharray` is a `` per SVG 2 §11.5; percentages resolve against the viewport diagonal normalized by sqrt(2), the same axis we already use for `stroke-width` (iter 8) and `stroke-dashoffset` (iter 18). The parser delegated to `parse_length_px` per token, which drops `%`, so a `stroke-dasharray="15% 30%"` collapsed to an empty array and the stroke rendered solid. Replace `parse_dash_intervals` with a variant that takes the viewport-diagonal extent and resolves each token: bare numbers and absolute units fall through `parse_length_px`, percent tokens scale against the supplied extent. Reftest impact (resvg-test-suite, htmlcss renderer): - consensus_pass_rate 0.8126 -> 0.8133 (1158/1425 -> 1159/1425) - painting_stroke-dasharray_percent-units: 0.51 -> 1.0 --- .../htmlcss/svg/paint/svg_shape_painter.rs | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/crates/grida/src/htmlcss/svg/paint/svg_shape_painter.rs b/crates/grida/src/htmlcss/svg/paint/svg_shape_painter.rs index e72c3e099..aaf50f654 100644 --- a/crates/grida/src/htmlcss/svg/paint/svg_shape_painter.rs +++ b/crates/grida/src/htmlcss/svg/paint/svg_shape_painter.rs @@ -464,7 +464,14 @@ fn paint_stroke( } if let Some(dash_attr) = get_attr(node, "stroke-dasharray") { - if let Some(intervals) = parse_dash_intervals(dash_attr) { + // SVG 2 §11.5: each entry in `stroke-dasharray` is a + // `` resolving against the viewport + // diagonal normalized by sqrt(2) — same axis as + // `stroke-width`. Per-token resolution lets `15% 30%` + // produce a real dashed stroke instead of silently dropping + // the percent tokens and rendering solid. + let dash_extent = axis_extent(viewport_box_for(ctx, node), Axis::D); + if let Some(intervals) = parse_dash_intervals_with_extent(dash_attr, dash_extent) { // SVG 2 §11.5: `stroke-dashoffset` is a `` // and percentages resolve against the viewport diagonal // normalized by sqrt(2) — same axis as `stroke-width`. Use @@ -623,15 +630,22 @@ fn points_bbox(pts: &[(f32, f32)]) -> Rect { Rect::new(minx, miny, maxx, maxy) } -fn parse_dash_intervals(s: &str) -> Option> { +fn parse_dash_intervals_with_extent(s: &str, dash_extent: f32) -> Option> { let s = s.trim(); if s.is_empty() || s.eq_ignore_ascii_case("none") { return None; } + let resolve = |p: &str| -> Option { + if let Some(num) = p.strip_suffix('%') { + let v: f32 = num.trim().parse().ok()?; + return Some(v / 100.0 * dash_extent); + } + parse_length_px(p) + }; let mut nums: Vec = s .split(|c: char| c.is_ascii_whitespace() || c == ',') .filter(|p| !p.is_empty()) - .filter_map(parse_length_px) + .filter_map(resolve) .collect(); if nums.is_empty() || nums.iter().all(|n| *n == 0.0) { return None; From de4b14648320334f4d724410ec8b0319dd5ea556 Mon Sep 17 00:00:00 2001 From: Universe Date: Thu, 30 Apr 2026 03:21:44 +0900 Subject: [PATCH 25/59] fix(htmlcss::svg): tspan/textPath honour conditional processing and -as-grouping - apply conditional-processing + display:none to - treat inside as a transparent grouping --- .../svg/paint/svg_container_painter.rs | 4 +-- .../htmlcss/svg/paint/svg_text_painter/mod.rs | 27 ++++++++++++++++++- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/crates/grida/src/htmlcss/svg/paint/svg_container_painter.rs b/crates/grida/src/htmlcss/svg/paint/svg_container_painter.rs index 80ed64f5f..2be4fce81 100644 --- a/crates/grida/src/htmlcss/svg/paint/svg_container_painter.rs +++ b/crates/grida/src/htmlcss/svg/paint/svg_container_painter.rs @@ -84,7 +84,7 @@ fn paint_switch_child(canvas: &Canvas, ctx: &PaintCtx<'_>, parent_id: NodeId) { } } -fn required_extensions_match(node: &DemoNode) -> bool { +pub(crate) fn required_extensions_match(node: &DemoNode) -> bool { match get_attr(node, "requiredExtensions").map(str::trim) { // Missing or empty list — selection passes per §5.8.2. None | Some("") => true, @@ -93,7 +93,7 @@ fn required_extensions_match(node: &DemoNode) -> bool { } } -fn system_language_match(node: &DemoNode) -> bool { +pub(crate) fn system_language_match(node: &DemoNode) -> bool { match get_attr(node, "systemLanguage").map(str::trim) { None => true, Some(s) => s diff --git a/crates/grida/src/htmlcss/svg/paint/svg_text_painter/mod.rs b/crates/grida/src/htmlcss/svg/paint/svg_text_painter/mod.rs index b38779e36..4caca3709 100644 --- a/crates/grida/src/htmlcss/svg/paint/svg_text_painter/mod.rs +++ b/crates/grida/src/htmlcss/svg/paint/svg_text_painter/mod.rs @@ -562,7 +562,32 @@ fn walk_glyphs( DemoNodeData::Element(data) => { let kind = ElementKind::from_local_name(data.name.local.as_ref()); match kind { - ElementKind::TSpan => { + ElementKind::TSpan | ElementKind::Anchor => { + // SVG 2 §5.10.1 / §11: conditional-processing + // attributes (`requiredExtensions`, + // `systemLanguage`) and `display:none` apply to + // `` just like any other rendered + // element. The container painter's `paint_node` + // gate doesn't fire for tspans (they're walked + // by the text painter, not dispatched), so + // re-apply the same checks here — otherwise a + // `` paints over + // the previous run instead of being skipped. + // + // SVG 2 §A: `` is a transparent grouping + // element — inside `` it's equivalent to + // a `` for layout/paint purposes (it + // adds hyperlink semantics on top, which the + // reftest renderer ignores). Walk its + // children with the same gate so `Text` + // emits glyphs instead of being silently + // dropped. + let skip = !super::svg_container_painter::required_extensions_match(child) + || !super::svg_container_painter::system_language_match(child) + || super::visibility::has_display_none(child); + if skip { + continue; + } let parent_shift = stack.last().map(|e| e.baseline_shift_dy).unwrap_or(0.0); stack.push(ElemAttrs::from_node(child, font_size, parent_shift)); walk_glyphs( From 56853da2f476ff48f547e43f15e7b6b9c0f665e9 Mon Sep 17 00:00:00 2001 From: Universe Date: Wed, 29 Apr 2026 16:25:45 +0900 Subject: [PATCH 26/59] fix(htmlcss::svg): treat feTurbulence numOctaves<=0 as transparent flood MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Filter Effects 1 §15.31 requires numOctaves to be a positive integer. Browsers (Chrome/Firefox/Safari/resvg/inkscape/batik all consensus) render an empty filter region for non-positive numOctaves — the noise loop in Blink's `fe_turbulence.cc` iterates `[0, num_octaves_)` and yields a zero sample, which composites as transparent. We were silently clamping to 1, which produced a visible noise pattern where the spec-correct output is empty. Detect non-positive numOctaves at parse time and emit a transparent `Flood` so the filtered element disappears (preventing the host rect's default `fill="black"` from leaking through if we instead returned `None` and bailed the filter). Fixes filters_feTurbulence_numOctaves=-1 and =0 (0.567 → 1.000 each). --- .../svg/resources/svg_filter_builder.rs | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/crates/grida/src/htmlcss/svg/resources/svg_filter_builder.rs b/crates/grida/src/htmlcss/svg/resources/svg_filter_builder.rs index bce58c1d5..bd508ba8b 100644 --- a/crates/grida/src/htmlcss/svg/resources/svg_filter_builder.rs +++ b/crates/grida/src/htmlcss/svg/resources/svg_filter_builder.rs @@ -1191,11 +1191,24 @@ fn parse_turbulence(node: &DemoNode) -> Option { // Negatives are clamped to 0 per Blink `fe_turbulence.cc:119-124`. let base_freq_x = fx.max(0.0); let base_freq_y = fy.max(0.0); - let num_octaves = get_attr(node, "numOctaves") + let num_octaves_raw = get_attr(node, "numOctaves") .and_then(|s| s.trim().parse::().ok()) - .unwrap_or(1) - // Blink caps at 9 (`fe_turbulence.cc:144`). - .clamp(1, 9) as u32; + .unwrap_or(1); + if num_octaves_raw <= 0 { + // SVG Filter Effects 1 §15.31: numOctaves "must be a positive + // integer". Non-positive values produce no noise — Blink's + // octave loop in `fe_turbulence.cc` (PaintingDataForCurrentFrame) + // iterates `[0, num_octaves_)`, so 0 / negative yields a zero + // sample. Chrome/Firefox/Safari/resvg/inkscape all render an + // empty filter region. Emit a transparent flood so the rect + // disappears (default fill="black" doesn't leak through). + return Some(FilterEffect::Flood { + color: Color::TRANSPARENT, + opacity: 0.0, + }); + } + // Blink caps at 9 (`fe_turbulence.cc:144`). + let num_octaves = num_octaves_raw.clamp(1, 9) as u32; let seed = get_attr(node, "seed") .and_then(parse_length_px) .unwrap_or(0.0); From f17ec413078edd430cd8f1cd20d89ffdedd9f408 Mon Sep 17 00:00:00 2001 From: Universe Date: Wed, 29 Apr 2026 16:42:46 +0900 Subject: [PATCH 27/59] fix(htmlcss::svg): compose chained clip-path on clipPath/child with cycle detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CSS Masking 1 §6.4 / SVG 2 §14.3.5: a `clip-path=url(#…)` on a `` element or one of its direct shape children composes another clipping region. Previously we silently ignored every non-empty chained reference, so `` and `` inside a clipPath had no effect. Mirror the iter 19/21 mask-cycle plumbing for clipPath: - `ClipFrame` linked-list on `PaintCtx::clip_chain` (same shape as `MaskFrame`) tracks active clipPath ids while resolving. - `chained_clip_path` helper resolves a chained `url(#id)` via `resolve_to_path` recursively, with the chain pushed; the helper returns `None` when the chained target is already in flight (cycle → break per Chrome/Safari/resvg behavior). - The clipPath's own chained reference intersects with its post- transform path (composes after clipPath's `transform=`). - A child's chained reference intersects with the child's post-transform path before its `clip-rule` fill type is set — Skia `PathOp::Intersect` reads each input's fill type, so the rule must be applied first for evenodd children. Fixes: - masking_clipPath_recursive / recursive-on-self / recursive-on-child: 0.638 → 1.000 - masking_clipPath_clip-path-on-self / on-self-2: 0.84 → 1.000 - masking_clipPath_clip-path-on-child / on-children: 0.84 → 1.000 Net: +6 consensus fixtures (1098 → 1104, 0.7705 → 0.7747). `clip-path-on-child-with-transform` (0.537 → 0.518) still fails — the chained intersection happens in clipPath-local space but the chained clipPath was resolved in user-space, mismatching when the outer clipPath has its own `transform=`. Pass-floor unchanged for that fixture; bigger refactor needed to compose clip-space-aware. --- .../svg/paint/scoped_svg_paint_state.rs | 38 ++++++++ .../src/htmlcss/svg/resources/clipper.rs | 89 ++++++++++++------- 2 files changed, 96 insertions(+), 31 deletions(-) diff --git a/crates/grida/src/htmlcss/svg/paint/scoped_svg_paint_state.rs b/crates/grida/src/htmlcss/svg/paint/scoped_svg_paint_state.rs index 1446cc91f..86c23ed5c 100644 --- a/crates/grida/src/htmlcss/svg/paint/scoped_svg_paint_state.rs +++ b/crates/grida/src/htmlcss/svg/paint/scoped_svg_paint_state.rs @@ -101,6 +101,14 @@ pub struct PaintCtx<'a> { /// the element unmasked. Same linked-list shape as `use_chain` so /// `PaintCtx` stays `Copy`. pub mask_chain: Option<&'a MaskFrame<'a>>, + /// Stack of `` elements currently being resolved, + /// innermost last. Mirrors `mask_chain` for `clip-path=` chained + /// references on clipPath elements or their direct children. CSS + /// Masking 1 §6.4 leaves clip-path cycles to "implementation + /// defined"; Chrome/Safari/resvg break the cycle at the first + /// repeat (treating the inner reference as absent), and the + /// reftest expected PNGs follow that behavior. + pub clip_chain: Option<&'a ClipFrame<'a>>, } /// One link in the active-mask stack. Holds the mask element's id so @@ -124,6 +132,28 @@ impl<'a> MaskFrame<'a> { } } +/// One link in the active-clipPath stack. Holds the clipPath element +/// id so the chain walk can detect cycles when a chained +/// `clip-path=url(#…)` reference points back at a clipPath already in +/// flight. +#[derive(Clone, Copy)] +pub struct ClipFrame<'a> { + pub clip_id: NodeId, + pub parent: Option<&'a ClipFrame<'a>>, +} + +impl<'a> ClipFrame<'a> { + pub fn contains(&self, id: NodeId) -> bool { + if self.clip_id == id { + return true; + } + match self.parent { + Some(p) => p.contains(id), + None => false, + } + } +} + /// One link in the `` recursion stack. Carries the `` /// element id (for inheritance lookup) and the resolved target id /// (for cycle detection — non-ancestor cycles like `#a → #b → #a` @@ -183,6 +213,14 @@ impl<'a> PaintCtx<'a> { initial_viewport, use_chain: None, mask_chain: None, + clip_chain: None, + } + } + + pub fn with_clip_chain(self, frame: &'a ClipFrame<'a>) -> Self { + Self { + clip_chain: Some(frame), + ..self } } diff --git a/crates/grida/src/htmlcss/svg/resources/clipper.rs b/crates/grida/src/htmlcss/svg/resources/clipper.rs index 943a20297..000a2063f 100644 --- a/crates/grida/src/htmlcss/svg/resources/clipper.rs +++ b/crates/grida/src/htmlcss/svg/resources/clipper.rs @@ -30,7 +30,7 @@ use crate::htmlcss::svg::dom::element::{get_attr, ElementKind}; use crate::htmlcss::svg::dom::href::{href_attr, same_document_fragment}; use crate::htmlcss::svg::dom::path_d::parse_path; use crate::htmlcss::svg::layout::transform::transform_origin_for; -use crate::htmlcss::svg::paint::scoped_svg_paint_state::PaintCtx; +use crate::htmlcss::svg::paint::scoped_svg_paint_state::{ClipFrame, PaintCtx}; use super::svg_resources::Resources; @@ -63,15 +63,23 @@ pub fn resolve_to_path(ctx: &PaintCtx<'_>, clip_id: NodeId, object_bbox: Rect) - _ => ClipPathUnits::UserSpaceOnUse, }; - // SVG 2 says a `clip-path` chained on the `` itself - // composes another clipping region, but resvg's handling — which - // our reference PNGs follow — ignores the chained reference when - // it cycles back to the same element (the most common case in the - // test suite). We can't represent chained clipPaths via the path - // strategy regardless, so the simplest correct approximation is - // to treat them as absent here. `mask=` / `filter=` on a clipPath - // are likewise unimplemented and ignored. - let _ = has_nontrivial_attr; // helper retained for any future chain probe + // SVG 2 §14.3.5 / CSS Masking 1 §6.4: a `clip-path=` chained on + // the `` element itself composes another clipping + // region. A chain to an *empty* (or unresolvable) clipPath is the + // "fully clipped" sentinel — return an empty path. Otherwise we + // recursively resolve the chained clipPath and intersect; cycles + // are broken via `clip_chain` (Chrome/Safari/resvg break at the + // first repeat, treating the inner reference as absent). + if references_empty_clip_path(dom, resources, get_attr(node, "clip-path")) { + return Some(Path::default()); + } + + let frame = ClipFrame { + clip_id, + parent: ctx.clip_chain, + }; + let inner_ctx = ctx.with_clip_chain(&frame); + let chained_self = chained_clip_path(&inner_ctx, get_attr(node, "clip-path"), object_bbox); // `clip-rule` cascades through ancestors of each shape — both // through `` wrappers inside the clipPath and (via CSS @@ -79,20 +87,9 @@ pub fn resolve_to_path(ctx: &PaintCtx<'_>, clip_id: NodeId, object_bbox: Rect) - // resolve the clipper's own inherited rule once and pass it as the // default into the child walk. Children's own `clip-rule` (or a // wrapping ``'s) overrides. - // SVG 2 §14.3.5 / CSS Masking 1 §6.4: a `clip-path=` chained on - // the `` element itself composes another clipping - // region. Generic composition is out of scope for the path - // strategy, but a chain to an *empty* (or unresolvable) clipPath - // is the common spec-edge case and clips everything — the rect - // ends up fully clipped. Detect that here so we return an empty - // path instead of silently ignoring the chained reference. - if references_empty_clip_path(dom, resources, get_attr(node, "clip-path")) { - return Some(Path::default()); - } - let initial_rule = inherited_clip_rule(dom, node); let mut acc: Option = None; - walk_clipper_children(dom, resources, node, initial_rule, &mut acc)?; + walk_clipper_children(&inner_ctx, node, initial_rule, object_bbox, &mut acc)?; // SVG 2 §14.3.5: a clipPath with no effective children clips // everything (output is empty). Distinguish that from the @@ -130,9 +127,35 @@ pub fn resolve_to_path(ctx: &PaintCtx<'_>, clip_id: NodeId, object_bbox: Rect) - } } + // Compose the clipPath's own chained `clip-path=` reference (after + // its transform), per CSS Masking 1 §6.4. Cycles were skipped by + // `chained_clip_path` returning `None`. + if let Some(chained) = chained_self { + path = skia_safe::op(&path, &chained, PathOp::Intersect).unwrap_or_default(); + } + Some(path) } +/// Resolve the chained `clip-path=url(#…)` reference on a clipPath or +/// one of its direct children to a path in user space. Returns `None` +/// when the attribute is absent, malformed, the referenced clipPath +/// is already in flight on `ctx.clip_chain` (cycle — break it), or +/// resolution otherwise fails. +fn chained_clip_path(ctx: &PaintCtx<'_>, attr: Option<&str>, object_bbox: Rect) -> Option { + let raw = attr + .map(str::trim) + .filter(|s| !s.is_empty() && !s.eq_ignore_ascii_case("none"))?; + let id = super::svg_resources::parse_url_ref(raw)?; + let target_id = ctx.resources.lookup(id)?; + if let Some(chain) = ctx.clip_chain { + if chain.contains(target_id) { + return None; + } + } + resolve_to_path(ctx, target_id, object_bbox) +} + /// Walk a clipPath's direct children, building each shape's /// contribution into `acc`. Per SVG 1.1 §14.3.5 only direct shape /// children (path/rect/circle/ellipse/line/polygon/polyline/use) @@ -141,12 +164,14 @@ pub fn resolve_to_path(ctx: &PaintCtx<'_>, clip_id: NodeId, object_bbox: Rect) - /// `g-is-not-a-valid-child.svg`). Per-shape `clip-rule` falls back to /// the cascade resolved by the caller into `inherited_rule`. fn walk_clipper_children( - dom: &DemoDom, - resources: &Resources, + ctx: &PaintCtx<'_>, parent: &DemoNode, inherited_rule: Option, + object_bbox: Rect, acc: &mut Option, ) -> Option<()> { + let dom = ctx.dom; + let resources = ctx.resources; for child_id in parent.children.iter().copied() { let child = dom.node(child_id); let DemoNodeData::Element(child_data) = &child.data else { @@ -175,11 +200,20 @@ fn walk_clipper_children( child_path = child_path.with_transform(&t); } + // Set fill type before any boolean op — Skia `PathOp::Intersect` + // uses each input's fill type to decide what's "inside". let rule = read_clip_rule_attr(child) .or(inherited_rule) .unwrap_or(PathFillType::Winding); child_path.set_fill_type(rule); + // Child's own chained `clip-path=` composes another clipping + // region around this shape. Cycles back to a clipPath already + // in flight on `ctx.clip_chain` are broken. + if let Some(chained) = chained_clip_path(ctx, get_attr(child, "clip-path"), object_bbox) { + child_path = skia_safe::op(&child_path, &chained, PathOp::Intersect)?; + } + *acc = Some(match acc.take() { None => child_path, Some(prev) => skia_safe::op(&prev, &child_path, PathOp::Union)?, @@ -278,13 +312,6 @@ fn deref_use_target( resources.lookup(id) } -fn has_nontrivial_attr(node: &csscascade::dom::DemoNode, name: &str) -> bool { - match get_attr(node, name).map(str::trim) { - Some("none") | Some("") | None => false, - Some(_) => true, - } -} - fn build_child_path( _dom: &DemoDom, _resources: &Resources, From a40d693e75ecf186163c2bb2394a737be1fbccc1 Mon Sep 17 00:00:00 2001 From: Universe Date: Wed, 29 Apr 2026 16:59:01 +0900 Subject: [PATCH 28/59] fix(htmlcss::svg): resolve transform-origin against the right reference box for gradient/pattern MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CSS Transforms 1 §5: `transform-origin` resolves against the element's transform-box. For `` / `` / ``, the transform-box that `gradientTransform` / `patternTransform` operates in is *not* the SVG viewport — it's the gradient/pattern's own coordinate space: - `gradientUnits=objectBoundingBox` (default): unit square (0..1). - `gradientUnits=userSpaceOnUse`: SVG viewport. - `patternUnits=objectBoundingBox` (default) with patternW/H: tile rect = bbox-scaled pattern box. - `patternUnits=userSpaceOnUse`: tile rect in user space. Previously `transform_origin_for` always resolved against the SVG viewport, so an `objectBoundingBox` gradient with `transform-origin="center"` ended up pivoting at the viewport center (e.g. (100, 100) for a 200x200 viewBox) instead of the unit-square center (0.5, 0.5). That moved the gradient/pattern origin far off the bbox, distorting the result. The `userSpaceOnUse` pattern fixture passed by coincidence — its declared pattern w/h matched the viewport dimensions. Adds `transform_origin_in_box(node, ref_box)` — the same parser used by `transform_origin_for`, but with an explicit reference box the caller chooses. Pattern.rs / gradient.rs pass the right box based on units. Pattern wrapping moved to after the tile rect is known so the reference box can be the actual tile. Fixes: - structure_transform-origin_on-gradient-object-bounding-box: 0.622 → 1.000 - structure_transform-origin_on-gradient-user-space-on-use: 0.666 → 1.000 - structure_transform-origin_on-pattern-object-bounding-box: 0.664 → 1.000 Pass-rate: 1104 → 1107 (+3, 0.7747 → 0.7768). --- .../grida/src/htmlcss/svg/layout/transform.rs | 23 ++++++++++++ .../src/htmlcss/svg/resources/gradient.rs | 24 +++++++++++- .../src/htmlcss/svg/resources/pattern.rs | 37 ++++++++++--------- 3 files changed, 66 insertions(+), 18 deletions(-) diff --git a/crates/grida/src/htmlcss/svg/layout/transform.rs b/crates/grida/src/htmlcss/svg/layout/transform.rs index 65c73ebba..217593265 100644 --- a/crates/grida/src/htmlcss/svg/layout/transform.rs +++ b/crates/grida/src/htmlcss/svg/layout/transform.rs @@ -26,6 +26,29 @@ pub(crate) fn transform_origin_for(ctx: &PaintCtx<'_>, node: &DemoNode) -> (f32, viewport_box_for(ctx, node) }; + parse_transform_origin(&raw, (rx, ry, rw, rh)) +} + +/// Resolve the `transform-origin` / `transform-box` declarations on +/// `node` against an explicit reference box `(rx, ry, rw, rh)`. The +/// box is the coordinate system in which the resulting origin will be +/// applied — for `` / `` / +/// `` with `gradientUnits=objectBoundingBox` (or the +/// pattern equivalent) that's the tile / unit-bbox space, not the +/// element's SVG viewport. Returns `(0, 0)` when no `transform-origin` +/// is set. +pub(crate) fn transform_origin_in_box( + node: &DemoNode, + ref_box: (f32, f32, f32, f32), +) -> (f32, f32) { + let Some(raw) = read_attr_or_style(node, "transform-origin") else { + return (0.0, 0.0); + }; + parse_transform_origin(&raw, ref_box) +} + +fn parse_transform_origin(raw: &str, ref_box: (f32, f32, f32, f32)) -> (f32, f32) { + let (rx, ry, rw, rh) = ref_box; let tokens: Vec<&str> = raw.split_ascii_whitespace().collect(); let parse_axis = |tok: &str, axis_size: f32, axis_origin: f32| -> Option { let t = tok.trim(); diff --git a/crates/grida/src/htmlcss/svg/resources/gradient.rs b/crates/grida/src/htmlcss/svg/resources/gradient.rs index 19ccf07ee..03b328aad 100644 --- a/crates/grida/src/htmlcss/svg/resources/gradient.rs +++ b/crates/grida/src/htmlcss/svg/resources/gradient.rs @@ -85,7 +85,29 @@ pub fn resolve_to_shader( m } }; - let local = Matrix::concat(&bbox_matrix, &common.transform); + // SVG 2 §8.3 / CSS Transforms 1: `transform-origin` shifts the + // pivot of `gradientTransform`. The pivot lives in the same space + // `gradientTransform` operates in — for `gradientUnits=objectBoundingBox` + // that's the unit square (0..1), for `userSpaceOnUse` it's the + // SVG viewport. Wrap as `T(o) ∘ M ∘ T(-o)` before composing with + // `bbox_matrix`. + let gradient_transform = { + let origin_box = match common.units { + GradientUnits::ObjectBoundingBox => (0.0, 0.0, 1.0, 1.0), + GradientUnits::UserSpaceOnUse => (0.0, 0.0, viewport.0, viewport.1), + }; + let origin = + crate::htmlcss::svg::layout::transform::transform_origin_in_box(node, origin_box); + if origin == (0.0, 0.0) { + common.transform + } else { + let mut wrapped = Matrix::translate(origin); + wrapped.pre_concat(&common.transform); + wrapped.pre_concat(&Matrix::translate((-origin.0, -origin.1))); + wrapped + } + }; + let local = Matrix::concat(&bbox_matrix, &gradient_transform); // NOTE: do NOT pass an explicit `Some(ColorSpace::new_srgb())` here. // skia-safe 0.93.x has a use-after-free in diff --git a/crates/grida/src/htmlcss/svg/resources/pattern.rs b/crates/grida/src/htmlcss/svg/resources/pattern.rs index cbe96c286..7bc0a5067 100644 --- a/crates/grida/src/htmlcss/svg/resources/pattern.rs +++ b/crates/grida/src/htmlcss/svg/resources/pattern.rs @@ -88,23 +88,6 @@ pub fn build_shader(ctx: &PaintCtx<'_>, node: NodeId, bbox: Rect) -> Option, node: NodeId, bbox: Rect) -> Option Date: Thu, 30 Apr 2026 03:22:10 +0900 Subject: [PATCH 29/59] fix(htmlcss::svg): text baseline / decoration / anchor / kerning / xml:space corrections Eight related fixes to text painting: - read alignment-baseline at text root, fall back to dominant-baseline - per-tspan alignment-baseline override - 1/3em / 1/5em for baseline-shift super/sub - cap_height for BaselineKind::Hanging - place overline at font ascent (line-box top), not cap height - honour font-kerning:none from CSS style declarations - honour xml:space="preserve" on / / - resolve text-anchor per anchored chunk --- .../htmlcss/svg/paint/svg_text_painter/mod.rs | 355 +++++++++++++++--- 1 file changed, 295 insertions(+), 60 deletions(-) diff --git a/crates/grida/src/htmlcss/svg/paint/svg_text_painter/mod.rs b/crates/grida/src/htmlcss/svg/paint/svg_text_painter/mod.rs index 4caca3709..081a85508 100644 --- a/crates/grida/src/htmlcss/svg/paint/svg_text_painter/mod.rs +++ b/crates/grida/src/htmlcss/svg/paint/svg_text_painter/mod.rs @@ -80,8 +80,10 @@ fn resolve_baseline(value: Option<&str>) -> BaselineKind { Some("hanging") => BaselineKind::Hanging, Some("mathematical") => BaselineKind::Math, Some("ideographic") => BaselineKind::IdeographicUnder, - Some("text-before-edge") | Some("text-top") => BaselineKind::TextOver, - Some("text-after-edge") | Some("text-bottom") => BaselineKind::TextUnder, + Some("text-before-edge") | Some("text-top") | Some("before-edge") => BaselineKind::TextOver, + Some("text-after-edge") | Some("text-bottom") | Some("after-edge") => { + BaselineKind::TextUnder + } // `auto`, `alphabetic`, `use-script`, `no-change`, `reset-size` and // anything unknown all collapse to alphabetic. SVG 2 §10.9.2: in // horizontal flow `auto` resolves to `alphabetic`. `use-script` @@ -99,19 +101,32 @@ fn baseline_offset(metrics: &skia_safe::FontMetrics, kind: BaselineKind) -> f32 let ascent = metrics.ascent; // negative in Skia let descent = metrics.descent; // positive let x_height = metrics.x_height; // positive (0 if font doesn't expose) + let cap_height = metrics.cap_height; // positive (0 if font doesn't expose) match kind { BaselineKind::Alphabetic => 0.0, BaselineKind::TextOver => -ascent, BaselineKind::TextUnder => -descent, BaselineKind::Central => -(ascent + descent) / 2.0, BaselineKind::XMiddle => x_height / 2.0, - // CSS Inline Layout 3 §5.1.1: hanging baseline ≈ em-square top - // (Tibetan / Indic scripts). Full ascent up from alphabetic. - // Browsers vary; Blink uses cap-height-ish (~0.7 * ascent). - // Em-top (full ascent) matches the resvg-test-suite expecteds - // most consistently for Latin glyphs without an OS/2 hanging - // baseline declared in the font. - BaselineKind::Hanging => -ascent, + // CSS Inline Layout 3 §5.1.1: hanging baseline ≈ cap-height + // top in the dominant script. Blink's `SimpleFontData:: + // GetBaseline(kHangingBaseline)` returns `cap_height` when + // OS/2 doesn't declare an explicit hanging metric, putting + // the glyph's cap-line on the user's y. Resvg-test-suite + // expecteds match this: e.g. `long` renders with the top of "long"'s + // cap at the user-y, not the full em-top. Earlier we used + // `-ascent` (em-top), which over-shifted by ~30%. Falls back + // to `0.8 * (-ascent)` when the font doesn't expose + // `cap_height` (Devanagari script in + // `dominant-baseline_hanging.svg` lands closer there). + BaselineKind::Hanging => { + if cap_height > 0.0 { + cap_height + } else { + -ascent * 0.8 + } + } BaselineKind::Math => -ascent / 2.0, BaselineKind::IdeographicUnder => -descent, } @@ -150,9 +165,27 @@ pub fn paint(canvas: &Canvas, ctx: &PaintCtx<'_>, root_id: NodeId, node: &DemoNo }; let font = Font::from_typeface(typeface, font_size); - // 2. Resolve dominant-baseline → constant y-offset added to every - // glyph's pen y, so the chosen baseline lands on the user y. - let baseline = resolve_baseline(read_inherited(ctx, node, "dominant-baseline").as_deref()); + // 2. Resolve dominant-baseline / alignment-baseline → constant + // y-offset added to every glyph's pen y, so the chosen baseline + // lands on the user y. SVG 2 §11.10.2.6 + CSS Inline Layout 3 + // §3.3: `alignment-baseline` on the layout root takes + // precedence over `dominant-baseline`; `auto` / `baseline` / + // absent falls back to `dominant-baseline`. `read_inherited` + // already handles `inherit` by walking ancestors. + let baseline = { + let ab = read_inherited(ctx, node, "alignment-baseline"); + let active = match ab.as_deref().map(str::trim) { + Some(v) + if !v.is_empty() + && !v.eq_ignore_ascii_case("auto") + && !v.eq_ignore_ascii_case("baseline") => + { + Some(v.to_string()) + } + _ => read_inherited(ctx, node, "dominant-baseline"), + }; + resolve_baseline(active.as_deref()) + }; let (_line_spacing, metrics) = font.metrics(); let baseline_y = baseline_offset(&metrics, baseline); @@ -203,9 +236,36 @@ pub fn paint(canvas: &Canvas, ctx: &PaintCtx<'_>, root_id: NodeId, node: &DemoNo // advance, and every fixture rendering "Text" scores ~64% from the // resulting accumulated drift (Skia's `Font::measureText` only // applies the legacy `kern` table, not GPOS). + // CSS Fonts 4 §6.5 `font-kerning`: `auto` / `normal` apply + // kerning; `none` disables it. We approximate "kerning off" by + // skipping the GPOS shaping pass and using standalone per-glyph + // widths from `font.measure_str` (which only applies the legacy + // `kern` table, not GPOS pair adjustments — same fallback Blink + // hits when GPOS is suppressed). Per SVG 2, `font-kerning` is + // not a presentation attribute — the resvg-test-suite's + // `as-property.svg` documents this behavior ("must be inside + // style, otherwise ignored"). Read from `style="…"` (or its + // ancestor cascade) only, never from the bare attribute. + let kerning_off = inherited_style_only(ctx, node, "font-kerning") + .as_deref() + .map(str::trim) + .map(|v| v.eq_ignore_ascii_case("none")) + .unwrap_or(false); let shaped_text: String = glyph_attrs.iter().map(|g| g.ch).collect(); - let (shaped_glyphs, shaped_total) = shape_text(&shaped_text, &font); - let kerned_advances = compute_kerned_advances(&glyph_attrs, &shaped_glyphs, shaped_total); + let kerned_advances = if kerning_off { + let mut buf = [0u8; 4]; + glyph_attrs + .iter() + .map(|g| { + let s = g.ch.encode_utf8(&mut buf); + let (w, _) = font.measure_str(s, None); + w + }) + .collect::>() + } else { + let (shaped_glyphs, shaped_total) = shape_text(&shaped_text, &font); + compute_kerned_advances(&glyph_attrs, &shaped_glyphs, shaped_total) + }; let mut glyphs = resolve_positions( &glyph_attrs, @@ -215,18 +275,15 @@ pub fn paint(canvas: &Canvas, ctx: &PaintCtx<'_>, root_id: NodeId, node: &DemoNo wm, v_perp_x, &kerned_advances, + &metrics, ); - // 4. Apply text-anchor per anchored chunk (SVG 2 §11.4.4). - let anchor = match read_inherited(ctx, node, "text-anchor") - .as_deref() - .map(str::trim) - { - Some("middle") => TextAnchor::Middle, - Some("end") => TextAnchor::End, - _ => TextAnchor::Start, - }; - apply_anchor(&mut glyphs, anchor, wm); + // 4. Apply text-anchor per anchored chunk (SVG 2 §11.4.4). Each + // chunk's anchor is resolved by walking ancestors of the + // element that started the chunk (the source of the first + // glyph), not the root ``. Lets a `` shift only its own chunk. + apply_anchor(ctx, &mut glyphs, wm); // 4b. Map any `` glyphs onto their referenced path. Runs // after anchor so the chunk's anchor shift is naturally folded @@ -286,6 +343,13 @@ pub(super) struct GlyphAttr { /// through nesting; `baseline-shift: baseline` adds 0 (i.e. equals /// the parent shift). Applied as an extra dy at resolve time. baseline_shift_dy: f32, + /// Per-tspan `alignment-baseline` override (innermost frame on the + /// stack at emit time wins). When `Some`, the glyph's y becomes + /// `pen_y + baseline_offset(metrics, kind) + baseline_shift_dy` + /// instead of using the root's baseline_y. Lets a single tspan + /// inside a `` use a different + /// baseline. SVG 2 §11.10.2 / CSS Inline Layout 3 §3.3. + alignment_baseline_kind: Option, } /// Resolved geometry for one `` element. Built once at @@ -325,6 +389,12 @@ struct ElemAttrs { /// Cumulative `baseline-shift` for this stack frame in SVG y /// direction. Equals `parent.baseline_shift_dy + parse(local)`. baseline_shift_dy: f32, + /// Local `alignment-baseline` on this element only (no ancestor + /// walk — that would conflict with the root's `dominant-baseline` + /// which is already applied uniformly). When set, every glyph + /// emitted at or below this stack frame picks up this kind as + /// its `alignment_baseline_kind` override. + alignment_baseline: Option, } impl ElemAttrs { @@ -332,6 +402,18 @@ impl ElemAttrs { let local_shift = read_local(node, "baseline-shift") .map(|v| parse_baseline_shift(&v, font_size)) .unwrap_or(0.0); + let alignment_baseline = read_local(node, "alignment-baseline").and_then(|v| { + let trimmed = v.trim(); + if trimmed.is_empty() + || trimmed.eq_ignore_ascii_case("auto") + || trimmed.eq_ignore_ascii_case("baseline") + || trimmed.eq_ignore_ascii_case("inherit") + { + None + } else { + Some(resolve_baseline(Some(trimmed))) + } + }); Self { // Coord lists may use `em`/`ex` units; resolve them against // the current font-size rather than CSS initial 16px. @@ -344,6 +426,7 @@ impl ElemAttrs { consumed: 0, is_text_path: false, baseline_shift_dy: parent_shift_dy + local_shift, + alignment_baseline, } } } @@ -408,17 +491,22 @@ fn parse_spacing_length(value: &str, font_size: f32) -> f32 { /// Parse `baseline-shift` to a SVG-y delta (down is positive). /// /// CSS Inline Layout 3 §5: `baseline` (or `0`) means no shift relative -/// to the parent baseline; `super` raises by ~0.5em; `sub` lowers by -/// ~0.5em; a `` resolves against the current font-size and -/// raises by that amount; a `` is taken raw and raises by that -/// amount. Positive values raise; we negate the sign because SVG y -/// grows downward. +/// to the parent baseline; `super`/`sub` use the font's own +/// superscript/subscript metrics (`OS/2.ySuperscriptYOffset` / +/// `ySubscriptYOffset`). Blink approximates with ~0.333em / ~0.2em +/// when those metrics aren't reachable — that matches the observed +/// resvg-test-suite expecteds. Earlier we used 0.5em which over-shot +/// the baseline by a noticeable margin in the diff for `super.svg` / +/// `sub.svg` / `with-rotate.svg` and the nested-super variants. +/// `` resolves against the current font-size and raises +/// by that amount; `` raises raw. SVG y grows downward, so +/// "raise" → negative delta. fn parse_baseline_shift(value: &str, font_size: f32) -> f32 { let v = value.trim(); match v { "" | "0" | "baseline" | "initial" | "auto" | "inherit" | "unset" => 0.0, - "super" => -0.5 * font_size, - "sub" => 0.5 * font_size, + "super" => -font_size / 3.0, + "sub" => font_size / 5.0, _ => { if let Some(pct) = v.strip_suffix('%') { pct.trim() @@ -493,6 +581,7 @@ fn flatten_glyphs( last_was_space: false, has_emitted: false, }; + let root_preserve = xml_space_preserve(root); walk_glyphs( ctx, root_id, @@ -503,13 +592,52 @@ fn flatten_glyphs( &mut paths, &mut out, &mut ws, + root_preserve, ); - while matches!(out.last(), Some(g) if g.ch == ' ') { - out.pop(); + if !root_preserve { + // SVG 1.1 §10.15 default xml:space: strip trailing whitespace + // of the entire content. Skip when root is in `preserve` mode. + while matches!(out.last(), Some(g) if g.ch == ' ') { + out.pop(); + } } (out, paths) } +/// Read the effective `xml:space` on `node`. `xml:space="preserve"` +/// suppresses XML whitespace collapse for this element and its +/// descendants until overridden. We don't walk ancestors here — +/// callers thread the inherited value down through `walk_glyphs` so +/// a child's local `xml:space="default"` can override. +/// +/// `html5ever` stores the xml-namespaced attribute with local name +/// `space` (the prefix `xml:` is stripped into a namespace), so +/// `get_attr` lookup uses the bare local name. +fn xml_space_preserve(node: &DemoNode) -> bool { + matches!(read_xml_space(node), Some("preserve")) +} + +fn xml_space_for(node: &DemoNode, parent_preserve: bool) -> bool { + match read_xml_space(node) { + Some("preserve") => true, + Some("default") => false, + _ => parent_preserve, + } +} + +fn read_xml_space(node: &DemoNode) -> Option<&str> { + let DemoNodeData::Element(data) = &node.data else { + return None; + }; + for attr in &data.attrs { + let local = attr.name.local.as_ref(); + if local == "space" || local == "xml:space" { + return Some(attr.value.as_ref().trim()); + } + } + None +} + /// XML-whitespace collapse state, threaded through the recursive walk /// so collapsing works across `` boundaries: e.g. `Hello /// world` produces a single space between `o` and @@ -532,11 +660,30 @@ fn walk_glyphs( paths: &mut Vec, out: &mut Vec, ws: &mut WhitespaceState, + preserve: bool, ) { for &cid in &node.children { let child = ctx.dom.node(cid); match &child.data { DemoNodeData::Text(s) => { + if preserve { + // SVG 1.1 §10.15 `xml:space="preserve"`: \n/\r/\t + // become space, but spaces are NOT collapsed and + // leading/trailing are NOT stripped. Reset the + // collapse-state cursor so spaces flanking this + // run are kept. + for raw in s.chars() { + let ch = if matches!(raw, '\n' | '\r' | '\t') { + ' ' + } else { + raw + }; + ws.last_was_space = ch == ' '; + ws.has_emitted = true; + emit_glyph(ch, node_id, stack, path_stack, out); + } + continue; + } for raw in s.chars() { let ch = if matches!(raw, '\n' | '\r' | '\t') { ' ' @@ -590,8 +737,18 @@ fn walk_glyphs( } let parent_shift = stack.last().map(|e| e.baseline_shift_dy).unwrap_or(0.0); stack.push(ElemAttrs::from_node(child, font_size, parent_shift)); + let child_preserve = xml_space_for(child, preserve); walk_glyphs( - ctx, cid, child, font_size, stack, path_stack, paths, out, ws, + ctx, + cid, + child, + font_size, + stack, + path_stack, + paths, + out, + ws, + child_preserve, ); stack.pop(); } @@ -611,8 +768,18 @@ fn walk_glyphs( tp_attrs.dy.clear(); tp_attrs.is_text_path = true; stack.push(tp_attrs); + let child_preserve = xml_space_for(child, preserve); walk_glyphs( - ctx, cid, child, font_size, stack, path_stack, paths, out, ws, + ctx, + cid, + child, + font_size, + stack, + path_stack, + paths, + out, + ws, + child_preserve, ); stack.pop(); path_stack.pop(); @@ -638,6 +805,11 @@ fn emit_glyph( path_stack: &[usize], out: &mut Vec, ) { + // Innermost stack frame with a local `alignment-baseline` wins. + // Walk inner→outer until the first non-`None` is found; the root + // doesn't set this (its `dominant-baseline`/`alignment- + // baseline` is already folded into `baseline_y` for all glyphs). + let alignment_baseline_kind = stack.iter().rev().find_map(|e| e.alignment_baseline); let mut attr = GlyphAttr { ch, x: None, @@ -651,6 +823,7 @@ fn emit_glyph( // push time (parent + local). The innermost frame holds the // value to apply to this glyph. baseline_shift_dy: stack.last().map(|e| e.baseline_shift_dy).unwrap_or(0.0), + alignment_baseline_kind, }; for elem in stack.iter().rev() { let i = elem.consumed; @@ -836,6 +1009,7 @@ fn resolve_positions( wm: WritingMode, v_perp_x: f32, kerned_advances: &[f32], + metrics: &skia_safe::FontMetrics, ) -> Vec { let mut pen_x = 0.0f32; let mut pen_y = 0.0f32; @@ -908,6 +1082,13 @@ fn resolve_positions( // baseline, but for a simple MVP we drop the perpendicular // baseline shift in vertical so glyphs stack on the same x. let user_rotate = g.rotate.unwrap_or(0.0); + // Per-glyph baseline override: a tspan with its own + // `alignment-baseline` overrides the root's baseline_y for + // glyphs in that subtree. + let glyph_baseline_y = match g.alignment_baseline_kind { + Some(kind) => baseline_offset(metrics, kind), + None => baseline_y, + }; let (gx, gy, glyph_rotate) = if wm.is_vertical() { // Glyph at (pen_x + perp, pen_y), rotated 90° CW. The // perpendicular offset places the rotated baseline on the @@ -916,7 +1097,11 @@ fn resolve_positions( } else if g.text_path_idx.is_some() { (pen_x, pen_y + g.baseline_shift_dy, user_rotate) } else { - (pen_x, pen_y + baseline_y + g.baseline_shift_dy, user_rotate) + ( + pen_x, + pen_y + glyph_baseline_y + g.baseline_shift_dy, + user_rotate, + ) }; out.push(ResolvedGlyph { ch: g.ch, @@ -1062,32 +1247,39 @@ impl ArcLengthMapper { } } -/// Apply `text-anchor` per anchored chunk. Each chunk's effective width -/// is the sum of its glyph advances; middle subtracts half, end -/// subtracts the whole, start is a no-op. (Blink: `ApplyAnchoring` in -/// `svg_text_layout_algorithm.cc`.) -fn apply_anchor(glyphs: &mut [ResolvedGlyph], anchor: TextAnchor, wm: WritingMode) { - if matches!(anchor, TextAnchor::Start) { - return; - } +/// Apply `text-anchor` per anchored chunk. Each chunk's effective +/// width is the sum of its glyph advances; middle subtracts half, end +/// subtracts the whole, start is a no-op. The anchor is read from the +/// element that started the chunk (the source of the first glyph), +/// walking ancestors via CSS inheritance. Lets a +/// `` shift only the chunk it spawned, +/// not the whole ``. Blink: `ApplyAnchoring` in +/// `svg_text_layout_algorithm.cc`. +fn apply_anchor(ctx: &PaintCtx<'_>, glyphs: &mut [ResolvedGlyph], wm: WritingMode) { let mut i = 0; while i < glyphs.len() { let cid = glyphs[i].chunk_id; + let chunk_source = glyphs[i].source; let mut j = i; let mut width = 0.0; while j < glyphs.len() && glyphs[j].chunk_id == cid { width += glyphs[j].advance; j += 1; } + let anchor = match read_inherited(ctx, ctx.dom.node(chunk_source), "text-anchor") + .as_deref() + .map(str::trim) + { + Some("middle") => TextAnchor::Middle, + Some("end") => TextAnchor::End, + _ => TextAnchor::Start, + }; let shift = match anchor { TextAnchor::Middle => -width / 2.0, TextAnchor::End => -width, TextAnchor::Start => 0.0, }; if shift != 0.0 { - // In vertical writing-mode, the inline axis is Y, so the - // anchor shift moves glyphs along Y. Per Blink - // `ApplyAnchoring` (svg_text_layout_algorithm.cc:541-597). for g in &mut glyphs[i..j] { if wm.is_vertical() { g.y += shift; @@ -1378,13 +1570,14 @@ fn paint_anchor_decoration( // Default ~30% of font size above the baseline. -font.size() * 0.3 }); - // Overline sits on the cap height — for fonts that don't expose - // `cap_height` we fall back to the ascent. - let overline_position = if metrics.cap_height > 0.0 { - -metrics.cap_height - } else { - metrics.ascent - }; + // Overline sits at the top of the em box (font ascent), with + // ~one underline-thickness of air above the cap. CSS Text + // Decoration 3 §3.2.4 leaves position implementation-defined; + // Blink/Chromium use the line-box top (approximately + // `metrics.ascent`), and the resvg-test-suite expecteds match + // that — placing the bar visually above the cap, not touching + // it. `ascent` is negative (Skia y-up). + let overline_position = metrics.ascent; // Build coalesced runs: groups of consecutive glyph indices // (after sorting and dedup), broken when a glyph's `chunk_id` @@ -1886,12 +2079,54 @@ fn read_local(node: &DemoNode, name: &str) -> Option { if let Some(v) = get_attr(node, name) { return Some(v.to_string()); } - if let Some(style) = get_attr(node, "style") { - for decl in style.split(';') { - if let Some((k, v)) = decl.split_once(':') { - if k.trim().eq_ignore_ascii_case(name) { - return Some(v.trim().to_string()); - } + read_local_style_only(node, name) +} + +/// Like [`read_local`] but ignores the bare attribute — only reads +/// from `style="…"`. Used for non-presentation CSS properties like +/// `font-kerning` that SVG 2 specifies as not-an-attribute. +fn read_local_style_only(node: &DemoNode, name: &str) -> Option { + let style = get_attr(node, "style")?; + for decl in style.split(';') { + if let Some((k, v)) = decl.split_once(':') { + if k.trim().eq_ignore_ascii_case(name) { + return Some(v.trim().to_string()); + } + } + } + None +} + +/// Walk the ancestor chain reading a non-presentation CSS property +/// from `style="…"` only. Mirrors [`read_inherited`]'s `inherit` +/// keyword handling but skips the bare-attribute hop. +fn inherited_style_only(ctx: &PaintCtx<'_>, node: &DemoNode, name: &str) -> Option { + fn is_skip(v: &str) -> bool { + matches!( + v.trim().to_ascii_lowercase().as_str(), + "inherit" | "initial" | "unset" | "revert" + ) + } + if let Some(v) = read_local_style_only(node, name) { + if !is_skip(&v) { + return Some(v); + } + } + let mut current = node.parent; + while let Some(id) = current { + let n = ctx.dom.node(id); + if let Some(v) = read_local_style_only(n, name) { + if !is_skip(&v) { + return Some(v); + } + } + current = n.parent; + } + for use_id in super::scoped_svg_paint_state::use_chain_iter(ctx.use_chain) { + let n = ctx.dom.node(use_id); + if let Some(v) = read_local_style_only(n, name) { + if !is_skip(&v) { + return Some(v); } } } From 04914908f4f704b47ec4633a64ef601e31d0d1ca Mon Sep 17 00:00:00 2001 From: Universe Date: Wed, 29 Apr 2026 18:50:48 +0900 Subject: [PATCH 30/59] fix(htmlcss::svg): per-tspan group opacity via save_layer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SVG 2 §6.13: `opacity` is a presentation attribute that wraps the rendered output. Previously a `` was silently ignored — the container painter dispatches `TSpan => {}` and the text painter applied no per-element layer. Implementation: in `paint_glyph_groups`, walk the source's ancestor chain up to (but not including) the root ``, multiplying each ancestor's `opacity`. When the cumulative is < 1, open a save_layer with that alpha around the run, paint as before, restore. Multiplicative ancestor walk matches SVG layer composition: a tspan at 0.5 inside a `` ends up at 0.25. The text root's opacity is consumed by `paint_root_node`, so we stop the walk there — `` doesn't double-apply. text_tspan_with-opacity: 0.876 → 0.942 (held below 0.95 floor by font-metric drift; the change is spec-correct). Pass-rate held at 1123 (0.7881). Mask / clip-path / filter on tspan are still no-ops; they require larger plumbing (computing the run's bounding box for filter region, lighting the chained clipPath chain through text geometry) — separate scope. --- .../htmlcss/svg/paint/svg_text_painter/mod.rs | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/crates/grida/src/htmlcss/svg/paint/svg_text_painter/mod.rs b/crates/grida/src/htmlcss/svg/paint/svg_text_painter/mod.rs index 081a85508..17db5cb7b 100644 --- a/crates/grida/src/htmlcss/svg/paint/svg_text_painter/mod.rs +++ b/crates/grida/src/htmlcss/svg/paint/svg_text_painter/mod.rs @@ -1313,6 +1313,23 @@ fn paint_glyph_groups(canvas: &Canvas, ctx: &PaintCtx<'_>, glyphs: &[ResolvedGly let run = &glyphs[i..j]; let node = ctx.dom.node(source); + // Per-tspan group `opacity`: applied to the composited run as + // a save_layer (not multiplied into per-glyph fill alpha, + // which would over-darken at kerning overlaps). SVG 2 §6.13: + // `opacity` is a presentation attribute on `` / + // `` / `` that wraps the rendered output. We walk + // ancestors source→`` so a `` + // inside a `` ends up at 0.25. + let group_alpha = ancestor_group_opacity(ctx, node); + let group_layer_opened = if group_alpha < 1.0 { + let mut p = SkPaint::default(); + p.set_alpha_f(group_alpha.max(0.0)); + canvas.save_layer(&skia_safe::canvas::SaveLayerRec::default().paint(&p)); + true + } else { + false + }; + // Fill — default is black (SVG 2 §11.3) when no inherited // value resolves. let fill_color = match read_inherited(ctx, node, "fill").as_deref().map(str::trim) { @@ -1364,10 +1381,39 @@ fn paint_glyph_groups(canvas: &Canvas, ctx: &PaintCtx<'_>, glyphs: &[ResolvedGly } } + if group_layer_opened { + canvas.restore(); + } i = j; } } +/// Walk from `node` up to (but not including) the root ``, +/// multiplying each ancestor's `opacity`. Matches the way SVG layers +/// compose — a tspan with `opacity=0.5` inside a `` +/// composites at 0.25. The text root's own opacity is consumed by +/// `paint_root_node` already, so we stop at the ``'s parent +/// (which is the typical traversal scope here). +fn ancestor_group_opacity(ctx: &PaintCtx<'_>, node: &DemoNode) -> f32 { + let mut acc = super::effects::group_opacity(node); + let mut current = node.parent; + while let Some(id) = current { + let n = ctx.dom.node(id); + if let DemoNodeData::Element(d) = &n.data { + let kind = ElementKind::from_local_name(d.name.local.as_ref()); + if kind == ElementKind::Text { + // Root opacity is applied by the container + // painter when it dispatched into us; don't double- + // apply. + break; + } + } + acc *= super::effects::group_opacity(n); + current = n.parent; + } + acc +} + /// Bitset of which line decorations a `text-decoration` declaration /// requested. SVG 1.1 / CSS Text Decoration 3 take the same keywords. #[derive(Debug, Clone, Copy, Default)] From 1a82a2d2647ed5cd12a119186c253749e19b005d Mon Sep 17 00:00:00 2001 From: Universe Date: Thu, 30 Apr 2026 03:22:18 +0900 Subject: [PATCH 31/59] fix(htmlcss::svg): marker shorthand cascade and end-of-path angle backpatch - treat the bare 'marker' shorthand as CSS-only - route marker resolution through the CSS cascade - backpatch trailing marker angle at subpath boundary - backpatch marker-end angle at end of path --- .../htmlcss/svg/paint/svg_marker_painter.rs | 72 +++++++++++++++---- 1 file changed, 59 insertions(+), 13 deletions(-) diff --git a/crates/grida/src/htmlcss/svg/paint/svg_marker_painter.rs b/crates/grida/src/htmlcss/svg/paint/svg_marker_painter.rs index c55c8e887..6992b5373 100644 --- a/crates/grida/src/htmlcss/svg/paint/svg_marker_painter.rs +++ b/crates/grida/src/htmlcss/svg/paint/svg_marker_painter.rs @@ -70,6 +70,18 @@ fn collect_positions(path: &Path) -> Vec { match verb { Verb::Move => { let p = pts[0]; + // If there's a pending Mid marker from the previous + // subpath's last drawn segment, backpatch its angle + // using only that segment's outgoing tangent — there + // is no continuing draw across a Move boundary, so the + // bisector rule that `backpatch_prev_angle` applies + // for inline vertices doesn't apply here. Without + // this, the trailing marker of subpath N gets + // `angle = 0` (uninitialized) when subpath N+1 is + // started by a bare Move. + if let (Some(prev_out), Some(prev)) = (prev_out_slope, out.last_mut()) { + prev.angle = slope_to_angle(prev_out); + } // First emit a position for the moveto. Angle is // backpatched when we see the first segment of this // subpath (its incoming `out_slope` becomes our @@ -220,6 +232,15 @@ fn collect_positions(path: &Path) -> Vec { Verb::Done => break, } } + // End-of-path: the trailing marker (about to become marker-end) + // has no next-segment incoming slope to bisect against, so use + // the last drawn segment's outgoing tangent only. Mirrors the + // Move-boundary patch above. Without this, marker-end on a path + // ending in a curve renders horizontally instead of along the + // curve's exit tangent. + if let (Some(prev_out), Some(last)) = (prev_out_slope, out.last_mut()) { + last.angle = slope_to_angle(prev_out); + } if let Some(first) = out.first_mut() { first.kind = MarkerKind::Start; } @@ -298,18 +319,40 @@ fn bisecting_angle(in_s: (f32, f32), out_s: (f32, f32)) -> f32 { /// the `marker` shorthand). SVG 2 §11.6.1 marks all four as /// inherited; we walk ancestors looking for the first explicit value /// that isn't `none`. +/// +/// The bare `marker` shorthand is a CSS-only property — SVG 2 does +/// NOT list it as a presentation attribute. The resvg-test-suite's +/// `the-marker-property.svg` documents this ("Should be ignored") +/// by setting `` and expecting no markers. +/// `marker-start` / `marker-mid` / `marker-end` ARE presentation +/// attributes, so the bare-attribute path is fine for those. fn read_marker_url(ctx: &PaintCtx<'_>, node: &DemoNode, prop: &str) -> Option { - fn read(node: &DemoNode, name: &str) -> Option { - if let Some(v) = get_attr(node, name) { - let v = v.trim(); - if !v.is_empty() { - return Some(v.to_string()); - } + use crate::htmlcss::svg::style::cascade::cascade_property; + let dom = Some(ctx.dom); + let sheet = Some(&ctx.resources.stylesheet); + // Per-property: marker-start/mid/end ARE presentation attributes, + // so cascade reads attribute, inline style, and `, so puppeteer's + // `addStyleTag` fails. Inject a