Merge into development-branch : fix-1577967 : Code : Mir

Reviewer	Review Type	Date Requested	Status
Mir CI Bot	continuous-integration		Approve on 2016-06-08
Daniel van Vugt			Abstain on 2016-06-08
Cemil Azizoglu (community)			Approve on 2016-05-16
Alan Griffiths			Approve on 2016-05-11
Chris Halse Rogers		2016-05-10	Approve on 2016-05-11
Review via email: mp+294283@code.qastaging.launchpad.net

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2016-05-10:

#

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/967/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Chris Halse Rogers (raof) wrote on 2016-05-11:

#

LGTM. Should we be marking all the mir_surface_* functions which implicitly deal with a default BufferStream as deprecated?

Certainly we should be marking mir_surface_{get,set}_swap_interval as deprecated. I lean towards marking mir_surface_get_buffer_stream as deprecated, also, although there's not a direct replacement.

review: Approve

Revision history for this message

Alan Griffiths (alan-griffiths) wrote on 2016-05-11:

#

Triggered rebuild as a quick looks suggested extremely slow test hardware leading to timeouts.

~~~~

+ uint32_t output_id;
+
};

Unnecessary whitespace

review: Approve

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2016-05-11:

#

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/971/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Kevin DuBois (kdub) wrote on 2016-05-11:

#

> Certainly we should be marking mir_surface_{get,set}_swap_interval as
> deprecated. I lean towards marking mir_surface_get_buffer_stream as
> deprecated, also, although there's not a direct replacement.

There is a way in the current API to work without mir_surface_get_buffer_stream()

MirSurfaceSpec* spec = ...;
MirBufferStream* stream = mir_connection_buffer_stream_create_sync(..);
mir_surface_spec_set_streams(spec, {stream, 0, 0}, 1);
MirSurface* surface = mir_surface_create_sync(spec);

now the client knows which stream is in the surface, and could change/swap as it wants to.

Revision history for this message

Cemil Azizoglu (cemil-azizoglu) wrote on 2016-05-11:

#

>LGTM. Should we be marking all the mir_surface_* functions which implicitly deal with a default >BufferStream as deprecated?

+1 as the names are really confusing.

Needs fixings :

153 + legacy_default_stream(buffer_stream),

Since the notion of "default stream" is not going away, we should just call it "default_stream".
-------------------------------------------

209 + auto resize_event = mir_event_get_resize_event(&e);
210 + size = { mir_resize_event_get_width(resize_event), mir_resize_event_get_height(resize_event) };
211 + if (legacy_default_stream)
212 + legacy_default_stream->set_size(size);

The if statement should enclose the whole thing as it only makes sense when there is a default stream.

review: Needs Fixing

Revision history for this message

Kevin DuBois (kdub) wrote on 2016-05-16:

#

can add some deprecated tags to discourage use in the headers.

re legacy_default_stream, I think tagging legacy_ on front indicates that this bit of functionality is really only around for legacy purposes. Its not going away yet, but it is still around for legacy reasons. OTOH, its an internal member variable, so will change name and add comment to this effect.

The size member variable is used in get_parameters, and should be changed when the resize event occurs.

Revision history for this message

Cemil Azizoglu (cemil-azizoglu) wrote on 2016-05-16:

#

LGTM

review: Approve

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2016-05-19:

#

review: Needs Fixing (continuous-integration)

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2016-05-19:

#

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/998/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2016-05-20:

#

A few failures:

16:48:22 9: [ FAILED ] 3 tests, listed below:
16:48:22 9: [ FAILED ] ServerStartup.creates_endpoint_on_filesystem
16:48:22 9: [ FAILED ] ServerStartup.after_server_sigkilled_can_start_new_instance
16:48:22 9: [ FAILED ] UnresponsiveClient.does_not_hang_server

They are not pre-existing bugs or known failures...

Revision history for this message

Daniel van Vugt (vanvugt) wrote on 2016-05-25:

#

Just a reminder - the CI failures appear to be consistent and unique to this branch. So I guess the branch is faulty.

review: Needs Fixing

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2016-06-08:

#

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/1110/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Daniel van Vugt (vanvugt) on 2016-06-08:

#

review: Abstain

Revision history for this message

Kevin DuBois (kdub) wrote on 2016-06-08:

#

CI was ran against rev3508... rev3509 has fix for valgrind+ci.

Revision history for this message

Mir CI Bot (mir-ci-bot) wrote on 2016-06-08:

#

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/1112/rebuild

PASSED: Continuous integration, rev:3509
https://mir-jenkins.ubuntu.com/job/mir-ci/1112/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/1231
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/1281
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1272
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/1272
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1241
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=vivid+overlay/1241/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/1241
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/1241/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1241
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/1241/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/1241
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/1241/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/1241
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/1241/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/1112/rebuild

review: Approve (continuous-integration)

Mir

Merge lp://qastaging/~kdub/mir/fix-1577967 into lp://qastaging/mir

Commit message

Description of the change

Preview Diff

Subscribers