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

Unwanted output from Selectable() when using DoubleClick flag #2503

Closed
agersant opened this issue Apr 20, 2019 · 6 comments
Closed

Unwanted output from Selectable() when using DoubleClick flag #2503

agersant opened this issue Apr 20, 2019 · 6 comments
Labels

Comments

@agersant
Copy link

agersant commented Apr 20, 2019

Version/Branch of Dear ImGui:

Version: 1.66b
Branch: master

Back-end/Renderer/Compiler/OS

Back-ends: imgui-gfx-renderer (provided by imgui-rs Rust bindings)
Compiler: MSVC
Operating System: Windows

My Issue/Question:

In my program using imgui, I show users a list of items which can be selected (single click) or edited (double click). I use Selectable() to represent these items, with the AllowDoubleClick flag enabled.

When performing a double click on any of the items, the Selectable() function returns true on three distinct frames instead of the expected two.

After following the corresponding imgui code path, I ended up reading the explanations at the top of ButtonBehavior. My understanding from it is that the three frames returning true are:

  • Frame N+6 for PressedOnClickRelease (first click)
  • Frame N+4 for PressedOnDoubleClick (second click)
  • Frame N+6 for PressedOnClickRelease (second click)

I am not sure whether that is intended behavior or not. If it is intended, I would like to hear suggestions on how I could discard the third true output (which has unwanted consequences in my program). I did not see anything in the ButtonBehavior reference table which could help.

Screenshots/Video

N/A

Standalone, minimal, complete and verifiable example: (see #2261)
Rust code below. Hopefully this is fairly transparent considering the bindings are mostly pass-through to the C++ API:

let mut flags = ImGuiSelectableFlags::empty();
flags.set(ImGuiSelectableFlags::AllowDoubleClick, true);
if ui.selectable(
	im_str!("Foo"),
	false,
	flags,
	ImVec2::new(0.0, 0.0),
) {
	println!("SELECTABLE");
	if ui.imgui().is_mouse_double_clicked(ImMouseButton::Left) {
		println!("DOUBLE CLICK");
	} else if {
		println!("CLICK");
	}
}

I would expect the following output when double clicking the item:

SELECTABLE
CLICK
SELECTABLE
DOUBLE CLICK

Instead I get:

SELECTABLE
SINGLE CLICK
SELECTABLE
DOUBLE CLICK
SELECTABLE
SINGLE CLICK

Many thanks in advance!

@ocornut
Copy link
Owner

ocornut commented Apr 20, 2019

Hello Antoine,

I have confirmed the issue and will look into it. I don't know what's the best course of action to solve this yet.

I did not see anything in the ButtonBehavior reference table which could help.

It's indeed happening because Selectable() call ButtonBehavior() with 2 flags and the combinatioin is not explicitely described in the comments.

Btw it's not clear if you looked at the 1.66 version or the latest one, but note that in recent 1.70 WIP commits this section got a lots of details added. Either way this is not solving your problem!

@ocornut
Copy link
Owner

ocornut commented Apr 20, 2019

Here's the patch to make ButtonBehavior() with both ImGuiButtonFlags_PressedOnClickRelease | ImGuiButtonFlags_PressedOnDoubleClick` flags not return true on the second mouse release.

patch.zip

I believe this is the right solution as I don't think users could have been meaningfully relying on the current behavior if they tried to test for single click. I'll sit on this to think for a bit of possible side-efects, feedback welcome!

@agersant
Copy link
Author

Thank you for jumping on this so quickly! The solution looks good to me, although I don't have a good way to test it until it makes its way to cimgui and imgui-rs (but I'm very patient).

@ocornut
Copy link
Owner

ocornut commented Apr 21, 2019

@agersant cimgui is auto generated so you should be able to rebuild it.
(and imgui-rs should be generated from cimgui metadata lua/json output if it's not already)

@agersant
Copy link
Author

imgui-rs is manually written (imgui-rs/imgui-rs#139 (comment)) and lagging behind cimgui so testing isn't quite as straightforward :(

I'm under no time constraints so I'm happy as long as a fix eventually gets merged.

ocornut added a commit that referenced this issue May 3, 2019
… true on the mouse button releas efollowing the double-click. Only first mouse release + second mouse down (double-click) returns true. Likewise for internal ButtonBehavior() with both _PressedOnClickRelease | _PressedOnDoubleClick. (#2503)
@ocornut
Copy link
Owner

ocornut commented May 3, 2019

This is now merged. Added some tests to clarify this behavior as well.

@ocornut ocornut closed this as completed May 3, 2019
@ocornut ocornut added the inputs label May 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants