feat(react-grab): render overlay canvas in HDR on capable displays#481
Open
aidenybai wants to merge 5 commits into
Open
feat(react-grab): render overlay canvas in HDR on capable displays#481aidenybai wants to merge 5 commits into
aidenybai wants to merge 5 commits into
Conversation
Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
commit: |
- Gate the P3 brightness boost behind an opt-in extendedRange flag so only the float16 canvas layers get past-SDR-white channels; the frozen-glow box-shadow (a DOM consumer that gamut-clamps) stays SDR-correct on HDR displays. - Require configureHighDynamicRange in the HDR probe so supportsHdr() fully reflects the canvas capability the color boost assumes. - Drop the redundant inner try/catch and round P3 channel output. Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
A persistent full-viewport canvas in extended HDR mode forces the OS compositor to reserve HDR headroom, which dims all surrounding SDR page content while the subtle highlight accent isn't bright enough to read as HDR. This is inherent compositor behavior for full-screen HDR surfaces, not a fixable bug, so revert to the SDR (wide-gamut P3) overlay. Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
The previous attempt dimmed the whole page: an extended-range canvas with no headroom limit makes the compositor reserve the display's full HDR headroom, darkening surrounding SDR content while the subtle accent barely exceeds the lowered reference white. Cap the headroom with CSS `dynamic-range-limit: constrained` (the spec value for SDR+HDR coexistence) so the highlight glows brighter-than-white without dimming the rest of the page. Gate HDR on `dynamic-range-limit` support so browsers that can enter extended mode but cannot constrain it fall back to SDR instead of regressing to page-wide dimming. The boost stays opt-in (extendedRange) so plain-CSS consumers like the frozen-glow box-shadow remain SDR-correct. Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
…ge dims Switch from constrained to no-limit headroom: the highlight now glows far brighter than SDR white (4x) and the compositor dims the surrounding SDR page, which is the intended dramatic HDR effect. no-limit also overrides any inherited dynamic-range-limit from the host page. Drop the dynamic-range-limit support gate from detection since we no longer constrain. Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 42d8c73. Configure here.
| // the intended dramatic effect. `no-limit` also overrides any inherited | ||
| // dynamic-range-limit from the host page. | ||
| canvasRef.configureHighDynamicRange?.({ mode: "extended" }); | ||
| canvasRef.style.setProperty("dynamic-range-limit", "no-limit"); |
There was a problem hiding this comment.
Unconstrained HDR dims entire page
High Severity
The PR describes avoiding page-wide SDR dimming by gating HDR on CSS.supports("dynamic-range-limit", "constrained") and setting dynamic-range-limit: constrained on the overlay canvas. The change turns on extended HDR with no-limit and never checks constrained support, matching the naive HDR path the PR says regresses UX by dimming the whole app while the accent barely reads as HDR.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 42d8c73. Configure here.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/react-grab/src/utils/overlay-color.ts">
<violation number="1" location="packages/react-grab/src/utils/overlay-color.ts:15">
P1: HDR_BRIGHTNESS_MULTIPLIER=4 + dynamic-range-limit:no-limit produces page-dimming blowout, contradicting the PR's constraint-based design.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| const SRGB_COMPONENTS = "210, 57, 192"; | ||
| const P3_COMPONENTS = "0.84 0.19 0.78"; | ||
| const P3_BASE_COMPONENTS = [0.84, 0.19, 0.78] as const; | ||
| const HDR_BRIGHTNESS_MULTIPLIER = 4; |
Contributor
There was a problem hiding this comment.
P1: HDR_BRIGHTNESS_MULTIPLIER=4 + dynamic-range-limit:no-limit produces page-dimming blowout, contradicting the PR's constraint-based design.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/react-grab/src/utils/overlay-color.ts, line 15:
<comment>HDR_BRIGHTNESS_MULTIPLIER=4 + dynamic-range-limit:no-limit produces page-dimming blowout, contradicting the PR's constraint-based design.</comment>
<file context>
@@ -4,15 +4,15 @@ import { supportsHdr } from "./supports-hdr.js";
const SRGB_COMPONENTS = "210, 57, 192";
const P3_BASE_COMPONENTS = [0.84, 0.19, 0.78] as const;
-const HDR_BRIGHTNESS_MULTIPLIER = 2;
+const HDR_BRIGHTNESS_MULTIPLIER = 4;
export const overlayColor = (alpha: number, extendedRange = false): string => {
</file context>
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
2 participants
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.
What
Renders the React Grab overlay highlights (selection, drag, grabbed, labels) in HDR on capable displays/browsers, so the pink accent glows brighter-than-SDR-white — without dimming the rest of the page.
The problem with naive HDR canvas
A first attempt put the full-screen overlay canvas into extended-range HDR mode. On HDR displays this dimmed the entire app: an extended canvas with no headroom limit makes the OS compositor reserve the display's full HDR headroom, which it does by mapping SDR reference white down — so all SDR content darkens while a subtle accent barely exceeds the lowered white. Net result: everything dims and the highlight doesn't even read as HDR.
The fix: constrain the headroom
Per the W3C CSS Color HDR spec,
dynamic-range-limitapplies to canvas elements, and for an"extended"canvas it caps the HDR headroom the browser requests. Settingdynamic-range-limit: constrained— the spec value meant for "a mix of SDR and HDR content [that] can be comfortably viewed together" — lets the highlight exceed reference white while the compositor reserves only a sliver of headroom, so surrounding SDR content stays put. Shipped in Chromium 136+.How
utils/supports-hdr.ts(new): cached check that enables HDR only when all of these hold —(dynamic-range: high)matches, a probe 2D canvas backsfloat16,configureHighDynamicRangeexists, andCSS.supports("dynamic-range-limit", "constrained"). That last gate is the safety net: a browser that can enter extended mode but can't constrain it falls back to SDR instead of regressing to page-wide dimming. Also carries the ambient type augmentations for the experimental HDR canvas API (not in the TS DOM lib yet), in a real module so they reach every importer.components/overlay-canvas.tsx: when HDR is supported, the main canvas and offscreen layers use{ colorType: "float16" }; the visible canvas callsconfigureHighDynamicRange({ mode: "extended" })and setsdynamic-range-limit: constrained. Falls back to the existingunorm8/ SDR path otherwise.utils/overlay-color.ts:overlayColor(alpha, extendedRange)scales the Display P3 channels past SDR white (2×, clamped down by the constrained headroom) only whenextendedRangeis set — opt-in for the four canvas highlight colors. Plain-CSS consumers like the frozen-glowbox-shadowstay SDR-correct (extended-range channels in a normal CSS context would gamut-clamp into a distorted hue).Notes
dynamic-range-limitgate ensures we never trade page-wide dimming for a brighter accent.Verification
pnpm build✅pnpm typecheck✅pnpm lint✅pnpm format✅Summary by cubic
Render the overlay canvas in HDR in
react-grabon capable browsers, using full headroom (dynamic-range-limit: no-limit) so the pink highlight blows out and the SDR page intentionally dims.HDR is gated on
(dynamic-range: high), float16 2D +configureHighDynamicRange; the 4× P3 brightness boost only applies to the HDR canvas layers, while CSS effects and unsupported browsers stay SDR.Written for commit 42d8c73. Summary will update on new commits.