Merge lp:~dandrader/unity8/pixelAlignedWindow into lp:unity8

[Daniel d'Andrada]

Ensure mouse and window movement are pixel-aligned

To avoid blurry renderings (LP: #1510382)

* Are there any related MPs required for this MP to build/function as expected? Please list.
There MPs required by its prerequisite.

* Did you perform an exploratory manual test run of your code change and any related functionality?
Yes

* If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
N/A

* If you changed the UI, has there been a design review?
N/A

[Unity8 CI Bot (unity8-ci-bot)]

FAILED: Continuous integration, rev:2400
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/1258/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/1700
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1650
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/1650
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1643/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1643/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1643/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1643/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1643/console
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1643/console

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/1258/rebuild

review: Needs Fixing (continuous-integration)

[Lukáš Tinkl (lukas-kde)]

I wonder how can one reproduce it (seeing the screenshots in the attached BR); I've never experienced these glitches. Also, would be nice to have a test, at least for the window movement part.

review: Needs Information

[Daniel d'Andrada (dandrader)]

> I wonder how can one reproduce it (seeing the screenshots in the attached BR);
> I've never experienced these glitches. Also, would be nice to have a test, at
> least for the window movement part.

I also never caught my attention. But by close visual inspection and adding some logging I did notice (and read proof) that cursor and window were not pixel aligned on my laptop and indeed a bit blurry as a result. The cursor definitely looks better now.

How perceptible is this issue depends on the pixel density of your display and how big the grid unit is. In your case (a 2160p laptop display) it might be hard to see the difference.

Will see about the test.

[Albert Astals Cid (aacid)]

The window is a very noticeable improvement.

On the cursor though i get visual noise with the patch
if i move the mouse horizontally to the right the vertical line of the pointing cursor seems to have some ghost a few pixels to its left
I tried recording it with the phone but i guess the camera is not as good/bad as my eyes

can you try to reproduce?

review: Needs Fixing

[Daniel d'Andrada (dandrader)]

On 20/05/2016 10:17, Albert Astals Cid wrote:
> Review: Needs Fixing
>
> The window is a very noticeable improvement.
>
> On the cursor though i get visual noise with the patch
> if i move the mouse horizontally to the right the vertical line of the pointing cursor seems to have some ghost a few pixels to its left
> I tried recording it with the phone but i guess the camera is not as good/bad as my eyes
>
> can you try to reproduce?

Please apply that patch:

http://pastebin.ubuntu.com/16520939/

And see if you get any non-integer position.

Also please send me a screenshot on startup, before you move the mouse
so it will be pixel aligned at (0,0) and when you think it looks bad.

[Albert Astals Cid (aacid)]

Ok, so the ghosting happens without the patch, may be an issue somewhere else, let's not block on that.

How sensible is a test for this? Maybe something that connects to the x/y position of the window/cursor when running all the other tests and asserts if the position is not an integer?

review: Needs Information

[Daniel d'Andrada (dandrader)]

Can't properly test. QtQuickTest::mouseEvent (which is what eventually gets called when you call mouseMove() from a qml test) converts movement from QPointF to QPoint, truncating it. So I can't emulate a subpixel-precision mouse movement in a qml test.

[Albert Astals Cid (aacid)]

Ok

 * Did you perform an exploratory manual test run of the code change and any related functionality?
Yes

 * Did CI run pass?
Parent branch has unity-api dep, ran fine on my machine.

review: Approve

[Unity8 CI Bot (unity8-ci-bot)]

FAILED: Continuous integration, rev:2400
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/1342/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/1780
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=vivid+overlay,testname=qmluitests.sh/876
    UNSTABLE: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=qmluitests.sh/876
    UNSTABLE: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=yakkety,testname=qmluitests.sh/876
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/1806
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1753
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/1753
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/1753
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1746
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1746/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1746
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1746/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/1746
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/1746/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1746
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1746/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1746
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1746/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/1746
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/1746/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1746
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1746/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1746
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1746/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/1746
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/1746/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/1342/rebuild

review: Needs Fixing (continuous-integration)

[Unity8 CI Bot (unity8-ci-bot)]

PASSED: Continuous integration, rev:2400
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/1388/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/1854
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=vivid+overlay,testname=qmluitests.sh/937
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=qmluitests.sh/937
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=yakkety,testname=qmluitests.sh/937
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/1880
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1818
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/1818
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/1818
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1809
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1809/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1809
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1809/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/1809
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/1809/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1809
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1809/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1809
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1809/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/1809
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/1809/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1809
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1809/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1809
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1809/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/1809
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/1809/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/1388/rebuild

review: Approve (continuous-integration)

=== modified file 'plugins/Cursor/MousePointer.cpp'
--- plugins/Cursor/MousePointer.cpp	2016-05-19 19:51:50 +0000
+++ plugins/Cursor/MousePointer.cpp	2016-05-19 19:51:50 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2015 Canonical, Ltd.
+ * Copyright (C) 2015-2016 Canonical, Ltd.
  *
  * This program is free software: you can redistribute it and/or modify it under
  * the terms of the GNU Lesser General Public License version 3, as published by
@@ -43,7 +43,12 @@
         Q_EMIT mouseMoved();
     }
 
-    qreal newX = x() + movement.x();
+    m_accumulatedMovement += movement;
+    // don't apply the fractional part
+    QPointF appliedMovement(int(m_accumulatedMovement.x()), int(m_accumulatedMovement.y()));
+    m_accumulatedMovement -= appliedMovement;
+
+    qreal newX = x() + appliedMovement.x();
     if (newX < 0) {
         Q_EMIT pushedLeftBoundary(qAbs(newX), buttons);
         newX = 0;
@@ -53,7 +58,7 @@
     }
     setX(newX);
 
-    qreal newY = y() + movement.y();
+    qreal newY = y() + appliedMovement.y();
     if (newY < 0) {
         newY = 0;
     } else if (newY > parentItem()->height()) {

=== modified file 'plugins/Cursor/MousePointer.h'
--- plugins/Cursor/MousePointer.h	2016-05-19 19:51:50 +0000
+++ plugins/Cursor/MousePointer.h	2016-05-19 19:51:50 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2015 Canonical, Ltd.
+ * Copyright (C) 2015-2016 Canonical, Ltd.
  *
  * This program is free software: you can redistribute it and/or modify it under
  * the terms of the GNU Lesser General Public License version 3, as published by
@@ -56,6 +56,9 @@
     QPointer<QWindow> m_registeredWindow;
     QString m_cursorName;
     QString m_themeName;
+
+    // Accumulated, unapplied, mouse movement.
+    QPointF m_accumulatedMovement;
 };
 
 #endif // MOUSEPOINTER_H

=== modified file 'qml/Stages/WindowDecoration.qml'
--- qml/Stages/WindowDecoration.qml	2016-04-27 15:01:10 +0000
+++ qml/Stages/WindowDecoration.qml	2016-05-19 19:51:50 +0000
@@ -58,8 +58,11 @@
         if (priv.dragging) {
             Mir.cursorName = "grabbing";
             var pos = mapToItem(root.target.parent, mouseX, mouseY);
-            root.target.x = pos.x - priv.distanceX;
-            root.target.y = Math.max(pos.y - priv.distanceY, PanelState.panelHeight);
+            // Use integer coordinate values to ensure that target is left in a pixel-aligned
+            // position. Mouse movement could have subpixel precision, yielding a fractional
+            // mouse position.
+            root.target.x = Math.round(pos.x - priv.distanceX);
+            root.target.y = Math.round(Math.max(pos.y - priv.distanceY, PanelState.panelHeight));
         }
     }