htmlcss: promote paint-opacity + box-padding to L0.exact#685
Merged
softmarshmallow merged 3 commits intohtmlcssfrom Apr 22, 2026
Merged
htmlcss: promote paint-opacity + box-padding to L0.exact#685softmarshmallow merged 3 commits intohtmlcssfrom
softmarshmallow merged 3 commits intohtmlcssfrom
Conversation
Two Chromium-parity fixes for the cg htmlcss renderer, both driven to
byte-exact via the dev-cg-htmlcss-feature loop.
- paint-opacity: replace `set_alpha((opacity * 255.0) as u8)` with
`set_alpha_f(opacity)` in htmlcss/paint.rs so opacity compositing is
float-native, matching Blink's `SkCanvas::saveLayerAlphaf` path.
The u8 truncation diverged by 1 per channel at non-255-aligned
opacity values (0.5 → 127 vs Chromium's 126; 0.25 → 63 vs 64).
paint-opacity fixture goes from 0.9600 → 1.0000 similarity.
- box-padding: fixture-hygiene only (no code change). Removed
unrelated border-radius on .outer/.inner, replaced font-shaped
inner "content" text with explicit `width: 80px; height: 24px`,
pinned `.label { width: 200px; height: 16px }` to eliminate
font-advance-width-driven flex item sizing. Fixture goes from
0.9932 → 1.0000 similarity.
Both fixtures also strip inner text from their visual probes to
avoid text-under-opacity-layer / font-shaping noise in the
pre-gate phase.
L0.exact now gates box-dimensions, box-padding, and paint-opacity at
floor 1.0.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
3 Skipped Deployments
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughReplace 8-bit quantized layer alpha with Skia's floating-point alpha when creating opacity/save-layers in the canvas paint path; update HTML/CSS fixtures and test-suite entries/viewport and add fixture-authoring guidance. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/grida-canvas/src/htmlcss/paint.rs (1)
118-120: Consolidate opacity handling across painter paths to avoid precision loss.The htmlcss layer fix correctly uses
set_alpha_f(style.opacity), but related painter code still quantizes opacity:painter/painter.rs(lines 403, 593, 2632) andpainter/gradient.rs(lines 88, 137, 190, 277) convert to integer alpha via(opacity * 255.0) as u32/u8. This mixing of quantized and float-based opacity can cause subtle rendering mismatches. Standardize on float-based opacity (e.g.,set_alpha_f,set_alpha_f(gradient.opacity)) or clearly document why specific paths require integer quantization.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/grida-canvas/src/htmlcss/paint.rs` around lines 118 - 120, The codebase mixes float-based alpha (Paint::set_alpha_f) with integer-quantized alpha casts (e.g., (opacity * 255.0) as u8/u32) in painter/painter.rs and painter/gradient.rs; update those places to use float alpha consistently by calling set_alpha_f(...) with the original opacity (e.g., set_alpha_f(gradient.opacity) or set_alpha_f(style.opacity)) instead of computing integer alpha, and remove the (opacity * 255.0) as ... conversions so all painting paths use float alpha and avoid precision loss.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@crates/grida-canvas/src/htmlcss/paint.rs`:
- Around line 118-120: The codebase mixes float-based alpha (Paint::set_alpha_f)
with integer-quantized alpha casts (e.g., (opacity * 255.0) as u8/u32) in
painter/painter.rs and painter/gradient.rs; update those places to use float
alpha consistently by calling set_alpha_f(...) with the original opacity (e.g.,
set_alpha_f(gradient.opacity) or set_alpha_f(style.opacity)) instead of
computing integer alpha, and remove the (opacity * 255.0) as ... conversions so
all painting paths use float alpha and avoid precision loss.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 30f89a69-a8b6-4a73-af24-8c4f541cd798
📒 Files selected for processing (5)
crates/grida-canvas/src/htmlcss/paint.rsfixtures/test-html/L0/box-padding.htmlfixtures/test-html/L0/paint-opacity.htmlfixtures/test-html/suites/L0.coverage.jsonfixtures/test-html/suites/L0.exact.json
Add a short section to the test-html README clarifying that captions next to specimens are fine — the mistake is letting them drive layout or forgetting that hide-text.css already neutralizes color/shaping noise when text is incidental. Motivated by a reftest iteration where labels were stripped instead of pinning the enclosing dimensions.
Extend the Labeled specimens bullet to spell out two things the test-html README already says: keep label text short, and pin the dimensions of any container holding a label so font-advance-width differences can't propagate into geometry. Also point to `hide-text.css` as the text-neutralizer so labels stay useful to future readers instead of getting stripped mid-reftest cycle.
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
1 participant
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.
Summary
Two Chromium-parity promotions for the cg htmlcss renderer, driven through the
dev-cg-htmlcss-feature5-phase loop.1.
paint-opacity— float-native alpha fixcrates/grida-canvas/src/htmlcss/paint.rs:120usedset_alpha((opacity * 255.0) as u8). Rustas u8truncates toward zero, so opacity values that don't align to u8 boundaries diverged from Blink by 1 unit per channel.opacity: 0.5on black-over-white yieldedrgb(127,127,127)in cg vsrgb(126,126,126)in Chromium;0.25yielded(191,191,191)vs(190,190,190).SkCanvas::saveLayerAlphaf(bounds, float)→SkPaint::setAlphaf. WebRender similarly carriesf32end-to-end.set_alpha_f(style.opacity). skia-safe's float-native API maps straight toSkPaint::setAlphaf.0.9600(19,200 / 480,000 diff pixels). After:1.0000(0 / 480,000).2.
box-padding— fixture stabilization (no code change)The 912-pixel divergence on box-padding was entirely fixture noise, not a renderer bug. Three anti-patterns from the skill:
.outer { border-radius: 8px }/.inner { border-radius: 4px }— decoration unrelated to the padding subject; removed..innercontent was font-shaped text"content"; its advance width varied ~1px between engines and propagated to outer block width via stretch. Replaced with explicitwidth: 80px; height: 24px..labelwas auto-sized by text advance width, driving flex-itemmax-contentwidth, which then drove.outerstretch width. Pinned with.label { width: 200px; height: 16px }.Before: similarity
0.9932(912 / 133,200 diff pixels). After:1.0000(0 / 153,600). No cg code change needed.3. Suite promotions
fixtures/test-html/suites/L0.exact.jsonnow gates three fixtures atfloor: 1.0,threshold: 0,aa: false:box-dimensions(pre-existing)box-padding(new — this PR)paint-opacity(new — this PR)paint-opacity.htmlalso strips inner text from its opacity boxes (text-under-opacity-layer hits a second-order Chromium compositing path, even withcolor: transparent).box-paddingviewport bumped from 222 → 256 to match the new natural document height after the label width change.Test plan
cargo check -p cg -p grida-canvas-wasm -p grida-devcleancargo test -p cg --lib htmlcsspasses (149/149, including the pre-existingtest_render_opacity)just fmtclean; lefthook pre-commit (oxfmt + cargo-fmt + clippy) passes@grida/reftestshowsL0.exactat floor=1.0 for all three fixtures after the changeFollow-ups
@grida/reftest compareCLI silently drops--thresholdand--jsonflags (defaults win regardless of CLI arg). Library path works correctly. Small bug, separate PR.paint-border-radius(0.9894) andpaint-background-solid(0.9792) remain inL0.coverage; next candidates for future iterations.Summary by CodeRabbit
Bug Fixes
Tests
Documentation