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

Clicks not registered #37

Closed
adcox opened this issue Jun 14, 2016 · 6 comments
Closed

Clicks not registered #37

adcox opened this issue Jun 14, 2016 · 6 comments

Comments

@adcox
Copy link
Contributor

adcox commented Jun 14, 2016

Compiled and ran example-demo on OS X 10.11.5 Mid-2015 Macbook Pro. The trackpad on these laptops has two click "types": one is triggered with a light tap, the other is triggered by a more forceful push. The second option always has the desired effect of clicking on the ImGui element. The first option, however, does not. Often the click is not registered by ImGui and the button isn't pressed or the menu item doesn't expand.

I added a logger line to the mouse press event in the ofApp::mousePressed function and it catches every single mouse press. I also tested the vanilla ImGui examples (https://github.com/ocornut/imgui) and they work perfectly.

This leads me to believe that there is some synchronization issue between the OF window detecting the mouse event and that event being passed down to the BaseEngine and other classes. A more forceful click lasts longer, so perhaps it is picked up over multiple frames and transmitted properly.

@jvcleave
Copy link
Owner

can you see if it makes it here?

int button = event.button;

@adcox
Copy link
Contributor Author

adcox commented Jun 14, 2016

It does make it there; a print statement located just before the if() statement is hit twice for every mouse click, regardless of whether it is light, firm, and whether or not the gui makes the appropriate reaction.

@jvcleave
Copy link
Owner

So using this on my Macbook Pro (2012ish) with "Tap to Click" set in the preferences

void EngineGLFW::onMousePressed(ofMouseEventArgs& event)
{
    ofLogVerbose() << "EngineGLFW::onMousePressed: " << ofGetFrameNum();
    int button = event.button;
    if(button >= 0 && button < 5)
    {
        remapToGLFWConvention(button);
        ImGuiIO& io = ImGui::GetIO();
        io.MouseDown[button] = true;
    }
}

void EngineGLFW::onMouseReleased(ofMouseEventArgs& event)
{
    ofLogVerbose() << "EngineGLFW::onMouseReleased: " << ofGetFrameNum();

What appears to be happening is that the onMousePressed/onMouseReleased are called on the same frame with the "short clicks"

Setting
ofSetVerticalSync(false);
in ofApp seems to solve it or at least make it much harder

@jvcleave
Copy link
Owner

However I am not seeing a double hit on "EngineGLFW::onMousePressed:" like you are with your print statement

@adcox
Copy link
Contributor Author

adcox commented Jun 14, 2016

I implemented your logging and can confirm that the cases that don't work correspond to both MousePressed and MouseReleased being called in the same frame. The double hit was due to some code I was playing around with; I removed it and get only a single hit for each click now.

In my opinion, a more elegant solution is to implement some logic that queues the mouse events such that MousePressed and MouseReleased are called in successive frames rather than the same frame so that io.MouseDown[button] has time to be interpreted. I'm not sure if this is within the scope of your code; perhaps it is something I should implement at the App level? Placing the print statements in the ofApp.cpp event functions results in very similar results, although there are cases where both the Pressed() and Released() events occurred in the same from but the click was successfully registered.

Thanks for tracking that down!

@jvcleave
Copy link
Owner

yeah - I really am trying to stay out of implementing logic on behalf of the user. The addon's intention is really just to make it a bit easier to get ImGui working with OF (and on the platforms that OF supports).

I know that ImGui's examples run with vertical sync off and OF has it on by default. I guess if it were really bothering someone on a certain platform they could just extend BaseEngine for specific tweaks (or just modify the existing one)

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 a pull request may close this issue.

2 participants