Skip to content

Fix Alt-based quick measurement to show document space coordinates instead of viewport space #2995

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

Jump to bottom
Merged
merged 5 commits into from
Aug 18, 2025
Merged

Fix Alt-based quick measurement to show document space coordinates instead of viewport space #2995

merged 5 commits into from
Aug 18, 2025

Conversation

vineetpuranik
Copy link
Contributor

@vineetpuranik vineetpuranik commented Aug 5, 2025 •
edited
Loading

Fixes #2941

This PR addresses the bug where the quick measurement feature (triggered by holding Alt while hovering over a shape) displayed distances in viewport coordinates instead of document coordinates. This caused the measured distance to vary incorrectly based on zoom level.

Changes made

  • Updated the logic to compute the bounding boxes (selected_bounds, hovered_bounds) in document space using bounding_box_document.
  • Applied the document_to_viewport transform only once during rendering, preserving measurement accuracy.
  • Refactored variable names and added clear inline comments for maintainability.

Before

  • Measured distances between shapes would change when zooming in/out.

After

  • Measured distances remain consistent regardless of zoom level.
  • Overlays are correctly drawn by transforming document-space bounding boxes at render time.

Visual verification

Screencast.from.08-04-2025.11.43.41.PM.webm

@vineetpuranik vineetpuranik marked this pull request as ready for review August 5, 2025 06:46
@Keavon
Copy link
Member

Keavon commented Aug 18, 2025

!build

@github-actions GitHub Actions
Copy link

📦 Build Complete for 4370612
https://db26d9e2.graphite.pages.dev

Keavon
Keavon approved these changes Aug 18, 2025
View reviewed changes
Copy link
Member

@Keavon Keavon left a comment

Choose a reason for hiding this comment

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

💯

Thank you for the valuable contribution. And sorry for the delay! It normally doesn't take me this long to get to problem-free PRs but I was busier than usual.

@Keavon Keavon enabled auto-merge (squash) August 18, 2025 06:23
@Keavon
Copy link
Member

Keavon commented Aug 18, 2025

Please also leave a comment in the associated issue so, as a conversation participant, GitHub allows me to assign it to you.

@Keavon Keavon merged commit 0a4a822 into GraphiteEditor:master Aug 18, 2025
4 checks passed
@vineetpuranik vineetpuranik mentioned this pull request Aug 18, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Keavon Keavon Keavon approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Quick measurement with Alt is incorrectly showing viewport coordinates instead of document coordinates
2 participants
@vineetpuranik @Keavon