Skip to content
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

Add auto-panning when pointer goes beyond viewport edge with Select tool #1625

Merged

Conversation

elbertronnie
Copy link
Collaborator

@elbertronnie elbertronnie commented Feb 24, 2024

Partly closes #1527

Notes:

  • This PR adds code for shifting viewport when using the select tool when it is in Dragging, DrawingBox, ResizingBounds or DraggingPivot state.
  • The only major difference in this implementation from the given reference video in the issue is that the shift only occurs on mouse movement. Keeping the mouse stationary beyond the edge will not shift the viewport.

Copy link
Member

@0HyperCube 0HyperCube left a comment

Choose a reason for hiding this comment

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

Looks good so far - although it would be nice if this updated when there was no mouse motion.

To implement this I think we could probably send a message on each animation frame which is received in the init_after_frontend_ready so that as well as calling poll_node_graph_evaluation we could also do something like:

call_closure_with_editor_and_handle(|editor, handle| {
	for message in editor.handle_message(BroadcastMessage::TriggerEvent(BroadcastEvent::AnimationFrame)) {
		handle.send_frontend_message_to_js(message);
	}
});

Then subscribe to the broadcast event so that we can update the viewport position.

@elbertronnie
Copy link
Collaborator Author

I have made the changes so that the viewport shifts when there is no mouse motion.
A new Message is created in SelectToolMessage named as PointerOutsideViewport. When the mouse moves outside the viewport, it subscribes to AnimationFrame broadcast message. After coming inside the viewport, it unsubscribes.
The logic of shifting viewport runs when a PointerOutsideViewport message is recieved.

  • Is the approach used correct?
  • Should I create a new struct SelectToolPointerKeys to manage axis_align, snap_angle, center and duplicate with a single name? It was getting cluttered very easily since all these parameters had to be passed often.

@0HyperCube
Copy link
Member

@elbertronnie thanks for this change. The approach seems reasonable, and I think that it would be good to have a struct to group the select tool modifier keys.

@elbertronnie
Copy link
Collaborator Author

I have grouped all modifier keys into SelectToolPointerKeys and cleaned up the code

@Keavon
Copy link
Member

Keavon commented Feb 27, 2024

!build

Copy link

📦 Build Complete for 42e9d16
https://5359d6ef.graphite.pages.dev

@Keavon
Copy link
Member

Keavon commented Feb 27, 2024

Great work! Could you slow it down by about 50? It goes really fast right now.

Could you also please add it to these tools?:

[list moved to #1527]

Please try to structure it so as much code as possible can be reused across the implementations.

@0HyperCube
Copy link
Member

@elbertronnie thanks for these changes - it feels much smoother now with the animation frame callback.

I fixed a small bug whereby during the dragging of the bounding box, the shape would shift around (due to the message ordering - the modification to original_bound_transform happens immediately, whereas the messages are handled later, but putting all of the modifications in messages allows for this to be resolved).

@Keavon
Copy link
Member

Keavon commented Feb 27, 2024

!build

@elbertronnie
Copy link
Collaborator Author

Could you also please add it to these tools?

@Keavon I will do it but I won't be able to contribute for a week since my college exams are going on.

@Keavon
Copy link
Member

Keavon commented Feb 27, 2024

Ok, let's merge this one today and then do the other tools as a followup PR once you're done with exams!

Copy link

📦 Build Complete for c7358fb
https://8de5d82d.graphite.pages.dev

@elbertronnie elbertronnie changed the title Add code to shift viewport if mouse is beyond edge Add code to shift viewport if mouse is beyond edge in Select Tool Feb 28, 2024
@Keavon Keavon force-pushed the shift-viewport-mouse-beyond-edge branch from c7358fb to 3424db4 Compare February 29, 2024 07:42
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.

This works nicely, thanks! I capped the speed with an overextension of 50px beyond the viewport bounds so it doesn't fly off uncontrollably. I love how fluid this feels, much better than a lot of other apps I've tried!

@Keavon Keavon changed the title Add code to shift viewport if mouse is beyond edge in Select Tool Add auto-panning when pointer goes beyond viewport edge with the Select tool Feb 29, 2024
@Keavon Keavon changed the title Add auto-panning when pointer goes beyond viewport edge with the Select tool Add auto-panning when pointer goes beyond viewport edge with Select tool Feb 29, 2024
@Keavon Keavon enabled auto-merge (squash) February 29, 2024 07:48
@Keavon Keavon merged commit 66cd73d into GraphiteEditor:master Feb 29, 2024
2 checks passed
Keavon added a commit that referenced this pull request Feb 29, 2024
…ool (#1625)

* Add code to shift viewport if mouse is beyond edge

* Allow shifting viewport if mouse is stationary too

* Group all modifier keys into SelectToolPointerKeys

* Cleanup message ordering to remove shifting during resize

* Slow down shifting by half

* Clamp speed; code review cleanup

---------

Co-authored-by: 0hypercube <0hypercube@gmail.com>
Co-authored-by: Keavon Chambers <keavon@keavon.com>
Keavon added a commit that referenced this pull request Feb 29, 2024
…ool (#1625)

* Add code to shift viewport if mouse is beyond edge

* Allow shifting viewport if mouse is stationary too

* Group all modifier keys into SelectToolPointerKeys

* Cleanup message ordering to remove shifting during resize

* Slow down shifting by half

* Clamp speed; code review cleanup

---------

Co-authored-by: 0hypercube <0hypercube@gmail.com>
Co-authored-by: Keavon Chambers <keavon@keavon.com>
@elbertronnie elbertronnie deleted the shift-viewport-mouse-beyond-edge branch April 14, 2024 14:24
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.

Autoscroll when mouse is outside viewport and dragging
3 participants