davidedmundson added a comment.
"Test plan" simply needs to be a list of things you've tried doing so the maintainers can point out if there's any parts you've not thought about. So saying if you tested annotations and what types of files you opened, checked with zoom = 100%..that sort of thing. INLINE COMMENTS > pagepainter.cpp:75-76 > > + int dScaledWidth = floor(scaledWidth * dpr) + 1; > + int dScaledHeight = floor(scaledHeight * dpr) + 1; > + const QRect dLimits(QRectF(limits.x() * dpr, limits.y() * dpr, > limits.width() * dpr, limits.height() * dpr).toAlignedRect()); Why? (maybe ceil() ? > pagepainter.cpp:410 > // highlight composition (product: highlight color * > destcolor) > + /* > + QRect highlightRect = r.geometry( dScaledWidth, > dScaledHeight ).translated( -dScaledCrop.topLeft() ).intersected( dLimits ); If the code below works, I'd avoid changing it. Painting something on top isn't quite the same as the exitsing multiplying the two values together, and the old code has that //for odt or epub hack swapping black pixels for white ones. > pagepainter.cpp:911 > > -void PagePainter::scalePixmapOnImage ( QImage & dest, const QPixmap * src, > - int scaledWidth, int scaledHeight, const QRect & cropRect, > QImage::Format format ) Can you explain on Phabricator the changes you've made in PagePainter, and that benchmarking you did. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D6268 To: hetzenecker, davidedmundson Cc: #okular, aacid
