Skip to content

Desktop: Make shutdown more robust and fix panic caused by invalid viewport scale#3783

Open
timon-schelling wants to merge 2 commits intomasterfrom
fix-zero-size-viewport-crash
Open

Desktop: Make shutdown more robust and fix panic caused by invalid viewport scale#3783
timon-schelling wants to merge 2 commits intomasterfrom
fix-zero-size-viewport-crash

Conversation

@timon-schelling
Copy link
Member

fixes


thread 'main' (6871) panicked at editor/src/messages/viewport/viewport_message_handler.rs:526:5:
assertion `left == right` failed: Cannot multiply with diffent scale
  left: NaN
 right: NaN
 

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request addresses a panic caused by an invalid viewport scale (NaN). The changes introduce checks in both the Rust backend and the TypeScript frontend to validate the scale value. While the fix is correct for the reported issue, my review includes suggestions to make these checks more robust by also handling non-finite values like Infinity, which could cause similar problems. I've recommended using is_finite() in Rust and Number.isFinite() in TypeScript for more comprehensive validation.

match message {
ViewportMessage::Update { x, y, width, height, scale } => {
assert_ne!(scale, 0., "Viewport scale cannot be zero");
assert!(scale > 0., "Viewport scale must be greater than zero");

Choose a reason for hiding this comment

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

high

While this change correctly prevents NaN scale values which caused the original panic, it still allows Infinity. An infinite scale could be passed from the frontend and lead to other calculations resulting in Infinity or NaN, potentially causing other issues. Using is_finite() would make this assertion more robust.

Suggested change
assert!(scale > 0., "Viewport scale must be greater than zero");
assert!(scale.is_finite() && scale > 0., "Viewport scale must be finite and greater than zero");

Comment on lines +45 to +47
if (!scale || scale <= 0) {
continue;
}

Choose a reason for hiding this comment

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

medium

The check !scale works for NaN and 0, but using Number.isFinite() is more explicit and robust. It will correctly handle Infinity, -Infinity, and NaN, which can occur if logicalWidth is zero. This ensures that only valid, finite, positive scale values are sent to the backend.

Suggested change
if (!scale || scale <= 0) {
continue;
}
if (!Number.isFinite(scale) || scale <= 0) {
continue;
}

@timon-schelling timon-schelling changed the title Fix panic caused by invalid viewport scale Desktop: Make shutdown more robust and fix panic caused by invalid viewport scale Feb 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

Comments