Skip to content

Fix footprint transform #4280

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
timon-schelling wants to merge 2 commits into
base: master
Choose a base branch
from
+34 −24
Open

Fix footprint transform #4280

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
40486dc
Fix footprint scale
timon-schelling Jun 24, 2026
cd2121a
Footprint in physical pixels
timon-schelling Jun 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions node-graph/nodes/gstd/src/render_background.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ async fn render_background<'a: 'n>(

let data = match foreground_data {
RenderOutputType::Texture(foreground_texture) => {
let doc_to_screen = (glam::DAffine2::from_scale(glam::DVec2::splat(render_params.scale)) * render_params.footprint.transform).as_affine2();
let doc_to_screen = render_params.footprint.transform.as_affine2();
let blended = pipeline
.run::<CompositeBackground>(&CompositeBackgroundArgs {
foreground: foreground_texture.as_ref(),
Expand All @@ -52,6 +52,8 @@ async fn render_background<'a: 'n>(
} => {
let mut render = SvgRender::new();

let logical_transform = glam::DAffine2::from_scale(glam::DVec2::splat(1.0 / render_params.scale)) * render_params.footprint.transform;

@cubic-dev-ai cubic-dev-ai Bot Jun 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Unconditional 1.0 / render_params.scale can create non-finite transforms when scale is 0. Guard scale before inversion to avoid invalid SVG transform output.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At node-graph/nodes/gstd/src/render_background.rs, line 55:

<comment>Unconditional `1.0 / render_params.scale` can create non-finite transforms when scale is 0. Guard scale before inversion to avoid invalid SVG transform output.</comment>

<file context>
@@ -52,6 +52,8 @@ async fn render_background<'a: 'n>(
 		} => {
 			let mut render = SvgRender::new();
 
+			let logical_transform = glam::DAffine2::from_scale(glam::DVec2::splat(1.0 / render_params.scale)) * render_params.footprint.transform;
+
 			if render_params.viewport_zoom > 0. {
</file context>


if render_params.viewport_zoom > 0. {
let draw_checkerboard = |render: &mut SvgRender, rect: vello::kurbo::Rect, pattern_origin: glam::DVec2, checker_id_prefix: &str| {
let checker_id = format!("{checker_id_prefix}-{}", generate_uuid());
Expand Down Expand Up @@ -79,6 +81,7 @@ async fn render_background<'a: 'n>(
if render_params.scale > 0. {
let logical_resolution = render_params.footprint.resolution.as_dvec2() / render_params.scale;
let logical_footprint = Footprint {
transform: logical_transform,
resolution: logical_resolution.round().as_uvec2().max(glam::UVec2::ONE),
..render_params.footprint
};
Expand All @@ -101,7 +104,7 @@ async fn render_background<'a: 'n>(
}

let logical_resolution = render_params.footprint.resolution.as_dvec2() / render_params.scale;
render.wrap_with_transform(render_params.footprint.transform, Some(logical_resolution));
render.wrap_with_transform(logical_transform, Some(logical_resolution));

let background = SvgRenderOutput::from(render);
assert!(background.svg_defs.is_empty());
Expand Down
21 changes: 11 additions & 10 deletions node-graph/nodes/gstd/src/render_cache.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -345,12 +345,10 @@ pub async fn render_output_cache<'a: 'n>(
return data.eval(context.into_context()).await;
}

let device_scale = render_params.scale;
let zoom = footprint.scale_magnitudes().x;
let zoom = footprint.scale_magnitudes().x / render_params.scale;

@cubic-dev-ai cubic-dev-ai Bot Jun 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Unvalidated render_params.scale is used as a divisor, allowing divide-by-zero/non-finite transform math. Guard invalid scale and fall back to direct render before computing zoom/transforms.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At node-graph/nodes/gstd/src/render_cache.rs, line 348:

<comment>Unvalidated `render_params.scale` is used as a divisor, allowing divide-by-zero/non-finite transform math. Guard invalid scale and fall back to direct render before computing zoom/transforms.</comment>

<file context>
@@ -345,12 +345,10 @@ pub async fn render_output_cache<'a: 'n>(
 
-	let device_scale = render_params.scale;
-	let zoom = footprint.scale_magnitudes().x;
+	let zoom = footprint.scale_magnitudes().x / render_params.scale;
 	let rotation = footprint.decompose_rotation();
 
</file context>
Suggested change
let zoom = footprint.scale_magnitudes().x / render_params.scale;
let scale = render_params.scale;
if !scale.is_finite() || scale <= 0. {
log::warn!("render_output_cache: invalid render scale ({scale}), falling back to direct render");
let context = OwnedContextImpl::from(ctx.clone()).with_footprint(*footprint).with_vararg(Box::new(render_params.clone()));
return data.eval(context.into_context()).await;
}
let zoom = footprint.scale_magnitudes().x / scale;

let rotation = footprint.decompose_rotation();

let viewport_origin_offset = footprint.transform.translation;
let device_origin_offset = viewport_origin_offset * device_scale;
let device_origin_offset = footprint.transform.translation;
let viewport_bounds_device = AxisAlignedBbox {
start: -device_origin_offset,
end: footprint.resolution.as_dvec2() - device_origin_offset,
Expand All @@ -361,7 +359,7 @@ pub async fn render_output_cache<'a: 'n>(
let cache_key = CacheKey::new(
max_region_area,
render_params.render_mode as u64,
device_scale,
render_params.scale,
zoom,
rotation,
render_params.for_export,
Expand All @@ -387,8 +385,8 @@ pub async fn render_output_cache<'a: 'n>(
ctx.clone(),
render_params,
&footprint.transform,
&viewport_origin_offset,
device_scale,
&device_origin_offset,
render_params.scale,
)
.await;
new_regions.push(region);
Expand All @@ -406,7 +404,9 @@ pub async fn render_output_cache<'a: 'n>(

let executor = executor.expect("GPU executor not available");
let output_texture = executor.request_texture(physical_resolution).await;
let combined_metadata = composite_cached_regions(&all_regions, &output_texture, &device_origin_offset, &footprint.transform, &executor);

let logical_viewport_transform = DAffine2::from_scale(DVec2::splat(1.0 / render_params.scale)) * footprint.transform;
let combined_metadata = composite_cached_regions(&all_regions, &output_texture, &device_origin_offset, &logical_viewport_transform, executor);

RenderOutput {
data: RenderOutputType::Texture(output_texture.into()),
Expand All @@ -433,7 +433,7 @@ where
let tile_count = (max_tile - min_tile) + IVec2::ONE;
let region_pixel_size = (tile_count * TILE_SIZE as i32).as_uvec2();

let tile_global_offset = min_tile.as_dvec2() * (TILE_SIZE as f64 / device_scale) + *viewport_origin_offset;
let tile_global_offset = min_tile.as_dvec2() * TILE_SIZE as f64 + *viewport_origin_offset;
let region_transform = DAffine2::from_translation(-tile_global_offset) * *viewport_transform;
let region_footprint = Footprint {
transform: region_transform,
Expand All @@ -449,7 +449,8 @@ where
unreachable!("render_missing_region: expected texture output from Vello render");
};

let pixel_to_document = region_transform.inverse();
let logical_region_transform = DAffine2::from_scale(DVec2::splat(1.0 / device_scale)) * region_transform;
let pixel_to_document = logical_region_transform.inverse();
result.metadata.apply_transform(pixel_to_document);

let memory_size = (region_pixel_size.x * region_pixel_size.y) as usize * BYTES_PER_PIXEL;
Expand Down
20 changes: 12 additions & 8 deletions node-graph/nodes/gstd/src/render_node.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,15 +88,17 @@ async fn render<'a: 'n>(
let mut render_params = render_params.clone();
render_params.footprint = *footprint;

let logical_transform = glam::DAffine2::from_scale(glam::DVec2::splat(1.0 / render_params.scale)) * footprint.transform;

let RenderIntermediate { ty, mut metadata } = data;
metadata.apply_transform(footprint.transform);
metadata.apply_transform(logical_transform);

let data = match (render_params.render_output_type, ty) {
(RenderOutputTypeRequest::Svg, RenderIntermediateType::Svg(data)) => {
let logical_resolution = render_params.footprint.resolution.as_dvec2() / render_params.scale;

let mut render = SvgRender::from(data.as_ref());
render.wrap_with_transform(render_params.footprint.transform, Some(logical_resolution));
render.wrap_with_transform(logical_transform, Some(logical_resolution));

let output = SvgRenderOutput::from(render);
assert!(output.svg_defs.is_empty());
Expand All @@ -108,11 +110,9 @@ async fn render<'a: 'n>(
}
(RenderOutputTypeRequest::Vello, RenderIntermediateType::Vello(data)) => {
let (scene, context) = data.as_ref();
let scale = render_params.scale;
let physical_resolution = render_params.footprint.resolution;

let scale_transform = glam::DAffine2::from_scale(glam::DVec2::splat(scale));
let footprint_transform = scale_transform * render_params.footprint.transform;
let footprint_transform = render_params.footprint.transform;
let footprint_transform_vello = vello::kurbo::Affine::new(footprint_transform.to_cols_array());

let mut transformed_scene = vello::Scene::new();
Expand Down Expand Up @@ -152,19 +152,23 @@ async fn create_context<'a: 'n>(
render_config: RenderConfig,
data: impl Node<Context<'static>, Output = RenderOutput>,
) -> RenderOutput {
let footprint = render_config.viewport;

let render_output_type = match render_config.export_format {
ExportFormat::Svg => RenderOutputTypeRequest::Svg,
ExportFormat::Raster => RenderOutputTypeRequest::Vello,
};

let logical_viewport = render_config.viewport;
let footprint = Footprint {
transform: glam::DAffine2::from_scale(glam::DVec2::splat(render_config.scale)) * logical_viewport.transform,
..logical_viewport
};

let render_params = RenderParams {
render_mode: render_config.render_mode,
for_export: render_config.for_export,
render_output_type,
scale: render_config.scale,
viewport_zoom: footprint.scale_magnitudes().x,
viewport_zoom: logical_viewport.scale_magnitudes().x,
..Default::default()
};

Expand Down
10 changes: 6 additions & 4 deletions node-graph/nodes/gstd/src/render_pixel_preview.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ pub async fn render_pixel_preview<'a: 'n>(
let physical_scale = render_params.scale;

let footprint = *ctx.footprint();
let viewport_zoom = footprint.scale_magnitudes().x * physical_scale;
let viewport_zoom = footprint.scale_magnitudes().x;

if render_params.render_mode != RenderMode::PixelPreview || !matches!(render_params.render_output_type, RenderOutputTypeRequest::Vello) || viewport_zoom <= 1. {
let context = OwnedContextImpl::from(ctx).into_context();
Expand All @@ -32,6 +32,7 @@ pub async fn render_pixel_preview<'a: 'n>(
let logical_resolution = physical_resolution.as_dvec2() / physical_scale;

let logical_footprint = Footprint {
transform: DAffine2::from_scale(DVec2::splat(1. / physical_scale)) * footprint.transform,
resolution: logical_resolution.as_uvec2().max(UVec2::ONE),
..footprint
};
Expand All @@ -45,7 +46,7 @@ pub async fn render_pixel_preview<'a: 'n>(
let upstream_resolution = upstream_size.as_uvec2().max(UVec2::ONE);

let upstream_footprint = Footprint {
transform: DAffine2::from_scale(DVec2::splat(1. / physical_scale)) * DAffine2::from_translation(-upstream_min),
transform: DAffine2::from_translation(-upstream_min),
resolution: upstream_resolution,
quality: footprint.quality,
};
Expand All @@ -55,7 +56,8 @@ pub async fn render_pixel_preview<'a: 'n>(

let RenderOutputType::Texture(ref source_texture) = result.data else { return result };

let transform = DAffine2::from_translation(-upstream_min) * footprint.transform.inverse() * DAffine2::from_scale(logical_resolution);
let logical_transform = DAffine2::from_scale(DVec2::splat(1. / physical_scale)) * footprint.transform;
let transform = DAffine2::from_translation(-upstream_min) * logical_transform.inverse() * DAffine2::from_scale(logical_resolution);

let resampled = pipeline
.run::<PixelPreview>(&PixelPreviewArgs {
Expand All @@ -69,7 +71,7 @@ pub async fn render_pixel_preview<'a: 'n>(

result
.metadata
.apply_transform(footprint.transform * DAffine2::from_translation(upstream_min) * DAffine2::from_scale(DVec2::splat(physical_scale)));
.apply_transform(logical_transform * DAffine2::from_translation(upstream_min) * DAffine2::from_scale(DVec2::splat(physical_scale)));
Comment on lines 72 to +74

@gemini-code-assist gemini-code-assist Bot Jun 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

With upstream_render_params.scale set to 1.0, the upstream metadata is already in logical space, so we no longer need to multiply by physical_scale to undo the mismatch.

Suggested change
result
.metadata
.apply_transform(footprint.transform * DAffine2::from_translation(upstream_min) * DAffine2::from_scale(DVec2::splat(physical_scale)));
.apply_transform(logical_transform * DAffine2::from_translation(upstream_min) * DAffine2::from_scale(DVec2::splat(physical_scale)));
result
.metadata
.apply_transform(logical_transform * DAffine2::from_translation(upstream_min));


result
}
Expand Down
Loading