From 898af0fcf3e14ab01ff1277da94919861199aacf Mon Sep 17 00:00:00 2001 From: "J.D. Purcell" Date: Sun, 29 Dec 2024 19:48:31 -0500 Subject: [PATCH] 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 | 55 ++++++++++++++++++++++++++ src/logicalpixelfitter.h | 33 ++++++++++++++++ src/mainwindow.cpp | 47 ++++++++++------------- src/mainwindow.h | 8 ++++ src/qvgraphicsview.cpp | 79 +++++++++++++------------------------- src/qvgraphicsview.h | 11 ++++-- src/src.pri | 2 + 7 files changed, 152 insertions(+), 83 deletions(-) create mode 100644 src/logicalpixelfitter.cpp create mode 100644 src/logicalpixelfitter.h diff --git a/src/logicalpixelfitter.cpp b/src/logicalpixelfitter.cpp new file mode 100644 index 00000000..babcb7b1 --- /dev/null +++ b/src/logicalpixelfitter.cpp @@ -0,0 +1,55 @@ +#include "logicalpixelfitter.h" +#include + +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(value); + const int valueRoundedUp = valueRoundedDown + 1; + const int physicalPixelsDrawn = qRound(value * logicalScale); + const int physicalPixelsShownIfRoundingUp = qRound(valueRoundedUp * logicalScale); + return physicalPixelsDrawn >= physicalPixelsShownIfRoundingUp ? valueRoundedUp : valueRoundedDown; +} + +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((value + 0.5) * logicalScale) - 1; + return maxPhysicalPixelForValue / logicalScale; +} diff --git a/src/logicalpixelfitter.h b/src/logicalpixelfitter.h new file mode 100644 index 00000000..f2951fb8 --- /dev/null +++ b/src/logicalpixelfitter.h @@ -0,0 +1,33 @@ +#ifndef LOGICALPIXELFITTER_H +#define LOGICALPIXELFITTER_H + +#include +#include + +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 diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 3f5ead94..1aae576d 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -286,30 +286,24 @@ void MainWindow::paintEvent(QPaintEvent *event) QPainter painter(this); const QColor &backgroundColor = customBackgroundColor.isValid() ? customBackgroundColor : painter.background().color(); - - // Find the top of the viewport to account for the menu bar if it's inside the window - // and/or the label that displays titlebar text in full screen mode. - const int viewportY = graphicsView->mapTo(this, QPoint()).y(); - // On macOS, part of the viewport may be additionally covered with the window's translucent - // titlebar due to full size content view. - const int unobscuredViewportY = qMax(getTitlebarOverlap(), viewportY); + const ViewportPosition viewportPos = getViewportPosition(); // Erase the area above the viewport, i.e. fill it with the painter's default color. - const QRect headerRect = QRect(0, 0, width(), viewportY); + const QRect headerRect = QRect(0, 0, width(), viewportPos.widgetY); if (headerRect.isValid()) { painter.eraseRect(headerRect); } // Fill the viewport with the background color. - const QRect viewportRect = rect().adjusted(0, viewportY, 0, 0); + const QRect viewportRect = rect().adjusted(0, viewportPos.widgetY, 0, 0); if (viewportRect.isValid()) { painter.fillRect(viewportRect, backgroundColor); } // If there's an error message, draw it centered inside the unobscured area of the viewport. - const QRect unobscuredViewportRect = rect().adjusted(0, unobscuredViewportY, 0, 0); + const QRect unobscuredViewportRect = rect().adjusted(0, viewportPos.widgetY + viewportPos.obscuredHeight, 0, 0); if (getCurrentFileDetails().errorData.hasError && unobscuredViewportRect.isValid()) { const QVImageCore::ErrorData &errorData = getCurrentFileDetails().errorData; @@ -591,31 +585,18 @@ void MainWindow::setWindowSize() const QSize minWindowSize = (screenSize * minWindowResizedPercentage).boundedTo(hardLimitSize); const QSize maxWindowSize = (screenSize * qMax(maxWindowResizedPercentage, minWindowResizedPercentage)).boundedTo(hardLimitSize); const QSizeF imageSize = graphicsView->getEffectiveOriginalSize(); - const qreal logicalPixelScale = graphicsView->devicePixelRatioF(); + const LogicalPixelFitter fitter = graphicsView->getPixelFitter(); const bool enforceMinSizeBothDimensions = false; - 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); + QSize targetSize = fitter.snapSize(imageSize); const bool limitToMin = targetSize.width() < minWindowSize.width() && targetSize.height() < minWindowSize.height(); const bool limitToMax = targetSize.width() > maxWindowSize.width() || targetSize.height() > maxWindowSize.height(); if (limitToMin || limitToMax) { - const QSizeF enforcedSize = gvReverseRoundSize((limitToMin ? minWindowSize : maxWindowSize)); + const QSizeF enforcedSize = fitter.unsnapSize(limitToMin ? minWindowSize : maxWindowSize); const qreal fitRatio = qMin(enforcedSize.width() / imageSize.width(), enforcedSize.height() / imageSize.height()); - targetSize = gvRoundSizeF(imageSize * fitRatio); + targetSize = fitter.snapSize(imageSize * fitRatio); } if (enforceMinSizeBothDimensions) @@ -1225,3 +1206,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; +} diff --git a/src/mainwindow.h b/src/mainwindow.h index 7db69a21..7c7059e2 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -26,6 +26,12 @@ class MainWindow : public QMainWindow QString previousPath; }; + struct ViewportPosition + { + int widgetY; + int obscuredHeight; + }; + explicit MainWindow(QWidget *parent = nullptr); ~MainWindow() override; @@ -113,6 +119,8 @@ class MainWindow : public QMainWindow int getTitlebarOverlap() const; + ViewportPosition getViewportPosition() const; + const QVImageCore::FileDetails& getCurrentFileDetails() const { return graphicsView->getCurrentFileDetails(); } public slots: diff --git a/src/qvgraphicsview.cpp b/src/qvgraphicsview.cpp index bbc32363..b75d7540 100644 --- a/src/qvgraphicsview.cpp +++ b/src/qvgraphicsview.cpp @@ -402,21 +402,14 @@ void QVGraphicsView::zoomToFit() 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(); // 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(); qreal targetRatio; @@ -449,8 +442,8 @@ void QVGraphicsView::zoomToFit() // 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 @@ -590,6 +583,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)); +} + QRect QVGraphicsView::getContentRect() const { // Avoid using loadedPixmapItem and the active transform because the pixmap may have expensive scaling applied @@ -600,36 +599,25 @@ 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 { -#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); 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::epsilon(); -#endif setTransform(getTransformWithNoScaling().scale(value, value)); } @@ -651,24 +639,6 @@ qreal QVGraphicsView::getDpiAdjustment() const return isOneToOnePixelSizingEnabled ? devicePixelRatioF() : 1.0; } -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; -} - void QVGraphicsView::handleDpiAdjustmentChange() { if (appliedDpiAdjustment == getDpiAdjustment()) @@ -682,6 +652,11 @@ void QVGraphicsView::handleDpiAdjustmentChange() expensiveScaleTimer->start(); } +MainWindow* QVGraphicsView::getMainWindow() const +{ + return qobject_cast(window()); +} + void QVGraphicsView::settingsUpdated() { auto &settingsManager = qvApp->getSettingsManager(); diff --git a/src/qvgraphicsview.h b/src/qvgraphicsview.h index aa217249..4587db2c 100644 --- a/src/qvgraphicsview.h +++ b/src/qvgraphicsview.h @@ -2,6 +2,7 @@ #define QVGRAPHICSVIEW_H #include "qvimagecore.h" +#include "logicalpixelfitter.h" #include #include #include @@ -9,6 +10,8 @@ #include #include +class MainWindow; + class QVGraphicsView : public QGraphicsView { Q_OBJECT @@ -63,15 +66,13 @@ 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; } - static int roundToCompleteLogicalPixel(const qreal value, const qreal logicalScale); - - static qreal reverseLogicalPixelRounding(const int value, const qreal logicalScale); - signals: void cancelSlideshow(); @@ -116,6 +117,8 @@ class QVGraphicsView : public QGraphicsView void handleDpiAdjustmentChange(); + MainWindow* getMainWindow() const; + private slots: void animatedFrameChanged(QRect rect); diff --git a/src/src.pri b/src/src.pri index 33fd6c3f..69634aa7 100644 --- a/src/src.pri +++ b/src/src.pri @@ -12,6 +12,7 @@ SOURCES += \ $$PWD/qvimagecore.cpp \ $$PWD/qvshortcutdialog.cpp \ $$PWD/actionmanager.cpp \ + $$PWD/logicalpixelfitter.cpp \ $$PWD/settingsmanager.cpp \ $$PWD/shortcutmanager.cpp @@ -34,6 +35,7 @@ HEADERS += \ $$PWD/qvimagecore.h \ $$PWD/qvshortcutdialog.h \ $$PWD/actionmanager.h \ + $$PWD/logicalpixelfitter.h \ $$PWD/settingsmanager.h \ $$PWD/shortcutmanager.h