Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/OgreWidget.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,8 @@
if (e->type() == QEvent::NativeGesture) {
auto *gestureEvent = static_cast<QNativeGestureEvent*>(e);
if (gestureEvent->gestureType() == Qt::ZoomNativeGesture) {
int delta = static_cast<int>(gestureEvent->value() * 200 * mCamera->getCameraSpeed());
// value() is typically ±0.01..0.1 per gesture event; scale to usable range
Ogre::Real delta = static_cast<Ogre::Real>(gestureEvent->value() * 5.0);

Check warning on line 272 in src/OgreWidget.cpp

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Replace the redundant type with "auto".

See more on https://sonarcloud.io/project/issues?id=fernandotonon_QtMeshEditor&issues=AZz5DRTR5wKcoBid-yO0&open=AZz5DRTR5wKcoBid-yO0&pullRequest=198
mCamera->zoomByDelta(delta);
e->accept();
return true;
Expand Down
50 changes: 42 additions & 8 deletions src/SpaceCamera.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -218,10 +218,27 @@

void SpaceCamera::wheelEvent(QWheelEvent *event)
{
int delta = event->angleDelta().y() * 10 / 60 * getCameraSpeed();
zoom(delta);
Ogre::Real xDelta = event->angleDelta().x() / 120.0f;

Check warning on line 221 in src/SpaceCamera.cpp

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

implicit conversion from 'int' to 'float' may lose precision

See more on https://sonarcloud.io/project/issues?id=fernandotonon_QtMeshEditor&issues=AZz5DROi5wKcoBid-yOy&open=AZz5DROi5wKcoBid-yOy&pullRequest=198
Ogre::Real yDelta = event->angleDelta().y() / 120.0f;

Check warning on line 222 in src/SpaceCamera.cpp

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

implicit conversion from 'int' to 'float' may lose precision

See more on https://sonarcloud.io/project/issues?id=fernandotonon_QtMeshEditor&issues=AZz5DROi5wKcoBid-yOz&open=AZz5DROi5wKcoBid-yOz&pullRequest=198

pan(Ogre::Vector2 ( event->angleDelta().x() * 10 / 60 * getCameraSpeed() , 0.0 ));
if (event->modifiers().testFlag(Qt::ControlModifier))
{
// Ctrl + scroll = zoom (for mouse wheel users)
zoom(yDelta);
}
else if (event->source() == Qt::MouseEventSynthesizedBySystem)
{
// Trackpad two-finger swipe = pan in both axes
Ogre::Real panScale = 3.0f;
pan(Ogre::Vector2(xDelta * panScale, yDelta * panScale));
}
Comment on lines +229 to +234
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

In Qt (5 and 6), is QWheelEvent::source() == Qt::MouseEventSynthesizedBySystem a reliable way to detect two-finger touchpad scroll across macOS, Windows, and Linux? What is the recommended cross-platform detection signal (e.g., pixelDelta, pointingDevice type, phase)?

💡 Result:

No. QWheelEvent::source() == Qt::MouseEventSynthesizedBySystem is not a reliable (or portable) “two‑finger touchpad scroll” detector across macOS/Windows/Linux (and not even reliably on macOS): Qt explicitly warns that it was “tempting and wrong” to equate “synthesized” mouse events with “from touch”, and in practice both mouse-wheel and touchpad wheel events can report MouseEventSynthesizedBySystem. [1], [2]

Recommended cross‑platform approach (Qt 5 & Qt 6): don’t detect “touchpad”, detect high‑resolution scrolling

Use the wheel event properties that describe how the scroll is delivered, and handle both smoothly:

  1. Prefer pixelDelta() when non-null (high‑resolution / touch-like scrolling); otherwise fall back to angleDelta() (traditional wheel “steps”). [3]
  2. Use phase() (when provided) to know whether you’re starting/updating/ending a scroll sequence; for non‑phase‑aware mice it is NoScrollPhase. [4]
  3. Use inverted() to respect “natural scrolling” direction (trackpads, some mice). (Documented alongside wheel event construction in Qt docs.) [3]

Qt 6: if you truly need device-based detection, use the pointing device (not source())

In Qt 6, QWheelEvent carries a QPointingDevice* device, and Qt documents that for touchpad-originated wheel events device()->type() will be QInputDevice::DeviceType::Touchpad. [3]
That’s the closest Qt provides to an explicit “this was a touchpad” classification; availability/accuracy still depends on platform backend/device discovery, but it’s the intended API in Qt 6—not source(). [1], [3]

Practical rule of thumb (best portability):

  • Treat pixelDelta()!=QPoint(0,0) as “smooth/pixel scrolling” (often touchpad / precision scrolling), and otherwise treat it as a wheel. [3]
  • Use device()->type()==Touchpad only as an optional refinement in Qt 6. [3]

Sources:
[1] Qt blog (“Input Events in Qt 6”) on why QMouseEvent::source() was misleading for inferring touch origin (qt.io)
[2] StackOverflow report that both real mouse wheel and touchpad can show MouseEventSynthesizedBySystem (stackoverflow.com)
[3] QWheelEvent docs (Qt 6) including pixelDelta/angleDelta/phase/source/device() and note about device()->type()==Touchpad (doc.qt.io)
[4] Qt 5.7 changelog note: QWheelEvent::phase() returns NoScrollPhase for non-phase-aware mice (sources.debian.org)

Citations:


🏁 Script executed:

# Find and examine SpaceCamera.cpp around lines 229-234
fd SpaceCamera.cpp --type f

Repository: fernandotonon/QtMeshEditor

Length of output: 89


🏁 Script executed:

# Get the size and examine the wheel event handling context
wc -l src/SpaceCamera.cpp

Repository: fernandotonon/QtMeshEditor

Length of output: 93


🏁 Script executed:

# Read the wheel event handling context
cat -n src/SpaceCamera.cpp | sed -n '200,260p'

Repository: fernandotonon/QtMeshEditor

Length of output: 1976


🏁 Script executed:

# Check Qt version and includes to understand which Qt version is being used
rg "QT_VERSION|include.*Qt.*Core|QWheelEvent" src/SpaceCamera.cpp -A 2

Repository: fernandotonon/QtMeshEditor

Length of output: 226


🏁 Script executed:

# Check Qt version in CMakeLists.txt or project configuration
fd "CMakeLists.txt|\.pro|\.pri|conanfile" --type f | head -5

Repository: fernandotonon/QtMeshEditor

Length of output: 187


🏁 Script executed:

# Check Qt version in CMakeLists.txt
rg "find_package.*Qt|QT_VERSION|qt_version" CMakeLists.txt -i

Repository: fernandotonon/QtMeshEditor

Length of output: 1502


Touchpad detection via MouseEventSynthesizedBySystem is unreliable; use pixel-delta as fallback.

Qt's MouseEventSynthesizedBySystem flag is not reliable for distinguishing touchpad events across macOS, Windows, and Linux—both real mouse wheel and touchpad scroll can report it, and some platforms may not set it at all. This causes touchpad scrolling to incorrectly fall through to zoom (line 238) instead of panning.

Harden detection by checking for high-resolution scrolling:

Fix: Add pixel-delta fallback
-    else if (event->source() == Qt::MouseEventSynthesizedBySystem)
+    else if (event->source() == Qt::MouseEventSynthesizedBySystem
+             || !event->pixelDelta().isNull())
     {
         // Trackpad two-finger swipe = pan in both axes
         Ogre::Real panScale = 3.0f;
         pan(Ogre::Vector2(xDelta * panScale, yDelta * panScale));
     }

Qt 6 (which this project uses) documents that pixelDelta() indicates high-resolution/touch-like scrolling (typically trackpad), while angleDelta() is traditional wheel "steps." Falling back to pixel-delta detection ensures portable touchpad handling.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/SpaceCamera.cpp` around lines 229 - 234, The touchpad detection using
event->source() == Qt::MouseEventSynthesizedBySystem is unreliable; change the
logic in the scroll handling branch (the block that currently calls
pan(Ogre::Vector2(xDelta * panScale, yDelta * panScale)) and later falls through
to zoom) to also check event->pixelDelta() (Qt 6 high-resolution scrolling) as a
fallback: treat non-zero pixelDelta() (or a sufficiently large pixelDelta
magnitude) as a touchpad/trackpad scroll and call pan(...) with the scaled pixel
deltas, otherwise fall back to angleDelta() behavior (wheel steps) and call
zoom(...). Update references around MouseEventSynthesizedBySystem, pixelDelta(),
angleDelta(), pan(), and zoom() accordingly so touchpad scrolling consistently
triggers panning across platforms.

else
{
// Mouse scroll wheel = zoom
zoom(yDelta);
if (std::abs(xDelta) > 0.01f)
pan(Ogre::Vector2(xDelta * 3.0f, 0.0f));
}

event->accept();
}
Expand Down Expand Up @@ -331,20 +348,37 @@
//////////////////////////////////////////////////////////////////////////////////
//Private Methods

void SpaceCamera::zoomByDelta(int delta)
void SpaceCamera::zoomByDelta(Ogre::Real delta)
{
zoom(delta);
}

void SpaceCamera::zoom(const int delta)
void SpaceCamera::zoom(Ogre::Real delta)
{
Ogre::Vector3 zTranslation(0, 0, delta);
mCameraNode->translate(zTranslation,Ogre::Node::TS_PARENT);
// Proportional zoom: step size scales with distance to target,
// so zooming feels consistent regardless of model size.
Ogre::Real distance = std::abs(mCameraNode->getPosition().z);
Ogre::Real minDistance = 0.01f;

Choose a reason for hiding this comment

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

P2 Badge Clamp zoom minimum to camera near clip distance

Using a hardcoded minDistance = 0.01f lets the camera stop much closer to the target than the configured near clip plane (setNearClipDistance(1.0f) in SpaceCamera::SpaceCamera), so zooming in can place the focal region inside the near plane and make geometry disappear/clamp oddly at close range. This regression comes from the new minimum-distance clamp; it should be derived from the active near clip distance (or another scene-scale-aware lower bound) instead of 0.01.

Useful? React with 👍 / 👎.

Ogre::Real zoomFactor = 0.15f; // 15% of distance per scroll tick

Ogre::Real step = delta * std::max(distance, minDistance) * zoomFactor;

Ogre::Vector3 newPos = mCameraNode->getPosition();
newPos.z += step;

// Don't pass through the target
if (newPos.z > -minDistance)
newPos.z = -minDistance;

mCameraNode->setPosition(newPos);
}

void SpaceCamera::pan(const Ogre::Real& deltaX, const Ogre::Real& deltaY)
{
mTarget->translate(deltaX,deltaY,0,Ogre::Node::TS_LOCAL );
// Proportional pan: step scales with distance to target
Ogre::Real distance = std::abs(mCameraNode->getPosition().z);
Ogre::Real scale = std::max(distance, 0.01f) * 0.01f;
mTarget->translate(deltaX * scale, deltaY * scale, 0, Ogre::Node::TS_LOCAL);
}

void SpaceCamera::pan(const Ogre::Vector2& translation)
Expand Down
4 changes: 2 additions & 2 deletions src/SpaceCamera.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ class SpaceCamera : Ogre::FrameListener
virtual void mouseMoveEvent(QMouseEvent *event);
virtual void wheelEvent(QWheelEvent *event);

void zoomByDelta(int delta);
void zoomByDelta(Ogre::Real delta);

protected:
SpaceCamera(){} // for testing purposes
Expand Down Expand Up @@ -104,7 +104,7 @@ class SpaceCamera : Ogre::FrameListener
float mAnimDuration = 0.5f;

void setKeyMapping();
void zoom(const int delta);
void zoom(Ogre::Real delta);
void pan(const Ogre::Real& deltaX, const Ogre::Real& deltaY);
void pan(const Ogre::Vector2& translation);
void arcBall(const Ogre::Vector2& rotation);
Expand Down
Loading