Fix #4619: Fallback to identity transform for zero-width layers in tr…

[shubhtrek]

Closes #4619

Summary

When a layer is collapsed to zero width or height, its transform matrix has a determinant of 0 and cannot be inverted. Previously, adding a small 1e-4 epsilon to the diagonal caused the inverted translation terms to blow up, displacing the transform cage away from the layer.

This PR replaces the matrix perturbation with a fallback to DAffine2::IDENTITY, allowing the bounding box to be computed directly in viewport space so the transform cage stays properly aligned over the layer.

Preview

[cubic-dev-ai]

No issues found across 2 files

Confidence score: 5/5

• Automated review surfaced no issues in the provided summaries.
• No files require special attention.

_{Re-trigger cubic}

[cubic-dev-ai]

1 issue found across 2 files (changes from recent commits).

Confidence score: 5/5

• editor/src/messages/tool/common_functionality/shapes/shape_utility.rs duplicates fallback transform logic from select_tool.rs, which could let the two paths drift and handle transforms inconsistently; consider extracting a shared helper.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="editor/src/messages/tool/common_functionality/shapes/shape_utility.rs">

<violation number="1" location="editor/src/messages/tool/common_functionality/shapes/shape_utility.rs:253">
P3: `transform_cage_overlays` now duplicates verbatim the fallback logic added to `select_tool.rs`: the `find` + singular-det fallback (which re-implements `create_bounding_box_transform`), the `transform_tampered` `.any()` scan, and the `filter_map` bounds computation. Both copies must be kept in sync, and the new fallback semantics make any drift silently change behavior in one tool but not the other. Extract the three pieces into one shared helper (leave `create_bounding_box_transform` where it is, but move it plus a `selection_bounds(...)` helper into a common module) and call it from both `select_tool.rs` and `transform_cage_overlays`.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P3: transform_cage_overlays now duplicates verbatim the fallback logic added to select_tool.rs: the find + singular-det fallback (which re-implements create_bounding_box_transform), the transform_tampered .any() scan, and the filter_map bounds computation. Both copies must be kept in sync, and the new fallback semantics make any drift silently change behavior in one tool but not the other. Extract the three pieces into one shared helper (leave create_bounding_box_transform where it is, but move it plus a selection_bounds(...) helper into a common module) and call it from both select_tool.rs and transform_cage_overlays.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At editor/src/messages/tool/common_functionality/shapes/shape_utility.rs, line 253:

<comment>`transform_cage_overlays` now duplicates verbatim the fallback logic added to `select_tool.rs`: the `find` + singular-det fallback (which re-implements `create_bounding_box_transform`), the `transform_tampered` `.any()` scan, and the `filter_map` bounds computation. Both copies must be kept in sync, and the new fallback semantics make any drift silently change behavior in one tool but not the other. Extract the three pieces into one shared helper (leave `create_bounding_box_transform` where it is, but move it plus a `selection_bounds(...)` helper into a common module) and call it from both `select_tool.rs` and `transform_cage_overlays`.</comment>

<file context>
@@ -244,30 +244,41 @@ pub fn update_radius_sign(end: DVec2, start: DVec2, layer: LayerNodeIdentifier,
 		.find(|layer| !document.network_interface.is_artboard(&layer.to_node(), &[]))
-		.map(|layer| document.metadata().transform_to_viewport_with_first_transform_node_if_group(layer, &document.network_interface))
+		.map(|layer| {
+			let transform = document.metadata().transform_to_viewport_with_first_transform_node_if_group(layer, &document.network_interface);
+			if transform.matrix2.determinant() == 0. {
+				document.metadata().document_to_viewport
</file context>

[0HyperCube]

Thanks for the contribution and also cleaning up the duplicated code for the transform bounds.

I am testing on your commit sha 4206e48 and I'm still able to reproduce the original issue (draw rectangle, set scale on x-axis to zero, observe offset)? Perhaps I had not correctly identified the issue in my earlier debugging.

I'd ideally like to see some tests since this edge case behaviour may easily regress in future.

[0HyperCube]

Consider adding a method for this to node-graph/libraries/core-types/src/glam_ext.rs e.g. is_singular().

[cubic-dev-ai]

6 issues found across 9 files (changes from recent commits).

Confidence score: 3/5

• glam_ext.rs also treats nonzero collinear axes as invalid, so it can discard a valid axis when constructing the fallback basis. Preserve one axis and synthesize its perpendicular companion.
• glam_ext.rs can report an infinite-determinant transform as invertible even though its inverse may contain NaN or infinity. Reject non-finite determinants before callers rely on inverse().
• transformation.rs still uses the raw singular to transform in the conjugation, so rotating a child under a zero-scaled parent can make it disappear. Avoid applying that singular transform in the conjugation.
• pivot.rs uses the original singular transform after building bounds with a replacement basis, so multi-layer offsets along the collapsed axis can be wrong. Make the offset calculation use the replacement basis too.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="node-graph/libraries/core-types/src/glam_ext.rs">

<violation number="1" location="node-graph/libraries/core-types/src/glam_ext.rs:62">
P2: `is_invertible` accepts transforms with an infinite determinant, even though their inverse can contain NaN or infinity. Since callers use this method before calling `inverse()`, reject non-finite determinants and verify the complete inverse is finite.</violation>

<violation number="2" location="node-graph/libraries/core-types/src/glam_ext.rs:88">
P1: This arm also handles singular matrices whose two axes are nonzero and collinear, not only matrices with two invalid axes. Preserve one valid axis and synthesize its perpendicular companion for `(true, true)`; otherwise zero-scale layers with skew lose their orientation and the cage becomes misaligned.</violation>
</file>

<file name="tools/third-party-licenses/src/cargo.rs">

<violation number="1" location="tools/third-party-licenses/src/cargo.rs:101">
P3: The temp file is removed only on the success path. When `cargo about generate` fails or `read_to_string` errors, the early `return Err` skips `remove_file`, leaving the file behind. Because the name derives only from the PID in the shared (often world-writable) temp dir, a later run that reuses the PID silently reuses whatever file or symlink sits at that predictable path, and `cargo about -o` writes through a pre-created symlink before it is deleted. Use `tempfile::NamedTempFile` (or a unique random name with cleanup on every error path) so failed, interrupted, or concurrent runs cannot collide with or read a stale file.</violation>
</file>

<file name="editor/src/messages/portfolio/document/utility_types/transformation.rs">

<violation number="1" location="editor/src/messages/portfolio/document/utility_types/transformation.rs:565">
P2: This repairs only the inverse of a singular downstream transform; the raw singular `to` remains on the right side of the conjugation. A selected child under a zero-scaled parent can therefore disappear during rotation or another axis-changing transform; use the same invertible fallback for both sides of the conjugation.</violation>
</file>

<file name="editor/src/messages/tool/common_functionality/pivot.rs">

<violation number="1" location="editor/src/messages/tool/common_functionality/pivot.rs:248">
P2: The bounds now use a replacement invertible basis, but `transform_from_normalized` still applies the original singular transform afterward. With multiple selected layers, offsets along the collapsed axis are therefore discarded and the custom pivot can remain on the first layer instead of the selection bounds; use the same `to_invertible()` transform for the final composition.</violation>
</file>

<file name="node-graph/libraries/rendering/src/renderer.rs">

<violation number="1" location="node-graph/libraries/rendering/src/renderer.rs:2060">
P2: The singular-reference fallback drops later items' transforms in multi-item layers. Preserve each item's placement when building viewport bounds, or choose a metadata representation that does not apply the singular first transform to every target; otherwise glyphs or gradient controls with `Ti != T0` are reported at the wrong position.</violation>
</file>

<sub>Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic</sub>

[cubic-dev-ai]

P1: This arm also handles singular matrices whose two axes are nonzero and collinear, not only matrices with two invalid axes. Preserve one valid axis and synthesize its perpendicular companion for (true, true); otherwise zero-scale layers with skew lose their orientation and the cage becomes misaligned.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At node-graph/libraries/core-types/src/glam_ext.rs, line 88:

<comment>This arm also handles singular matrices whose two axes are nonzero and collinear, not only matrices with two invalid axes. Preserve one valid axis and synthesize its perpendicular companion for `(true, true)`; otherwise zero-scale layers with skew lose their orientation and the cage becomes misaligned.</comment>

<file context>
@@ -35,3 +35,99 @@ impl FallibleVec2Operations for DVec2 {
+				let dir = y_axis.normalize();
+				x_axis = DVec2::new(dir.y, -dir.x);
+			}
+			_ => {
+				x_axis = DVec2::X;
+				y_axis = DVec2::Y;
</file context>

Suggested change

	_ => {
	x_axis = DVec2::X;
	y_axis = DVec2::Y;
	}
	(true, true) => {
	let dir = x_axis.normalize();
	y_axis = DVec2::new(-dir.y, dir.x);
	}
	(false, false) => {
	x_axis = DVec2::X;
	y_axis = DVec2::Y;
	}

[cubic-dev-ai]

P2: is_invertible accepts transforms with an infinite determinant, even though their inverse can contain NaN or infinity. Since callers use this method before calling inverse(), reject non-finite determinants and verify the complete inverse is finite.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At node-graph/libraries/core-types/src/glam_ext.rs, line 62:

<comment>`is_invertible` accepts transforms with an infinite determinant, even though their inverse can contain NaN or infinity. Since callers use this method before calling `inverse()`, reject non-finite determinants and verify the complete inverse is finite.</comment>

<file context>
@@ -35,3 +35,99 @@ impl FallibleVec2Operations for DVec2 {
+
+	#[inline]
+	fn is_invertible(&self) -> bool {
+		self.matrix2.determinant().recip().is_finite()
+	}
+
</file context>

Suggested change

	self.matrix2.determinant().recip().is_finite()
	let determinant = self.matrix2.determinant();
	determinant.is_finite() && determinant != 0. && self.inverse().is_finite()

[cubic-dev-ai]

P2: This repairs only the inverse of a singular downstream transform; the raw singular to remains on the right side of the conjugation. A selected child under a zero-scaled parent can therefore disappear during rotation or another axis-changing transform; use the same invertible fallback for both sides of the conjugation.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At editor/src/messages/portfolio/document/utility_types/transformation.rs, line 565:

<comment>This repairs only the inverse of a singular downstream transform; the raw singular `to` remains on the right side of the conjugation. A selected child under a zero-scaled parent can therefore disappear during rotation or another axis-changing transform; use the same invertible fallback for both sides of the conjugation.</comment>

<file context>
@@ -565,7 +562,7 @@ impl<'a> Selected<'a> {
 		let Some(&original_transform) = original_transform else { return };
 		let to = document_metadata.downstream_transform_to_viewport(layer);
-		let new = to.inverse() * transformation * to * original_transform;
+		let new = to.to_invertible().inverse() * transformation * to * original_transform;
 		responses.add(GraphOperationMessage::TransformSet {
 			layer,
</file context>

Suggested change

	let new = to.to_invertible().inverse() * transformation * to * original_transform;
	let new = to.to_invertible().inverse() * transformation * to.to_invertible() * original_transform;

[cubic-dev-ai]

P2: The bounds now use a replacement invertible basis, but transform_from_normalized still applies the original singular transform afterward. With multiple selected layers, offsets along the collapsed axis are therefore discarded and the custom pivot can remain on the first layer instead of the selection bounds; use the same to_invertible() transform for the final composition.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At editor/src/messages/tool/common_functionality/pivot.rs, line 248:

<comment>The bounds now use a replacement invertible basis, but `transform_from_normalized` still applies the original singular transform afterward. With multiple selected layers, offsets along the collapsed axis are therefore discarded and the custom pivot can remain on the first layer instead of the selection bounds; use the same `to_invertible()` transform for the final composition.</comment>

<file context>
@@ -243,12 +243,9 @@ impl Pivot {
 				document
 					.metadata()
-					.bounding_box_with_transform(layer, transform.inverse() * document.metadata().transform_to_viewport(layer))
+					.bounding_box_with_transform(layer, transform.to_invertible().inverse() * document.metadata().transform_to_viewport(layer))
 			})
 			.reduce(graphene_std::renderer::Quad::combine_bounds);
</file context>

[cubic-dev-ai]

P2: The singular-reference fallback drops later items' transforms in multi-item layers. Preserve each item's placement when building viewport bounds, or choose a metadata representation that does not apply the singular first transform to every target; otherwise glyphs or gradient controls with Ti != T0 are reported at the wrong position.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At node-graph/libraries/rendering/src/renderer.rs, line 2060:

<comment>The singular-reference fallback drops later items' transforms in multi-item layers. Preserve each item's placement when building viewport bounds, or choose a metadata representation that does not apply the singular first transform to every target; otherwise glyphs or gradient controls with `Ti != T0` are reported at the wrong position.</comment>

<file context>
@@ -2055,17 +2055,17 @@ fn collect_vector_items_metadata<'a>(
-				reference_transform.inverse()
+			let item_relative_transform = if transform == reference_transform {
+				DAffine2::IDENTITY
+			} else if reference_transform.is_invertible() {
+				reference_transform.inverse() * transform
 			} else {
</file context>

[cubic-dev-ai]

P3: The temp file is removed only on the success path. When cargo about generate fails or read_to_string errors, the early return Err skips remove_file, leaving the file behind. Because the name derives only from the PID in the shared (often world-writable) temp dir, a later run that reuses the PID silently reuses whatever file or symlink sits at that predictable path, and cargo about -o writes through a pre-created symlink before it is deleted. Use tempfile::NamedTempFile (or a unique random name with cleanup on every error path) so failed, interrupted, or concurrent runs cannot collide with or read a stale file.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At tools/third-party-licenses/src/cargo.rs, line 101:

<comment>The temp file is removed only on the success path. When `cargo about generate` fails or `read_to_string` errors, the early `return Err` skips `remove_file`, leaving the file behind. Because the name derives only from the PID in the shared (often world-writable) temp dir, a later run that reuses the PID silently reuses whatever file or symlink sits at that predictable path, and `cargo about -o` writes through a pre-created symlink before it is deleted. Use `tempfile::NamedTempFile` (or a unique random name with cleanup on every error path) so failed, interrupted, or concurrent runs cannot collide with or read a stale file.</comment>

<file context>
@@ -95,7 +97,8 @@ fn run() -> Result<Output, Error> {
 
-	let stdout = String::from_utf8(output.stdout).map_err(|e| Error::Utf8(e, "cargo about generate returned invalid UTF-8".into()))?;
+	let stdout = fs::read_to_string(&temp_path).map_err(|e| Error::Io(e, "Failed to read cargo about output file".into()))?;
+	let _ = fs::remove_file(&temp_path);
 
 	serde_json::from_str(&stdout).map_err(|e| Error::Json(e, "Failed to parse cargo about generate JSON".into()))
</file context>