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

Upgrade to > Qt5.15 -6 and ImGUI > v1.87 + fixes keyboard and mouse handling & widget samples #44

Merged
merged 8 commits into from
Apr 14, 2023

Conversation

moezb
Copy link

@moezb moezb commented Oct 14, 2022

No description provided.

@moezb moezb changed the title Upgrade to > Qt5.15 -6 and ImGUI > v1.87 + fixes keyboard in widget mode Upgrade to > Qt5.15 -6 and ImGUI > v1.87 + fixes keyboard and mouse handling & widget samples Oct 26, 2022
…I v1.87

This also fixes a couple of problems with mouse clik/double click.

# We're using qt versionless targets so our minimal Qt version is 5.15, we support Qt6 as well
# See https://www.qt.io/blog/versionless-cmake-targets-qt-5.15
find_package(Qt6 COMPONENTS COMPONENTS Core Quick Gui Widgets OpenGLWidgets )
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra space before the parenthesis and before "Core".

endif()
endif()

message(STATUS Qt ${QT_DEFAULT_MAJOR_VERSION})
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This message should be a bit clearer to the user. Like "Compiling with Qt X". I saw it, and was just a bit puzzled.

@@ -21,7 +32,10 @@ target_link_libraries(
qt_imgui_quick
PUBLIC
imgui
Qt5::Gui
Qt::Gui
# We use the new Qt Rendering Hardware Interface (RHI) when buzilding with Qt and thgus we need openglwidgets qt module
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/buzilding with Qt and thgus/building with Qt and thus/

@@ -34,7 +48,10 @@ target_link_libraries(
qt_imgui_widgets
PUBLIC
imgui
Qt5::Widgets
Qt::Widgets
# We use the new Qt Rendering Hardware Interface (RHI) when buzilding with Qt and thgus we need openglwidgets qt module
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.

#endif

#ifdef Q_OS_LINUX
#include "platform/linux_key_mappings.cpp"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The one for linux is entirely empty. Is there a recipe one can follow to make the mappings? I checked the backend directory on ImGui, but I was expecting something like X11/Wayland, and that doesn't seem to be the case.


DemoWidget *widget=new DemoWidget();
widget->setWindowTitle("QtImGui widget example");
QWidget* window=new QWidget();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this should be heap allocated. It seems to be leaking right now.


switch (event->type()) {
case QEvent::MouseMove:
onMouseMove(dynamic_cast<QMouseEvent*>(event));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This cast, and all of the above, can be static_cast.

case QMouseEvent::Leave:
onMouseLeave();
break;
case QEvent::MouseButtonDblClick: case QEvent::MouseButtonPress:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably split the line, so there is one case per line.

@suy
Copy link
Collaborator

suy commented Apr 11, 2023

The PR doesn't compile yet on Qt 5 and/or Linux. I'll see if I can provide a linux implementation for the key mapping. For the compile errors on Qt 5, I needed to apply this changes:

diff --git i/src/ImGuiRenderer.cpp w/src/ImGuiRenderer.cpp
index da7c76f..306e088 100644
--- i/src/ImGuiRenderer.cpp
+++ w/src/ImGuiRenderer.cpp
@@ -5,7 +5,7 @@
 #include <QMouseEvent>
 #include <QClipboard>
 #include <QCursor>
-#include <QWidget> 
+#include <QDebug>
 
 
 #ifdef ANDROID
@@ -347,7 +347,7 @@ void ImGuiRenderer::onMouseMove(QMouseEvent* event) {
     assert(event != nullptr);
     ImGuiIO& io = ImGui::GetIO();
     if (!MouseTracked)  MouseTracked = true;
-    MousePos = ImVec2(static_cast<float>(event->position().x()), static_cast<float>(event->position().y()));
+    MousePos = ImVec2(static_cast<float>(event->pos().x()), static_cast<float>(event->pos().y()));
     io.AddMousePosEvent(MousePos.x, MousePos.y);
 }
 
diff --git i/src/ImGuiRenderer.h w/src/ImGuiRenderer.h
index 6c29f84..4ea6d42 100644
--- i/src/ImGuiRenderer.h
+++ w/src/ImGuiRenderer.h
@@ -1,5 +1,6 @@
 #pragma once
 
+#include <QWindow> // WId
 #include <QOpenGLExtraFunctions>
 #include <QObject>
 #include <QPoint>
diff --git i/src/QtImGui.cpp w/src/QtImGui.cpp
index 5da2c1b..ebb812a 100644
--- i/src/QtImGui.cpp
+++ w/src/QtImGui.cpp
@@ -83,7 +83,7 @@ public:
     }
 
     QPointF mapToGlobal(const QPointF& pos) const override {
-        return w->mapToGlobal(pos);
+        return w->mapToGlobal(pos.toPoint());
     }
 
     WId nativeHandle() const override {
@@ -166,7 +166,7 @@ public:
     }
      
     QPointF mapToGlobal(const QPointF& pos) const override {
-        return w->mapToGlobal(pos);
+        return w->mapToGlobal(pos.toPoint());
     }
     
     WId nativeHandle() const override {

@seanchas116 seanchas116 changed the base branch from master to upgrade-imgui April 14, 2023 10:03
@seanchas116 seanchas116 merged commit 3ce55a3 into seanchas116:upgrade-imgui Apr 14, 2023
@seanchas116
Copy link
Owner

Looks like it doesn't compile on some platforms so I merged it to the separate upgrade-imgui branch.

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 this pull request may close these issues.

None yet

3 participants