-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add guide lines feature #3644
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.
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
base: master
Are you sure you want to change the base?
Add guide lines feature #3644
Conversation
|
It still looks strange when rotating a document with guides in: rotating_document_with_guide.mp4Also the snapping doesn't work when the document is rotated. |
0HyperCube
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems to work well; I have a few suggestions for the code structure. I won't comment on the JS changes.
Please mark the PR as «Ready to review» when you are happy with it.
editor/src/messages/portfolio/document/overlays/guide_overlays.rs
Outdated
Show resolved
Hide resolved
editor/src/messages/portfolio/document/overlays/guide_overlays.rs
Outdated
Show resolved
Hide resolved
editor/src/messages/portfolio/document/document_message_handler.rs
Outdated
Show resolved
Hide resolved
Co-authored-by: James Lindsay <[email protected]>
Co-authored-by: James Lindsay <[email protected]>
|
!build |
|
|
Some of the ~750 lines of code is for snapping, right? At least when I glanced over it last, I thought I saw that. In testing this build link, I am not seeing any snapping to objects in the document. The "Move Guide" hint should be removed since the user is already dragging the guide. More review to follow when I have time. My previously stated concern that the line count seems way too high still applies and I'm hesitant to merge it before that can be either resolved or justified to me as to why it's about double the number of lines I would expect from this feature. |
| let transform = document | ||
| .navigation_handler | ||
| .calculate_offset_transform(viewport.center_in_viewport_space().into(), &document.document_ptz); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use document.metadata().transform_to_viewport directly. There should be no need to use the calculate_offset_transform anywhere in your code since it is already cached.

Adds feature for guide lines that can be dragged from the rulers to help with alignment .closes #2601

image of guide lines: