epezent / implot

Immediate Mode Plotting
MIT License
4.55k stars 503 forks source link

Ability to query point, line and rect drags for their click, hovered and held states #507

Closed pozemka closed 11 months ago

pozemka commented 11 months ago

Hello again! Following #506 PR.

To be able to query states of draggable items I've added new optional arguments:

IMPLOT_API bool DragPoint(int id, double* x, double* y, const ImVec4& col, float size = 4, ImPlotDragToolFlags flags = 0, 
    bool* clicked = nullptr, bool* hovered = nullptr, bool* held = nullptr);

When clicked, hovered or held provided it will be set to results of ImGui::ButtonBehavior function.

Example code:

bool is_clicked = false;
ImPlot::DragPoint(key.id, &x, &y, ImVec4(1.0f, 0.0f, 1.0f, 1.0f), line_weight, 0, &is_clicked);
if (is_clicked)
{
  //process click
}
epezent commented 11 months ago

Three comments:

  1. Can you add comment to these functions in the header: "This function returns true when user interaction causes the provided coordinates to change. Additional user interactions can be retrieved through the optional output parameters."
  2. Let's name the arguments out_clicked, out_hovered, out_held to be consistent with ImGui::ButtonBehavior
  3. DragRect calls ImGui::ButtonBehavior button behavior five times -- once for the center, and once for each corner. We need to handle this somehow. A couple options:
    • change the output args to bool out_xxx[5]
    • aggregate the outputs across all calls to ButtonBehavior, e.g. *is_clicked = *is_clicked || clicked;
    • just report the state change of the center, as is

The first option feels heavy, the last one feels misleading. So perhaps the second option is the best generalization. Also, DragRect might need to report "hovered" when the cursor is inside the rect (but not necessarily hovering one of the control points).

Thoughts?

pozemka commented 11 months ago
  1. Comments added
  2. Arguments renamed
  3. DragRect is tricky. 🤔 In my opinion most useful would be to know if it was clicked anywhere. Actually I can imagine scenarios when user would be interested in clicks on corner points or edges but then we could also imagine need for a RMB, Scroll etc. So I think it is better to provide facility to handle most common case.

I've added handling of events on entire rectangle:

if (input) {
    // Entire rectangle
    ImGuiID r_id = id + 5;
    ImGui::KeepAliveID(r_id);
    bool is_hovered = false;
    bool is_held = false;
    bool is_clicked = ImGui::ButtonBehavior(rect, r_id, &is_hovered, &is_held);
    if (out_clicked)
        *out_clicked = is_clicked;
    if (out_hovered)
        *out_hovered = is_hovered;
    if (out_held)
        *out_held = is_held;
}

But I not sure about the r_id value for this ButtonBehavior. In the loop earlier it had values id+1 .. id+4 so id + 5 seems correct?

Also I have updated example code to print info about state of draggable items (only one item per plot though): image

epezent commented 11 months ago

@pozemka, I just pushed a few changes:

  1. We can't use a ButtonBehavior for the entire rect because it prevents the mouse from passing through and interacting with the plot. This is problematic if the rect covers or exceeds the plot extents (basically makes the entire plot unusable). But, I agree with you on the rect needing to provide click/hover events for its interior, so I added some custom logic to handle this case.
  2. I updated the demo examples to show something a bit more interesting/practical (changes line thickness when hovered etc.)
  3. Just a few preferential changes to the documentation and code.

Let me know what you think and we'll merge this... dragrect

pozemka commented 11 months ago

I've looked through your commit. Everything seems fine. My initial thought was thath this code

const bool mouse_inside = rect_grab.Contains(ImGui::GetMousePos());
const bool mouse_clicked = ImGui::IsMouseClicked(0);
if (input && mouse_inside) {
    if (out_clicked) *out_clicked = *out_clicked || mouse_clicked;
    if (out_hovered) *out_hovered = true;        
}

Makes previous hover/click checks unnecessary. But parts of the grab points are outside of the rect so they should be handled too. Only thing I could imagine as missing is that the rect itself has no held state (grab points have it). So I have added ImGui::IsMouseDown.

epezent commented 11 months ago

So I have added ImGui::IsMouseDown.

Yea, I was on the fence about doing that. Seemed a little weird to say the rect is held when the user isn't necessarily holding a moveable element. But, for consistency, I think it's the right call.

Merging ...