Skip to content

Commit

Permalink
ImGuiIntegration: ensure too fast mouse clicks aren't ignored.
Browse files Browse the repository at this point in the history
If both a press and a release happened in the same frame, ImGui wouldn't
know about them.

Co-authored-by: Marco Melorio <m.melorio@icloud.com>
  • Loading branch information
mosra and Marco Melorio committed Oct 21, 2019
1 parent 045fda0 commit 84fa6d8
Show file tree
Hide file tree
Showing 5 changed files with 76 additions and 4 deletions.
4 changes: 4 additions & 0 deletions doc/changelog-integration.dox
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ namespace Magnum {
- @ref ImGuiIntegration::Context::handleKeyPressEvent() "ImGuiIntegration::Context::handleKey*Event()"
now handles the @m_class{m-label m-default} **Space** key as well (see
[mosra/magnum-integration#44](https://github.com/mosra/magnum-integration/pull/44))
- @ref ImGuiIntegration::Context::handleMousePressEvent() and
@ref ImGuiIntegration::Context::handleMouseReleaseEvent() are fixed to
avoid ignoring mouse clicks if both press and release happens in the same
frame
- Assorted @ref DartIntegration updates (see
[mosra/magnum-integration#47](https://github.com/mosra/magnum-integration/pull/47)):
- @ref DartIntegration::convertShapeNode() can now handle cones as well
Expand Down
13 changes: 12 additions & 1 deletion src/Magnum/ImGuiIntegration/Context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -248,9 +248,20 @@ void Context::newFrame() {

_timeline.nextFrame();

ImGui::GetIO().DeltaTime = _timeline.previousFrameDuration();
ImGuiIO& io = ImGui::GetIO();
io.DeltaTime = _timeline.previousFrameDuration();

/* Fire delayed mouse events. This sets MouseDown both in case the press
happened in this frame but also if both press and release happened at
the same frame */
for(const Int buttonId: {0, 1, 2})
io.MouseDown[buttonId] = _mousePressed[buttonId] || _mousePressedInThisFrame[buttonId];

ImGui::NewFrame();

/* It's a new frame, clear any indicators for received mouse presses in
this frame */
_mousePressedInThisFrame = {};
}

void Context::drawFrame() {
Expand Down
1 change: 1 addition & 0 deletions src/Magnum/ImGuiIntegration/Context.h
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,7 @@ class MAGNUM_IMGUIINTEGRATION_EXPORT Context {
GL::Mesh _mesh;
Vector2 _supersamplingRatio,
_eventScaling;
BoolVector3 _mousePressed, _mousePressedInThisFrame;

private:
template<class KeyEvent> bool handleKeyEvent(KeyEvent& event, bool value);
Expand Down
15 changes: 12 additions & 3 deletions src/Magnum/ImGuiIntegration/Context.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -140,21 +140,30 @@ template<class MouseEvent> bool Context::handleMouseEvent(MouseEvent& event, boo
ImGuiIO& io = ImGui::GetIO();
io.MousePos = ImVec2(Vector2(event.position())*_eventScaling);

std::size_t buttonId;
switch(event.button()) {
case MouseEvent::Button::Left:
io.MouseDown[0] = value;
buttonId = 0;
break;
case MouseEvent::Button::Right:
io.MouseDown[1] = value;
buttonId = 1;
break;
case MouseEvent::Button::Middle:
io.MouseDown[2] = value;
buttonId = 2;
break;

/* Unknown button, do nothing */
default: return false;
}

/* Instead of setting io.MouseDown directly, we delay this until the
newFrame() call in order to prevent mouse clicks from being ignored when
both a press and a release happens in the same frame. Apart from this
happening when the app can't render fast enough, for some reason it also
happens with SDL2 on macOS -- press delayed by a significant */
_mousePressed.set(buttonId, value);
if(value) _mousePressedInThisFrame.set(buttonId, true);

return io.WantCaptureMouse;
}

Expand Down
47 changes: 47 additions & 0 deletions src/Magnum/ImGuiIntegration/Test/ContextGLTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ struct ContextGLTest: GL::OpenGLTester {
void relayoutRefreshFonts();

void mouseInput();
void mouseInputTooFast();
void keyInput();
void textInput();

Expand All @@ -149,6 +150,7 @@ ContextGLTest::ContextGLTest() {
&ContextGLTest::relayoutRefreshFonts,

&ContextGLTest::mouseInput,
&ContextGLTest::mouseInputTooFast,
&ContextGLTest::keyInput,
&ContextGLTest::textInput,

Expand Down Expand Up @@ -477,29 +479,52 @@ void ContextGLTest::mouseInput() {
MouseEvent right{Button::Right, {3, 4}, {}};
MouseEvent middle{Button::Middle, {5, 6}, {}};

/* In order to avoid imgui ignoring mouse clicks if both press and release
happens in the same frame, the events are propagated to it only during
newFrame() -- which means we need to check *after* it gets called. See
mouseInputTooFast() below for more. */

c.handleMousePressEvent(left);
Utility::System::sleep(1);
c.newFrame();
CORRADE_VERIFY(ImGui::IsMouseDown(0)); /* left */
CORRADE_COMPARE(Vector2(ImGui::GetMousePos()), (Vector2{1.0f, 2.0f}));
c.drawFrame();

c.handleMousePressEvent(right);
Utility::System::sleep(1);
c.newFrame();
CORRADE_VERIFY(ImGui::IsMouseDown(1)); /* right */
CORRADE_COMPARE(Vector2(ImGui::GetMousePos()), (Vector2{3.0f, 4.0f}));
c.drawFrame();

c.handleMousePressEvent(middle);
Utility::System::sleep(1);
c.newFrame();
CORRADE_VERIFY(ImGui::IsMouseDown(2)); /* middle */
CORRADE_COMPARE(Vector2(ImGui::GetMousePos()), (Vector2{5.0f, 6.0f}));
c.drawFrame();

c.handleMouseReleaseEvent(right);
Utility::System::sleep(1);
c.newFrame();
CORRADE_VERIFY(!ImGui::IsMouseDown(1)); /* right */
CORRADE_COMPARE(Vector2(ImGui::GetMousePos()), (Vector2{3.0f, 4.0f}));
c.drawFrame();

c.handleMouseReleaseEvent(left);
Utility::System::sleep(1);
c.newFrame();
CORRADE_VERIFY(!ImGui::IsMouseDown(0)); /* left */
CORRADE_COMPARE(Vector2(ImGui::GetMousePos()), (Vector2{1.0f, 2.0f}));
c.drawFrame();

c.handleMouseReleaseEvent(middle);
Utility::System::sleep(1);
c.newFrame();
CORRADE_VERIFY(!ImGui::IsMouseDown(2)); /* middle */
CORRADE_COMPARE(Vector2(ImGui::GetMousePos()), (Vector2{5.0f, 6.0f}));
c.drawFrame();

/* Mouse Movement */
MouseEvent move{Button{}, {1, 2}, {}};
Expand All @@ -522,6 +547,28 @@ void ContextGLTest::mouseInput() {
CORRADE_VERIFY(!c.handleMouseReleaseEvent(unknownButton));
}

void ContextGLTest::mouseInputTooFast() {
Context c{{200, 200}};

CORRADE_VERIFY(!ImGui::IsMouseDown(0));

/* It's not reported immediately */
MouseEvent left{Button::Left, {1, 2}, {}};
c.handleMousePressEvent(left);
c.handleMouseReleaseEvent(left);
CORRADE_VERIFY(!ImGui::IsMouseDown(0));

/* Only during the newFrame call */
c.newFrame();
CORRADE_VERIFY(ImGui::IsMouseDown(0));
c.drawFrame();

/* And mouse up is reported in the next one */
c.newFrame();
CORRADE_VERIFY(!ImGui::IsMouseDown(0));
c.drawFrame();
}

void ContextGLTest::keyInput() {
Context c{{}};

Expand Down

0 comments on commit 84fa6d8

Please sign in to comment.