From 71782aec20c68b4b22e05a0bbee8bdb7dafc6715 Mon Sep 17 00:00:00 2001 From: Andre Rummler Date: Fri, 25 Sep 2026 15:09:21 +0200 Subject: [PATCH] Fix wheel scale hardening --- sources/diagramevent/diagrameventaddimage.cpp | 38 +++++++++++++------ sources/diagramevent/diagrameventaddpdf.cpp | 28 ++++++++++---- 2 files changed, 47 insertions(+), 19 deletions(-) diff --git a/sources/diagramevent/diagrameventaddimage.cpp b/sources/diagramevent/diagrameventaddimage.cpp index 5dc22aa03..8c2e11a5e 100644 --- a/sources/diagramevent/diagrameventaddimage.cpp +++ b/sources/diagramevent/diagrameventaddimage.cpp @@ -287,18 +287,32 @@ void DiagramEventAddImage::wheelEvent(QGraphicsSceneWheelEvent *event) return; } - // scaleFactorX(), not QGraphicsItem's own scale(): see the right-click - // rotate comment in mousePressEvent for why. Wheel-scaling only ever - // runs while !m_pressed (guarded above), i.e. before any drag-resize - // has anchored the pivot to the origin (see mouseMoveEvent), so the - // pivot here is still the default boundingRect().center() and this - // scales the image in place around its own middle, exactly like - // before. - qreal scaling = m_image->scaleFactorX(); - event->delta() > 1? scaling += 0.01 : scaling -= 0.01; - if (scaling>0.01 && scaling <= 2) { - m_image->setScaleFactorX(scaling); - m_image->setScaleFactorY(scaling); + // scaleFactorX()/scaleFactorY(), not QGraphicsItem's own scale(): see + // the right-click rotate comment in mousePressEvent for why. Wheel- + // scaling only ever runs while !m_pressed (guarded above), i.e. before + // any drag-resize has anchored the pivot to the origin (see + // mouseMoveEvent), so the pivot here is still the default + // boundingRect().center() and this scales the image in place around + // its own middle, exactly like before. + // + // Step each axis from its own current value rather than reading X and + // writing it back to both: scaleFactorX and scaleFactorY cannot + // actually differ at this point today (every other mutator in this + // class -- the drag-resize branch above, and this same wheelEvent -- + // only ever sets them to the same value, and mouseReleaseEvent commits + // and ends this tool on any left-button release, so a handle-based + // non-uniform resize can never happen first and leave this instance + // still alive). Stepping both from their own value rather than + // collapsing Y to X costs nothing today and removes the trap if that + // invariant ever stops holding. + qreal scalingX = m_image->scaleFactorX(); + qreal scalingY = m_image->scaleFactorY(); + const qreal step = event->delta() > 1 ? 0.01 : -0.01; + scalingX += step; + scalingY += step; + if (scalingX > 0.01 && scalingX <= 2 && scalingY > 0.01 && scalingY <= 2) { + m_image->setScaleFactorX(scalingX); + m_image->setScaleFactorY(scalingY); } event->setAccepted(true); diff --git a/sources/diagramevent/diagrameventaddpdf.cpp b/sources/diagramevent/diagrameventaddpdf.cpp index d4af74058..d0f360565 100644 --- a/sources/diagramevent/diagrameventaddpdf.cpp +++ b/sources/diagramevent/diagrameventaddpdf.cpp @@ -135,21 +135,35 @@ void DiagramEventAddPdf::mouseDoubleClickEvent(QGraphicsSceneMouseEvent *event) */ void DiagramEventAddPdf::wheelEvent(QGraphicsSceneWheelEvent *event) { - if (!m_is_added || !m_image || event->modifiers() != Qt::CTRL) { + // event->modifiers() & Qt::ControlModifier, not != Qt::CTRL: the same + // exact-equality bug already found and fixed elsewhere this session -- + // Ctrl held together with any other modifier would silently fail to + // register as Ctrl at all. + if (!m_is_added || !m_image || !(event->modifiers() & Qt::ControlModifier)) { return; } - // scaleFactorX(), not QGraphicsItem's own scale(): see + // scaleFactorX()/scaleFactorY(), not QGraphicsItem's own scale(): see // DiagramEventAddImage's identical fix for why. No drag-to-resize // exists here, and the pivot is never touched elsewhere in this // class, so it stays at its default boundingRect().center() and this // scales the page in place around its own middle, exactly like // before. - qreal scaling = m_image->scaleFactorX(); - event->delta() > 1 ? scaling += 0.01 : scaling -= 0.01; - if (scaling > 0.01 && scaling <= 2) { - m_image->setScaleFactorX(scaling); - m_image->setScaleFactorY(scaling); + // + // Step each axis from its own current value rather than reading X and + // writing it back to both: scaleFactorX and scaleFactorY cannot + // actually differ here today (nothing in this class ever sets them to + // different values, and there is no drag-resize at all), but stepping + // both independently costs nothing and removes the trap if that ever + // changes. + qreal scalingX = m_image->scaleFactorX(); + qreal scalingY = m_image->scaleFactorY(); + const qreal step = event->delta() > 1 ? 0.01 : -0.01; + scalingX += step; + scalingY += step; + if (scalingX > 0.01 && scalingX <= 2 && scalingY > 0.01 && scalingY <= 2) { + m_image->setScaleFactorX(scalingX); + m_image->setScaleFactorY(scalingY); } event->setAccepted(true);