More fixes when fitting image with display scaling

Extend logical pixel "rounding" to handle case where menu bar is visible at top of window which influences the calculation.

src/logicalpixelfitter.cpp

@@ -0,0 +1,57 @@
+#include "logicalpixelfitter.h"
+#include <QtMath>
+
+LogicalPixelFitter::LogicalPixelFitter(const qreal logicalScale, const QPoint offset)
+    : logicalScale(logicalScale), offset(offset)
+{
+}
+
+int LogicalPixelFitter::snapWidth(const qreal value) const
+{
+    return snap(value + offset.x(), logicalScale) - offset.x();
+}
+
+int LogicalPixelFitter::snapHeight(const qreal value) const
+{
+    return snap(value + offset.y(), logicalScale) - offset.y();
+}
+
+QSize LogicalPixelFitter::snapSize(const QSizeF size) const
+{
+    return QSize(snapWidth(size.width()), snapHeight(size.height()));
+}
+
+qreal LogicalPixelFitter::unsnapWidth(const int value) const
+{
+    return unsnap(value + offset.x(), logicalScale) - offset.x();
+}
+
+qreal LogicalPixelFitter::unsnapHeight(const int value) const
+{
+    return unsnap(value + offset.y(), logicalScale) - offset.y();
+}
+
+QSizeF LogicalPixelFitter::unsnapSize(const QSize size) const
+{
+    return QSizeF(unsnapWidth(size.width()), unsnapHeight(size.height()));
+}
+
+int LogicalPixelFitter::snap(const qreal value, const qreal logicalScale)
+{
+    const int valueRoundedDown = qFloor(qAbs(value));
+    const int valueRoundedUp = valueRoundedDown + 1;
+    const int physicalPixelsDrawn = qRound(qAbs(value) * logicalScale);
+    const int physicalPixelsShownIfRoundingUp = qRound(valueRoundedUp * logicalScale);
+    const int result = physicalPixelsDrawn >= physicalPixelsShownIfRoundingUp ? valueRoundedUp : valueRoundedDown;
+    return result * (value >= 0 ? 1 : -1);
+}
+
+qreal LogicalPixelFitter::unsnap(const int value, const qreal logicalScale)
+{
+    // For a given input value, its physical pixels fall within [value-0.5,value+0.5), so
+    // calculate the first physical pixel of the next value (rounding up if between pixels),
+    // and the pixel prior to that is the last one within the current value.
+    const int maxPhysicalPixelForValue = qCeil((qAbs(value) + 0.5) * logicalScale) - 1;
+    const qreal result = maxPhysicalPixelForValue / logicalScale;
+    return result * (value >= 0 ? 1 : -1);
+}

src/logicalpixelfitter.h

@@ -0,0 +1,33 @@
+#ifndef LOGICALPIXELFITTER_H
+#define LOGICALPIXELFITTER_H
+
+#include <QPoint>
+#include <QSize>
+
+class LogicalPixelFitter
+{
+public:
+    LogicalPixelFitter(const qreal logicalScale, const QPoint offset);
+
+    int snapWidth(const qreal value) const;
+
+    int snapHeight(const qreal value) const;
+
+    QSize snapSize(const QSizeF size) const;
+
+    qreal unsnapWidth(const int value) const;
+
+    qreal unsnapHeight(const int value) const;
+
+    QSizeF unsnapSize(const QSize size) const;
+
+    static int snap(const qreal value, const qreal logicalScale);
+
+    static qreal unsnap(const int value, const qreal logicalScale);
+
+private:
+    const qreal logicalScale;
+    const QPoint offset;
+};
+
+#endif // LOGICALPIXELFITTER_H

src/mainwindow.cpp

@@ -284,9 +284,10 @@ void MainWindow::paintEvent(QPaintEvent *event)
 
     QPainter painter(this);
 
-    const int viewportY = qMax(getTitlebarOverlap(), graphicsView->mapTo(this, QPoint()).y());
-    const QRect headerRect = QRect(0, 0, width(), viewportY);
-    const QRect viewportRect = rect().adjusted(0, viewportY, 0, 0);
+    const ViewportPosition viewportPos = getViewportPosition();
+    const int adjustedViewportY = viewportPos.widgetY + viewportPos.obscuredHeight;
+    const QRect headerRect = QRect(0, 0, width(), adjustedViewportY);
+    const QRect viewportRect = rect().adjusted(0, adjustedViewportY, 0, 0);
 
     if (headerRect.isValid())
     {
@@ -743,28 +744,15 @@ void MainWindow::setWindowSize(const bool isFromTransform)
     const QSizeF imageSize = graphicsView->getEffectiveOriginalSize() * (isZoomFixed ? graphicsView->getZoomLevel() : 1.0);
     const int fitOverscan = graphicsView->getFitOverscan();
     const QSize fitOverscanSize = QSize(fitOverscan * 2, fitOverscan * 2);
-    const qreal logicalPixelScale = graphicsView->devicePixelRatioF();
+    const LogicalPixelFitter fitter = graphicsView->getPixelFitter();
 
-    const auto gvRoundSizeF = [logicalPixelScale](const QSizeF value) {
-        return QSize(
-            QVGraphicsView::roundToCompleteLogicalPixel(value.width(), logicalPixelScale),
-            QVGraphicsView::roundToCompleteLogicalPixel(value.height(), logicalPixelScale)
-        );
-    };
-    const auto gvReverseRoundSize = [logicalPixelScale](const QSize value) {
-        return QSizeF(
-            QVGraphicsView::reverseLogicalPixelRounding(value.width(), logicalPixelScale),
-            QVGraphicsView::reverseLogicalPixelRounding(value.height(), logicalPixelScale)
-        );
-    };
-
-    QSize targetSize = gvRoundSizeF(imageSize) - fitOverscanSize;
+    QSize targetSize = fitter.snapSize(imageSize) - fitOverscanSize;
 
     if (targetSize.width() > maxWindowSize.width() || targetSize.height() > maxWindowSize.height())
     {
-        const QSizeF enforcedSize = gvReverseRoundSize(maxWindowSize) + fitOverscanSize;
+        const QSizeF enforcedSize = fitter.unsnapSize(maxWindowSize) + fitOverscanSize;
         const qreal fitRatio = qMin(enforcedSize.width() / imageSize.width(), enforcedSize.height() / imageSize.height());
-        targetSize = gvRoundSizeF(imageSize * fitRatio) - fitOverscanSize;
+        targetSize = fitter.snapSize(imageSize * fitRatio) - fitOverscanSize;
     }
 
     targetSize = targetSize.expandedTo(minWindowSize).boundedTo(maxWindowSize);
@@ -1450,3 +1438,15 @@ int MainWindow::getTitlebarOverlap() const
 
     return 0;
 }
+
+MainWindow::ViewportPosition MainWindow::getViewportPosition() const
+{
+    ViewportPosition result;
+    // This accounts for anything that may be above the viewport such as the menu bar (if it's inside
+    // the window) and/or the label that displays titlebar text in full screen mode.
+    result.widgetY = windowHandle() ? graphicsView->mapTo(this, QPoint()).y() : 0;
+    // On macOS, part of the viewport may be additionally covered with the window's translucent
+    // titlebar due to full size content view.
+    result.obscuredHeight = qMax(getTitlebarOverlap() - result.widgetY, 0);
+    return result;
+}

src/mainwindow.h

@@ -26,6 +26,12 @@ class MainWindow : public QMainWindow
         QString previousPath;
     };
 
+    struct ViewportPosition
+    {
+        int widgetY;
+        int obscuredHeight;
+    };
+
     explicit MainWindow(QWidget *parent = nullptr, const QJsonObject &windowSessionState = {});
     ~MainWindow() override;
 
@@ -137,6 +143,8 @@ class MainWindow : public QMainWindow
 
     int getTitlebarOverlap() const;
 
+    ViewportPosition getViewportPosition() const;
+
     const QVImageCore::FileDetails& getCurrentFileDetails() const { return graphicsView->getCurrentFileDetails(); }
 
     qint64 getLastActivatedTimestamp() const { return lastActivated.msecsSinceReference(); }

src/qvgraphicsview.cpp

@@ -719,23 +719,16 @@ void QVGraphicsView::recalculateZoom()
     if (viewSize.isEmpty())
         return;
 
-    const qreal logicalPixelScale = devicePixelRatioF();
-    const auto gvRound = [logicalPixelScale](const qreal value) {
-        return roundToCompleteLogicalPixel(value, logicalPixelScale);
-    };
-    const auto gvReverseRound = [logicalPixelScale](const int value) {
-        return reverseLogicalPixelRounding(value, logicalPixelScale);
-    };
-
-    const qreal fitXRatio = gvReverseRound(viewSize.width()) / imageSize.width();
-    const qreal fitYRatio = gvReverseRound(viewSize.height()) / imageSize.height();
+    const LogicalPixelFitter fitter = getPixelFitter();
+    const qreal fitXRatio = fitter.unsnapWidth(viewSize.width()) / imageSize.width();
+    const qreal fitYRatio = fitter.unsnapHeight(viewSize.height()) / imageSize.height();
 
     qreal targetRatio;
 
     // Each mode will check if the rounded image size already produces the desired fit,
     // in which case we can use exactly 1.0 to avoid unnecessary scaling
-    const int imageOverflowX = gvRound(imageSize.width()) - viewSize.width();
-    const int imageOverflowY = gvRound(imageSize.height()) - viewSize.height();
+    const int imageOverflowX = fitter.snapWidth(imageSize.width()) - viewSize.width();
+    const int imageOverflowY = fitter.snapHeight(imageSize.height()) - viewSize.height();
 
     switch (calculatedZoomMode.value()) {
     case Qv::CalculatedZoomMode::ZoomToFit:
@@ -754,8 +747,8 @@ void QVGraphicsView::recalculateZoom()
             // If the fit ratios are extremely close, it's possible that both are sufficient to
             // contain the image, but one results in the opposing dimension getting rounded down
             // to just under the view size, so use the larger of the two ratios in that case.
-            const bool isOverallFitToXRatio = gvRound(imageSize.height() * fitXRatio) == viewSize.height();
-            const bool isOverallFitToYRatio = gvRound(imageSize.width() * fitYRatio) == viewSize.width();
+            const bool isOverallFitToXRatio = fitter.snapHeight(imageSize.height() * fitXRatio) == viewSize.height();
+            const bool isOverallFitToYRatio = fitter.snapWidth(imageSize.width() * fitYRatio) == viewSize.width();
             if (isOverallFitToXRatio || isOverallFitToYRatio)
                 targetRatio = qMax(fitXRatio, fitYRatio);
             else
@@ -924,6 +917,12 @@ QSizeF QVGraphicsView::getEffectiveOriginalSize() const
     return getTransformWithNoScaling().mapRect(QRectF(QPoint(), getCurrentFileDetails().loadedPixmapSize)).size() / getDpiAdjustment();
 }
 
+LogicalPixelFitter QVGraphicsView::getPixelFitter() const
+{
+    const MainWindow::ViewportPosition viewportPos = getMainWindow()->getViewportPosition();
+    return LogicalPixelFitter(devicePixelRatioF(), QPoint(0, viewportPos.widgetY + viewportPos.obscuredHeight));
+}
+
 void QVGraphicsView::matchContentCenter(const QRect target)
 {
     const QPointF delta = QRectF(getContentRect()).center() - QRectF(target).center();
@@ -940,38 +939,27 @@ QRect QVGraphicsView::getContentRect() const
     const qreal effectiveTransformScale = zoomLevel / appliedDpiAdjustment;
     const QTransform effectiveTransform = getTransformWithNoScaling().scale(effectiveTransformScale, effectiveTransformScale);
     const QRectF contentRect = effectiveTransform.mapRect(loadedPixmapBoundingRect);
-    const qreal logicalPixelScale = devicePixelRatioF();
-    const auto gvRound = [logicalPixelScale](const qreal value) {
-        return roundToCompleteLogicalPixel(qAbs(value), logicalPixelScale) * (value >= 0 ? 1 : -1);
-    };
-    return QRect(gvRound(contentRect.left()), gvRound(contentRect.top()), gvRound(contentRect.width()), gvRound(contentRect.height()));
+    const QSize snappedSize = getPixelFitter().snapSize(contentRect.size());
+    const bool isHorizontalReversed = effectiveTransform.m11() < 0 || effectiveTransform.m21() < 0;
+    const bool isVerticalReversed = effectiveTransform.m22() < 0 || effectiveTransform.m12() < 0;
+    const QPointF topLeftCorrection = QPointF(
+        isHorizontalReversed ? contentRect.width() - snappedSize.width() : 0,
+        isVerticalReversed ? contentRect.height() - snappedSize.height() : 0
+    );
+    return QRect((contentRect.topLeft() + topLeftCorrection).toPoint(), snappedSize);
 }
 
 QRect QVGraphicsView::getUsableViewportRect(const bool addOverscan) const
 {
-#ifdef COCOA_LOADED
-    int obscuredHeight = QVCocoaFunctions::getObscuredHeight(window()->windowHandle());
-#else
-    int obscuredHeight = 0;
-#endif
     QRect rect = viewport()->rect();
-    rect.setTop(obscuredHeight);
+    rect.setTop(getMainWindow()->getViewportPosition().obscuredHeight);
     if (addOverscan)
         rect.adjust(-fitOverscan, -fitOverscan, fitOverscan, fitOverscan);
     return rect;
 }
 
 void QVGraphicsView::setTransformScale(qreal value)
 {
-#ifndef Q_OS_MACOS
-    // If fractional display scaling is in use, when attempting to target a given size, the resulting error
-    // can be [0,1) unlike the typical [0,0.5] without scaling or integer scaling. This is because the
-    // image origin which is always at an integer logical pixel becomes potentially a fractional physical
-    // pixel due to the display scaling, adding to the overall error. As a result, tiny rounding errors can
-    // cause us to miss the size we were targetting, so increase the scale just a hair to compensate.
-    if (value != std::floor(value))
-        value *= 1.0 + std::numeric_limits<double>::epsilon();
-#endif
     setTransform(getTransformWithNoScaling().scale(value, value));
 }
 
@@ -1157,21 +1145,3 @@ void QVGraphicsView::resetTransformation()
     setTransform(QTransform::fromScale(scale, scale));
     matchContentCenter(oldRect);
 }
-
-int QVGraphicsView::roundToCompleteLogicalPixel(const qreal value, const qreal logicalScale)
-{
-    const int valueRoundedDown = qFloor(value);
-    const int valueRoundedUp = valueRoundedDown + 1;
-    const int physicalPixelsDrawn = qRound(value * logicalScale);
-    const int physicalPixelsShownIfRoundingUp = qRound(valueRoundedUp * logicalScale);
-    return physicalPixelsDrawn >= physicalPixelsShownIfRoundingUp ? valueRoundedUp : valueRoundedDown;
-}
-
-qreal QVGraphicsView::reverseLogicalPixelRounding(const int value, const qreal logicalScale)
-{
-    // For a given input value, its physical pixels fall within [value-0.5,value+0.5), so
-    // calculate the first physical pixel of the next value (rounding up if between pixels),
-    // and the pixel prior to that is the last one within the current value.
-    int maxPhysicalPixelForValue = qCeil((value + 0.5) * logicalScale) - 1;
-    return maxPhysicalPixelForValue / logicalScale;
-}

src/qvgraphicsview.h

@@ -4,6 +4,7 @@
 #include "qvnamespace.h"
 #include "qvimagecore.h"
 #include "axislocker.h"
+#include "logicalpixelfitter.h"
 #include "scrollhelper.h"
 #include <optional>
 #include <QGraphicsView>
@@ -80,17 +81,15 @@ class QVGraphicsView : public QGraphicsView
 
     QSizeF getEffectiveOriginalSize() const;
 
+    LogicalPixelFitter getPixelFitter() const;
+
     const QVImageCore::FileDetails& getCurrentFileDetails() const { return imageCore.getCurrentFileDetails(); }
     const QPixmap& getLoadedPixmap() const { return imageCore.getLoadedPixmap(); }
     const QMovie& getLoadedMovie() const { return imageCore.getLoadedMovie(); }
     qreal getZoomLevel() const { return zoomLevel; }
 
     int getFitOverscan() const { return fitOverscan; }
 
-    static int roundToCompleteLogicalPixel(const qreal value, const qreal logicalScale);
-
-    static qreal reverseLogicalPixelRounding(const int value, const qreal logicalScale);
-
 signals:
     void cancelSlideshow();
 

src/src.pri

@@ -1,5 +1,4 @@
 SOURCES += \
-    $$PWD/axislocker.cpp \
     $$PWD/main.cpp \
     $$PWD/mainwindow.cpp \
     $$PWD/openwith.cpp \
@@ -14,6 +13,8 @@ SOURCES += \
     $$PWD/qvimagecore.cpp \
     $$PWD/qvshortcutdialog.cpp \
     $$PWD/actionmanager.cpp \
+    $$PWD/axislocker.cpp \
+    $$PWD/logicalpixelfitter.cpp \
     $$PWD/scrollhelper.cpp \
     $$PWD/settingsmanager.cpp \
     $$PWD/shortcutmanager.cpp \
@@ -24,7 +25,6 @@ win32:!CONFIG(NO_WIN32):SOURCES += $$PWD/qvwin32functions.cpp
 linux:!CONFIG(NO_X11):SOURCES += $$PWD/qvlinuxx11functions.cpp
 
 HEADERS += \
-    $$PWD/axislocker.h \
     $$PWD/mainwindow.h \
     $$PWD/openwith.h \
     $$PWD/qvfileenumerator.h \
@@ -39,6 +39,8 @@ HEADERS += \
     $$PWD/qvimagecore.h \
     $$PWD/qvshortcutdialog.h \
     $$PWD/actionmanager.h \
+    $$PWD/axislocker.h \
+    $$PWD/logicalpixelfitter.h \
     $$PWD/scrollhelper.h \
     $$PWD/settingsmanager.h \
     $$PWD/shortcutmanager.h \
@@ -49,7 +51,7 @@ win32:!CONFIG(NO_WIN32):HEADERS += $$PWD/qvwin32functions.h
 linux:!CONFIG(NO_X11):HEADERS += $$PWD/qvlinuxx11functions.h
 
 FORMS += \
-        $$PWD/mainwindow.ui \
+    $$PWD/mainwindow.ui \
     $$PWD/qvopenwithdialog.ui \
     $$PWD/qvoptionsdialog.ui \
     $$PWD/qvaboutdialog.ui \