Skip to content

refactor(extensions): order PathStyle shader coordinates - #10644

Open
chrisgervang wants to merge 3 commits into
masterfrom
codex/path-style-shader-cleanup
Open

refactor(extensions): order PathStyle shader coordinates#10644
chrisgervang wants to merge 3 commits into
masterfrom
codex/path-style-shader-cleanup

Conversation

@chrisgervang

@chrisgervang chrisgervang commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Goal

Make the PathStyleExtension shader easier to maintain without changing how paths render.

Offset paths need a widened mesh so the shifted stroke still fits. Before this change, dash values were calculated from that widened mesh and then repaired afterward: several coordinates were rescaled, the path-length phase was rescaled, and the justified period was calculated a second time. Adding a new dash-space value meant remembering to repair it in the later offset block.

This was identified as follow-up work in the earlier shader-order review.

Stack (merge bottom-up): #10644#10646#10647#10648#10649

Changes

  • Introduce one explicitly ordered PathStyle pipeline for PathLayer:
    1. widen the mesh for the offset stroke, as before;
    2. map PathLayer coordinates back to the pre-offset stroke;
    3. recover the pre-offset half-width;
    4. convert dash units and calculate dash phase once.
  • Keep dash- and offset-specific declarations/setup in their capability shaders, while removing their competing vs:#main-end and fs:#main-end injections.
  • Centralize deferred fragment finalization so dash rejection still happens after PathLayer evaluates derivatives, followed by the non-AA offset boundary.
  • Add preprocessing assertions for offset-only, dash-only, combined path-dash/offset, antialiased, and non-antialiased variants.

There are no public API, uniform, attribute, core PathLayer, WGSL, or golden-image changes.

Render equivalence

A strict same-machine A/B rendered all 43 cases in path-dash.spec.ts from the branch point, v9.4.0-beta.3 (7f1e9e6301), and this change. Every captured image was byte-for-byte identical. The matrix includes segment/path modes, all dash units, justification, positive/negative/large offsets, flat and billboard paths, 3D/elevated paths, rounded caps, analytic AA, subpixel patterns, and camera clipping.

No checked-in golden image changed.

Performance and resource impact

This refactor does not add vertex attributes, interstage varyings, uniforms, buffers, bindings, draw calls, or per-instance storage. It only changes how the existing PathStyle shader stages are assembled. Segment dash plus offset gains one vertex division to recover the original stroke width; path mode removes the old second period/phase repair. The hardware measurement below covers both sides of that tradeoff.

Apple M1 Max, Chrome/ANGLE Metal, WebGL2 timestamp queries, 3840x2160. The table is the median of four alternating parent/head rounds with 30 samples per variant and workload in each round. It compares the direct feature snapshots 7f1e9e6301 and 805b7efcb5; later merge-only synchronization does not change the feature trees.

100K-path workload baseline (7f1e9e6301) cleanup delta
segment dash 2.8258 ms 2.8254 ms -0.05%
path dash, widths 2.9211 ms 2.9203 ms -0.04%
offset only 2.5665 ms 2.5667 ms +0.03%
all controls 2.9275 ms 2.8990 ms -0.97%

The fragment-heavy workload was also noise-scale. These results show no measurable regression. The repeatable all-controls trend is consistent with removing duplicate path-period work, but this sample is intentionally not presented as a general speedup claim.

Validation

  • yarn build
  • yarn lint
  • yarn vitest run --project headless test/modules/extensions/path.spec.ts test/modules/layers/path-antialiasing.spec.ts test/modules/extensions/clip.spec.ts — 32/32 passed
  • RENDER_TEST_DEVICE=webgl yarn vitest run --project render test/render/test-cases/path-dash.spec.ts test/render/test-cases/path-layer.spec.ts test/render/test-cases/polygon-layer.spec.ts — 61 passed, four intentional backend-selection skips
  • pre-commit affected tests — 19/19 passed
  • git diff --exit-code 7f1e9e6301 -- test/render/golden-images

Stacked follow-ups

Further opportunities

These higher-risk ideas remain separate so the landed visual baselines continue to protect them:

  • encapsulate normalized PathTessellator traversal behind a typed reader and pure metric accumulator;
  • align the vertex and fragment justified-period helpers for extremely small periods.

Note

Medium Risk
Refactors PathLayer GLSL assembly for dash and offset (coordinate spaces and fragment discard order), but scope is internal shader plumbing with no public API change and tests aimed at render equivalence.

Overview
PathStyleExtension for PathLayer now merges a shared pathStylePipelineShaders stage before dash- and offset-specific injections, so offset remapping and dash unit conversion run in a fixed order instead of depending on merge order and post-hoc “repair” of widened geometry.

The pipeline’s vertex stage maps vPathPosition / path length back to the pre-offset stroke, recovers strokeHalfWidth from widened DECKGL_FILTER_SIZE, then computes dash arrays and phase once (dropping the old second restoredDashPeriod pass). Fragment fs:#main-end handling moves here too: deferred dash discard/coverage after PathLayer derivatives, then the non-AA offset hard clip. dashShaders and offsetShaders keep declarations and dash fragment setup only; their competing vs:#main-end / fs:#main-end hooks are removed.

path.spec.ts adds preprocessing checks for offset-only, dash-only, combined, and AA vs non-AA fragment ordering.

Reviewed by Cursor Bugbot for commit 2b3766b. Bugbot is set up for automated code reviews on this repo. Configure here.

@coveralls

coveralls commented Sep 3, 2026

Copy link
Copy Markdown

Coverage Status

coverage: 82.484% (+0.01%) from 82.473% — codex/path-style-shader-cleanup into master

@chrisgervang
chrisgervang marked this pull request as ready for review September 3, 2026 04:55
@chrisgervang chrisgervang added this to the v9.4 milestone Sep 3, 2026
@greptile-apps

greptile-apps Bot commented Sep 3, 2026

Copy link
Copy Markdown

Greptile Summary

Refactors PathStyleExtension’s PathLayer shader assembly into a single ordered coordinate pipeline without intending to alter rendering.

  • Remaps offset-widened geometry before deriving dash coordinates and phase.
  • Centralizes deferred dash and offset fragment finalization.
  • Adds preprocessing assertions for offset-only, dash-only, combined, antialiased, and non-antialiased variants.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
modules/extensions/src/path-style/path-style-extension.ts Registers the shared PathStyle pipeline before merging dash- and offset-specific declarations and defines.
modules/extensions/src/path-style/shaders.glsl.ts Consolidates offset remapping, dash-coordinate conversion, and deferred fragment finalization into one ordered shader pipeline.
test/modules/extensions/path.spec.ts Adds preprocessing checks that verify stage ordering and feature-specific shader variants.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[PathLayer computes stroke width] --> B[Offset capability widens mesh]
  B --> C[Shared pipeline remaps path coordinates]
  C --> D[Recover pre-offset half-width]
  D --> E[Convert dash units and phase]
  E --> F[PathLayer fragment derivative evaluation]
  F --> G[Deferred dash rejection and coverage]
  G --> H[Non-AA offset boundary clip]
Loading

Reviews (3): Last reviewed commit: "Merge master into PathStyle shader clean..." | Re-trigger Greptile

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants