Mir

Merge lp:~cemil-azizoglu/mir/create-chain-from-render-surface into lp:mir

Proposed by Cemil Azizoglu
Status: Merged
Approved by: Cemil Azizoglu
Approved revision: no longer in the source branch.
Merged at revision: 3868
Proposed branch: lp:~cemil-azizoglu/mir/create-chain-from-render-surface
Merge into: lp:mir
Prerequisite: lp:~cemil-azizoglu/mir/separate_out_render_surface_tests
Diff against target: 822 lines (+404/-57)
15 files modified
playground/mir_demo_client_chain_jumping_buffers.c (+19/-10)
playground/mir_demo_client_prerendered_frames.c (+12/-4)
src/client/error_render_surface.cpp (+11/-4)
src/client/error_render_surface.h (+1/-0)
src/client/error_stream.cpp (+14/-12)
src/client/mir_connection.cpp (+13/-0)
src/client/mir_connection.h (+4/-0)
src/client/mir_render_surface.h (+1/-0)
src/client/mir_render_surface_api.cpp (+15/-0)
src/client/render_surface.cpp (+25/-13)
src/client/render_surface.h (+3/-0)
src/client/symbols.map (+1/-0)
src/include/client/mir_toolkit/mir_render_surface.h (+14/-2)
tests/acceptance-tests/staging/test_render_surface.cpp (+158/-2)
tests/unit-tests/client/test_mir_render_surface.cpp (+113/-10)
To merge this branch: bzr merge lp:~cemil-azizoglu/mir/create-chain-from-render-surface
Reviewer Review Type Date Requested Status
Kevin DuBois (community) Approve
Mir CI Bot continuous-integration Approve
Review via email: mp+312211@code.launchpad.net

This proposal supersedes a proposal from 2016-11-30.

Commit message

Get presentation chain from a render surface.

Description of the change

Get presentation chain from a render surface.

I also modified the two presentation chain examples in the playground to use render surfaces.

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

PASSED: Continuous integration, rev:3856
https://mir-jenkins.ubuntu.com/job/mir-ci/2295/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/2982
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/3047
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/3039
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/3039
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/3039
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/3011
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/3011/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3011
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3011/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/3011
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/3011/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/3011
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3011/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/3011
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3011/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3011
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3011/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:3857
https://mir-jenkins.ubuntu.com/job/mir-ci/2302/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/2996
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/3061
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/3053
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/3053
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/3053
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/3025
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/3025/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3025
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3025/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/3025
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/3025/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/3025
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3025/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/3025
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3025/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3025
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3025/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:3858
https://mir-jenkins.ubuntu.com/job/mir-ci/2303/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/2998
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/3063
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/3055
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/3055
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/3055
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/3027
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/3027/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3027
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3027/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/3027
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/3027/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/3027
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3027/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/3027
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3027/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3027
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3027/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)
Revision history for this message
Kevin DuBois (kdub) wrote :

(just repeating myself from email chain)
The signature looks good to me. I think its more straightforward internally and for the client if MirRenderSurface doesn't have one-time factory-like methods (the _get_ ones), but rather there are create/destroy (discussed in email chain). So, I guess on this point, I'm 'needs info' about if we're going to use create/destroy as opposed to get (and can wait for the email chain to figure that out).
Like, the function is described as
+ * Obtain the presentation chain backing a given render surface
but what's really going on is an object is created that has the same lifetime as MirRenderSurface, and it precludes the use of mir_render_surface_get_buffer_stream

prior problem, may as well fix in this MP:
121: + throw std::runtime_error(error);
Recently popped up in one of my MP's I guess the 'standard mir' way is to use BOOST_THROW_EXCEPTION instead of just throw:
https://code.launchpad.net/~mir-team/mir/mesa-auth-extensions/+merge/312045

needs info:
+ EXPECT_THAT(bs2, Eq(nullptr));
Returning an "error object" would seem more appropriate to the mir client api?

needs fixing:
+ auto determine_physical_size = [](MirRenderSurface* rs) -> geom::Size
+ {
+ int width = -1;
+ int height = -1;
+ mir_render_surface_get_size(rs, &width, &height);
+ return {width, height};
+ };
repeated code, and not really needed. The client is not required to create its buffers (or bufferstreams) based from the logical size that the rs has.

review: Needs Fixing
Revision history for this message
Kevin DuBois (kdub) wrote :

Just to clarify, the create/destroy vs get is probably for a followup mp, not part of the 'needs fixing'

Revision history for this message
Cemil Azizoglu (cemil-azizoglu) wrote :

> (just repeating myself from email chain)
> The signature looks good to me. I think its more straightforward internally
> and for the client if MirRenderSurface doesn't have one-time factory-like
> methods (the _get_ ones), but rather there are create/destroy (discussed in
> email chain). So, I guess on this point, I'm 'needs info' about if we're going
> to use create/destroy as opposed to get (and can wait for the email chain to
> figure that out).
> Like, the function is described as
> + * Obtain the presentation chain backing a given render surface
> but what's really going on is an object is created that has the same lifetime
> as MirRenderSurface, and it precludes the use of
> mir_render_surface_get_buffer_stream

Ack, will wait for the outcome of the discussion.

>
> prior problem, may as well fix in this MP:
> 121: + throw std::runtime_error(error);
> Recently popped up in one of my MP's I guess the 'standard mir' way is to use
> BOOST_THROW_EXCEPTION instead of just throw:
> https://code.launchpad.net/~mir-team/mir/mesa-auth-extensions/+merge/312045

Fixed, also fixed those in error_stream.cpp

>
> needs info:
> + EXPECT_THAT(bs2, Eq(nullptr));
> Returning an "error object" would seem more appropriate to the mir client api?
>

Though seemingly innocent, this will require a bunch of superfluous code changes for not much return (due to the throw coming from render_surface.cpp, not mir_connection.cpp where the error stream code is, along with necessary constructs like the surface_map). There are also other cases where we return nullptr (e.g. in buffer_stream_api.cpp, so it's not totally inconsistent). We can more easily do this once, the render surfaces become the authoritative way to obtain content (as opposed to being able to do that through MirConnection, too).

> needs fixing:
> + auto determine_physical_size = [](MirRenderSurface* rs) -> geom::Size
> + {
> + int width = -1;
> + int height = -1;
> + mir_render_surface_get_size(rs, &width, &height);
> + return {width, height};
> + };
> repeated code, and not really needed. The client is not required to create its
> buffers (or bufferstreams) based from the logical size that the rs has.

Fixed.

Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:3859
https://mir-jenkins.ubuntu.com/job/mir-ci/2305/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/3000
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/3065
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/3057
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/3057
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/3057
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/3029
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/3029/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3029
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3029/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/3029
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/3029/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/3029
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3029/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/3029
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3029/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3029
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3029/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)
Revision history for this message
Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:3860
https://mir-jenkins.ubuntu.com/job/mir-ci/2306/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/3001
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/3066
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/3058
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/3058
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/3058
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/3030
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/3030/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3030
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/3030/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/3030
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/3030/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/3030
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/3030/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/3030
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/3030/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3030
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/3030/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Did anyone else notice how many related client objects we have now?...

  MirSurface
  MirBufferStream
  MirPresentationChain
  MirRenderSurface

Surely someone who understands what these all are could try and reduce the number? (and shorten their names)

Revision history for this message
Chris Halse Rogers (raof) wrote :

Of course we could collapse those all into one type, just like we could collapse *all* Mir types into void*.

But each of those types relates to a thing with distinct properties and uses, so the only way you'll reduce the number of types is by conflating two concepts.

Revision history for this message
Kevin DuBois (kdub) wrote :

> > (just repeating myself from email chain)
> > The signature looks good to me. I think its more straightforward internally
> > and for the client if MirRenderSurface doesn't have one-time factory-like
> > methods (the _get_ ones), but rather there are create/destroy (discussed in
> > email chain). So, I guess on this point, I'm 'needs info' about if we're
> going
> > to use create/destroy as opposed to get (and can wait for the email chain to
> > figure that out).
> > Like, the function is described as
> > + * Obtain the presentation chain backing a given render surface
> > but what's really going on is an object is created that has the same
> lifetime
> > as MirRenderSurface, and it precludes the use of
> > mir_render_surface_get_buffer_stream
>
> Ack, will wait for the outcome of the discussion.
>

Sure, would even be happy to spike the idea.

> >
> > prior problem, may as well fix in this MP:
> > 121: + throw std::runtime_error(error);
> > Recently popped up in one of my MP's I guess the 'standard mir' way is to
> use
> > BOOST_THROW_EXCEPTION instead of just throw:
> > https://code.launchpad.net/~mir-team/mir/mesa-auth-extensions/+merge/312045
>
> Fixed, also fixed those in error_stream.cpp
>
> >
> > needs info:
> > + EXPECT_THAT(bs2, Eq(nullptr));
> > Returning an "error object" would seem more appropriate to the mir client
> api?
> >
>
> Though seemingly innocent, this will require a bunch of superfluous code
> changes for not much return (due to the throw coming from render_surface.cpp,
> not mir_connection.cpp where the error stream code is, along with necessary
> constructs like the surface_map). There are also other cases where we return
> nullptr (e.g. in buffer_stream_api.cpp, so it's not totally inconsistent). We
> can more easily do this once, the render surfaces become the authoritative way
> to obtain content (as opposed to being able to do that through MirConnection,
> too).
>

So sounds like something that we will do, once its easier to move the code in src/client. The MirConnection object particularly needs a cleanup to make this easier.

> > needs fixing:
> > + auto determine_physical_size = [](MirRenderSurface* rs) -> geom::Size
> > + {
> > + int width = -1;
> > + int height = -1;
> > + mir_render_surface_get_size(rs, &width, &height);
> > + return {width, height};
> > + };
> > repeated code, and not really needed. The client is not required to create
> its
> > buffers (or bufferstreams) based from the logical size that the rs has.
>
> Fixed.

Revision history for this message
Kevin DuBois (kdub) :
review: Approve
Revision history for this message
Kevin DuBois (kdub) wrote :

> Did anyone else notice how many related client objects we have now?...
>
> MirSurface
> MirBufferStream
> MirPresentationChain
> MirRenderSurface
>
> Surely someone who understands what these all are could try and reduce the
> number? (and shorten their names)

As Chris mentioned, they have different purposes, so they have different names.
As talked about before, the names might be off, but they're hard to move.

MirSurface -> the window, contains input, graphics, etc
MirRenderSurface-> the graphics content
MirPresentationChain -> a buffer-based submission
MirBufferStream -> the legacy type, with swap-based submissions

Now, I think that MirRenderSurface and MirPresentationChain are pretty darn close as far as what they are, and possibly could be consolidated. This gives small benefit in my estimation though, and makes the MirBufferStream usage stranger.

MirBufferStream is largely a helper object provided for legacy/convenience reasons. It is built internally on MirPresentationChain+MirBuffers these days.

Revision history for this message
Kevin DuBois (kdub) wrote :

+ //TODO: Figure out how to handle mir_buffer_usage_hardware once
+ // EGL is made to support RSs.
+

I think its also valuable (in terms of saving work) to figure out how to chase the MirBufferUsage field out of MirBufferStream.

(in the same line as https://code.launchpad.net/~mir-team/mir/android-buffer-alloc-extension/+merge/312293)

Revision history for this message
Kevin DuBois (kdub) wrote :

> + //TODO: Figure out how to handle mir_buffer_usage_hardware once
> + // EGL is made to support RSs.
> +
>
> I think its also valuable (in terms of saving work) to figure out how to chase
> the MirBufferUsage field out of MirBufferStream.
>
> (in the same line as https://code.launchpad.net/~mir-team/mir/android-buffer-
> alloc-extension/+merge/312293)

oh, and maybe not in this MP, as its a bit of a separate discussion ^_^

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'playground/mir_demo_client_chain_jumping_buffers.c'
2--- playground/mir_demo_client_chain_jumping_buffers.c 2016-11-16 12:50:00 +0000
3+++ playground/mir_demo_client_chain_jumping_buffers.c 2016-12-01 23:49:58 +0000
4@@ -21,6 +21,7 @@
5 #include <mir_toolkit/mir_buffer_stream.h>
6 #include <mir_toolkit/mir_surface.h>
7 #include <mir_toolkit/mir_presentation_chain.h>
8+#include <mir_toolkit/mir_render_surface.h>
9 #include <mir_toolkit/mir_buffer.h>
10 #include <mir_toolkit/version.h>
11 #include <sys/types.h>
12@@ -174,9 +175,17 @@
13 unsigned int spare_buffer = 0;
14
15 MirPresentationChain* chain[num_chains];
16+ MirRenderSurface* render_surface[num_chains];
17 for(unsigned int i = 0u; i < num_chains; i++)
18 {
19- chain[i] = mir_connection_create_presentation_chain_sync(connection);
20+ render_surface[i] = mir_connection_create_render_surface_sync(connection, chain_width, chain_height);
21+ if (!mir_render_surface_is_valid(render_surface[i]))
22+ {
23+ printf("could not create render surface\n");
24+ return -1;
25+ }
26+
27+ chain[i] = mir_render_surface_get_presentation_chain(render_surface[i]);
28 if (!mir_presentation_chain_is_valid(chain[i]))
29 {
30 printf("could not create MirPresentationChain\n");
31@@ -193,14 +202,14 @@
32
33 //Arrange a 2x2 grid of chains within surface
34 MirSurfaceSpec* spec = mir_connection_create_spec_for_normal_surface(connection, width, height, format);
35- mir_surface_spec_add_presentation_chain(
36- spec, chain_width, chain_height, displacement_x, displacement_y, chain[0]);
37- mir_surface_spec_add_presentation_chain(
38- spec, chain_width, chain_height, chain_width, displacement_y, chain[1]);
39- mir_surface_spec_add_presentation_chain(
40- spec, chain_width, chain_height, displacement_x, chain_height, chain[2]);
41- mir_surface_spec_add_presentation_chain(
42- spec, chain_width, chain_height, chain_width, chain_height, chain[3]);
43+ mir_surface_spec_add_render_surface(
44+ spec, render_surface[0], chain_width, chain_height, displacement_x, displacement_y);
45+ mir_surface_spec_add_render_surface(
46+ spec, render_surface[1], chain_width, chain_height, chain_width, displacement_y);
47+ mir_surface_spec_add_render_surface(
48+ spec, render_surface[2], chain_width, chain_height, displacement_x, chain_height);
49+ mir_surface_spec_add_render_surface(
50+ spec, render_surface[3], chain_width, chain_height, chain_width, chain_height);
51 MirSurface* surface = mir_surface_create_sync(spec);
52 mir_surface_spec_release(spec);
53
54@@ -256,7 +265,7 @@
55 for (unsigned int i = 0u; i < num_buffers; i++)
56 mir_buffer_release(buffer_available[i].buffer);
57 for (unsigned int i = 0u; i < num_chains; i++)
58- mir_presentation_chain_release(chain[i]);
59+ mir_render_surface_release(render_surface[i]);
60 mir_surface_release_sync(surface);
61 mir_connection_release(connection);
62 return 0;
63
64=== modified file 'playground/mir_demo_client_prerendered_frames.c'
65--- playground/mir_demo_client_prerendered_frames.c 2016-11-16 12:50:00 +0000
66+++ playground/mir_demo_client_prerendered_frames.c 2016-12-01 23:49:58 +0000
67@@ -21,6 +21,7 @@
68 #include <mir_toolkit/mir_buffer_stream.h>
69 #include <mir_toolkit/mir_surface.h>
70 #include <mir_toolkit/mir_presentation_chain.h>
71+#include <mir_toolkit/mir_render_surface.h>
72 #include <mir_toolkit/mir_buffer.h>
73 #include <mir_toolkit/version.h>
74 #include <sys/types.h>
75@@ -146,7 +147,14 @@
76 return -1;
77 }
78
79- MirPresentationChain* chain = mir_connection_create_presentation_chain_sync(connection);
80+ MirRenderSurface* render_surface = mir_connection_create_render_surface_sync(connection, width, height);
81+ if (!mir_render_surface_is_valid(render_surface))
82+ {
83+ printf("could not create a render surface\n");
84+ return -1;
85+ }
86+
87+ MirPresentationChain* chain = mir_render_surface_get_presentation_chain(render_surface);
88 if (!mir_presentation_chain_is_valid(chain))
89 {
90 printf("could not create MirPresentationChain\n");
91@@ -161,8 +169,8 @@
92 }
93
94 MirSurfaceSpec* spec = mir_connection_create_spec_for_normal_surface(connection, width, height, format);
95- mir_surface_spec_add_presentation_chain(
96- spec, width, height, displacement_x, displacement_y, chain);
97+ mir_surface_spec_add_render_surface(
98+ spec, render_surface, width, height, displacement_x, displacement_y);
99 MirSurface* surface = mir_surface_create_sync(spec);
100 if (!mir_surface_is_valid(surface))
101 {
102@@ -224,7 +232,7 @@
103
104 for (i = 0u; i < num_prerendered_frames; i++)
105 mir_buffer_release(buffer_available[i].buffer);
106- mir_presentation_chain_release(chain);
107+ mir_render_surface_release(render_surface);
108 mir_surface_release_sync(surface);
109 mir_connection_release(connection);
110 return 0;
111
112=== modified file 'src/client/error_render_surface.cpp'
113--- src/client/error_render_surface.cpp 2016-11-22 18:44:52 +0000
114+++ src/client/error_render_surface.cpp 2016-12-01 23:49:58 +0000
115@@ -18,6 +18,8 @@
116
117 #include "error_render_surface.h"
118
119+#include <boost/throw_exception.hpp>
120+
121 namespace mcl = mir::client;
122
123 mcl::ErrorRenderSurface::ErrorRenderSurface(
124@@ -34,17 +36,17 @@
125
126 mir::frontend::BufferStreamId mcl::ErrorRenderSurface::stream_id() const
127 {
128- throw std::runtime_error(error);
129+ BOOST_THROW_EXCEPTION(std::runtime_error(error));
130 }
131
132 mir::geometry::Size mcl::ErrorRenderSurface::size() const
133 {
134- throw std::runtime_error(error);
135+ BOOST_THROW_EXCEPTION(std::runtime_error(error));
136 }
137
138 void mcl::ErrorRenderSurface::set_size(mir::geometry::Size /*size*/)
139 {
140- throw std::runtime_error(error);
141+ BOOST_THROW_EXCEPTION(std::runtime_error(error));
142 }
143
144 bool mcl::ErrorRenderSurface::valid() const
145@@ -57,7 +59,12 @@
146 MirPixelFormat /*format*/,
147 MirBufferUsage /*buffer_usage*/)
148 {
149- throw std::runtime_error(error);
150+ BOOST_THROW_EXCEPTION(std::runtime_error(error));
151+}
152+
153+MirPresentationChain* mcl::ErrorRenderSurface::get_presentation_chain()
154+{
155+ BOOST_THROW_EXCEPTION(std::runtime_error(error));
156 }
157
158 char const* mcl::ErrorRenderSurface::get_error_message() const
159
160=== modified file 'src/client/error_render_surface.h'
161--- src/client/error_render_surface.h 2016-11-22 18:44:52 +0000
162+++ src/client/error_render_surface.h 2016-12-01 23:49:58 +0000
163@@ -41,6 +41,7 @@
164 int width, int height,
165 MirPixelFormat format,
166 MirBufferUsage buffer_usage) override;
167+ MirPresentationChain* get_presentation_chain() override;
168 private:
169 std::string const error;
170 MirConnection* const connection_;
171
172=== modified file 'src/client/error_stream.cpp'
173--- src/client/error_stream.cpp 2016-11-14 21:59:19 +0000
174+++ src/client/error_stream.cpp 2016-12-01 23:49:58 +0000
175@@ -18,6 +18,8 @@
176
177 #include "error_stream.h"
178
179+#include <boost/throw_exception.hpp>
180+
181 namespace mcl = mir::client;
182
183 mcl::ErrorStream::ErrorStream(
184@@ -57,53 +59,53 @@
185
186 MirSurfaceParameters mcl::ErrorStream::get_parameters() const
187 {
188- throw std::runtime_error(error);
189+ BOOST_THROW_EXCEPTION(std::runtime_error(error));
190 }
191 std::shared_ptr<mcl::ClientBuffer> mcl::ErrorStream::get_current_buffer()
192 {
193- throw std::runtime_error(error);
194+ BOOST_THROW_EXCEPTION(std::runtime_error(error));
195 }
196 uint32_t mcl::ErrorStream::get_current_buffer_id()
197 {
198- throw std::runtime_error(error);
199+ BOOST_THROW_EXCEPTION(std::runtime_error(error));
200 }
201 EGLNativeWindowType mcl::ErrorStream::egl_native_window()
202 {
203- throw std::runtime_error(error);
204+ BOOST_THROW_EXCEPTION(std::runtime_error(error));
205 }
206 MirWaitHandle* mcl::ErrorStream::next_buffer(std::function<void()> const&)
207 {
208- throw std::runtime_error(error);
209+ BOOST_THROW_EXCEPTION(std::runtime_error(error));
210 }
211 std::shared_ptr<mcl::MemoryRegion> mcl::ErrorStream::secure_for_cpu_write()
212 {
213- throw std::runtime_error(error);
214+ BOOST_THROW_EXCEPTION(std::runtime_error(error));
215 }
216
217 int mcl::ErrorStream::swap_interval() const
218 {
219- throw std::runtime_error(error);
220+ BOOST_THROW_EXCEPTION(std::runtime_error(error));
221 }
222 MirWaitHandle* mcl::ErrorStream::set_swap_interval(int)
223 {
224- throw std::runtime_error(error);
225+ BOOST_THROW_EXCEPTION(std::runtime_error(error));
226 }
227 MirNativeBuffer* mcl::ErrorStream::get_current_buffer_package()
228 {
229- throw std::runtime_error(error);
230+ BOOST_THROW_EXCEPTION(std::runtime_error(error));
231 }
232 MirPlatformType mcl::ErrorStream::platform_type()
233 {
234- throw std::runtime_error(error);
235+ BOOST_THROW_EXCEPTION(std::runtime_error(error));
236 }
237 MirWaitHandle* mcl::ErrorStream::set_scale(float)
238 {
239- throw std::runtime_error(error);
240+ BOOST_THROW_EXCEPTION(std::runtime_error(error));
241 }
242
243 mir::geometry::Size mcl::ErrorStream::size() const
244 {
245- throw std::runtime_error(error);
246+ BOOST_THROW_EXCEPTION(std::runtime_error(error));
247 }
248
249 void mcl::ErrorStream::buffer_available(mir::protobuf::Buffer const&) {}
250
251=== modified file 'src/client/mir_connection.cpp'
252--- src/client/mir_connection.cpp 2016-11-30 09:18:39 +0000
253+++ src/client/mir_connection.cpp 2016-12-01 23:49:58 +0000
254@@ -1172,6 +1172,19 @@
255 return display_configuration->take_snapshot();
256 }
257
258+std::shared_ptr<mcl::PresentationChain> MirConnection::create_presentation_chain_with_id(
259+ MirRenderSurface* render_surface,
260+ mir::protobuf::BufferStream const& a_protobuf_bs)
261+{
262+ if (!client_buffer_factory)
263+ client_buffer_factory = platform->create_buffer_factory();
264+ auto chain = std::make_shared<mcl::PresentationChain>(
265+ this, a_protobuf_bs.id().value(), server, client_buffer_factory, buffer_factory);
266+
267+ surface_map->insert(render_surface->stream_id(), chain);
268+ return chain;
269+}
270+
271 void MirConnection::create_presentation_chain(
272 mir_presentation_chain_callback callback,
273 void *context)
274
275=== modified file 'src/client/mir_connection.h'
276--- src/client/mir_connection.h 2016-11-30 09:02:39 +0000
277+++ src/client/mir_connection.h 2016-12-01 23:49:58 +0000
278@@ -66,6 +66,7 @@
279 class AsyncBufferFactory;
280 class MirBuffer;
281 class BufferStream;
282+class PresentationChain;
283
284 namespace rpc
285 {
286@@ -169,6 +170,9 @@
287 void create_presentation_chain(
288 mir_presentation_chain_callback callback,
289 void *context);
290+ std::shared_ptr<mir::client::PresentationChain> create_presentation_chain_with_id(
291+ MirRenderSurface* render_surface,
292+ mir::protobuf::BufferStream const& a_protobuf_bs);
293 void release_presentation_chain(MirPresentationChain* context);
294
295 void release_consumer_stream(MirBufferStream*);
296
297=== modified file 'src/client/mir_render_surface.h'
298--- src/client/mir_render_surface.h 2016-11-22 18:44:52 +0000
299+++ src/client/mir_render_surface.h 2016-12-01 23:49:58 +0000
300@@ -36,6 +36,7 @@
301 int width, int height,
302 MirPixelFormat format,
303 MirBufferUsage buffer_usage) = 0;
304+ virtual MirPresentationChain* get_presentation_chain() = 0;
305 virtual char const* get_error_message() const = 0;
306 virtual ~MirRenderSurface() = default;
307 protected:
308
309=== modified file 'src/client/mir_render_surface_api.cpp'
310--- src/client/mir_render_surface_api.cpp 2016-11-29 14:18:01 +0000
311+++ src/client/mir_render_surface_api.cpp 2016-12-01 23:49:58 +0000
312@@ -189,6 +189,21 @@
313 return nullptr;
314 }
315
316+MirPresentationChain* mir_render_surface_get_presentation_chain(
317+ MirRenderSurface* render_surface)
318+try
319+{
320+ mir::require(render_surface);
321+ auto connection = connection_map.connection(static_cast<void*>(render_surface));
322+ auto rs = connection->connection_surface_map()->render_surface(render_surface);
323+ return rs->get_presentation_chain();
324+}
325+catch (std::exception const& ex)
326+{
327+ MIR_LOG_UNCAUGHT_EXCEPTION(ex);
328+ return nullptr;
329+}
330+
331 void mir_render_surface_get_size(MirRenderSurface* render_surface, int* width, int* height)
332 {
333 mir::require(render_surface && width && height);
334
335=== modified file 'src/client/render_surface.cpp'
336--- src/client/render_surface.cpp 2016-11-30 09:18:39 +0000
337+++ src/client/render_surface.cpp 2016-12-01 23:49:58 +0000
338@@ -27,7 +27,6 @@
339 #include <boost/throw_exception.hpp>
340
341 namespace mcl = mir::client;
342-namespace mclr = mcl::rpc;
343 namespace geom = mir::geometry;
344 namespace mp = mir::protobuf;
345 namespace mf = mir::frontend;
346@@ -61,24 +60,37 @@
347 MirPixelFormat format,
348 MirBufferUsage buffer_usage)
349 {
350- if (!stream_from_id)
351+ if (chain_from_id || stream_from_id)
352+ BOOST_THROW_EXCEPTION(std::logic_error("Content already handed out"));
353+
354+ protobuf_bs->set_pixel_format(format);
355+ protobuf_bs->set_buffer_usage(buffer_usage);
356+ stream_from_id = connection_->create_client_buffer_stream_with_id(width,
357+ height,
358+ this,
359+ *protobuf_bs);
360+ if (buffer_usage == mir_buffer_usage_hardware)
361 {
362- protobuf_bs->set_pixel_format(format);
363- protobuf_bs->set_buffer_usage(buffer_usage);
364- stream_from_id = connection_->create_client_buffer_stream_with_id(width,
365- height,
366- this,
367- *protobuf_bs);
368- if (buffer_usage == mir_buffer_usage_hardware)
369- {
370- platform->use_egl_native_window(
371- wrapped_native_window, dynamic_cast<EGLNativeSurface*>(stream_from_id.get()));
372- }
373+ platform->use_egl_native_window(
374+ wrapped_native_window, dynamic_cast<EGLNativeSurface*>(stream_from_id.get()));
375 }
376
377 return stream_from_id.get();
378 }
379
380+MirPresentationChain* mcl::RenderSurface::get_presentation_chain()
381+{
382+ if (chain_from_id || stream_from_id)
383+ BOOST_THROW_EXCEPTION(std::logic_error("Content already handed out"));
384+
385+ chain_from_id = connection_->create_presentation_chain_with_id(this,
386+ *protobuf_bs);
387+ //TODO: Figure out how to handle mir_buffer_usage_hardware once
388+ // EGL is made to support RSs.
389+
390+ return reinterpret_cast<MirPresentationChain*>(chain_from_id.get());
391+}
392+
393 geom::Size mcl::RenderSurface::size() const
394 {
395 std::lock_guard<decltype(size_mutex)> lk(size_mutex);
396
397=== modified file 'src/client/render_surface.h'
398--- src/client/render_surface.h 2016-11-22 18:44:52 +0000
399+++ src/client/render_surface.h 2016-12-01 23:49:58 +0000
400@@ -39,6 +39,7 @@
401 {
402 class ClientPlatform;
403 class BufferStream;
404+class PresentationChain;
405 class RenderSurface : public MirRenderSurface
406 {
407 public:
408@@ -57,6 +58,7 @@
409 int width, int height,
410 MirPixelFormat format,
411 MirBufferUsage buffer_usage) override;
412+ MirPresentationChain* get_presentation_chain() override;
413
414 private:
415 MirConnection* const connection_;
416@@ -64,6 +66,7 @@
417 std::shared_ptr<ClientPlatform> platform;
418 std::shared_ptr<mir::protobuf::BufferStream> protobuf_bs;
419 std::shared_ptr<mir::client::BufferStream> stream_from_id;
420+ std::shared_ptr<mir::client::PresentationChain> chain_from_id;
421
422 std::mutex mutable size_mutex;
423 geometry::Size desired_size;
424
425=== modified file 'src/client/symbols.map'
426--- src/client/symbols.map 2016-12-01 18:48:07 +0000
427+++ src/client/symbols.map 2016-12-01 23:49:58 +0000
428@@ -390,6 +390,7 @@
429 mir_connection_create_render_surface;
430 mir_connection_create_render_surface_sync;
431 mir_render_surface_get_buffer_stream;
432+ mir_render_surface_get_presentation_chain;
433 mir_render_surface_is_valid;
434 mir_render_surface_get_error_message;
435 mir_render_surface_get_size;
436
437=== modified file 'src/include/client/mir_toolkit/mir_render_surface.h'
438--- src/include/client/mir_toolkit/mir_render_surface.h 2016-11-29 14:18:01 +0000
439+++ src/include/client/mir_toolkit/mir_render_surface.h 2016-12-01 23:49:58 +0000
440@@ -125,6 +125,9 @@
441 * \param [in] usage Requested buffer usage
442 *
443 * \return The buffer stream contained in the given render surface
444+ * or 'nullptr' if it, or
445+ * mir_render_surface_get_presentation_chain(), has already
446+ * been called once
447 */
448 MirBufferStream* mir_render_surface_get_buffer_stream(
449 MirRenderSurface* render_surface,
450@@ -133,13 +136,22 @@
451 MirBufferUsage usage);
452
453 /**
454+ * Obtain the presentation chain backing a given render surface
455+ *
456+ * \return The chain contained in the given render surface
457+ * or 'nullptr' if it, or
458+ * mir_render_surface_get_buffer_stream(), has already
459+ * been called once
460+ */
461+MirPresentationChain* mir_render_surface_get_presentation_chain(
462+ MirRenderSurface* render_surface);
463+
464+/**
465 * Set the MirSurfaceSpec to display content contained in a render surface
466 *
467 * \warning: The initial call to mir_surface_spec_add_render_surface will set
468 * the bottom-most content, and subsequent calls will stack the
469 * content on top.
470- * \warning: It's an error to call this function with a render surface
471- * that holds no content.
472 *
473 * \param spec The surface_spec to be updated
474 * \param render_surface The render surface containing the content to be displayed
475
476=== modified file 'tests/acceptance-tests/staging/test_render_surface.cpp'
477--- tests/acceptance-tests/staging/test_render_surface.cpp 2016-11-29 22:59:26 +0000
478+++ tests/acceptance-tests/staging/test_render_surface.cpp 2016-12-01 23:49:58 +0000
479@@ -18,6 +18,7 @@
480
481 #include "mir_toolkit/mir_client_library.h"
482 #include "mir_toolkit/mir_render_surface.h"
483+#include "mir_toolkit/mir_presentation_chain.h"
484
485 #include "mir/geometry/size.h"
486 #include "mir_test_framework/headless_in_process_server.h"
487@@ -57,7 +58,7 @@
488 mir_connection_release(connection);
489 }
490
491-TEST_F(RenderSurfaceTest, can_hand_out_buffer_streams)
492+TEST_F(RenderSurfaceTest, can_hand_out_buffer_stream)
493 {
494 auto connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__);
495
496@@ -86,6 +87,36 @@
497 mir_connection_release(connection);
498 }
499
500+TEST_F(RenderSurfaceTest, hands_out_buffer_stream_only_once)
501+{
502+ auto physical_size = logical_size;
503+ auto connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__);
504+
505+ auto rs = mir_connection_create_render_surface_sync(
506+ connection, logical_size.width.as_int(), logical_size.height.as_int());
507+
508+ auto bs = mir_render_surface_get_buffer_stream(
509+ rs,
510+ physical_size.width.as_int(), physical_size.height.as_int(),
511+ mir_pixel_format_abgr_8888,
512+ mir_buffer_usage_hardware);
513+
514+ ASSERT_THAT(bs, NotNull());
515+ EXPECT_TRUE(mir_buffer_stream_is_valid(bs));
516+ EXPECT_THAT(mir_buffer_stream_get_error_message(bs), StrEq(""));
517+
518+ auto bs2 = mir_render_surface_get_buffer_stream(
519+ rs,
520+ physical_size.width.as_int(), physical_size.height.as_int(),
521+ mir_pixel_format_abgr_8888,
522+ mir_buffer_usage_hardware);
523+
524+ EXPECT_THAT(bs2, Eq(nullptr));
525+
526+ mir_render_surface_release(rs);
527+ mir_connection_release(connection);
528+}
529+
530 TEST_F(RenderSurfaceTest, dont_have_to_release_buffer_stream)
531 {
532 auto physical_size = logical_size;
533@@ -131,7 +162,7 @@
534 mir_connection_release(connection);
535 }
536
537-TEST_F(RenderSurfaceTest, content_can_be_constructed_after_surface_creation)
538+TEST_F(RenderSurfaceTest, stream_can_be_constructed_after_surface_creation)
539 {
540 int const width{800}, height{600};
541 MirPixelFormat const format{mir_pixel_format_abgr_8888};
542@@ -160,3 +191,128 @@
543 mir_surface_release_sync(surface);
544 mir_connection_release(connection);
545 }
546+
547+TEST_F(RenderSurfaceTest, can_hand_out_presentation_chain)
548+{
549+ auto connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__);
550+
551+ auto rs = mir_connection_create_render_surface_sync(
552+ connection, logical_size.width.as_int(), logical_size.height.as_int());
553+
554+ auto pc = mir_render_surface_get_presentation_chain(rs);
555+
556+ ASSERT_THAT(pc, NotNull());
557+ EXPECT_TRUE(mir_presentation_chain_is_valid(pc));
558+
559+ mir_render_surface_release(rs);
560+ mir_connection_release(connection);
561+}
562+
563+TEST_F(RenderSurfaceTest, hands_out_presentation_chain_only_once)
564+{
565+ auto connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__);
566+
567+ auto rs = mir_connection_create_render_surface_sync(
568+ connection, logical_size.width.as_int(), logical_size.height.as_int());
569+
570+ auto pc = mir_render_surface_get_presentation_chain(rs);
571+
572+ ASSERT_THAT(pc, NotNull());
573+ EXPECT_TRUE(mir_presentation_chain_is_valid(pc));
574+
575+ auto pc2 = mir_render_surface_get_presentation_chain(rs);
576+
577+ EXPECT_THAT(pc2, Eq(nullptr));
578+
579+ mir_render_surface_release(rs);
580+ mir_connection_release(connection);
581+}
582+
583+TEST_F(RenderSurfaceTest, chain_can_be_constructed_after_surface_creation)
584+{
585+ int const width{800}, height{600};
586+ MirPixelFormat const format{mir_pixel_format_abgr_8888};
587+
588+ auto connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__);
589+
590+ auto rs = mir_connection_create_render_surface_sync(
591+ connection, logical_size.width.as_int(), logical_size.height.as_int());
592+ auto spec = mir_connection_create_spec_for_normal_surface(connection,
593+ width, height,
594+ format);
595+ mir_surface_spec_add_render_surface(spec, rs, width, height, 0, 0);
596+ auto surface = mir_surface_create_sync(spec);
597+ mir_surface_spec_release(spec);
598+ auto pc = mir_render_surface_get_presentation_chain(rs);
599+
600+ EXPECT_THAT(surface, IsValid());
601+ EXPECT_THAT(mir_surface_get_buffer_stream(surface), Eq(nullptr));
602+ EXPECT_TRUE(mir_presentation_chain_is_valid(pc));
603+
604+ mir_render_surface_release(rs);
605+ mir_surface_release_sync(surface);
606+ mir_connection_release(connection);
607+}
608+
609+TEST_F(RenderSurfaceTest, dont_have_to_release_presentation_chain)
610+{
611+ auto connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__);
612+
613+ auto rs = mir_connection_create_render_surface_sync(
614+ connection, logical_size.width.as_int(), logical_size.height.as_int());
615+ auto pc = mir_render_surface_get_presentation_chain(rs);
616+
617+ ASSERT_THAT(pc, NotNull());
618+ EXPECT_TRUE(mir_presentation_chain_is_valid(pc));
619+
620+ mir_render_surface_release(rs);
621+ mir_connection_release(connection);
622+}
623+
624+TEST_F(RenderSurfaceTest, excepts_on_stream_request_if_chain_handed_out)
625+{
626+ auto physical_size = logical_size;
627+ auto connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__);
628+
629+ auto rs = mir_connection_create_render_surface_sync(
630+ connection, logical_size.width.as_int(), logical_size.height.as_int());
631+ auto pc = mir_render_surface_get_presentation_chain(rs);
632+
633+ ASSERT_THAT(pc, NotNull());
634+ EXPECT_TRUE(mir_presentation_chain_is_valid(pc));
635+
636+ auto bs = mir_render_surface_get_buffer_stream(
637+ rs,
638+ physical_size.width.as_int(), physical_size.height.as_int(),
639+ mir_pixel_format_abgr_8888,
640+ mir_buffer_usage_hardware);
641+
642+ EXPECT_THAT(bs, Eq(nullptr));
643+
644+ mir_render_surface_release(rs);
645+ mir_connection_release(connection);
646+}
647+
648+TEST_F(RenderSurfaceTest, excepts_on_chain_request_if_stream_handed_out)
649+{
650+ auto physical_size = logical_size;
651+ auto connection = mir_connect_sync(new_connection().c_str(), __PRETTY_FUNCTION__);
652+
653+ auto rs = mir_connection_create_render_surface_sync(
654+ connection, logical_size.width.as_int(), logical_size.height.as_int());
655+ auto bs = mir_render_surface_get_buffer_stream(
656+ rs,
657+ physical_size.width.as_int(), physical_size.height.as_int(),
658+ mir_pixel_format_abgr_8888,
659+ mir_buffer_usage_hardware);
660+
661+ ASSERT_THAT(bs, NotNull());
662+ EXPECT_TRUE(mir_buffer_stream_is_valid(bs));
663+
664+ auto pc = mir_render_surface_get_presentation_chain(rs);
665+
666+ EXPECT_THAT(pc, Eq(nullptr));
667+
668+ mir_render_surface_release(rs);
669+ mir_connection_release(connection);
670+}
671
672=== modified file 'tests/unit-tests/client/test_mir_render_surface.cpp'
673--- tests/unit-tests/client/test_mir_render_surface.cpp 2016-11-30 16:58:48 +0000
674+++ tests/unit-tests/client/test_mir_render_surface.cpp 2016-12-01 23:49:58 +0000
675@@ -298,7 +298,7 @@
676 EXPECT_THAT(bs, NotNull());
677 }
678
679-TEST_F(MirRenderSurfaceTest, render_surface_creation_of_buffer_stream_more_than_once_returns_same_object)
680+TEST_F(MirRenderSurfaceTest, excepts_on_creation_of_buffer_stream_more_than_once)
681 {
682 connection->connect("MirRenderSurfaceTest", connected_callback, 0)->wait_for_all();
683 auto id = 123;
684@@ -316,15 +316,14 @@
685
686 EXPECT_CALL(*mock_platform, use_egl_native_window(native_window,_));
687
688- auto bs1 = rs.get_buffer_stream(2, 2, mir_pixel_format_abgr_8888,
689- mir_buffer_usage_hardware);
690-
691- auto bs2 = rs.get_buffer_stream(2, 2, mir_pixel_format_abgr_8888,
692- mir_buffer_usage_hardware);
693-
694- EXPECT_THAT(bs1, NotNull());
695- EXPECT_THAT(bs2, NotNull());
696- EXPECT_THAT(bs1, Eq(bs2));
697+ auto bs = rs.get_buffer_stream(2, 2, mir_pixel_format_abgr_8888,
698+ mir_buffer_usage_hardware);
699+
700+ EXPECT_THROW(
701+ { rs.get_buffer_stream(2, 2, mir_pixel_format_abgr_8888,
702+ mir_buffer_usage_hardware); },
703+ std::logic_error);
704+ EXPECT_THAT(bs, NotNull());
705 }
706
707 TEST_F(MirRenderSurfaceTest, render_surface_creation_of_buffer_stream_with_hardware_usage_installs_new_native_window)
708@@ -394,10 +393,114 @@
709
710 EXPECT_TRUE(callback.invoked);
711 EXPECT_THAT(callback.resulting_render_surface, NotNull());
712+
713 auto rs = connection->connection_surface_map()->render_surface(
714 static_cast<void*>(callback.resulting_render_surface));
715+
716 EXPECT_THAT(rs->get_error_message(),
717 StrEq("Error processing buffer stream response during render "
718 "surface creation: no ID in response (disconnected?)"));
719 EXPECT_FALSE(reinterpret_cast<mcl::RenderSurface*>(rs->valid()));
720 }
721+
722+TEST_F(MirRenderSurfaceTest, render_surface_can_create_presentation_chain)
723+{
724+ connection->connect("MirRenderSurfaceTest", connected_callback, 0)->wait_for_all();
725+ auto id = 123;
726+
727+ mp::BufferStream protobuf_bs;
728+ mp::BufferStreamId bs_id;
729+
730+ bs_id.set_value(id);
731+ *protobuf_bs.mutable_id() = bs_id;
732+
733+ auto native_window = mock_platform->create_egl_native_window(nullptr);
734+
735+ mcl::RenderSurface rs(
736+ connection.get(), native_window, nullptr, mt::fake_shared(protobuf_bs), {});
737+
738+ auto pc = rs.get_presentation_chain();
739+
740+ EXPECT_THAT(pc, NotNull());
741+}
742+
743+TEST_F(MirRenderSurfaceTest, excepts_on_creation_of_presentation_chain_more_than_once)
744+{
745+ connection->connect("MirRenderSurfaceTest", connected_callback, 0)->wait_for_all();
746+ auto id = 123;
747+
748+ mp::BufferStream protobuf_bs;
749+ mp::BufferStreamId bs_id;
750+
751+ bs_id.set_value(id);
752+ *protobuf_bs.mutable_id() = bs_id;
753+
754+ auto native_window = mock_platform->create_egl_native_window(nullptr);
755+
756+ mcl::RenderSurface rs(
757+ connection.get(), native_window, mock_platform, mt::fake_shared(protobuf_bs), {});
758+
759+ auto pc = rs.get_presentation_chain();
760+ EXPECT_THAT(pc, NotNull());
761+
762+ EXPECT_THROW(
763+ { rs.get_presentation_chain(); },
764+ std::logic_error);
765+}
766+
767+TEST_F(MirRenderSurfaceTest, excepts_on_creation_of_chain_after_stream)
768+{
769+ connection->connect("MirRenderSurfaceTest", connected_callback, 0)->wait_for_all();
770+ auto id = 123;
771+
772+ mp::BufferStream protobuf_bs;
773+ mp::BufferStreamId bs_id;
774+
775+ bs_id.set_value(id);
776+ *protobuf_bs.mutable_id() = bs_id;
777+
778+ auto native_window = mock_platform->create_egl_native_window(nullptr);
779+
780+ mcl::RenderSurface rs(connection.get(),
781+ native_window,
782+ mock_platform,
783+ mt::fake_shared(protobuf_bs),
784+ {});
785+
786+ auto bs = rs.get_buffer_stream(2, 2, mir_pixel_format_abgr_8888,
787+ mir_buffer_usage_hardware);
788+
789+ EXPECT_THAT(bs, NotNull());
790+ EXPECT_THROW(
791+ { rs.get_presentation_chain(); },
792+ std::logic_error);
793+}
794+
795+TEST_F(MirRenderSurfaceTest, excepts_on_creation_of_stream_after_chain)
796+{
797+ connection->connect("MirRenderSurfaceTest", connected_callback, 0)->wait_for_all();
798+ auto id = 123;
799+
800+ mp::BufferStream protobuf_bs;
801+ mp::BufferStreamId bs_id;
802+
803+ bs_id.set_value(id);
804+ *protobuf_bs.mutable_id() = bs_id;
805+
806+ auto native_window = mock_platform->create_egl_native_window(nullptr);
807+
808+ mcl::RenderSurface rs(connection.get(),
809+ native_window,
810+ mock_platform,
811+ mt::fake_shared(protobuf_bs),
812+ {});
813+
814+ auto pc = rs.get_presentation_chain();
815+
816+ EXPECT_THAT(pc, NotNull());
817+ EXPECT_THROW(
818+ { rs.get_buffer_stream(2, 2,
819+ mir_pixel_format_abgr_8888,
820+ mir_buffer_usage_hardware); },
821+ std::logic_error);
822+}

Subscribers

People subscribed via source and target branches