fix(paint): unbreak shadows, bound the opacity layer, make the banding fix real - #154
Merged
LeadcodeDev merged 1 commit intoAug 8, 2026
Conversation
…g fix real Seven confirmed audit findings in the paint pass, plus one regression guard for a side effect the scroll-wrap fix would otherwise have introduced. - `overflow: hidden` erased the node's own outset `box-shadow`: the clip was installed before the shadow was drawn, so the shadow was clipped away by the very box it was supposed to sit outside. - `backdrop-filter` was neutralised whenever `opacity < 1` on the same node. - The opacity `save_layer` was allocated with no bounds — one full-viewport layer per node. On a 60-frame scenario this dominated everything else: 42-60s down to 0.5s. - Leaf painters were handed the border-box origin with the padding insets zeroed, so every leaf ignored `padding`. `Codeblock` stays a deliberate, now-documented exception: it reads `style.padding` itself and paints its own background from the border box, so honouring the general contract for it too would double-apply padding. - Transform percentages were resolved against `max(width, height)` on both axes instead of per-axis. - Scrolling tiled backgrounds walked out of frame: the offset grew linearly with time forever while the draw loops only overscan by one tile, so the pattern left a growing blank band. The offset now wraps into one tile period, and `draw_bg_grid_dots`'s x-loop overscans symmetrically like its y-loop already did (it started at 0, with no left margin). - Both documented gradient banding mitigations were inert. The render surfaces are created with no Skia `ColorSpace`, which short-circuits the conversion, so tagging the shader's colors with `srgb_linear` was a silent no-op — and subdividing an already-sRGB lerp is a mathematical identity (17x the stops, zero visual effect). `subdivide_gradient_stops` now does the gamma round-trip itself on plain `f32`s, which works whatever color space the destination surface ends up carrying. The wrap above needed one companion fix, found while verifying it rather than reported: geometry is periodic on `spacing` and survives a wrap, but the dot pulse is a `sin` of position and is not. Fed canvas-local coordinates, every dot's radius and alpha stepped at once, every `spacing / speed` seconds. The pulse now reads the unwrapped scroll track, and the test asserting this also asserts that the naive version still steps — so it cannot quietly become vacuous. Tests: 153 + 117 + 6 + 3 + 5 + 215 + 3 pass on this branch alone.
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Aug 9, 2026
LeadcodeDev added a commit
that referenced
this pull request
Aug 10, 2026
…g fix real (#154) Seven confirmed audit findings in the paint pass, plus one regression guard for a side effect the scroll-wrap fix would otherwise have introduced. - `overflow: hidden` erased the node's own outset `box-shadow`: the clip was installed before the shadow was drawn, so the shadow was clipped away by the very box it was supposed to sit outside. - `backdrop-filter` was neutralised whenever `opacity < 1` on the same node. - The opacity `save_layer` was allocated with no bounds — one full-viewport layer per node. On a 60-frame scenario this dominated everything else: 42-60s down to 0.5s. - Leaf painters were handed the border-box origin with the padding insets zeroed, so every leaf ignored `padding`. `Codeblock` stays a deliberate, now-documented exception: it reads `style.padding` itself and paints its own background from the border box, so honouring the general contract for it too would double-apply padding. - Transform percentages were resolved against `max(width, height)` on both axes instead of per-axis. - Scrolling tiled backgrounds walked out of frame: the offset grew linearly with time forever while the draw loops only overscan by one tile, so the pattern left a growing blank band. The offset now wraps into one tile period, and `draw_bg_grid_dots`'s x-loop overscans symmetrically like its y-loop already did (it started at 0, with no left margin). - Both documented gradient banding mitigations were inert. The render surfaces are created with no Skia `ColorSpace`, which short-circuits the conversion, so tagging the shader's colors with `srgb_linear` was a silent no-op — and subdividing an already-sRGB lerp is a mathematical identity (17x the stops, zero visual effect). `subdivide_gradient_stops` now does the gamma round-trip itself on plain `f32`s, which works whatever color space the destination surface ends up carrying. The wrap above needed one companion fix, found while verifying it rather than reported: geometry is periodic on `spacing` and survives a wrap, but the dot pulse is a `sin` of position and is not. Fed canvas-local coordinates, every dot's radius and alpha stepped at once, every `spacing / speed` seconds. The pulse now reads the unwrapped scroll track, and the test asserting this also asserts that the naive version still steps — so it cannot quietly become vacuous. Tests: 153 + 117 + 6 + 3 + 5 + 215 + 3 pass on this branch alone.
11 tasks
This was referenced Aug 11, 2026
LeadcodeDev added a commit
that referenced
this pull request
Aug 11, 2026
…ers (#175) Investigating the "per-node visual effects" gap produced a negative result rather than a feature, and this commit ships the evidence so the next attempt does not have to rediscover it. `effects` is a field of `Scene` only, so an effect covers the whole frame. The obvious workaround — render normally, then crop the effect to a node's rect from `render_scene_hits` — does not merely cost too much, it is wrong. A background node with `pixelate` applied that way contaminates **5904 of 10000 pixels** inside an overlapping sibling's own box; a sample pixel goes from pure blue to `[155, 0, 99]`. `buffer_crop_strategy_leaks_effect_onto_ overlapping_sibling` pins that: it passes, and what it documents is the rejection. The same approach also costs x1.47 on a real scenario (mega-showcase scene 0, 1920x1080, release: 6.0ms/frame becomes 8.8ms), but the leak is the disqualifying part, not the cost. The cost benchmark is kept `#[ignore]`d because its other measurement is the one that matters for whoever implements this properly: bounding the CPU effect pass to a node's real box instead of the full frame is **x14.57** faster at a 13.9x area ratio. That is the same argument PR #154 already made for the `save_layer` it bounded (42-60s down to 0.5s over 60 frames), and it says any real implementation must bound its layer to the node. No production file is touched. The feature is blocked on three things that sit outside one file: a node's device-space rect exists only inside `paint_node` (animation presets resolve into `CssStyle.transform`, so a rect computed from `run_layout` alone is silently wrong for most animated content); `post_effects.rs` lives in the crate that depends on the one holding `paint_pass.rs`, not the reverse; and `CssStyle` is the only per-node channel that reaches `BoxNode` without teaching `box_builder` a new field.
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 freeto 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.
Round 3 du chantier d'audit — lot passe de peinture, 7 constats vérifiés (+ 1 garde-fou que j'ai ajouté en vérifiant).
overflow: hiddenvs ombrebox-shadowextérieur du nœud était découpé par la boîte même hors de laquelle il est censé déborder.backdrop-filteropacity < 1sur le même nœud.padding.save_layersans boundsmax(width, height)sur les deux axes au lieu d'un axe par axe.Codeblockreste une exception délibéréeLe contrat
Painterpromet un canvas déjà translaté sur l'origine de la content-box.Codeblocklitstyle.paddinglui-même et peint son fond depuis la border-box : honorer le contrat général pour lui aussi doublerait le padding. L'exception est maintenant documentée dans le code plutôt que subie.Pourquoi les deux mitigations de banding ne faisaient rien
Les surfaces de rendu sont créées sans
ColorSpaceSkia (ImageInfo::new(..., None),scene.rs). Cela court-circuite toute conversion colorimétrique — donc :ColorSpace::new_srgb_linear()était un no-op silencieux : les couleurs restaient en sRGB gamma-encodé.subdivide_gradient_stopsfait désormais l'aller-retour gamma lui-même sur desf32— ce qui fonctionne quel que soit l'espace colorimétrique que la surface de destination finira par porter. L'alpha n'est pas gamma-encodé et garde un lerp linéaire.Le garde-fou que le wrap rendait nécessaire
Trouvé en vérifiant le constat, pas rapporté par l'agent. La géométrie est périodique sur
spacinget traverse un wrap sans changer ; le pulse des points ne l'est pas — c'est unsinde la position. Alimenté en coordonnées canvas-locales, le rayon et l'alpha de tous les points sautaient d'un coup, toutes lesspacing / speedsecondes (≈ 0,67 s pour spacing 40 / speed 60), d'environ 17 %.Le pulse lit maintenant la piste de défilement non wrappée. Le test qui l'affirme vérifie aussi que la version naïve saute toujours — il ne peut donc pas devenir silencieusement vide.
Reste ouvert
Le wrap s'applique aussi aux trois presets non périodiques (
gradient_shift,halo,concentric_circles), qui animent déjà leur propre mouvement en interne. Pour eux, il substitue un artefact borné à une disparition non bornée : strictement meilleur, pas encore juste. Suivi séparément.Vérification
cargo test -p rustmotion -p rustmotion-core -p rustmotion-componentssur cette branche seule : 153 + 117 + 6 + 3 + 5 + 215 + 3 tests, 0 échec.