From ccda6d2a65571510c1dc50e7b2e9eee27722ee39 Mon Sep 17 00:00:00 2001 From: Oliver Sander Date: Fri, 19 Feb 2021 21:28:32 +0100 Subject: [PATCH] Remove device pixel ratio scaling from PixmapRequest The PixmapRequest constructor expected width and height in logical pixels, and scaled them to device pixels. However, not knowing what screen the request is for, it had to use qApp->devicePixelRatio() for the scaling. That value may not be correct if not all screens use the same scaling. Fix this by introducing a new constructor that takes the device pixel ratio as an additional argument, and deprecating the old constructor. --- autotests/documenttest.cpp | 2 +- core/document.cpp | 4 ++-- core/generator.cpp | 9 +++++++-- core/generator.h | 19 ++++++++++++++++--- part/magnifierview.cpp | 2 +- part/pageview.cpp | 12 ++++++------ part/presentationwidget.cpp | 7 ++++--- part/thumbnaillist.cpp | 2 +- 8 files changed, 38 insertions(+), 19 deletions(-) diff --git a/autotests/documenttest.cpp b/autotests/documenttest.cpp index 97bf39037..7740f2fc5 100644 --- a/autotests/documenttest.cpp +++ b/autotests/documenttest.cpp @@ -54,7 +54,7 @@ void DocumentTest::testCloseDuringRotationJob() ThreadWeaver::Queue::instance()->suspend(); // Request a pixmap. A RotationJob will be enqueued but not started - Okular::PixmapRequest *pixmapReq = new Okular::PixmapRequest(dummyDocumentObserver, 0, 100, 100, 1, Okular::PixmapRequest::NoFeature); + Okular::PixmapRequest *pixmapReq = new Okular::PixmapRequest(dummyDocumentObserver, 0, 100, 100, qApp->devicePixelRatio(), 1, Okular::PixmapRequest::NoFeature); m_document->requestPixmaps(QLinkedList() << pixmapReq); // Delete the document diff --git a/core/document.cpp b/core/document.cpp index 18cc746e3..9fb5bce1b 100644 --- a/core/document.cpp +++ b/core/document.cpp @@ -1519,7 +1519,7 @@ void DocumentPrivate::refreshPixmaps(int pageNumber) QVector pixmapsToRequest; for (; it != itEnd; ++it) { const QSize size = (*it).m_pixmap->size(); - PixmapRequest *p = new PixmapRequest(it.key(), pageNumber, size.width() / qApp->devicePixelRatio(), size.height() / qApp->devicePixelRatio(), 1, PixmapRequest::Asynchronous); + PixmapRequest *p = new PixmapRequest(it.key(), pageNumber, size.width(), size.height(), 1 /* dpr */, 1, PixmapRequest::Asynchronous); p->d->mForce = true; pixmapsToRequest << p; } @@ -1539,7 +1539,7 @@ void DocumentPrivate::refreshPixmaps(int pageNumber) if (tilesManager) { tilesManager->markDirty(); - PixmapRequest *p = new PixmapRequest(observer, pageNumber, tilesManager->width() / qApp->devicePixelRatio(), tilesManager->height() / qApp->devicePixelRatio(), 1, PixmapRequest::Asynchronous); + PixmapRequest *p = new PixmapRequest(observer, pageNumber, tilesManager->width(), tilesManager->height(), 1 /* dpr */, 1, PixmapRequest::Asynchronous); // Get the visible page rect NormalizedRect visibleRect; diff --git a/core/generator.cpp b/core/generator.cpp index 6e59a72dd..6b4957cf1 100644 --- a/core/generator.cpp +++ b/core/generator.cpp @@ -570,12 +570,17 @@ TextRequestPrivate *TextRequestPrivate::get(const TextRequest *req) } PixmapRequest::PixmapRequest(DocumentObserver *observer, int pageNumber, int width, int height, int priority, PixmapRequestFeatures features) + : PixmapRequest(observer, pageNumber, width, height, qApp->devicePixelRatio(), priority, features) +{ +} + +PixmapRequest::PixmapRequest(DocumentObserver *observer, int pageNumber, int width, int height, qreal dpr, int priority, PixmapRequestFeatures features) : d(new PixmapRequestPrivate) { d->mObserver = observer; d->mPageNumber = pageNumber; - d->mWidth = ceil(width * qApp->devicePixelRatio()); - d->mHeight = ceil(height * qApp->devicePixelRatio()); + d->mWidth = ceil(width * dpr); + d->mHeight = ceil(height * dpr); d->mPriority = priority; d->mFeatures = features; d->mForce = false; diff --git a/core/generator.h b/core/generator.h index 44ac21df2..a53d2935f 100644 --- a/core/generator.h +++ b/core/generator.h @@ -648,12 +648,25 @@ public: * * @param observer The observer. * @param pageNumber The page number. - * @param width The width of the page. - * @param height The height of the page. + * @param width The width of the page in logical pixels. + * @param height The height of the page in logical pixels. * @param priority The priority of the request. * @param features The features of generation. */ - PixmapRequest(DocumentObserver *observer, int pageNumber, int width, int height, int priority, PixmapRequestFeatures features); + [[deprecated("This PixmapRequest constructor is deprecated, use the one including the device pixel ratio")]] PixmapRequest(DocumentObserver *observer, int pageNumber, int width, int height, int priority, PixmapRequestFeatures features); + + /** + * Creates a new pixmap request. + * + * @param observer The observer. + * @param pageNumber The page number. + * @param width The width of the page in logical pixels. + * @param height The height of the page in logical pixels. + * @param dpr Device pixel ratio of the screen that the pixmap is intended for. + * @param priority The priority of the request. + * @param features The features of generation. + */ + PixmapRequest(DocumentObserver *observer, int pageNumber, int width, int height, qreal dpr, int priority, PixmapRequestFeatures features); /** * Destroys the pixmap request. diff --git a/part/magnifierview.cpp b/part/magnifierview.cpp index 02d81fd15..36694c027 100644 --- a/part/magnifierview.cpp +++ b/part/magnifierview.cpp @@ -128,7 +128,7 @@ void MagnifierView::requestPixmap() if (m_page && !m_page->hasPixmap(this, full_width, full_height, nrect)) { QLinkedList requestedPixmaps; - Okular::PixmapRequest *p = new Okular::PixmapRequest(this, m_current, full_width, full_height, PAGEVIEW_PRIO, Okular::PixmapRequest::Asynchronous); + Okular::PixmapRequest *p = new Okular::PixmapRequest(this, m_current, full_width, full_height, devicePixelRatioF(), PAGEVIEW_PRIO, Okular::PixmapRequest::Asynchronous); if (m_page->hasTilesManager(this)) { p->setTile(true); diff --git a/part/pageview.cpp b/part/pageview.cpp index 7d5f4db26..a8952bb52 100644 --- a/part/pageview.cpp +++ b/part/pageview.cpp @@ -4426,7 +4426,7 @@ void PageView::delayedResizeEvent() slotRequestVisiblePixmaps(); } -static void slotRequestPreloadPixmap(Okular::DocumentObserver *observer, const PageViewItem *i, const QRect expandedViewportRect, QLinkedList *requestedPixmaps) +static void slotRequestPreloadPixmap(PageView *pageView, const PageViewItem *i, const QRect expandedViewportRect, QLinkedList *requestedPixmaps) { Okular::NormalizedRect preRenderRegion; const QRect intersectionRect = expandedViewportRect.intersected(i->croppedGeometry()); @@ -4434,18 +4434,18 @@ static void slotRequestPreloadPixmap(Okular::DocumentObserver *observer, const P preRenderRegion = Okular::NormalizedRect(intersectionRect.translated(-i->uncroppedGeometry().topLeft()), i->uncroppedWidth(), i->uncroppedHeight()); // request the pixmap if not already present - if (!i->page()->hasPixmap(observer, i->uncroppedWidth(), i->uncroppedHeight(), preRenderRegion) && i->uncroppedWidth() > 0) { + if (!i->page()->hasPixmap(pageView, i->uncroppedWidth(), i->uncroppedHeight(), preRenderRegion) && i->uncroppedWidth() > 0) { Okular::PixmapRequest::PixmapRequestFeatures requestFeatures = Okular::PixmapRequest::Preload; requestFeatures |= Okular::PixmapRequest::Asynchronous; - const bool pageHasTilesManager = i->page()->hasTilesManager(observer); + const bool pageHasTilesManager = i->page()->hasTilesManager(pageView); if (pageHasTilesManager && !preRenderRegion.isNull()) { - Okular::PixmapRequest *p = new Okular::PixmapRequest(observer, i->pageNumber(), i->uncroppedWidth(), i->uncroppedHeight(), PAGEVIEW_PRELOAD_PRIO, requestFeatures); + Okular::PixmapRequest *p = new Okular::PixmapRequest(pageView, i->pageNumber(), i->uncroppedWidth(), i->uncroppedHeight(), pageView->devicePixelRatioF(), PAGEVIEW_PRELOAD_PRIO, requestFeatures); requestedPixmaps->push_back(p); p->setNormalizedRect(preRenderRegion); p->setTile(true); } else if (!pageHasTilesManager) { - Okular::PixmapRequest *p = new Okular::PixmapRequest(observer, i->pageNumber(), i->uncroppedWidth(), i->uncroppedHeight(), PAGEVIEW_PRELOAD_PRIO, requestFeatures); + Okular::PixmapRequest *p = new Okular::PixmapRequest(pageView, i->pageNumber(), i->uncroppedWidth(), i->uncroppedHeight(), pageView->devicePixelRatioF(), PAGEVIEW_PRELOAD_PRIO, requestFeatures); requestedPixmaps->push_back(p); p->setNormalizedRect(preRenderRegion); } @@ -4527,7 +4527,7 @@ void PageView::slotRequestVisiblePixmaps(int newValue) #ifdef PAGEVIEW_DEBUG kWarning() << "rerequesting visible pixmaps for page" << i->pageNumber() << "!"; #endif - Okular::PixmapRequest *p = new Okular::PixmapRequest(this, i->pageNumber(), i->uncroppedWidth(), i->uncroppedHeight(), PAGEVIEW_PRIO, Okular::PixmapRequest::Asynchronous); + Okular::PixmapRequest *p = new Okular::PixmapRequest(this, i->pageNumber(), i->uncroppedWidth(), i->uncroppedHeight(), devicePixelRatioF(), PAGEVIEW_PRIO, Okular::PixmapRequest::Asynchronous); requestedPixmaps.push_back(p); if (i->page()->hasTilesManager(this)) { diff --git a/part/presentationwidget.cpp b/part/presentationwidget.cpp index 5499e3972..f43865a9b 100644 --- a/part/presentationwidget.cpp +++ b/part/presentationwidget.cpp @@ -1360,6 +1360,7 @@ QScreen *PresentationWidget::defaultScreen() const void PresentationWidget::requestPixmaps() { + const qreal dpr = devicePixelRatioF(); PresentationFrame *frame = m_frames[m_frameIndex]; int pixW = frame->geometry.width(); int pixH = frame->geometry.height(); @@ -1368,7 +1369,7 @@ void PresentationWidget::requestPixmaps() QApplication::setOverrideCursor(QCursor(Qt::BusyCursor)); // request the pixmap QLinkedList requests; - requests.push_back(new Okular::PixmapRequest(this, m_frameIndex, pixW, pixH, PRESENTATION_PRIO, Okular::PixmapRequest::NoFeature)); + requests.push_back(new Okular::PixmapRequest(this, m_frameIndex, pixW, pixH, dpr, PRESENTATION_PRIO, Okular::PixmapRequest::NoFeature)); // restore cursor QApplication::restoreOverrideCursor(); // ask for next and previous page if not in low memory usage setting @@ -1389,7 +1390,7 @@ void PresentationWidget::requestPixmaps() pixW = nextFrame->geometry.width(); pixH = nextFrame->geometry.height(); if (!nextFrame->page->hasPixmap(this, pixW, pixH)) - requests.push_back(new Okular::PixmapRequest(this, tailRequest, pixW, pixH, PRESENTATION_PRELOAD_PRIO, requestFeatures)); + requests.push_back(new Okular::PixmapRequest(this, tailRequest, pixW, pixH, dpr, PRESENTATION_PRELOAD_PRIO, requestFeatures)); } int headRequest = m_frameIndex - j; @@ -1398,7 +1399,7 @@ void PresentationWidget::requestPixmaps() pixW = prevFrame->geometry.width(); pixH = prevFrame->geometry.height(); if (!prevFrame->page->hasPixmap(this, pixW, pixH)) - requests.push_back(new Okular::PixmapRequest(this, headRequest, pixW, pixH, PRESENTATION_PRELOAD_PRIO, requestFeatures)); + requests.push_back(new Okular::PixmapRequest(this, headRequest, pixW, pixH, dpr, PRESENTATION_PRELOAD_PRIO, requestFeatures)); } // stop if we've already reached both ends of the document diff --git a/part/thumbnaillist.cpp b/part/thumbnaillist.cpp index 7a259d1bb..c3361d885 100644 --- a/part/thumbnaillist.cpp +++ b/part/thumbnaillist.cpp @@ -649,7 +649,7 @@ void ThumbnailListPrivate::slotRequestVisiblePixmaps() m_visibleThumbnails.push_back(t); // if pixmap not present add it to requests if (!t->page()->hasPixmap(q, t->pixmapWidth(), t->pixmapHeight())) { - Okular::PixmapRequest *p = new Okular::PixmapRequest(q, t->pageNumber(), t->pixmapWidth(), t->pixmapHeight(), THUMBNAILS_PRIO, Okular::PixmapRequest::Asynchronous); + Okular::PixmapRequest *p = new Okular::PixmapRequest(q, t->pageNumber(), t->pixmapWidth(), t->pixmapHeight(), devicePixelRatioF(), THUMBNAILS_PRIO, Okular::PixmapRequest::Asynchronous); requestedPixmaps.push_back(p); } }