Boxed zoom takes wrong mouse location as a starting point for the rectangle

Open Beginner friendly
#248 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
1/5
Estimated time
Under an hour
Newbie friendliness
90/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Quiet
Tech stack
rust

Research direction

Read /src/plot.rs around lines 1132 and 1162, comparing the existing axis-drag coordinate handling with boxed zoom. Update the boxed-zoom starting position to use the pointer's press origin, then verify that right-click dragging begins the rectangle at the original mouse-down location.

Written by the indexing model from the issue text.

Description

bug

When zooming into a plot by right click + drag, the zoom does not take the mouse location where the user pressed down, but instead the mouse location where egui has decided that this is a drag action. But egui waits for a few pixels of mouse movement before declaring it as dragged. As a consequence it is very hard to place the initial corner of the zoom correctly. The issue is worse in slow applications, where it is easy to move the mouse a large distance between two consecutive frames.

The source of the bug is /src/plot.rs:1162:

// Save last click to allow boxed zooming
if response.drag_started() && response.dragged_by(self.boxed_zoom_pointer_button) {
    // it would be best for egui that input has a memory of the last click pos
    // because it's a common pattern
    mem.last_click_pos_for_zoom = response.hover_pos();
}

response.hover_pos() is not the position that we want here. The solution is simple:

// Save last click to allow boxed zooming
if response.drag_started() && response.dragged_by(self.boxed_zoom_pointer_button) {
    // it would be best for egui that input has a memory of the last click pos
    // because it's a common pattern
    mem.last_click_pos_for_zoom = ui.input(|i| i.pointer.press_origin());
}

I was initially worried about potential deadlocks, because ui.input() has a big warning about it, but the same code to get the proper mouse coordinates is already present in /src/plot.rs:1132 for zooming by dragging an axis, so it does not seem to be an issue.

(sorry for not forking, cloning, commiting, pushing and making a pull request. That's a lot of work for a simple one line change. I hope this is fine too)

Dominant language
Rust
Stars
467
Forks
107
Avg merge
15m
Merged PRs (30d)
3

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from emilk/egui_plot

All issues in emilk/egui_plot

Similar issues

More Rust issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.