Skip to content

fix(svg): preserve dimension contract under sw/sh and pixelRatio < 1#44

Merged
chiefcll merged 1 commit into
mainfrom
svg-dimension-regression
May 26, 2026
Merged

fix(svg): preserve dimension contract under sw/sh and pixelRatio < 1#44
chiefcll merged 1 commit into
mainfrom
svg-dimension-regression

Conversation

@chiefcll
Copy link
Copy Markdown
Contributor

Summary

#41 introduced two unintended changes to SVG texture dimensions that broke the existing texture-svg VRT (Loaded Event Test: Fail). Both narrow tweaks in loadSvg.

Regressions

1. Sub-1 stage.pixelRatio downscales the raster.
The examples app sets logicalPixelRatio = resolution / appHeight = 720 / 1080 = 0.667 (examples/index.ts:78). #41's Math.ceil(targetW * pixelRatio) multiplied by that, so a 181-wide SVG rasterized to 121 pixels — lower resolution than before, the opposite of the HiDPI sharpness goal. Tests 1, 2, 3 hit this.

2. sw/sh no longer drive texture size when w/h are unset.
Pre-fix, getImageData(sx, sy, sw, sh) returned an sw × sh ImageData regardless of canvas size, so a node that set only srcWidth/srcHeight got a texture sized to the source crop. #41 keyed targetW/H off width || naturalWidth, ignoring sw, so the texture became natural-size with the crop stretched into it. Test 4 (rocko2 partial: expects 81×218, was getting 181×218) hit this.

Fix

Two small adjustments to textureSvg.ts:57-59:

const targetW = width || sw || img.naturalWidth || img.width;
const targetH = height || sh || img.naturalHeight || img.height;
const ratio = pixelRatio > 1 ? pixelRatio : 1;
  • sw/sh fall through when w/h are null, restoring the "crop dims drive texture dims" contract.
  • pixelRatio > 1 ? pixelRatio : 1 clamps so sub-1 stages never downscale. HiDPI upscaling (the original intent) still applies.

The fallback to naturalWidth || width is intentional — naturalWidth is preferred (intrinsic SVG dims), with .width as the back-stop for browsers that return 0 for SVGs without explicit width attributes.

Verification

Captured a fresh chromium-ci texture-svg-1.png via the Docker runner. All five existing assertions pass:

Test Pre-this-PR This PR Expected
1. rocko (no w/h) 121×146 ✗ 181×218 ✓ 181×218
2. elevator (no w/h) 134×179 ✗ 200×268 ✓ 200×268
3. lightning (w=125, h=25) 84×17 ✗ 125×25 ✓ 125×25
4. rocko2 (srcW=81, no w/h) 121×146 ✗ 81×218 ✓ 81×218
5. 404 failure event Pass ✓ Pass ✓ (no dims)

loaded / failed events were never broken — they fire and carry a dimensions payload as before. This PR just makes that payload match the pre-#41 contract.

Test plan

  • pnpm build clean
  • pnpm test — 193/193 pass
  • Local + Docker VRT for texture-svg — all 5 assertions pass
  • Refreshed visual-regression/certified-snapshots/chromium-ci/texture-svg-1.png

🤖 Generated with Claude Code

…xelRatio < 1

PR #41 introduced two unintended dimension changes for SVG textures:

1. Multiplying targetW/H by stage.pixelRatio meant any app rendering at
   sub-1 logical pixel ratio (e.g. the examples app at 720p/1080p =
   0.667) downscaled SVG rasters below the requested size — making
   textures *less* sharp than pre-fix, the opposite of the intent.

2. A node that set only srcWidth/srcHeight (no w/h) used to produce a
   texture sized to the source crop, because the old getImageData(sx,
   sy, sw, sh) returned an sw×sh ImageData regardless of canvas size.
   The new code keyed targetW/H off width || naturalWidth, ignoring sw,
   so the texture became natural-size with a stretched crop drawn into
   it. This broke texture-svg.ts test #4 (rocko2 partial crop, expects
   81x218; was getting 181x218).

Fix:
  - Clamp the DPR multiplier to >= 1 so a sub-1 stage pixelRatio never
    downscales. HiDPI upscaling (the original goal) still applies.
  - Fall through to sw/sh when w/h aren't set, restoring the "crop dims
    drive texture dims" contract.

Refreshed the chromium-ci texture-svg-1.png snapshot — all 5 dimension
+ event assertions now pass (181x218, 200x268, 125x25, 81x218, failure).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@chiefcll chiefcll merged commit a64f053 into main May 26, 2026
1 check failed
@chiefcll chiefcll deleted the svg-dimension-regression branch May 26, 2026 21:43
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.

1 participant