Merge lp:~vanvugt/qtmir/unstretch into lp:qtmir

[Daniel van Vugt]

Implement non-stretchy resizing. This partly fixes LP: #1466510
and probably other use cases too.

Although the delay is still there, any stretching you do see
will be limited to the pixels on the right and bottom edges.

Reducing the delay is left as an exercise for the reader. :)

[Gerry Boland (gerboland)]

This indeed will stop the stretching, but will replace it with an app surface with weird borders instead. This is a small visual improvement, but will mask the core issue. So I would not consider it a proper fix for the attached bug.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:366
http://jenkins.qa.ubuntu.com/job/qtmir-ci/373/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-wily-amd64-ci/106
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtmir-wily-armhf-ci/106
        deb: http://jenkins.qa.ubuntu.com/job/qtmir-wily-armhf-ci/106/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/qtmir-ci/373/rebuild

review: Approve (continuous-integration)

[Daniel van Vugt (vanvugt)]

For rotation, yes I think we can maybe do better but not block this branch on that. And for general desktop resizing, we should not attempt to do anything "better".

There's a reason why you see the same lagging resize on other operating systems. I've seen it on Windows 10 and ChromeOS recently too. The reason is that it's more desirable to not "trust" the client, and never wait for the client to do anything, than it is to delay screen updates (miss frames) keeping things visually synchronized. A lagging client behind a smooth shell is better than a stuttering (but synchronized) shell+client.

Mir is built on the design principle that we never trust the client or wait for the client. Mir just acts on the latest information it has received at the time. And Unity8 should probably do the same.

What does help to reduce the visual lag in resizing is the length of the buffer queue. So MIR_SERVER_NBUFFERS=2 halves the problem. And we have other optimizations on the way that will also reduce the visibility of resize lag.

That all said, I think any additional work to improve the fix for the attached bug should be done separately.

[Daniel van Vugt (vanvugt)]

Also, "weird borders" is an overstatement. The goal is to try and make most of the client's pixels appear to not move or stretch during resizing. The only weirdness is on the right and bottom edges, only during rapid resizing, and only a small percentage of the overall surface is affected. In fact, in most cases where you're resizing a surface whose edges are one uniform colour you will not see any clamping artefacts at all. So this is much more desirable and less noticeable than having every single pixel stretched.

[Daniel van Vugt (vanvugt)]

I've now clarified the bug so that this branch is not seen as a full fix. It's just a related branch.

[Daniel d'Andrada (dandrader)]

This won't allow shell to intentionally scale window (ie,"strech" it but keeping the original aspect ratio).

lp:~dandrader/qtmir/mirSurface solve this issue by giving QML code the freedom to choose whether it wants to scale/stretch a surface or not. So it can implement resizing by stretching or clamping.

review: Disapprove

[Daniel van Vugt (vanvugt)]

Fair point. But can you point to a use case I can test for that?

We still need this change for the generic resizing case.

[Daniel van Vugt (vanvugt)]

What is the use case for wanting resizing by stretching? Mir had that (accidentally) for a while and it was widely hated because the stretching was so distracting. So now Mir always uses clamping, like every other desktop OS.

[Daniel d'Andrada (dandrader)]

On 27/08/2015 03:45, Daniel van Vugt wrote:
> What is the use case for wanting resizing by stretching? Mir had that (accidentally) for a while and it was widely hated because the stretching was so distracting. So now Mir always uses clamping, like every other desktop OS.

There's no use case for resizing by streching. But there is for
stretching for the sake of scale it. So you say this branch does not
impede QML from scaling (ie, stretching without changing the aspect
ratio) a surface in QML?

If so, I will try it out next week (once I'm back from the sprint) with
qml-demo-shell and tell you if that's indeed the case.

By the way, would be great if you rebased this against
lp:~dandrader/qtmir/mirSurface as it changes a lot of stuff and it's
gonna land ASAP.

[Daniel d'Andrada (dandrader)]

review: Needs Information

[Daniel van Vugt (vanvugt)]

If you implement scale, then you still want this branch. It would just change to something like:

    qreal u = static_cast<qreal>(width()) / (scale * textureSize.width());
    qreal v = static_cast<qreal>(height()) / (scale * textureSize.height());

[Daniel van Vugt (vanvugt)]

Just tested the unity8 desktop session. The stretching is quite bad during window resizing so it desperately needs this.

[Daniel d'Andrada (dandrader)]

On 03/09/15 06:52, Daniel van Vugt wrote:
> Just tested the unity8 desktop session. The stretching is quite bad during window resizing so it desperately needs this.

I meant to get to it this week. Even started but other things kept
getting in between.
My only problem with this is that it closes the door for a scaling
option. So I was working on a patch that does this stuff but also allows
QML to scale (ie, streching at will) if it wants to.

[Daniel van Vugt (vanvugt)]

It does not close the door on scaling. Scroll up to find I demonstrated a solution on 2015-08-31.

[Daniel d'Andrada (dandrader)]

On 08/09/15 03:07, Daniel van Vugt wrote:
> It does not close the door on scaling. Scroll up to find I demonstrated a solution on 2015-08-31.

But I think we need to have the scaling option included in this MP.
Otherwise it can block features like live thumbnails that are going to
be used in the desktop window switcher.

At the moment I'm thinking of an API like this:

MirSurfaceItem.fillMode, which could be either "Stretch" or "PadOrCrop"

[Daniel d'Andrada (dandrader)]

This is what I meant:
https://code.launchpad.net/~dandrader/qtmir/surfaceItemFillMode/+merge/270859

review: Disapprove

Unmerged revisions

366. By Daniel van Vugt on 2015-08-21

Implement non-stretchy resizing. This fixes most of LP: #1466510
and probably other use cases too.

Although the delay is still there, any stretching you do see
will be limited to the pixels on the right and bottom edges.

365. By CI Train Bot Account on 2015-08-20

Releasing 0.4.5+15.04.20150820-0ubuntu1

364. By Gerry Boland on 2015-08-20

Hotfix to work around bug 1483752

=== modified file 'src/modules/Unity/Application/mirsurfaceitem.cpp'
--- src/modules/Unity/Application/mirsurfaceitem.cpp	2015-08-20 12:38:34 +0000
+++ src/modules/Unity/Application/mirsurfaceitem.cpp	2015-08-21 10:06:54 +0000
@@ -431,13 +431,28 @@
         node->setMipmapFiltering(QSGTexture::None);
         node->setHorizontalWrapMode(QSGTexture::ClampToEdge);
         node->setVerticalWrapMode(QSGTexture::ClampToEdge);
-        node->setSubSourceRect(QRectF(0, 0, 1, 1));
     } else {
         if (textureUpdated) {
             node->markDirty(QSGNode::DirtyMaterial);
         }
     }
 
+    /*
+     * Set texture coordinates correctly so we never see stretching.
+     * The client is a separate process so we must excuse it for lagging
+     * behind a little. The key is to choose texture coordinates in such
+     * a way that even when the texture dimensions differ from the window
+     * dimensions, a single texel still ends up fitting a screen pixel
+     * perfectly.
+     * This way any difference in dimensions is only visible at the right
+     * and bottom edges, either cut off or filled in using the edge colour
+     * (that's what QSGTexture::ClampToEdge above is for).
+     */
+    const QSize &textureSize = m_textureProvider->t->textureSize();
+    qreal u = static_cast<qreal>(width()) / textureSize.width();
+    qreal v = static_cast<qreal>(height()) / textureSize.height();
+    node->setSubSourceRect(QRectF(0, 0, u, v));
+
     node->setTargetRect(QRectF(0, 0, width(), height()));
     node->setInnerTargetRect(QRectF(0, 0, width(), height()));