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 3 commits into
base: master
Choose a base branch
from
+32 −34
Open

Fix footprint transform #4280

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9d476a3
Fix footprint scale
timon-schelling Jun 24, 2026
f00b8c1
Footprint in physical pixels
timon-schelling Jun 25, 2026
9674ccc
Move scale conversion to `create_context`
timon-schelling Jul 4, 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
25 changes: 7 additions & 18 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;

@TrueDoctor TrueDoctor Jul 14, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

could the scale be just removed here?

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 @@ -381,16 +379,7 @@ pub async fn render_output_cache<'a: 'n>(
if missing_region.tiles.is_empty() {
continue;
}
let region = render_missing_region(
missing_region,
|ctx| data.eval(ctx),
ctx.clone(),
render_params,
&footprint.transform,
&viewport_origin_offset,
device_scale,
)
.await;
let region = render_missing_region(missing_region, |ctx| data.eval(ctx), ctx.clone(), render_params, &footprint.transform, &device_origin_offset).await;
new_regions.push(region);
}

Expand All @@ -406,7 +395,8 @@ 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 combined_metadata = composite_cached_regions(&all_regions, &output_texture, &device_origin_offset, &footprint.transform, executor);

RenderOutput {
data: RenderOutputType::Texture(output_texture.into()),
Expand All @@ -421,7 +411,6 @@ async fn render_missing_region<F, Fut>(
render_params: &RenderParams,
viewport_transform: &DAffine2,
viewport_origin_offset: &DVec2,
device_scale: f64,
) -> CachedRegion
where
F: Fn(Context<'static>) -> Fut,
Expand All @@ -433,7 +422,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 Down
22 changes: 14 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 @@ -93,10 +93,11 @@ async fn render<'a: 'n>(

let data = match (render_params.render_output_type, ty) {
(RenderOutputTypeRequest::Svg, RenderIntermediateType::Svg(data)) => {
let logical_transform = glam::DAffine2::from_scale(glam::DVec2::splat(1.0 / render_params.scale)) * footprint.transform;
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 +109,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 +151,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 All @@ -176,5 +179,8 @@ async fn create_context<'a: 'n>(
.with_vararg(Box::new(render_params))
.into_context();

data.eval(ctx).await
let mut result = data.eval(ctx).await;

result.metadata.apply_transform(glam::DAffine2::from_scale(glam::DVec2::splat(1. / render_config.scale)));
result
}
12 changes: 6 additions & 6 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 @@ -67,9 +69,7 @@ pub async fn render_pixel_preview<'a: 'n>(

result.data = RenderOutputType::Texture(resampled.into());

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

result
}
Expand Down
Loading