Skip to content

Sharpen template images with DPR-aware srcset and quality knob#18972

Open
jeffmerrick wants to merge 3 commits into
masterfrom
jeffmerrick/issue-18932
Open

Sharpen template images with DPR-aware srcset and quality knob#18972
jeffmerrick wants to merge 3 commits into
masterfrom
jeffmerrick/issue-18932

Conversation

@jeffmerrick
Copy link
Copy Markdown
Contributor

Proposed changes

Updates fingerprinted-img.html to cap the 1x src at maxWidth while extending srcset entries up to 2x maxWidth so high-DPI displays get sharp resources, and adds a quality parameter (default q90) for slots where lossy WebP artifacts are visible. Sets explicit maxWidth/quality on the hero and section-header partials and the superintelligence-infrastructure hero so each image fills its actual container width instead of defaulting to 800px.

Related issues (optional)

Fixes #18932

Cap the 1x src at maxWidth but extend srcset up to 2x maxWidth so
high-DPI displays get sharp images, add a `quality` parameter (default
q90, q95 for screenshot-heavy slots), and set explicit maxWidth/quality
on the hero, section-header, and superintelligence-infrastructure
callsites so they fill their actual container widths.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 13, 2026

Review

Targeted, well-scoped fix for the blurry-image regression from #18762. The DPR-aware srcset + quality knob is the right shape of change, defaults preserve existing behavior, and the per-call sizes look sensible for where the partial actually renders. A few notes — none are blockers.

Observations

layouts/partials/fingerprinted-img.html:35 — The fallback that appends the original width only fires when origImg.Width < $maxSrcsetWidth (strict). When the original is exactly 2 * maxWidth, neither the candidate loop (no 3000 in the candidate list, for example) nor the fallback emits it. For maxWidth: 1500 with a 3000w source, the largest srcset entry would be 2400w, so a 2x device only gets ~1.6x resolution. Probably fine in practice given typical asset sizes, but worth knowing. If you want belt-and-suspenders:

    {{/* If the original is at-or-below 2x maxWidth, include the original width too so the
         sharpest available resource is offered (the largest candidate may fall short of the
         original width — e.g. a 1472w source with a 1200 candidate). Skip when the original
         width exactly matches a candidate, otherwise the loop above already emitted it. */}}
    {{ if and (le $origImg.Width $maxSrcsetWidth) (not (in $candidates $origImg.Width)) }}
        {{ $resized := $origImg.Process $processArgs | fingerprint }}
        {{ $srcset = $srcset | append (printf \"%s %dw\" $resized.RelPermalink $origImg.Width) }}
    {{ end }}

layouts/partials/template-partials/template-hero.html:273 — The comment correctly notes that image_max_width (a CSS cap, lines 20/69/71) may shrink the rendered slot below 1500px, but the partial now always emits a 1500px-wide src (and sizes=...1500px). When a caller sets image_max_width: "60rem" (~960px), the browser will download a 1500w source and downscale. That's a small inefficiency, not a correctness bug — flagging in case you want to thread image_max_width through to maxWidth.

layouts/partials/fingerprinted-img.html:19 — The "Skip srcset for small images" comment was kept from the prior code but the trailing rationale (400px srcset covers 2x) reads oddly now that no srcset is emitted in that branch. Consider trimming to just: Skip srcset for very small images (maxWidth < 400) — src already covers 1x and 2x.

Behavior change for default callers — Callers that use the partial without passing maxWidth (the 800px default) will now get srcset entries up to 1600w instead of 800w. That's the intended fix, just noting it affects ~80 callsites of this partial; worth a quick spot-check that none of them are in tight CSS slots where a much-larger source would be wasteful. The 2x DPR ceiling makes this self-limiting in most cases.

Looks good

  • quality parameter with 90 default cleanly preserves prior behavior; the q95 bumps on screenshots/text-heavy slots are well-justified
  • Per-call maxWidth values match the actual container widths called out in comments (max-w-4xl ≈ 896 → cap 1000; page container ≈ 1472 → cap 1500; max-w-5xl ≈ 1024 on the superintelligence hero)
  • The in $candidates $origImg.Width dedupe correctly avoids double-emitting when the source width happens to match a candidate

Mention @claude if you'd like another pass or want me to take a stab at the image_max_width plumbing.

Switch the original-width fallback from `lt` to `le` so a source that's
exactly 2x maxWidth still ends up in the srcset (the candidate list
tops out at 3200, so e.g. a 3000w source with maxWidth=1500 would
otherwise cap at 2400w). The candidate-dedupe check prevents the
loosened bound from producing duplicate entries.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@pulumi-bot
Copy link
Copy Markdown
Collaborator

pulumi-bot commented May 13, 2026

@pulumi-bot
Copy link
Copy Markdown
Collaborator

pulumi-bot commented May 13, 2026

Lighthouse Performance Report

Commit: 66d188a | Metric definitions

Page Device Score FCP LCP TBT CLS SI
Homepage Mobile 🟡 66 2.9s 2.9s 715ms 0.019 6.6s
Homepage Desktop 🟢 95 0.8s 1.3s 32ms 0.004 1.3s
Install Pulumi Mobile 🔴 32 5.2s 7.8s 314ms 0.438 7.6s
Install Pulumi Desktop 🟡 80 1.2s 2.1s 22ms 0.011 2.9s
AWS Get Started Mobile 🟡 55 5.0s 8.1s 314ms 0.066 5.0s
AWS Get Started Desktop 🟡 85 1.2s 1.6s 0ms 0.024 2.8s

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.

Fix blurry images on product pages

2 participants