Unity 8

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

  1. pixelAlignedWindow
  2. Merge into trunk
Proposed by Daniel d'Andrada
Status: Merged
Approved by: Albert Astals Cid
Approved revision: 2400
Merged at revision: 2453
Proposed branch: lp://staging/~dandrader/unity8/pixelAlignedWindow
Merge into: lp://staging/unity8
Prerequisite: lp://staging/~dandrader/unity8/animatedCursors
Diff against target: 71 lines (+17/-6)
3 files modified
plugins/Cursor/MousePointer.cpp (+8/-3)
plugins/Cursor/MousePointer.h (+4/-1)
qml/Stages/WindowDecoration.qml (+5/-2)
To merge this branch: bzr merge lp://staging/~dandrader/unity8/pixelAlignedWindow
Reviewer Review Type Date Requested Status
Unity8 CI Bot continuous-integration Approve
Albert Astals Cid (community) Approve
Lukáš Tinkl (community) Needs Information
Review via email: mp+295262@code.staging.launchpad.net

Commit message

Ensure mouse and window movement are pixel-aligned

To avoid blurry renderings (LP: #1510382)

Description of the change

* 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

To post a comment you must log in.
Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

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)
Revision history for this message
Lukáš Tinkl (lukas-kde) wrote :

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
Revision history for this message
Daniel d'Andrada (dandrader) wrote :

> 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.

Revision history for this message
Albert Astals Cid (aacid) wrote :

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
Revision history for this message
Daniel d'Andrada (dandrader) wrote :

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.

Revision history for this message
Albert Astals Cid (aacid) wrote :

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
Revision history for this message
Daniel d'Andrada (dandrader) wrote :

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.

Revision history for this message
Albert Astals Cid (aacid) wrote :

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
Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

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)
Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

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)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches