Merge lp:~dandrader/qtubuntu/resizeToolTip into lp:qtubuntu

[Daniel d'Andrada]

Resize menus and toolips when told to do so

Fixes the following bug that you can see when running the childWindows branches + https://code.launchpad.net/~alan-griffiths/miral/fix-1652109/+merge/313782

1 - launch Kate
2 - Open some big text file (at least hundreds of lines long)
3 - scroll to the very top
4 - shorten the Kate window so that it doesn't not show many lines at once
5 - press and hold the down arrow on the scroll bar
6 - see the tooltip displaying some line numbers

expected outcome:
tooltip gets wider once it starts diplaying numbers bigger than 99

actual outcome:
numbers get squeezed once they get past 99

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

PASSED: Continuous integration, rev:364
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/165/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/3630
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3658
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3504
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3504/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3504
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3504/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3504
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3504/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3504
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3504/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3504
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3504/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3504
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3504/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Gerry Boland (gerboland)]

You're removing the ability for Qt to reposition an existing surface. I believe I've seen some Qt apps doing that for tooltips, resize & reposition a single surface which it used for all tooltips.

Thus I'm not happy with this, but the core point is correct (not setting the new geometry in the surface spec if surface also moved)

review: Needs Information

[Daniel d'Andrada (dandrader)]

On 05/01/2017 15:32, Gerry Boland wrote:
> Review: Needs Information
>
> You're removing the ability for Qt to reposition an existing surface. I believe I've seen some Qt apps doing that for tooltips, resize & reposition a single surface which it used for all tooltips.
>
> Thus I'm not happy with this, but the core point is correct (not setting the new geometry in the surface spec if surface also moved)

I'm not seeing the original code doing anything with the geometry
received from Qt (including position) for the so-called movable types.
How come the original code resizes and repositions "movable" types? Am I
missing something?

[Daniel d'Andrada (dandrader)]

On 17/01/2017 17:15, Daniel d'Andrada wrote:
> On 05/01/2017 15:32, Gerry Boland wrote:
>> Review: Needs Information
>>
>> You're removing the ability for Qt to reposition an existing surface. I believe I've seen some Qt apps doing that for tooltips, resize & reposition a single surface which it used for all tooltips.
>>
>> Thus I'm not happy with this, but the core point is correct (not setting the new geometry in the surface spec if surface also moved)
> I'm not seeing the original code doing anything with the geometry
> received from Qt (including position) for the so-called movable types.
> How come the original code resizes and repositions "movable" types? Am I
> missing something?
>
>
Got it now. Fixed.

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

PASSED: Continuous integration, rev:365
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/171/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/3839
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3867
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3710
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3710/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3710
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3710/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3710
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3710/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3710
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3710/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3710
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3710/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3710
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3710/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Gerry Boland (gerboland)]

/me wonders what happens if a client asks to move & resize a surface type that mir considered not-movable. Does it still get resized?

I guess we'll figure that out when we hit an example of that.

But confirmed this works ok with Kate and a few Qt demos

review: Approve

[Daniel d'Andrada (dandrader)]

On 18/01/2017 12:26, Gerry Boland wrote:
> Review: Approve
>
> /me wonders what happens if a client asks to move & resize a surface type that mir considered not-movable. Does it still get resized?
>

It's just a client request, server not obliged to comply. Furthermore,
movement requests only happens for child windows.

[Gerry Boland (gerboland)]

Testing this with silo 2313, with Kate, try opening the popup menus from the bottom-right panel (tabs style, text encoding..)

With this, the surface positions are incorrect, usually 0,0
Without this, the surface positions are more correct, just sized wrong.

I think the parent check is problematic here

review: Needs Fixing

[Daniel d'Andrada (dandrader)]

On 19/01/2017 12:41, Gerry Boland wrote:
> Review: Needs Fixing
>
> Testing this with silo 2313, with Kate, try opening the popup menus from the bottom-right panel (tabs style, text encoding..)
>
> With this, the surface positions are incorrect, usually 0,0
> Without this, the surface positions are more correct, just sized wrong.
>
> I think the parent check is problematic here

Yes, fixed.

[Lukáš Tinkl (lukas-kde)]

Would like to see the CI run after the recent changes (top approval changed)

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

PASSED: Continuous integration, rev:366
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/172/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/3867
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3895
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3740
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3740/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3740
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3740/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3740
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3740/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3740
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3740/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3740
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3740/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3740
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3740/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Gerry Boland (gerboland)]

Yes, better

review: Approve

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

PASSED: Continuous integration, rev:367
https://unity8-jenkins.ubuntu.com/job/lp-qtubuntu-ci/173/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/3882
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/3910
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3754
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/3754/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3754
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=zesty/3754/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3754
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/3754/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3754
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=zesty/3754/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3754
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/3754/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3754
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=zesty/3754/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)

=== modified file 'src/ubuntumirclient/window.cpp'
--- src/ubuntumirclient/window.cpp	2016-12-09 17:02:57 +0000
+++ src/ubuntumirclient/window.cpp	2017-01-19 18:27:48 +0000
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2014-2016 Canonical, Ltd.
+ * Copyright (C) 2014-2017 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
@@ -222,18 +222,6 @@
     return requiresParent(qtWindowTypeToMirSurfaceType(type));
 }
 
-bool isMovable(const Qt::WindowType type)
-{
-    auto mirType = qtWindowTypeToMirSurfaceType(type);
-    switch (mirType) {
-    case mir_surface_type_menu:
-    case mir_surface_type_tip:
-        return true;
-    default:
-        return false;
-    }
-}
-
 Spec makeSurfaceSpec(QWindow *window, MirPixelFormat pixelFormat, UbuntuWindow *parentWindowHandle,
                      MirConnection *connection)
 {
@@ -534,17 +522,33 @@
 
 void UbuntuSurface::updateGeometry(const QRect &newGeometry)
 {
-    qCDebug(ubuntumirclient,"updateGeometry(window=%p, width=%d, height=%d)", mWindow,
-            newGeometry.width(), newGeometry.height());
-
-    Spec spec;
-    if (isMovable(mWindow->type())) {
-        spec = Spec{makeSurfaceSpec(mWindow, mPixelFormat, mParentWindowHandle, mConnection)};
+
+    auto spec = Spec{mir_connection_create_spec_for_changes(mConnection)};
+
+    mir_surface_spec_set_width(spec.get(), newGeometry.width());
+    mir_surface_spec_set_height(spec.get(), newGeometry.height());
+
+    MirRectangle mirRect {0,0,0,0};
+
+    if (mParentWindowHandle) {
+        qCDebug(ubuntumirclient,"updateGeometry(window=%p, x=%d, y=%d, width=%d, height=%d, child)", mWindow,
+                newGeometry.x(), newGeometry.y(), newGeometry.width(), newGeometry.height());
+
+        mirRect.left = newGeometry.x() - mParentWindowHandle->window()->x();
+        mirRect.top = newGeometry.y() - mParentWindowHandle->window()->y();
+
     } else {
-        spec = Spec{mir_connection_create_spec_for_changes(mConnection)};
-        mir_surface_spec_set_width(spec.get(), newGeometry.width());
-        mir_surface_spec_set_height(spec.get(), newGeometry.height());
+        qCDebug(ubuntumirclient,"updateGeometry(window=%p, x=%d, y=%d, width=%d, height=%d, top-level)", mWindow,
+                newGeometry.x(), newGeometry.y(), newGeometry.width(), newGeometry.height());
+
+        mirRect.left = newGeometry.x();
+        mirRect.top = newGeometry.y();
     }
+
+    mir_surface_spec_set_placement(spec.get(), &mirRect,
+            mir_placement_gravity_northwest /* rect_gravity */, mir_placement_gravity_northwest /* surface_gravity */,
+            (MirPlacementHints)0, 0 /* offset_dx */, 0 /* offset_dy */);
+
     mir_surface_apply_spec(mMirSurface, spec.get());
 }