Fix OpenGL canvas under fractional DPI scaling
ClosedPublic

Authored by alvinhochun on Mar 28 2019, 5:41 PM.

Details

Summary
The canvas wasn't pixel-perfect even at 100% zoom when the
devicePixelRatio is non-integral. This change fixes the issue by
teaching Krita to perceive the widget/viewport size in the same manner
as Qt sets up the viewport, and to align the documentOffset to device
pixels.

The canvas image tends to be blurred or contains missing lines, depending on the widget size and document offset. The effect is best observed by using pixel art or images with 1px lines, like

. Wraparound mode can make the issue more observable.

What this patch fixes (under fractional DPI scaling):

  • The OpenGL canvas and coordinates converter now rounds the widget/viewport size to device pixel which matches the viewport set up by Qt.
  • The document offset always snaps to device pixels.
  • Observable result: The image is now always rendered pixel-perfect under 100% zoom and with square rotation angles.
  • (The leaking checkers issue that I mentioned on IRC is fixed in this patch.)

What this patch does not fix (High DPI issues specific to fractional DPI scaling):

  • The ruler is slightly off at certain documentOffset

What this patch does not fix (High DPI issues in general, non-exhaustive):

  • Mouse input (unsure about tablet input) for some operations (e.g. colour-picking, painting) are snapped to logical pixels, i.e. some pixels are inaccessible at 100% zoom.
  • (Minor) Document offset when panning is snapped to logical pixels, i.e. moving one px when panning may pan the canvas by 0, 1 or 2px depending on the current offset.
  • QPainter canvas is still a mess under high dpi
Test Plan

You can use any monitor for testing. They don't need to be high DPI. (Qt doesn't use fractional scaling by default so overriding is needed anyway. Unless you are using KDE.)

If on Windows, open a command prompt in the krita bin/ dir:

set QT_AUTO_SCREEN_SCALE_FACTOR=0
set QT_SCREEN_SCALE_FACTORS=1.5
krita.exe

Or with PowerShell:

$env:QT_AUTO_SCREEN_SCALE_FACTOR="0"
$env:QT_SCREEN_SCALE_FACTORS="1.5"
.\krita.exe

For Linux, this should work:

QT_AUTO_SCREEN_SCALE_FACTOR=0 QT_SCREEN_SCALE_FACTORS=1.5 ./krita

If you have two monitors, you can test with QT_SCREEN_SCALE_FACTORS=1.5;1 to simulate per-monitor DPI scaling.

Test with images with sharp edges that you can easily tell if the render is pixel-perfect. For more detailed inspection, you can take a screenshot and check the pixels in detail. Check the colours for any unwanted interpolation.

Diff Detail

Repository
R37 Krita
Lint
Lint Skipped
Unit
Unit Tests Skipped
alvinhochun requested review of this revision.Mar 28 2019, 5:41 PM
alvinhochun created this revision.
alvinhochun updated this revision to Diff 55489.Apr 5 2019, 5:26 PM
  • Starting to convert more stuff to use device pixel
  • Beginning to differentiate between device pixel and logical pixels

I think I really need to start by refractoring/simplifying KisCanvasController / KoCanvasController / KoCanvasControllerWidget and friends...

alvinhochun added inline comments.Apr 5 2019, 5:44 PM
libs/ui/canvas/kis_coordinates_converter.cpp
47

I have a suspicion that the original author chose QSizeF and QPointF for performance - that is, converting between integers and floating point adds some overhead? It was changed on commit c93a6dcc61b96d0387ae0f60f0edb8339f5f18a0.

Looks like premature optimization to me...

alvinhochun updated this revision to Diff 55758.Apr 8 2019, 5:38 PM
alvinhochun updated this revision to Diff 55845.Apr 9 2019, 5:50 PM

Speaking truly, I don't understand, what bug this huge refactoring is intended to fix. Please list the problems you are trying to resolve.

Right now the patch introduces at lease two huge regressions:

  1. Zoom with mouse wheel and Ctrl+Space centers around wrong point.
  2. Rotation with Shift+Space center around wrong point and drifts from place to place (some rounding issues).

The main point, I don't see what problem we are trying to solve?

libs/ui/canvas/kis_coordinates_converter.cpp
47

No, it was not an optimization. It was a fix to avoid rounding errors when doing canvas rotation with Shift+Space+LMB. The matrices are updated incrementally, so rounding errors build-up easily. Your refactoring causes this regression again.

184

Looks like atm this function duplicates functionality of setCanvasWidgetSize

alvinhochun marked 3 inline comments as done.
alvinhochun retitled this revision from (Don't accept) Fix OpenGL canvas with fractional DPI scaling to Fix OpenGL canvas under fractional DPI scaling.
alvinhochun edited the summary of this revision. (Show Details)

Speaking truly, I don't understand, what bug this huge refactoring is intended to fix. Please list the problems you are trying to resolve.

Right now the patch introduces at lease two huge regressions:

  1. Zoom with mouse wheel and Ctrl+Space centers around wrong point.
  2. Rotation with Shift+Space center around wrong point and drifts from place to place (some rounding issues).

    The main point, I don't see what problem we are trying to solve?

You're right, this kind of refactoring is not needed for the issue I'm solving here, thus I've reverted back to the first version and revised upon it.

Please also refer to the edited summary.

alvinhochun edited the summary of this revision. (Show Details)Apr 12 2019, 5:39 PM
alvinhochun edited the summary of this revision. (Show Details)Apr 13 2019, 11:02 AM

Moved some code from the converter to the canvas.

The patch works fine now! Please push! :)

(but don't forget to remove debugging statements first)

libs/ui/canvas/kis_canvas2.cpp
1073

debug--

libs/ui/canvas/kis_coordinates_converter.cpp
161

Please remove this debugging line :)

libs/ui/opengl/kis_opengl_canvas2.cpp
390

This one should also go :)

dkazakov accepted this revision.Apr 15 2019, 1:30 PM
This revision is now accepted and ready to land.Apr 15 2019, 1:30 PM
This revision was automatically updated to reflect the committed changes.