Merge lp://staging/~unity-team/qtubuntu/DPR2 into lp://staging/qtubuntu

PASSED: Continuous integration, rev:261
http://jenkins.qa.ubuntu.com/job/qtubuntu-ci/204/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtubuntu-vivid-armhf-ci/30
        deb: http://jenkins.qa.ubuntu.com/job/qtubuntu-vivid-armhf-ci/30/artifact/work/output/*zip*/output.zip

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

PASSED: Continuous integration, rev:262
http://jenkins.qa.ubuntu.com/job/qtubuntu-ci/205/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtubuntu-vivid-armhf-ci/31
        deb: http://jenkins.qa.ubuntu.com/job/qtubuntu-vivid-armhf-ci/31/artifact/work/output/*zip*/output.zip

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

In src/ubuntumirclient/window.cpp

"""
    // Window manager can give us a final size different from what we asked for
    // so let's check what we ended up getting
    {
        MirSurfaceParameters parameters;
        mir_surface_get_parameters(d->surface, &parameters);

        geometry.setWidth(divideAndRoundUp(parameters.width, devicePixelRatio()));
        geometry.setHeight(divideAndRoundUp(parameters.height, devicePixelRatio()));

        DLOG("[ubuntumirclient QPA] created surface has pixel size (%d, %d) and device-pixel size (%d, %d)",
                parameters.width, parameters.height, geometry.width(), geometry.height());
    }

    // Assume that the buffer size matches the surface size at creation time
    d->bufferSize = geometry.size();
"""

Here bufferSize will be initialized with a scaled size, but it should have the real size.
So bufferSize initialization should probably moved above the line "// Window manager can give us a final size different[...]".

PASSED: Continuous integration, rev:264
http://jenkins.qa.ubuntu.com/job/qtubuntu-ci/206/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtubuntu-vivid-armhf-ci/32
        deb: http://jenkins.qa.ubuntu.com/job/qtubuntu-vivid-armhf-ci/32/artifact/work/output/*zip*/output.zip

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

@Daniel, you were correct, I had mixed up a case where pixel != devicePixel. Since this is far too easy to screw up, I thought it a good idea to append "Px" to any variable that is in pixels, and so any other geometry is in device pixels. I think it helps distinguish them.

In src/ubuntumirclient/window.cpp:

"""
    DLOG("[ubuntumirclient QPA] creating surface at (%d, %d) with pixel size (%d, %d) with title '%s'\n",
            d->bufferSizePx.x(), d->bufferSizePx.y(), d->bufferSizePx.width(), d->bufferSizePx.height(), title.data());
"""

bufferSizePx has no x() and y() (as it's a QSize, not a QRect). I think code won't build if you enable debug logging?

PASSED: Continuous integration, rev:265
http://jenkins.qa.ubuntu.com/job/qtubuntu-ci/207/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtubuntu-vivid-armhf-ci/33
        deb: http://jenkins.qa.ubuntu.com/job/qtubuntu-vivid-armhf-ci/33/artifact/work/output/*zip*/output.zip

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

PASSED: Continuous integration, rev:266
http://jenkins.qa.ubuntu.com/job/qtubuntu-ci/214/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtubuntu-wily-armhf-ci/5
        deb: http://jenkins.qa.ubuntu.com/job/qtubuntu-wily-armhf-ci/5/artifact/work/output/*zip*/output.zip

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

FAILED: Continuous integration, rev:268
http://jenkins.qa.ubuntu.com/job/qtubuntu-ci/223/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtubuntu-wily-armhf-ci/14/console

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

FAILED: Continuous integration, rev:269
http://jenkins.qa.ubuntu.com/job/qtubuntu-ci/224/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtubuntu-wily-armhf-ci/15/console

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

FAILED: Continuous integration, rev:270
http://jenkins.qa.ubuntu.com/job/qtubuntu-ci/225/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtubuntu-wily-armhf-ci/16/console

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

PASSED: Continuous integration, rev:271
http://jenkins.qa.ubuntu.com/job/qtubuntu-ci/226/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtubuntu-wily-armhf-ci/17
        deb: http://jenkins.qa.ubuntu.com/job/qtubuntu-wily-armhf-ci/17/artifact/work/output/*zip*/output.zip

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

PASSED: Continuous integration, rev:272
http://jenkins.qa.ubuntu.com/job/qtubuntu-ci/227/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtubuntu-wily-armhf-ci/18
        deb: http://jenkins.qa.ubuntu.com/job/qtubuntu-wily-armhf-ci/18/artifact/work/output/*zip*/output.zip

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

PASSED: Continuous integration, rev:273
http://jenkins.qa.ubuntu.com/job/qtubuntu-ci/239/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtubuntu-wily-armhf-ci/30
        deb: http://jenkins.qa.ubuntu.com/job/qtubuntu-wily-armhf-ci/30/artifact/work/output/*zip*/output.zip

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

PASSED: Continuous integration, rev:274
http://jenkins.qa.ubuntu.com/job/qtubuntu-ci/241/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtubuntu-wily-armhf-ci/32
        deb: http://jenkins.qa.ubuntu.com/job/qtubuntu-wily-armhf-ci/32/artifact/work/output/*zip*/output.zip

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

PASSED: Continuous integration, rev:275
http://jenkins.qa.ubuntu.com/job/qtubuntu-ci/244/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtubuntu-wily-armhf-ci/35
        deb: http://jenkins.qa.ubuntu.com/job/qtubuntu-wily-armhf-ci/35/artifact/work/output/*zip*/output.zip

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

PASSED: Continuous integration, rev:276
http://jenkins.qa.ubuntu.com/job/qtubuntu-ci/245/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtubuntu-wily-armhf-ci/36
        deb: http://jenkins.qa.ubuntu.com/job/qtubuntu-wily-armhf-ci/36/artifact/work/output/*zip*/output.zip

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

PASSED: Continuous integration, rev:277
http://jenkins.qa.ubuntu.com/job/qtubuntu-ci/259/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtubuntu-wily-armhf-ci/50
        deb: http://jenkins.qa.ubuntu.com/job/qtubuntu-wily-armhf-ci/50/artifact/work/output/*zip*/output.zip

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

PASSED: Continuous integration, rev:278
http://jenkins.qa.ubuntu.com/job/qtubuntu-ci/260/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtubuntu-wily-armhf-ci/51
        deb: http://jenkins.qa.ubuntu.com/job/qtubuntu-wily-armhf-ci/51/artifact/work/output/*zip*/output.zip

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

Ok, should be good to go

Looks good to me (didn't test yet).

PASSED: Continuous integration, rev:280
http://jenkins.qa.ubuntu.com/job/qtubuntu-ci/262/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtubuntu-wily-armhf-ci/53
        deb: http://jenkins.qa.ubuntu.com/job/qtubuntu-wily-armhf-ci/53/artifact/work/output/*zip*/output.zip

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

And it works fine as well.

Why is that function in src/ubuntumirclient/utils.h inside an anonymous namespace?

PASSED: Continuous integration, rev:290
http://jenkins.qa.ubuntu.com/job/qtubuntu-ci/289/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/qtubuntu-wily-armhf-ci/80
        deb: http://jenkins.qa.ubuntu.com/job/qtubuntu-wily-armhf-ci/80/artifact/work/output/*zip*/output.zip

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

> Why is that function in src/ubuntumirclient/utils.h inside an anonymous
> namespace?

To stop it polluting the global namespace. The header file is private.

Why did you remove that chunk from UbuntuSurface constructor in src/ubuntumirclient/window.cpp? In any case that removal is unrelated to DPR work.

"""
        auto geom = mWindow->geometry();
        geom.setWidth(parameters.width);
        geom.setHeight(parameters.height);
        if (mWindowState == Qt::WindowFullScreen) {
            geom.setY(0);
        } else {
            geom.setY(panelHeight());
        }
"""

On 17/12/2015 11:52, Daniel d'Andrada wrote:
> Why did you remove that chunk from UbuntuSurface constructor in src/ubuntumirclient/window.cpp? In any case that removal is unrelated to DPR work.
>
> """
> auto geom = mWindow->geometry();
> geom.setWidth(parameters.width);
> geom.setHeight(parameters.height);
> if (mWindowState == Qt::WindowFullScreen) {
> geom.setY(0);
> } else {
> geom.setY(panelHeight());
> }
> """

And so is the panel hack refactoring you're doing.

> Why did you remove that chunk from UbuntuSurface constructor in
> src/ubuntumirclient/window.cpp? In any case that removal is unrelated to DPR
> work.
>
> """
> auto geom = mWindow->geometry();
> geom.setWidth(parameters.width);
> geom.setHeight(parameters.height);
> if (mWindowState == Qt::WindowFullScreen) {
> geom.setY(0);
> } else {
> geom.setY(panelHeight());
> }
> """

It was a bit related. I wanted to prevent UbuntuSurface having any knowledge of device-indepedent pixels, I wanted it to be purely pixel based. UbuntuSurface pretty much a wrapper for a MirSurface. So that small refactor allowed me to do that, plus eliminate a small bit of duplicate code.

Panel hack refactoring was to ensure the hack was applied at UbuntuWindow creation in a correct fashion.

The issue I found with the old hack was if window was created in fullscreen state, with y=0, then
  if (state == Qt::WindowFullScreen && geometry().y() != 0) {
was not entered, but
  } else if (geometry().y() == 0) {
was. So fullscreen surface got the panel hack applied.

I fixed it here as I was testing widget-based apps which did this.

Code looks ok

Works fine.