Skip to content

fix(geometry): resolve vw/vh against the scenario viewport - #227

Merged
LeadcodeDev merged 1 commit into
chantier/audit-2026-09from
fix/geometry-viewport-units
Sep 22, 2026
Merged

LeadcodeDev merged 1 commit into
chantier/audit-2026-09from
fix/geometry-viewport-units

Conversation

@LeadcodeDev

Copy link
Copy Markdown
Owner

Severity Medium, category coherence. Location: crates/rustmotion/src/cli/commands/geometry.rs:180

Impact

ConversionContext::default() carries LengthContext { viewport_width: 1920.0, viewport_height: 1080.0, .. } (rustmotion-core/src/css/units.rs:66-74), and taffy_bridge::size_to_dim/lp_to_lp/lp_to_lp_auto resolve every vw/vh/rem/em against it (taffy_bridge.rs:335-338). The renderer does NOT: render_with_new_pipeline_iter (engine/render/scene.rs:554-558) and render_scene_hits (scene.rs:792) both pass viewport_conversion_context(vw, vh), whose own doc comment calls the 1920×1080 default a 78% error on a 1080×1920 vertical video. So on any non-1920×1080 scenario the mandatory rustmotion validate gate checks a layout the renderer never produces: width: "50vw" is measured as 960px by the validator and painted as 540px, 50vh as 540px vs 960px. The file header promises the opposite ("so the geometry it checks matches what the renderer will actually paint"). The same file builds a correct LengthContext with the real viewport at line 477 for its transform-origin math, so the omission is confined to the layout pass. Both the resting walk (:180) and the --strict-anim walk (:1347) are affected.

Fix

Make viewport_conversion_context public (or move it next to run_layout in rustmotion-core) and call it from both geometry.rs sites: run_layout(&built.root, viewport_f, &viewport_conversion_context(viewport_f.0, viewport_f.1)). Better: stop exposing ConversionContext at the run_layout signature at all — derive it from the viewport argument inside run_layout, which makes the wrong context unrepresentable.

Evidence the audit read

let root_css = render::root_style(scene.layout.as_ref(), view.view_type.clone());
let built = build_scene_from_refs(children.iter(), viewport_f, root_css, None);
let layouts = run_layout(&built.root, viewport_f, &ConversionContext::default());

Based directly on the chantier branch.

Part of the September 2026 audit remediation chantier. Refs #220 (RM-05).

@LeadcodeDev LeadcodeDev added the bug Something isn't working label Sep 21, 2026
@LeadcodeDev LeadcodeDev self-assigned this Sep 21, 2026
@LeadcodeDev
LeadcodeDev force-pushed the fix/geometry-viewport-units branch from b59cf4f to f94bcad Compare September 21, 2026 23:39
@LeadcodeDev
LeadcodeDev force-pushed the fix/geometry-viewport-units branch 2 times, most recently from 99491fd to c1f4b85 Compare September 22, 2026 08:33
ConversionContext::default() carries LengthContext { viewport_width: 1920.0,
viewport_height: 1080.0, .. } (rustmotion-core/src/css/units.rs:66-74), and
taffy_bridge::size_to_dim/lp_to_lp/lp_to_lp_auto resolve every vw/vh/rem/em
against it (taffy_bridge.rs:335-338). The renderer does NOT:
render_with_new_pipeline_iter (engine/render/scene.rs:554-558) and
render_scene_hits (scene.rs:792) both pass viewport_conversion_context(vw,
vh), whose own doc comment calls the 1920×1080 default a 78% error on a
1080×1920 vertical video. So on any non-1920×1080 scenario the mandatory
rustmotion validate gate checks a layout the renderer never produces: width:
"50vw" is measured as 960px by the validator and painted as 540px, 50vh as
540px vs 960px. The file header promises the opposite ("so the geometry it
checks matches what the renderer will actually paint"). The same file builds
a correct LengthContext with the real viewport at line 477 for its
transform-origin math, so the omission is confined to the layout pass. Both
the resting walk (:180) and the --strict-anim walk (:1347) are affected.

Fix: Make viewport_conversion_context public (or move it next to run_layout
in rustmotion-core) and call it from both geometry.rs sites:
run_layout(&built.root, viewport_f,
&viewport_conversion_context(viewport_f.0, viewport_f.1)). Better: stop
exposing ConversionContext at the run_layout signature at all — derive it
from the viewport argument inside run_layout, which makes the wrong context
unrepresentable.

Refs #220
@LeadcodeDev
LeadcodeDev force-pushed the fix/geometry-viewport-units branch from c1f4b85 to fc20b4c Compare September 22, 2026 08:43
@LeadcodeDev
LeadcodeDev merged commit f46548b into chantier/audit-2026-09 Sep 22, 2026
2 of 3 checks passed
@LeadcodeDev
LeadcodeDev deleted the fix/geometry-viewport-units branch September 22, 2026 08:52
LeadcodeDev added a commit that referenced this pull request Sep 22, 2026
ConversionContext::default() carries LengthContext { viewport_width: 1920.0,
viewport_height: 1080.0, .. } (rustmotion-core/src/css/units.rs:66-74), and
taffy_bridge::size_to_dim/lp_to_lp/lp_to_lp_auto resolve every vw/vh/rem/em
against it (taffy_bridge.rs:335-338). The renderer does NOT:
render_with_new_pipeline_iter (engine/render/scene.rs:554-558) and
render_scene_hits (scene.rs:792) both pass viewport_conversion_context(vw,
vh), whose own doc comment calls the 1920×1080 default a 78% error on a
1080×1920 vertical video. So on any non-1920×1080 scenario the mandatory
rustmotion validate gate checks a layout the renderer never produces: width:
"50vw" is measured as 960px by the validator and painted as 540px, 50vh as
540px vs 960px. The file header promises the opposite ("so the geometry it
checks matches what the renderer will actually paint"). The same file builds
a correct LengthContext with the real viewport at line 477 for its
transform-origin math, so the omission is confined to the layout pass. Both
the resting walk (:180) and the --strict-anim walk (:1347) are affected.

Fix: Make viewport_conversion_context public (or move it next to run_layout
in rustmotion-core) and call it from both geometry.rs sites:
run_layout(&built.root, viewport_f,
&viewport_conversion_context(viewport_f.0, viewport_f.1)). Better: stop
exposing ConversionContext at the run_layout signature at all — derive it
from the viewport argument inside run_layout, which makes the wrong context
unrepresentable.

Refs #220
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant