Skip to content

Commit

Permalink
Fix more rounding errors
Browse files Browse the repository at this point in the history
  • Loading branch information
@jdpurcell
jdpurcell committed Dec 26, 2024
1 parent ea718c8 commit 8204e38
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 42 deletions.
16 changes: 10 additions & 6 deletions src/mainwindow.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -591,8 +591,6 @@ 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 int fitOverscan = graphicsView->getFitOverscan();
const QSize fitOverscanSize = QSize(fitOverscan * 2, fitOverscan * 2);
const qreal logicalPixelScale = graphicsView->devicePixelRatioF();
const bool enforceMinSizeBothDimensions = false;

Expand All @@ -602,16 +600,22 @@ void MainWindow::setWindowSize()
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 = gvRoundSizeF(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 viewSize = (limitToMin ? minWindowSize : maxWindowSize) + fitOverscanSize;
const qreal fitRatio = qMin(viewSize.width() / imageSize.width(), viewSize.height() / imageSize.height());
targetSize = gvRoundSizeF(imageSize * fitRatio) - fitOverscanSize;
const QSizeF enforcedSize = gvReverseRoundSize((limitToMin ? minWindowSize : maxWindowSize));
const qreal fitRatio = qMin(enforcedSize.width() / imageSize.width(), enforcedSize.height() / imageSize.height());
targetSize = gvRoundSizeF(imageSize * fitRatio);
}

if (enforceMinSizeBothDimensions)
Expand Down
79 changes: 47 additions & 32 deletions src/qvgraphicsview.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ QVGraphicsView::QVGraphicsView(QWidget *parent) : QGraphicsView(parent)
zoomMultiplier = 1.25;

// Initialize other variables
fitOverscan = 0;
zoomLevel = 1.0;
appliedDpiAdjustment = 1.0;
appliedExpensiveScaleZoomLevel = 0.0;
Expand Down Expand Up @@ -312,7 +311,7 @@ void QVGraphicsView::zoomAbsolute(const qreal absoluteLevel, const QPoint &pos)

// If we are zooming in, we have a point to zoom towards, the mouse is on top of the viewport, and cursor zooming is enabled
const QSize contentSize = getContentRect().size();
const QSize viewportSize = getUsableViewportRect(true).size();
const QSize viewportSize = getUsableViewportRect().size();
const bool reachedViewportSize = contentSize.width() >= viewportSize.width() || contentSize.height() >= viewportSize.height();
if (reachedViewportSize && pos != QPoint(-1, -1) && underMouse() && isCursorZoomEnabled)
{
Expand Down Expand Up @@ -340,7 +339,7 @@ void QVGraphicsView::applyExpensiveScaling()

// Don't go over the maximum scaling size - small tolerance added to cover rounding errors
const QSize contentSize = getContentRect().size();
const QSize maxSize = getUsableViewportRect(true).size() * (isScalingTwoEnabled ? 3 : 1) + QSize(2, 2);
const QSize maxSize = getUsableViewportRect().size() * (isScalingTwoEnabled ? 3 : 1) + QSize(2, 2);
if (contentSize.width() > maxSize.width() || contentSize.height() > maxSize.height())
{
// Return to original size
Expand Down Expand Up @@ -397,58 +396,62 @@ void QVGraphicsView::zoomToFit()
if (!getCurrentFileDetails().isPixmapLoaded)
return;

const QSizeF effectiveImageSize = getEffectiveOriginalSize();
const QSize viewSize = getUsableViewportRect(true).size();
const QSizeF imageSize = getEffectiveOriginalSize();
const QSize viewSize = getUsableViewportRect().size();

if (viewSize.isEmpty())
return;

const qreal fitXRatio = viewSize.width() / effectiveImageSize.width();
const qreal fitYRatio = viewSize.height() / effectiveImageSize.height();
const qreal logicalPixelScale = devicePixelRatioF();

const auto gvRound = [logicalPixelScale](const qreal value) {
return roundToCompleteLogicalPixel(value, logicalPixelScale);
};
const auto gvRoundSizeF = [logicalPixelScale](const QSizeF value) {
return QSize(
QVGraphicsView::roundToCompleteLogicalPixel(value.width(), logicalPixelScale),
QVGraphicsView::roundToCompleteLogicalPixel(value.height(), logicalPixelScale)
);
const auto gvReverseRound = [logicalPixelScale](const int value) {
return reverseLogicalPixelRounding(value, logicalPixelScale);
};

qreal targetRatio;
const qreal fitXRatio = gvReverseRound(viewSize.width()) / imageSize.width();
const qreal fitYRatio = gvReverseRound(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();

qreal targetRatio;

switch (cropMode) { // should be enum tbh
case 1: // only take into account height
if (gvRound(effectiveImageSize.height()) == viewSize.height())
if (imageOverflowY == 0)
targetRatio = 1.0;
else
targetRatio = fitYRatio;
break;
case 2: // only take into account width
if (gvRound(effectiveImageSize.width()) == viewSize.width())
if (imageOverflowX == 0)
targetRatio = 1.0;
else
targetRatio = fitXRatio;
break;
default:
if ((gvRound(effectiveImageSize.height()) == viewSize.height() && gvRound(effectiveImageSize.width()) <= viewSize.width()) ||
(gvRound(effectiveImageSize.width()) == viewSize.width() && gvRound(effectiveImageSize.height()) <= viewSize.height()))
// In rare cases, if the window sizing code just barely increased the size to enforce
// the minimum and intends for a tiny upscale to occur (e.g. to 100.3%), that could get
// misdetected as the special case for 1.0 here and leave an unintentional 1 pixel
// border. So if we match on only one dimension, make sure the other dimension will have
// at least a few pixels of border showing.
if ((imageOverflowX == 0 && (imageOverflowY == 0 || imageOverflowY <= -2)) ||
(imageOverflowY == 0 && (imageOverflowX == 0 || imageOverflowX <= -2)))
{
targetRatio = 1.0;
}
else
{
const QSize xRatioSize = gvRoundSizeF(effectiveImageSize * fitXRatio);
const QSize yRatioSize = gvRoundSizeF(effectiveImageSize * fitYRatio);
// 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.
if (xRatioSize.boundedTo(viewSize) == xRatioSize && yRatioSize.boundedTo(viewSize) == yRatioSize)
const bool isOverallFitToXRatio = gvRound(imageSize.height() * fitXRatio) == viewSize.height();
const bool isOverallFitToYRatio = gvRound(imageSize.width() * fitYRatio) == viewSize.width();
if (isOverallFitToXRatio || isOverallFitToYRatio)
targetRatio = qMax(fitXRatio, fitYRatio);
else
targetRatio = qMin(fitXRatio, fitYRatio);
Expand Down Expand Up @@ -599,7 +602,7 @@ QRect QVGraphicsView::getContentRect() const
return effectiveTransform.mapRect(loadedPixmapBoundingRect).toRect();
}

QRect QVGraphicsView::getUsableViewportRect(const bool addOverscan) const
QRect QVGraphicsView::getUsableViewportRect() const
{
#ifdef COCOA_LOADED
int obscuredHeight = QVCocoaFunctions::getObscuredHeight(window()->windowHandle());
Expand All @@ -608,18 +611,19 @@ QRect QVGraphicsView::getUsableViewportRect(const bool addOverscan) const
#endif
QRect rect = viewport()->rect();
rect.setTop(obscuredHeight);
if (addOverscan)
rect.adjust(-fitOverscan, -fitOverscan, fitOverscan, fitOverscan);
return rect;
}

void QVGraphicsView::setTransformScale(qreal value)
{
#ifdef Q_OS_WIN
// On Windows, the positioning of scaled pixels seems to follow a floor rule rather
// than rounding, so increase the scale just a hair to cover rounding errors in case
// the desired scale was targeting an integer pixel boundary.
value *= 1.0 + std::numeric_limits<double>::epsilon();
#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));
}
Expand All @@ -644,9 +648,20 @@ qreal QVGraphicsView::getDpiAdjustment() const

int QVGraphicsView::roundToCompleteLogicalPixel(const qreal value, const qreal logicalScale)
{
const int roundedLogicalPixel = qRound(value);
const bool isComplete = qRound(value * logicalScale) >= qRound(roundedLogicalPixel * logicalScale);
return roundedLogicalPixel - (isComplete ? 0 : 1);
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()
Expand Down
7 changes: 3 additions & 4 deletions src/qvgraphicsview.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,10 @@ class QVGraphicsView : public QGraphicsView
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();

Expand Down Expand Up @@ -106,7 +106,7 @@ class QVGraphicsView : public QGraphicsView

QRect getContentRect() const;

QRect getUsableViewportRect(const bool addOverscan = false) const;
QRect getUsableViewportRect() const;

void setTransformScale(qreal absoluteScale);

Expand Down Expand Up @@ -137,7 +137,6 @@ private slots:
int cropMode;
qreal zoomMultiplier;

int fitOverscan;
qreal zoomLevel;
qreal appliedDpiAdjustment;
qreal appliedExpensiveScaleZoomLevel;
Expand Down

0 comments on commit 8204e38

Please sign in to comment.