refactor(extensions): order PathStyle shader coordinates - #10644
Open
chrisgervang wants to merge 3 commits into
Open
refactor(extensions): order PathStyle shader coordinates#10644chrisgervang wants to merge 3 commits into
chrisgervang wants to merge 3 commits into
Conversation
Greptile SummaryRefactors PathStyleExtension’s PathLayer shader assembly into a single ordered coordinate pipeline without intending to alter rendering.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| 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]
Reviews (3): Last reviewed commit: "Merge master into PathStyle shader clean..." | Re-trigger Greptile
This was referenced Sep 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
vs:#main-endandfs:#main-endinjections.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.tsfrom 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
7f1e9e6301and805b7efcb5; later merge-only synchronization does not change the feature trees.7f1e9e6301)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 buildyarn lintyarn 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 passedRENDER_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 skipsgit diff --exit-code 7f1e9e6301 -- test/render/golden-imagesStacked follow-ups
dashMode: 'path'.Further opportunities
These higher-risk ideas remain separate so the landed visual baselines continue to protect them:
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
pathStylePipelineShadersstage 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, recoversstrokeHalfWidthfrom widenedDECKGL_FILTER_SIZE, then computes dash arrays and phase once (dropping the old secondrestoredDashPeriodpass). Fragmentfs:#main-endhandling moves here too: deferred dash discard/coverage after PathLayer derivatives, then the non-AA offset hard clip.dashShadersandoffsetShaderskeep declarations and dash fragment setup only; their competingvs:#main-end/fs:#main-endhooks are removed.path.spec.tsadds 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.