Merge lp:~robertcarr/mir/enable-inprocess-egl into lp:~mir-team/mir/trunk

[Robert Carr]

Provide a server side implementation of MirMesaEGLNativeDisplay and demonstrate inprocess EGL rendering through an example.

Inprocess demo is working

Machinery is all through graphics/egl/mesa_native_display.cpp

Android support is not present yet.

[Daniel van Vugt (vanvugt)]

Minor point: "ServerInstance" vs the old name "DisplayServer". "Instance" is a redundant word, more so than "Display" was :)

[Alan Griffiths (alan-griffiths)]

Why rename DisplayServer to ServerInstance? (I could understand it being renamed e.g. "Mir".)

Anyway, it is broken:

src/server/graphics/egl/mesa_native_display.cpp:19:32: fatal error: mir/display_server.h: No such file or directory

review: Needs Fixing

[Alan Griffiths (alan-griffiths)]

67 + auto client = new me::InprocessEGLClient(server);
68 + client->start();

Looks like a memory leak.

~~~~

363 + InprocessEGLClient(mir::DisplayServer* server);
...
853 + MesaNativeDisplayImpl(mir::DisplayServer *server)
854 + : graphics_platform(server->graphics_platform())

I don't think these should be taking a DisplayServer - they should take a interface onto the configuration that has the_surface_factory(); and the_graphics_platform()

review: Needs Fixing

[Alan Griffiths (alan-griffiths)]

I *think* the the_ready_to_run_handler() and associated baggage should be unnecessary with the revised run_mir() in https://code.launchpad.net/~alan-griffiths/mir/reuse-run_mir/+merge/155196

[Alan Griffiths (alan-griffiths)]

Actually, why isn't DefaultServerConfiguration constructing InprocessEGLClient and passing the_surface_factory() and the_graphics_platform() to the constructor?

[Robert Carr (robertcarr)]

Used the run_mir changes and some simplification around interfaces (never need mir::DisplayServer, just the graphics platform or surface factory) to simplify a lot of the errant complexity around this branch.

[PS Jenkins bot (ps-jenkins)]

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/mir-autolanding/96/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/181
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/64
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-autolanding/97/console

review: Needs Fixing (continuous-integration)

[Robert Carr (robertcarr)]

Assosciated mesa changes (https://bugs.launchpad.net/mir/+bug/1122394):

diff --git a/src/egl/main/egldisplay.c b/src/egl/main/egldisplay.c
index d587ee9..a65aa40 100644
--- a/src/egl/main/egldisplay.c
+++ b/src/egl/main/egldisplay.c
@@ -60,7 +60,7 @@
 #include <sys/stat.h>
 #endif
 #ifdef HAVE_MIR_PLATFORM
-#include <mir/client/mir_toolkit/mesa/native_display.h>
+#include <mir/client/mir_toolkit/mir_client_library.h>
 #endif

 /**
@@ -158,7 +158,7 @@ _eglNativePlatformDetectNativeDisplay(EGLNativeDisplayType nativeDisplay)
 #endif

 #ifdef HAVE_MIR_PLATFORM
- if (mir_egl_mesa_display_is_valid((MirMesaEGLNativeDisplay *)nativeDisplay))
+ if (mir_egl_native_display_is_valid(nativeDisplay))
       return _EGL_PLATFORM_MIR;
 #endif

@@ -196,7 +196,7 @@ _eglNativePlatformDetectNativeDisplay(EGLNativeDisplayType nativeDisplay)
 _EGLPlatformType
 _eglGetNativePlatform(EGLNativeDisplayType nativeDisplay)
 {
- _EGLPlatformType native_platform = _EGL_INVALID_PLATFORM;
+ static _EGLPlatformType native_platform = _EGL_INVALID_PLATFORM;
    char *detection_method = NULL;

    if (native_platform == _EGL_INVALID_PLATFORM) {
diff --git a/src/egl/main/egldriver.c b/src/egl/main/egldriver.c
index 292adad..ffdd146 100644
--- a/src/egl/main/egldriver.c
+++ b/src/egl/main/egldriver.c
@@ -200,8 +200,8 @@ _eglLoadModule(_EGLModule *mod)
    lib_handle lib;
    _EGLDriver *drv;

+ if (mod->Driver)
+ return EGL_TRUE;

    if (mod->BuiltIn) {
       lib = (lib_handle) NULL;

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:564
http://jenkins.qa.ubuntu.com/job/mir-ci/238/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/182
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/65
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/239/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=precise,flavor=amd64/45

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-ci/238/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:565
http://jenkins.qa.ubuntu.com/job/mir-ci/239/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/183
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/66
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/240
        deb: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/240/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=precise,flavor=amd64/48

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

review: Approve (continuous-integration)

[Kevin DuBois (kdub)]

needs fixings:
src/server/graphics/egl (and test directories, and the demo program)
we build these on android (which appears to just have mesa related things). We should disable it until its working

src/server/display_server.cpp
trailing whitespace in file

could the start function in the example program go in the constructor? (two step initialization)

non-blocking things that make the diff bigger:
src/server/CMakeLists.txt
moving things around

src/server/run_mir.cpp
whitespace

default_server_configuration.h
whitespace

review: Needs Fixing

[Kevin DuBois (kdub)]

needs infos:
We're going to need some sort of platform abstraction for this I'd imagine... something like a mgeglm:create_native_display() (except in a platform independent namespace) that the inprocess clients can use without having to worry about mesa/android differences. Should we make a work item for this?

This might be because i'm not steeped in mesa terminology, but "MesaNativeDisplay" doesn't give me a hint as to what this is doing. (I could argue that the framebuffer is a native display too!) Maybe RenderTargetWindow? MesaInternalWindowType?

[Kevin DuBois (kdub)]

> needs infos:
> We're going to need some sort of platform abstraction for this I'd imagine...
> something like a mgeglm:create_native_display() (except in a platform
> independent namespace) that the inprocess clients can use without having to
> worry about mesa/android differences. Should we make a work item for this?

related to this... src/server/graphics/android and src/server/graphics/gbm are both platforms... egl isn't really a platform in this sense. maybe mesa_native_display.cpp should go in src/server/graphics/gbm and we can get at it for mesa inprocess clients via the include/server/mir/graphics/platform.h abstraction?

[Alan Griffiths (alan-griffiths)]

76 + auto client = new me::InprocessEGLClient(graphics_platform, surface_factory);
77 + client->start();

Looks like a leak - at the very least it needs a comment explaining why it doesn't leak.

80 + std::shared_ptr<msh::SurfaceFactory> const surface_factory;
81 + std::shared_ptr<mg::Platform> const graphics_platform;

should be const.

Actually, loose this class and rewrite main():

int main(int argc, char const* argv[])
try
{
    mir::DefaultServerConfiguration config(argc, argv);

    std::shared_ptr<me::InprocessEGLClient> client;

    mir::run_mir(config, [&](mir::DisplayServer&)
    {
         client = std::make_shared<me::InprocessEGLClient>(
              config.the_surface_factory(),
              config.the_graphics_platform());
         client->start();
    });
}
catch (std::exception const& error)
...

~~~~

516 === modified file 'include/server/mir/server_configuration.h'
657 === modified file 'src/server/display_server.cpp'
1020 === modified file 'tests/integration-tests/cucumber/test_session_management_context.cpp'

Unnecessary changes to these files. (None are are needed.)

~~~~

375 + virtual ~InprocessEGLClient() {}

Why written explicitly? Why not "default"? Why virtual?

~~~~

402 -

Unnecessary

414 -
415 +

Wrong

~~~~

427 -/// All things Mir

It is a feeble doc comment, but please don't just remove it - replace it with something better!

~~~~

637 - display->context = connection;
638 + display->context = static_cast<void*>(connection);

Unnecessary?

~~~~

650 + display_server.cpp
651 run_mir.cpp
652 - display_server.cpp

Unnecessary.

review: Needs Fixing

[Robert Carr (robertcarr)]

Kevin:

>> src/server/graphics/egl (and test directories, and the demo program)
>> we build these on android (which appears to just have mesa related things). We should disable it until its working
Fixed

>> could the start function in the example program go in the constructor? (two step initialization)
Yes! using the initializer list now

>> non-blocking things that make the diff bigger:
>> src/server/CMakeLists.txt
>> moving things around
>>
>> src/server/run_mir.cpp
>> whitespace
>>
>> default_server_configuration.h
>> whitespace
>> src/server/display_server.cpp
>> trailing whitespace in file
Fixed!

>> We're going to need some sort of platform abstraction for this I'd imagine... something like a mgeglm:create_native_display() (except in >> a platform independent namespace) that the inprocess clients can use without having to worry about mesa/android differences. Should we />> make a work item for this?

I think maybe just a bug is fine?

>> This might be because i'm not steeped in mesa terminology, but "MesaNativeDisplay" doesn't give me a hint as to what this is doing. (I >>could argue that the framebuffer is a native display too!) Maybe RenderTargetWindow? MesaInternalWindowType?

Would it help if it were called MesaEGLNativeDisplay? I only chose to avoid this because of redundancy with the namespace. But redundancy isn't always bad.

>> related to this... src/server/graphics/android and src/server/graphics/gbm are both platforms... egl isn't really a platform in this
>> sense. maybe mesa_native_display.cpp should go in src/server/graphics/gbm and we can get at it for mesa inprocess clients via the include/>> server/mir/graphics/platform.h abstraction?

My thought here was that Mesa isn't really part of the "GBM" platform so to speak, so perhaps after android in server support lands there is graphics/egl/android/android_native_display.cpp and graphics/egl/mesa/mesa_native_display.cpp

[Robert Carr (robertcarr)]

Alan:

>>
>> 76 + auto client = new me::InprocessEGLClient(graphics_platform, surface_factory);
>> 77 + client->start();
>>
>> Looks like a leak - at the very least it needs a comment explaining why it doesn't leak.
>>
>> 80 + std::shared_ptr<msh::SurfaceFactory> const surface_factory;
>> 81 + std::shared_ptr<mg::Platform> const graphics_platform;
>>
>> should be const.
>>
>> Actually, loose this class and rewrite main():
>>
>> int main(int argc, char const* argv[])
>> try
>> {
>> mir::DefaultServerConfiguration config(argc, argv);
>>
>> std::shared_ptr<me::InprocessEGLClient> client;
>>
>> mir::run_mir(config, [&](mir::DisplayServer&)
>> {
>> client = std::make_shared<me::InprocessEGLClient>(
>> config.the_surface_factory(),
>> config.the_graphics_platform());
>> client->start();
>> });
>> }
>> catch (std::exception const& error)
>> ...
>>
>> ~~~~

Good call. Fixed.

>>
>> 516 === modified file 'include/server/mir/server_configuration.h'
>> 657 === modified file 'src/server/display_server.cpp'
>> 1020 === modified file 'tests/integration-tests/cucumber/test_session_management_context.cpp'
>>
>> Unnecessary changes to these files. (None are are needed.)
>>
>> ~~~~
Removed

>>
>> 375 + virtual ~InprocessEGLClient() {}
>>
>> Why written explicitly? Why not "default"? Why virtual?
>>

Removed

>> ~~~~
>>
>> 402 -
>>
>> Unnecessary
>>
>> 414 -
>> 415 +
>>
>> Wrong
>>
>> ~~~~
>>
>> 427 -/// All things Mir
>>
>> It is a feeble doc comment, but please don't just remove it - replace it with something better!
>>
>> ~~~~
>>
>> 637 - display->context = connection;
>> 638 + display->context = static_cast<void*>(connection);
>>
>> Unnecessary?
>>
>> ~~~~
>>
>> 650 + display_server.cpp
>> 651 run_mir.cpp
>> 652 - display_server.cpp
>>
>> Unnecessary.
>>
>>

Restored to original versions.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:575
http://jenkins.qa.ubuntu.com/job/mir-ci/264/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/211
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/94
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/265
        deb: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/265/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=precise,flavor=amd64/72

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-ci/264/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:578
http://jenkins.qa.ubuntu.com/job/mir-ci/265/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/212
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/95
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/266
        deb: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/266/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-ci/265/rebuild

review: Approve (continuous-integration)

[Alexandros Frantzis (afrantzis)]

I can't get this to run. I get:

libEGL debug: EGL user error 0x3001 (EGL_NOT_INITIALIZED) in eglInitialize

mir_demo_inprocess_egl: /storage/work/mir.bzr/enable-inprocess-egl/examples/demo-inprocess-egl/example_egl_helper.cpp:32: mir::examples::EGLHelper::EGLHelper(EGLNativeDisplayType, EGLNativeWindowType): Assertion `rc == 1' failed.

I am using the latest mesa from RAOF's git repository (https://github.com/RAOF/mesa.git)

review: Needs Information

[Alan Griffiths (alan-griffiths)]

> I can't get this to run. I get:
>
> libEGL debug: EGL user error 0x3001 (EGL_NOT_INITIALIZED) in eglInitialize
>
> mir_demo_inprocess_egl: /storage/work/mir.bzr/enable-inprocess-egl/examples
> /demo-inprocess-egl/example_egl_helper.cpp:32:
> mir::examples::EGLHelper::EGLHelper(EGLNativeDisplayType,
> EGLNativeWindowType): Assertion `rc == 1' failed.
>
> I am using the latest mesa from RAOF's git repository
> (https://github.com/RAOF/mesa.git)

I get a similar error. (mesa from ppa)

review: Needs Information

[Robert Carr (robertcarr)]

Corrected mesa patch:

diff --git a/src/egl/main/egldisplay.c b/src/egl/main/egldisplay.c
index da8136c..d587ee9 100644
--- a/src/egl/main/egldisplay.c
+++ b/src/egl/main/egldisplay.c
@@ -60,7 +60,7 @@
 #include <sys/stat.h>
 #endif
 #ifdef HAVE_MIR_PLATFORM
-#include <mir_client_library.h>
+#include <mir/client/mir_toolkit/mesa/native_display.h>
 #endif

 /**
@@ -158,7 +158,7 @@ _eglNativePlatformDetectNativeDisplay(EGLNativeDisplayType nativeDisplay)
 #endif

 #ifdef HAVE_MIR_PLATFORM
- if (mir_connection_is_valid(nativeDisplay))
+ if (mir_egl_mesa_display_is_valid((MirMesaEGLNativeDisplay *)nativeDisplay))
       return _EGL_PLATFORM_MIR;
 #endif

@@ -196,7 +196,7 @@ _eglNativePlatformDetectNativeDisplay(EGLNativeDisplayType nativeDisplay)
 _EGLPlatformType
 _eglGetNativePlatform(EGLNativeDisplayType nativeDisplay)
 {
- static _EGLPlatformType native_platform = _EGL_INVALID_PLATFORM;
+ _EGLPlatformType native_platform = _EGL_INVALID_PLATFORM;
    char *detection_method = NULL;

    if (native_platform == _EGL_INVALID_PLATFORM) {
diff --git a/src/egl/main/egldriver.c b/src/egl/main/egldriver.c
index ffdd146..292adad 100644
--- a/src/egl/main/egldriver.c
+++ b/src/egl/main/egldriver.c
@@ -200,8 +200,8 @@ _eglLoadModule(_EGLModule *mod)
    lib_handle lib;
    _EGLDriver *drv;

- if (mod->Driver)
- return EGL_TRUE;

    if (mod->BuiltIn) {
       lib = (lib_handle) NULL;

[Kevin DuBois (kdub)]

> Kevin:
> >> We're going to need some sort of platform abstraction for this I'd
> imagine... something like a mgeglm:create_native_display() (except in >> a
> platform independent namespace) that the inprocess clients can use without
> having to worry about mesa/android differences. Should we />> make a work item
> for this?
>
> I think maybe just a bug is fine?

Well, seems like a good time to head off a bug :)

include/server/mir/graphics/egl/mesa_native_display.h
We have a mir::graphics::egl::mesa::create_native_display that takes the Platform as an argument... seems more appropriate that we add a "create_native_display" in mg::Platform that calls mgeglm::create_native_display. This would make the example platform-independent too

>
> >> This might be because i'm not steeped in mesa terminology, but
> "MesaNativeDisplay" doesn't give me a hint as to what this is doing. (I
> >>could argue that the framebuffer is a native display too!) Maybe
> RenderTargetWindow? MesaInternalWindowType?
>
> Would it help if it were called MesaEGLNativeDisplay? I only chose to avoid
> this because of redundancy with the namespace. But redundancy isn't always
> bad.
I'm not too stuck on this one, I suppose we can leave it as it is

>
> >> related to this... src/server/graphics/android and src/server/graphics/gbm
> are both platforms... egl isn't really a platform in this
> >> sense. maybe mesa_native_display.cpp should go in src/server/graphics/gbm
> and we can get at it for mesa inprocess clients via the include/>>
> server/mir/graphics/platform.h abstraction?
>
> My thought here was that Mesa isn't really part of the "GBM" platform so to
> speak, so perhaps after android in server support lands there is
> graphics/egl/android/android_native_display.cpp and
> graphics/egl/mesa/mesa_native_display.cpp

Could we just model these as a different displays? I'm beginning to think that we do need one folder for displays that are not necessarily tied to a platform (eg, an SDL display that Elenimaria was looking at, or one for mir-as-mir-client, or one for this). I am still favoring this being in the gbm platform folder, as we can't really separate mesa from gbm so they're all one logical platform, even if mesa and gbm are distinct pieces when working on that platform

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:579
http://jenkins.qa.ubuntu.com/job/mir-ci/288/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/244
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/127/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/293/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-ci/288/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:581
http://jenkins.qa.ubuntu.com/job/mir-ci/292/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/248/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/131
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/297
        deb: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/297/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=precise,flavor=amd64/104

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-ci/292/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:583
http://jenkins.qa.ubuntu.com/job/mir-ci/293/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/250
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/133
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/298
        deb: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/298/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=precise,flavor=amd64/105

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-ci/293/rebuild

review: Approve (continuous-integration)

[Robert Carr (robertcarr)]

Pushed some changes to abstract creation of the native display (and ownership of! looks much nicer from client perspective...see inprocess_egl_client.cpp) through Platform as Kevin suggests.

[Kevin DuBois (kdub)]

ok! the abstraction is looking much better. The android platform returning a null is ok. Overall this looks good to me, a few minor things:

platform.h
mir_native_display() let's make this name indicate more that this is "inprocess egl"

android_platform.cpp
test_display_info.cpp
<others>
C-style cast (although if its tough to get around because these are EGL types, i suppose its ok)

display_server.cpp
addition of tab whitespace after member variable

native_display.h
I don't see what's driving the change to void*

mesa_native_display_container.cpp
we can considate the extern "C" with the one already in the file pretty easily

mesa_native_display.cpp
we use new/raw ptrs instead of std::shared_ptr

[Alan Griffiths (alan-griffiths)]

kdub has already mentioned my main points.

I'm also wondering if the example be cleaner if it threw exceptions rather than terminating via assert? (That permits a clean shutdown.)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:584
http://jenkins.qa.ubuntu.com/job/mir-ci/301/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/261
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/144
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/306
        deb: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/306/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=precise,flavor=amd64/115

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-ci/301/rebuild

review: Approve (continuous-integration)

[Alexandros Frantzis (afrantzis)]

> kdub has already mentioned my main points.
>
> I'm also wondering if the example be cleaner if it threw exceptions rather
> than terminating via assert? (That permits a clean shutdown.)

Tried this branch with the mesa patche applied. The example seems to work well, apart from aborting on shutdown :)

[Chris Halse Rogers (raof)]

I've got the mesa changes cleaned up and ready to go.

Question: wither mir_egl_native_display_is_valid(), from mir_client_library.h? My understanding is that this will now always return false, and is hence a filthy lie designed to trap implementors like me? ☺

Either mir_egl_native_display_is_valid() should return sensible values, or it should be removed.

review: Needs Information

[Alan Griffiths (alan-griffiths)]

Changing the signature of TestingServerConfiguration::exec() makes sense but would probably be better proposed separately).

However, it leads to confict with trunk:

tests/acceptance-tests/test_client_input.cpp:88:10: error: ‘void {anonymous}::FakeInputServerConfiguration::exec(mir::DisplayServer*)’ marked override, but does not override

review: Needs Fixing

[Alan Griffiths (alan-griffiths)]

Remove blocking vote

review: Abstain

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:586
http://jenkins.qa.ubuntu.com/job/mir-ci/329/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/306
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/189
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/334
        deb: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/334/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=precise,flavor=amd64/140

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-ci/329/rebuild

review: Approve (continuous-integration)

[Alexandros Frantzis (afrantzis)]

We should ensure the example runs and shuts down properly before merging this.

review: Needs Fixing

[Robert Carr (robertcarr)]

>> Either mir_egl_native_display_is_valid() should return sensible values, or it should be removed.

The old version still works. The new version exists to be overriden by the server (And for usage only by mesa)...I didn't want to have libmirserver conflict with client library symbols.

[Robert Carr (robertcarr)]

Fixed shutdown.

[Robert Carr (robertcarr)]

Renamed mg::Platform::mir_native_display->mg::Platform->shell_egl_display

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:588
http://jenkins.qa.ubuntu.com/job/mir-ci/340/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/323
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/206/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/345/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=precise,flavor=amd64/146/console

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-ci/340/rebuild

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:589
http://jenkins.qa.ubuntu.com/job/mir-ci/341/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/324
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/207
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/346
        deb: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/346/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-ci/341/rebuild

review: Approve (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:590
http://jenkins.qa.ubuntu.com/job/mir-ci/344/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/329
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/212
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/349
        deb: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/349/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=precise,flavor=amd64/149

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-ci/344/rebuild

review: Approve (continuous-integration)

[Kevin DuBois (kdub)]

just a few questions remaining...
> platform.h
> mir_native_display() let's make this name indicate more that this is
> "inprocess egl"
cool, much better name

> android_platform.cpp
> test_display_info.cpp
> <others>
> C-style cast (although if its tough to get around because these are EGL types,
> i suppose its ok)
these still remain... are they too difficult to remove?

> native_display.h
> I don't see what's driving the change to void*

> mesa_native_display.cpp
> we use new/raw ptrs instead of std::shared_ptr

still a few new/delete's

[Alexandros Frantzis (afrantzis)]

> > mesa_native_display.cpp
> > we use new/raw ptrs instead of std::shared_ptr
>
> still a few new/delete's

Perhaps we should drop the Impl class, and just inherit from MirMesaEGLNativeDisplay:

struct ShellMesaEGLNativeDisplay : MirMesaEGLNativeDisplay
{
    ShellMesaEGLNativeDisplay(std::shared_ptr<mg::Platform> const& graphics_platform)
        : graphics_platform(graphics_platform)
    {
        /* Fill in MirMesaEGLNativeDisplay fields */
        context = this;
        display_get_platform = native_display_get_platform;
        ...
    }

    static void native_display_get_platform(MirMesaEGLNativeDisplay* display, MirPlatformPackage* package)
    {
        auto native_disp = static_cast<ShellMesaEGLNativeDisplay*>(display);
        native_disp->display_get_platform(package);
    }

    void display_get_platform(MirPlatformPackage* package)
    {
        ...
    }

    std::shared_ptr<mg::Platform> const graphics_platform;
    ...
}

auto native_display = std::shared_ptr<MirMesaEGLNativeDisplay>(new ShellMesaEGLNativeDisplay(platform),
                                                               NativeDisplayDeleter());

[Robert Carr (robertcarr)]

>> these still remain... are they too difficult to remove?

No! fixed

> native_display.h
> I don't see what's driving the change to void*

Before it was MirConnection but we need to store server context as well (mesa_native_display.cpp: l48)

> mesa_native_display.cpp
> we use new/raw ptrs instead of std::shared_ptr

Removed one via alf's refactoring.

>> Perhaps we should drop the Impl class, and just inherit from MirMesaEGLNativeDisplay:
>>
Good call.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:595
http://jenkins.qa.ubuntu.com/job/mir-ci/349/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/335
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/218
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/354
        deb: http://jenkins.qa.ubuntu.com/job/mir-quantal-amd64-ci/354/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins:8080/job/mir-ci/349/rebuild

review: Approve (continuous-integration)

[Kevin DuBois (kdub)]

well, i can't speak to whether this works or not, but the code looks good. i'll +1 conditional on someone assuring me that the code works on gbm :)

review: Approve

[Chris Halse Rogers (raof)]

Looks good to me, and works with the Mesa currently in the PPA.

review: Approve

[Chris Halse Rogers (raof)]

Also now closes cleanly, fixing Alexandros' last review comment, so marking as approved.

=== modified file 'examples/CMakeLists.txt'
--- examples/CMakeLists.txt	2013-03-28 09:50:09 +0000
+++ examples/CMakeLists.txt	2013-04-10 16:59:23 +0000
@@ -120,5 +120,9 @@
   mir_eglplasma
 )
 
+if (MIR_PLATFORM STREQUAL "gbm")
+  add_subdirectory(demo-inprocess-egl)
+endif()
+
 install(TARGETS ${DEMO_CLIENTS} RUNTIME DESTINATION bin)
 

=== added directory 'examples/demo-inprocess-egl'
=== added file 'examples/demo-inprocess-egl/CMakeLists.txt'
--- examples/demo-inprocess-egl/CMakeLists.txt	1970-01-01 00:00:00 +0000
+++ examples/demo-inprocess-egl/CMakeLists.txt	2013-04-10 16:59:23 +0000
@@ -0,0 +1,11 @@
+add_executable(mir_demo_inprocess_egl
+  demo_inprocess_egl.cpp
+  inprocess_egl_client.cpp
+  example_egl_helper.cpp
+)
+
+target_link_libraries(mir_demo_inprocess_egl
+  mirserver
+  mirdraw
+)
+# TODO Install this demo?
\ No newline at end of file

=== added file 'examples/demo-inprocess-egl/demo_inprocess_egl.cpp'
--- examples/demo-inprocess-egl/demo_inprocess_egl.cpp	1970-01-01 00:00:00 +0000
+++ examples/demo-inprocess-egl/demo_inprocess_egl.cpp	2013-04-10 16:59:23 +0000
@@ -0,0 +1,48 @@
+/*
+ * Copyright © 2013 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Robert Carr <[e-mail]>
+ */
+
+#include "inprocess_egl_client.h"
+
+#include "mir/run_mir.h"
+#include "mir/default_server_configuration.h"
+
+#include <boost/exception/diagnostic_information.hpp>
+#include <iostream>
+
+namespace me = mir::examples;
+
+int main(int argc, char const* argv[])
+try
+{
+    mir::DefaultServerConfiguration config(argc, argv);
+
+    std::shared_ptr<me::InprocessEGLClient> client;
+    mir::run_mir(config, [&config, &client](mir::DisplayServer&)
+    {
+        client = std::make_shared<me::InprocessEGLClient>(
+              config.the_graphics_platform(),
+              config.the_surface_factory());
+    });
+
+    return 0;
+}
+catch (std::exception const& error)
+{
+    std::cerr << "ERROR: " << boost::diagnostic_information(error) << std::endl;
+    return 1;
+}

=== added file 'examples/demo-inprocess-egl/example_egl_helper.cpp'
--- examples/demo-inprocess-egl/example_egl_helper.cpp	1970-01-01 00:00:00 +0000
+++ examples/demo-inprocess-egl/example_egl_helper.cpp	2013-04-10 16:59:23 +0000
@@ -0,0 +1,78 @@
+/*
+ * Copyright © 2013 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Robert Carr <[e-mail]>
+ */
+
+#include "example_egl_helper.h"
+
+#include <assert.h>
+
+namespace me = mir::examples;
+
+me::EGLHelper::EGLHelper(EGLNativeDisplayType native_display, EGLNativeWindowType native_window)
+{
+    display = eglGetDisplay(native_display);
+    assert(display != EGL_NO_DISPLAY);
+
+    int major, minor, rc;
+    rc = eglInitialize(display, &major, &minor);
+    assert(rc == EGL_TRUE);
+    assert(major == 1);
+    assert(minor == 4);
+
+    EGLint attribs[] = {
+        EGL_SURFACE_TYPE, EGL_WINDOW_BIT,
+        EGL_RED_SIZE, 8,
+        EGL_GREEN_SIZE, 8,
+        EGL_BLUE_SIZE, 8,
+        EGL_ALPHA_SIZE, 8,
+        EGL_RENDERABLE_TYPE, EGL_OPENGL_ES2_BIT,
+        EGL_NONE };
+    EGLConfig egl_config;
+    int n;
+    rc = eglChooseConfig(display, attribs, &egl_config, 1, &n);
+    assert(rc == EGL_TRUE);
+    assert(n == 1);
+
+    surface = eglCreateWindowSurface(display, egl_config, native_window, nullptr);
+    assert(surface != EGL_NO_SURFACE);
+
+    EGLint context_attribs[] = { EGL_CONTEXT_CLIENT_VERSION, 2, EGL_NONE };
+    context = eglCreateContext(display, egl_config, EGL_NO_CONTEXT, context_attribs);
+    assert(surface != EGL_NO_CONTEXT);
+}
+
+me::EGLHelper::~EGLHelper()
+{
+    eglDestroySurface(display, surface);
+    eglDestroyContext(display, context);
+    eglTerminate(display);
+}
+
+EGLDisplay me::EGLHelper::the_display() const
+{
+    return display;
+}
+
+EGLContext me::EGLHelper::the_context() const
+{
+    return context;
+}
+
+EGLSurface me::EGLHelper::the_surface() const
+{
+    return surface;
+}

=== added file 'examples/demo-inprocess-egl/example_egl_helper.h'
--- examples/demo-inprocess-egl/example_egl_helper.h	1970-01-01 00:00:00 +0000
+++ examples/demo-inprocess-egl/example_egl_helper.h	2013-04-10 16:59:23 +0000
@@ -0,0 +1,53 @@
+/*
+ * Copyright © 2013 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Robert Carr <[e-mail]>
+ */
+
+#ifndef MIR_EXAMPLES_EGL_HELPER_H_
+#define MIR_EXAMPLES_EGL_HELPER_H_
+
+#include <EGL/egl.h>
+
+namespace mir
+{
+namespace examples
+{
+
+/// Simple helper class for managing an EGL rendering with a sensible default configuration.
+class EGLHelper
+{
+public:
+    EGLHelper(EGLNativeDisplayType native_display, EGLNativeWindowType native_surface);
+    virtual ~EGLHelper();
+    
+    EGLDisplay the_display() const;
+    EGLContext the_context() const;
+    EGLSurface the_surface() const;
+
+protected:
+    EGLHelper(EGLHelper const&) = delete;
+    EGLHelper& operator=(EGLHelper const&) = delete;
+
+private:
+    EGLDisplay display;
+    EGLContext context;
+    EGLSurface surface;
+};
+
+}
+} // namespace mir
+
+#endif // MIR_EXAMPLES_EGL_HELPER_H_

=== added file 'examples/demo-inprocess-egl/inprocess_egl_client.cpp'
--- examples/demo-inprocess-egl/inprocess_egl_client.cpp	1970-01-01 00:00:00 +0000
+++ examples/demo-inprocess-egl/inprocess_egl_client.cpp	2013-04-10 16:59:23 +0000
@@ -0,0 +1,83 @@
+/*
+ * Copyright © 2013 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Robert Carr <[e-mail]>
+ */
+
+#include "inprocess_egl_client.h"
+#include "example_egl_helper.h"
+
+#include "mir/shell/surface_factory.h"
+#include "mir/shell/surface.h"
+#include "mir/frontend/surface_creation_parameters.h"
+#include "mir/geometry/size.h"
+#include "mir/compositor/buffer_properties.h"
+#include "mir/graphics/platform.h"
+
+#include "mir/draw/graphics.h"
+
+#include <EGL/egl.h>
+
+#include <functional>
+
+#include <assert.h>
+
+namespace mf = mir::frontend;
+namespace mc = mir::compositor;
+namespace msh = mir::shell;
+namespace mg = mir::graphics;
+namespace me = mir::examples;
+namespace geom = mir::geometry;
+
+me::InprocessEGLClient::InprocessEGLClient(std::shared_ptr<mg::Platform> const& graphics_platform,
+                                           std::shared_ptr<msh::SurfaceFactory> const& surface_factory)
+  : graphics_platform(graphics_platform),
+    surface_factory(surface_factory),
+    client_thread(std::mem_fn(&InprocessEGLClient::thread_loop), this)
+{
+    client_thread.detach();
+}
+
+void me::InprocessEGLClient::thread_loop()
+{
+    geom::Size const surface_size = geom::Size{geom::Width{512},
+                                               geom::Height{512}};
+    
+    auto params = mf::a_surface().of_name("Inprocess EGL Demo")
+        .of_size(surface_size)
+        .of_buffer_usage(mc::BufferUsage::hardware)
+        .of_pixel_format(geom::PixelFormat::argb_8888);
+    auto surface = surface_factory->create_surface(params);
+    
+    surface->advance_client_buffer(); // TODO: What a wart!
+    
+    auto native_display = graphics_platform->shell_egl_display();
+    me::EGLHelper helper(reinterpret_cast<EGLNativeDisplayType>(native_display), reinterpret_cast<EGLNativeWindowType>(surface.get()));
+
+    auto rc = eglMakeCurrent(helper.the_display(), helper.the_surface(), helper.the_surface(), helper.the_context());
+    assert(rc == EGL_TRUE);
+    
+    mir::draw::glAnimationBasic gl_animation;
+    gl_animation.init_gl();
+    
+    for(;;)
+    {
+        gl_animation.render_gl();
+        rc = eglSwapBuffers(helper.the_display(), helper.the_surface());
+        assert(rc = EGL_TRUE);
+
+        gl_animation.step();
+    }
+}

=== added file 'examples/demo-inprocess-egl/inprocess_egl_client.h'
--- examples/demo-inprocess-egl/inprocess_egl_client.h	1970-01-01 00:00:00 +0000
+++ examples/demo-inprocess-egl/inprocess_egl_client.h	2013-04-10 16:59:23 +0000
@@ -0,0 +1,61 @@
+/*
+ * Copyright © 2013 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Robert Carr <[e-mail]>
+ */
+
+#ifndef MIR_EXAMPLES_INPROCESS_EGL_CLIENT_H_
+#define MIR_EXAMPLES_INPROCESS_EGL_CLIENT_H_
+
+#include <thread>
+#include <memory>
+
+namespace mir
+{
+namespace graphics
+{
+class Platform;
+}
+namespace shell
+{
+class SurfaceFactory;
+}
+
+namespace examples
+{
+
+/// Encapsulation of a simple threaded client demonstrating inprocess EGL rendering
+class InprocessEGLClient
+{
+public:
+    InprocessEGLClient(std::shared_ptr<graphics::Platform> const& graphics_platform,
+                       std::shared_ptr<shell::SurfaceFactory> const& surface_factory);
+
+protected:
+    InprocessEGLClient(InprocessEGLClient const&) = delete;
+    InprocessEGLClient& operator=(InprocessEGLClient const&) = delete;
+
+private:
+    std::shared_ptr<graphics::Platform> const graphics_platform;
+    std::shared_ptr<shell::SurfaceFactory> const surface_factory;
+    
+    std::thread client_thread;
+    void thread_loop();
+};
+
+}
+} // namespace mir
+
+#endif // MIR_EXAMPLES_INPROCESS_EGL_CLIENT_H_

=== modified file 'include/client/mir_toolkit/mir_client_library.h'
--- include/client/mir_toolkit/mir_client_library.h	2013-04-08 05:51:29 +0000
+++ include/client/mir_toolkit/mir_client_library.h	2013-04-10 16:59:23 +0000
@@ -103,13 +103,6 @@
 MirEGLNativeDisplayType mir_connection_get_egl_native_display(MirConnection *connection);
 
 /**
- * Validate an EGL native display
- *   \param [in] display the display
- *   \return             whether the display is valid for use with EGL
-  */
-int mir_egl_native_display_is_valid(MirEGLNativeDisplayType display);
-
-/**
  * Request a new Mir surface on the supplied connection with the supplied
  * parameters. The returned handle remains valid until the surface has been
  * released.

=== modified file 'include/server/mir/display_server.h'
--- include/server/mir/display_server.h	2013-03-19 09:51:24 +0000
+++ include/server/mir/display_server.h	2013-04-10 16:59:23 +0000
@@ -26,24 +26,6 @@
 /// All things Mir
 namespace mir
 {
-namespace compositor
-{
-
-class BufferAllocationStrategy;
-class BufferBundleFactory;
-
-}
-namespace frontend
-{
-
-class Communicator;
-
-}
-namespace graphics
-{
-class Display;
-}
-
 class ServerConfiguration;
 
 class DisplayServer

=== added directory 'include/server/mir/graphics/egl'
=== added file 'include/server/mir/graphics/egl/mesa_native_display.h'
--- include/server/mir/graphics/egl/mesa_native_display.h	1970-01-01 00:00:00 +0000
+++ include/server/mir/graphics/egl/mesa_native_display.h	2013-04-10 16:59:23 +0000
@@ -0,0 +1,42 @@
+/*
+ * Copyright © 2013 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 the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Robert Carr <[e-mail]>
+ */
+
+#ifndef MIR_GRAPHICS_EGL_NATIVE_DISPLAY_H_
+#define MIR_GRAPHICS_EGL_NATIVE_DISPLAY_H_
+
+#include "mir_toolkit/mesa/native_display.h"
+
+#include <memory>
+
+namespace mir
+{
+namespace graphics
+{
+class Platform;
+
+namespace egl
+{
+namespace mesa
+{
+std::shared_ptr<MirMesaEGLNativeDisplay> create_native_display(std::shared_ptr<Platform> const& platform);
+}
+}
+}
+} // namespace mir
+
+#endif // MIR_GRAPHICS_EGL_NATIVE_DISPLAY_H_

=== modified file 'include/server/mir/graphics/platform.h'
--- include/server/mir/graphics/platform.h	2013-03-21 03:32:59 +0000
+++ include/server/mir/graphics/platform.h	2013-04-10 16:59:23 +0000
@@ -20,9 +20,10 @@
 #ifndef MIR_GRAPHICS_PLATFORM_H_
 #define MIR_GRAPHICS_PLATFORM_H_
 
+#include <EGL/egl.h>
+
 #include <memory>
 
-
 namespace mir
 {
 namespace compositor
@@ -50,9 +51,11 @@
     Platform& operator=(const Platform& p) = delete;
 
     virtual std::shared_ptr<compositor::GraphicBufferAllocator> create_buffer_allocator(
-        const std::shared_ptr<BufferInitializer>& buffer_initializer) = 0;
+        std::shared_ptr<BufferInitializer> const& buffer_initializer) = 0;
     virtual std::shared_ptr<Display> create_display() = 0;
     virtual std::shared_ptr<PlatformIPCPackage> get_ipc_package() = 0;
+    
+    virtual EGLNativeDisplayType shell_egl_display() = 0;
 };
 
 // Create and return a new graphics platform.

=== modified file 'include/server/mir/shell/focus_sequence.h'
--- include/server/mir/shell/focus_sequence.h	2013-03-21 03:32:59 +0000
+++ include/server/mir/shell/focus_sequence.h	2013-04-10 16:59:23 +0000
@@ -42,8 +42,8 @@
 
 protected:
     FocusSequence() = default;
-    FocusSequence(const FocusSequence&) = delete;
-    FocusSequence& operator=(const FocusSequence&) = delete;
+    FocusSequence(FocusSequence const&) = delete;
+    FocusSequence& operator=(FocusSequence const&) = delete;
 };
 
 }

=== renamed file 'src/server/shell/surface.h' => 'include/server/mir/shell/surface.h'
=== modified file 'include/shared/mir_toolkit/mesa/native_display.h'
--- include/shared/mir_toolkit/mesa/native_display.h	2013-03-29 11:39:30 +0000
+++ include/shared/mir_toolkit/mesa/native_display.h	2013-04-10 16:59:23 +0000
@@ -44,9 +44,11 @@
     void (*surface_advance_buffer)(MirMesaEGLNativeDisplay* display,
                                    MirEGLNativeWindowType surface);
 
-    MirConnection* context;
+    void *context;
 };
 
+int mir_egl_mesa_display_is_valid(MirMesaEGLNativeDisplay* display);
+
 #ifdef __cplusplus
 } // extern "C"
 /**@}*/

=== modified file 'include/test/mir_test_doubles/mock_surface.h'
--- include/test/mir_test_doubles/mock_surface.h	2013-03-29 16:51:35 +0000
+++ include/test/mir_test_doubles/mock_surface.h	2013-04-10 16:59:23 +0000
@@ -19,7 +19,7 @@
 #ifndef MIR_TEST_DOUBLES_MOCK_SURFACE_H_
 #define MIR_TEST_DOUBLES_MOCK_SURFACE_H_
 
-#include "src/server/shell/surface.h"
+#include "mir/shell/surface.h"
 
 #include "mir/frontend/surface_creation_parameters.h"
 

=== modified file 'include/test/mir_test_framework/testing_server_configuration.h'
--- include/test/mir_test_framework/testing_server_configuration.h	2013-03-25 10:18:17 +0000
+++ include/test/mir_test_framework/testing_server_configuration.h	2013-04-10 16:59:23 +0000
@@ -37,7 +37,7 @@
     TestingServerConfiguration();
 
     // Code to run in server process
-    virtual void exec(DisplayServer* display_server);
+    virtual void exec();
 
     // Code to run in server process after server exits
     virtual void on_exit();

=== modified file 'src/client/gbm/mesa_native_display_container.cpp'
--- src/client/gbm/mesa_native_display_container.cpp	2013-04-02 09:23:02 +0000
+++ src/client/gbm/mesa_native_display_container.cpp	2013-04-10 16:59:23 +0000
@@ -35,7 +35,9 @@
 static void gbm_egl_display_get_platform(MirMesaEGLNativeDisplay* display,
                                          MirPlatformPackage* package)
 {
-    mir_connection_get_platform(display->context, package);
+    auto connection = static_cast<MirConnection*>(display->context);
+
+    mir_connection_get_platform(connection, package);
 }
 
 static void gbm_egl_surface_get_current_buffer(MirMesaEGLNativeDisplay* /* display */,
@@ -66,6 +68,11 @@
     mir_surface_get_parameters(ms,  surface_parameters);
 }
 
+int mir_egl_mesa_display_is_valid(MirMesaEGLNativeDisplay* display)
+{
+    return mcl::EGLNativeDisplayContainer::instance().validate(display);
+}
+
 }
 }
 

=== modified file 'src/client/mir_client_library.cpp'
--- src/client/mir_client_library.cpp	2013-04-08 05:31:01 +0000
+++ src/client/mir_client_library.cpp	2013-04-10 16:59:23 +0000
@@ -108,11 +108,6 @@
     return connection->egl_native_display();
 }
 
-int mir_egl_native_display_is_valid(MirEGLNativeDisplayType egl_display)
-{
-    return mcl::EGLNativeDisplayContainer::instance().validate(egl_display);
-}
-
 MirWaitHandle* mir_surface_create(
     MirConnection* connection,
     MirSurfaceParameters const* params,

=== modified file 'src/server/display_server.cpp'
--- src/server/display_server.cpp	2013-04-08 16:20:33 +0000
+++ src/server/display_server.cpp	2013-04-10 16:59:23 +0000
@@ -30,6 +30,7 @@
 
 namespace mc = mir::compositor;
 namespace mf = mir::frontend;
+namespace msh = mir::shell;
 namespace mg = mir::graphics;
 namespace mi = mir::input;
 

=== modified file 'src/server/graphics/CMakeLists.txt'
--- src/server/graphics/CMakeLists.txt	2013-03-21 03:32:59 +0000
+++ src/server/graphics/CMakeLists.txt	2013-04-10 16:59:23 +0000
@@ -11,14 +11,24 @@
   add_subdirectory(gbm/)
 endif()
 
-add_library(
-  mirgraphics STATIC
+set(
+  GRAPHICS_SOURCES
 
   gl_renderer.cpp
   display_configuration.cpp
   null_display_report.cpp
 )
 
+if (MIR_PLATFORM STREQUAL "gbm")
+  add_subdirectory(egl/)
+endif()
+
+add_library(
+  mirgraphics STATIC
+
+  ${GRAPHICS_SOURCES}
+)
+
 target_link_libraries(
   mirgraphics
 

=== modified file 'src/server/graphics/android/android_platform.cpp'
--- src/server/graphics/android/android_platform.cpp	2013-04-02 15:06:31 +0000
+++ src/server/graphics/android/android_platform.cpp	2013-04-10 16:59:23 +0000
@@ -48,6 +48,12 @@
     return std::make_shared<mg::PlatformIPCPackage>();
 }
 
+EGLNativeDisplayType mga::AndroidPlatform::shell_egl_display()
+{
+    // TODO: Implement
+    return static_cast<EGLNativeDisplayType>(0);
+}
+
 std::shared_ptr<mg::Platform> mg::create_platform(std::shared_ptr<DisplayReport> const& /*TODO*/)
 {
     return std::make_shared<mga::AndroidPlatform>();

=== modified file 'src/server/graphics/android/android_platform.h'
--- src/server/graphics/android/android_platform.h	2013-03-22 17:08:01 +0000
+++ src/server/graphics/android/android_platform.h	2013-04-10 16:59:23 +0000
@@ -36,6 +36,8 @@
             const std::shared_ptr<BufferInitializer>& buffer_initializer);
     std::shared_ptr<Display> create_display();
     std::shared_ptr<PlatformIPCPackage> get_ipc_package();
+    
+    EGLNativeDisplayType shell_egl_display();
 };
 
 }

=== added directory 'src/server/graphics/egl'
=== added file 'src/server/graphics/egl/CMakeLists.txt'
--- src/server/graphics/egl/CMakeLists.txt	1970-01-01 00:00:00 +0000
+++ src/server/graphics/egl/CMakeLists.txt	2013-04-10 16:59:23 +0000
@@ -0,0 +1,10 @@
+list(
+  APPEND GRAPHICS_SOURCES
+  ${CMAKE_CURRENT_SOURCE_DIR}/mesa_native_display.cpp
+)
+
+set(
+  GRAPHICS_SOURCES
+  ${GRAPHICS_SOURCES}
+  PARENT_SCOPE
+)
\ No newline at end of file

=== added file 'src/server/graphics/egl/mesa_native_display.cpp'
--- src/server/graphics/egl/mesa_native_display.cpp	1970-01-01 00:00:00 +0000
+++ src/server/graphics/egl/mesa_native_display.cpp	2013-04-10 16:59:23 +0000
@@ -0,0 +1,148 @@
+/*
+ * Copyright © 2013 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 the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Robert Carr <[e-mail]>
+ */
+
+#include "mir/display_server.h"
+#include "mir/graphics/egl/mesa_native_display.h"
+#include "mir/graphics/platform_ipc_package.h"
+#include "mir/graphics/platform.h"
+#include "mir/frontend/surface.h"
+#include "mir/compositor/buffer.h"
+#include "mir/compositor/buffer_ipc_package.h"
+
+#include "mir_toolkit/mesa/native_display.h"
+#include "mir_toolkit/c_types.h"
+
+#include <mutex>
+#include <set>
+
+namespace mg = mir::graphics;
+namespace mgeglm = mg::egl::mesa;
+namespace mf = mir::frontend;
+
+std::mutex valid_displays_guard;
+std::set<MirMesaEGLNativeDisplay*> valid_displays;
+
+namespace
+{
+struct ShellMesaEGLNativeDisplay : MirMesaEGLNativeDisplay
+{
+public:
+    ShellMesaEGLNativeDisplay(std::shared_ptr<mg::Platform> const& graphics_platform)
+        : graphics_platform(graphics_platform)
+    {
+        context = this;
+        this->display_get_platform = &ShellMesaEGLNativeDisplay::native_display_get_platform;
+        this->surface_get_current_buffer = &ShellMesaEGLNativeDisplay::native_display_surface_get_current_buffer;
+        this->surface_get_parameters = &ShellMesaEGLNativeDisplay::native_display_surface_get_parameters;
+        this->surface_advance_buffer = &ShellMesaEGLNativeDisplay::native_display_surface_advance_buffer;
+    }
+
+    static void native_display_get_platform(MirMesaEGLNativeDisplay* display, MirPlatformPackage* package)
+    {
+        auto native_disp = static_cast<ShellMesaEGLNativeDisplay*>(display);
+        if (!native_disp->platform_package)
+            native_disp->platform_package = native_disp->graphics_platform->get_ipc_package();
+        package->data_items = native_disp->platform_package->ipc_data.size();
+        for (int i = 0; i < package->data_items; i++)
+        {
+            package->data[i] = native_disp->platform_package->ipc_data[i];
+        }
+        package->fd_items = native_disp->platform_package->ipc_fds.size();
+        for (int i = 0; i < package->fd_items; i++)
+        {
+            package->fd[i] = native_disp->platform_package->ipc_fds[i];
+        }
+
+    }
+    
+    static void native_display_surface_get_current_buffer(MirMesaEGLNativeDisplay* /* display */, 
+                                                          MirEGLNativeWindowType surface,
+                                                          MirBufferPackage* package)
+    {
+        auto mir_surface = static_cast<mf::Surface*>(surface);
+
+        auto buffer = mir_surface->client_buffer();
+        auto buffer_package = buffer->get_ipc_package();
+        package->data_items = buffer_package->ipc_data.size();
+        for (int i = 0; i < package->data_items; i++)
+        {
+            package->data[i] = buffer_package->ipc_data[i];
+        }
+        package->fd_items = buffer_package->ipc_fds.size();
+        for (int i = 0; i < package->fd_items; i++)
+        {
+            package->fd[i] = buffer_package->ipc_fds[i];
+        }
+        package->stride = buffer_package->stride;
+    }
+
+    static void native_display_surface_get_parameters(MirMesaEGLNativeDisplay* /* display  */, 
+                                                      MirEGLNativeWindowType surface,
+                                                      MirSurfaceParameters* parameters)
+    {
+        auto mir_surface = static_cast<mf::Surface*>(surface);
+
+        parameters->width = mir_surface->size().width.as_uint32_t();
+        parameters->height = mir_surface->size().height.as_uint32_t();
+        parameters->pixel_format = static_cast<MirPixelFormat>(mir_surface->pixel_format());
+        parameters->buffer_usage = mir_buffer_usage_hardware;
+    }
+
+    static void native_display_surface_advance_buffer(MirMesaEGLNativeDisplay* /* display */, 
+                                                      MirEGLNativeWindowType surface)
+    {
+        auto mir_surface = static_cast<mf::Surface*>(surface);
+        mir_surface->advance_client_buffer();
+    }
+
+private:
+    std::shared_ptr<mg::Platform> graphics_platform;
+    std::shared_ptr<mg::PlatformIPCPackage> platform_package;
+};
+
+struct NativeDisplayDeleter
+{
+    void operator()(MirMesaEGLNativeDisplay* display)
+    {
+        std::unique_lock<std::mutex> lg(valid_displays_guard);
+        valid_displays.erase(display);
+        auto disp = static_cast<ShellMesaEGLNativeDisplay*>(display->context);
+        delete disp;
+    }
+};
+
+}
+
+extern "C"
+{
+int mir_egl_mesa_display_is_valid(MirMesaEGLNativeDisplay* display)
+{
+    std::unique_lock<std::mutex> lg(valid_displays_guard);
+    return valid_displays.find(display) != valid_displays.end();
+}
+}
+
+std::shared_ptr<MirMesaEGLNativeDisplay> mgeglm::create_native_display(std::shared_ptr<mg::Platform> const& platform)
+{
+    auto native_display = std::shared_ptr<ShellMesaEGLNativeDisplay>(new ShellMesaEGLNativeDisplay(platform), 
+                                                                     NativeDisplayDeleter());
+    std::unique_lock<std::mutex> lg(valid_displays_guard);
+    valid_displays.insert(native_display.get());
+    
+    return native_display;
+}

=== modified file 'src/server/graphics/gbm/gbm_platform.cpp'
--- src/server/graphics/gbm/gbm_platform.cpp	2013-03-13 04:54:15 +0000
+++ src/server/graphics/gbm/gbm_platform.cpp	2013-04-10 16:59:23 +0000
@@ -22,6 +22,7 @@
 #include "gbm_buffer_allocator.h"
 #include "gbm_display.h"
 #include "mir/graphics/platform_ipc_package.h"
+#include "mir/graphics/egl/mesa_native_display.h"
 #include "mir/logging/logger.h"
 #include "mir/logging/dumb_console_logger.h"
 
@@ -29,10 +30,11 @@
 
 #include <stdexcept>
 
-namespace mgg=mir::graphics::gbm;
-namespace mg=mir::graphics;
-namespace ml=mir::logging;
-namespace mc=mir::compositor;
+namespace mg = mir::graphics;
+namespace mgg = mg::gbm;
+namespace mgeglm = mg::egl::mesa;
+namespace ml = mir::logging;
+namespace mc = mir::compositor;
 
 namespace
 {
@@ -53,7 +55,8 @@
 }
 
 mgg::GBMPlatform::GBMPlatform(std::shared_ptr<DisplayReport> const& listener) :
-    listener(listener)
+    listener(listener),
+    native_display(0)
 {
     drm.setup();
     gbm.setup(drm);
@@ -83,6 +86,15 @@
     drm.auth_magic(magic);
 }
 
+EGLNativeDisplayType mgg::GBMPlatform::shell_egl_display()
+{
+    if (native_display)
+        return reinterpret_cast<EGLNativeDisplayType>(native_display.get());
+    native_display = mgeglm::create_native_display(this->shared_from_this());
+    
+    return reinterpret_cast<EGLNativeDisplayType>(native_display.get());
+}
+
 std::shared_ptr<mg::Platform> mg::create_platform(std::shared_ptr<DisplayReport> const& report)
 {
     return std::make_shared<mgg::GBMPlatform>(report);

=== modified file 'src/server/graphics/gbm/gbm_platform.h'
--- src/server/graphics/gbm/gbm_platform.h	2013-03-13 04:54:15 +0000
+++ src/server/graphics/gbm/gbm_platform.h	2013-04-10 16:59:23 +0000
@@ -23,6 +23,8 @@
 #include "mir/graphics/drm_authenticator.h"
 #include "gbm_display_helpers.h"
 
+#include "mir_toolkit/mesa/native_display.h"
+
 namespace mir
 {
 namespace graphics
@@ -42,6 +44,8 @@
             const std::shared_ptr<BufferInitializer>& buffer_initializer);
     std::shared_ptr<Display> create_display();
     std::shared_ptr<PlatformIPCPackage> get_ipc_package();
+    
+    EGLNativeDisplayType shell_egl_display();
 
     /* From DRMAuthenticator */
     void drm_auth_magic(drm_magic_t magic);
@@ -50,6 +54,8 @@
     helpers::GBMHelper gbm;
 
     std::shared_ptr<DisplayReport> listener;
+private:
+    std::shared_ptr<MirMesaEGLNativeDisplay> native_display;
 };
 
 }

=== modified file 'src/server/shell/application_session.cpp'
--- src/server/shell/application_session.cpp	2013-03-22 16:41:59 +0000
+++ src/server/shell/application_session.cpp	2013-04-10 16:59:23 +0000
@@ -17,7 +17,7 @@
  */
 
 #include "mir/shell/application_session.h"
-#include "surface.h"
+#include "mir/shell/surface.h"
 
 #include "mir/shell/surface_factory.h"
 

=== modified file 'src/server/shell/single_visibility_focus_mechanism.cpp'
--- src/server/shell/single_visibility_focus_mechanism.cpp	2013-03-22 16:41:59 +0000
+++ src/server/shell/single_visibility_focus_mechanism.cpp	2013-04-10 16:59:23 +0000
@@ -22,7 +22,7 @@
 #include "mir/shell/input_focus_selector.h"
 
 #include "mir/shell/session.h"
-#include "surface.h" // TODO: Cleanup headers ~racarr
+#include "mir/shell/surface.h"
 
 namespace mf = mir::frontend;
 namespace msh = mir::shell;

=== modified file 'src/server/shell/surface.cpp'
--- src/server/shell/surface.cpp	2013-03-22 16:41:59 +0000
+++ src/server/shell/surface.cpp	2013-04-10 16:59:23 +0000
@@ -16,8 +16,7 @@
  * Authored by: Alan Griffiths <[e-mail]>
  */
 
-#include "surface.h"
-
+#include "mir/shell/surface.h"
 #include "mir/shell/surface_builder.h"
 #include "mir/input/input_channel.h"
 

=== modified file 'src/server/shell/surface_source.cpp'
--- src/server/shell/surface_source.cpp	2013-03-21 03:32:59 +0000
+++ src/server/shell/surface_source.cpp	2013-04-10 16:59:23 +0000
@@ -18,11 +18,10 @@
 
 #include "mir/shell/surface_source.h"
 #include "mir/shell/surface_builder.h"
+#include "mir/shell/surface.h"
 #include "mir/frontend/surface.h"
 #include "mir/input/input_channel_factory.h"
 
-#include "surface.h"
-
 #include <cassert>
 
 namespace ms = mir::surfaces;

=== modified file 'tests/acceptance-tests/test_client_input.cpp'
--- tests/acceptance-tests/test_client_input.cpp	2013-04-09 08:50:40 +0000
+++ tests/acceptance-tests/test_client_input.cpp	2013-04-10 16:59:23 +0000
@@ -85,7 +85,7 @@
     {
     }
     
-    void exec(mir::DisplayServer* /* display_server */) override
+    void exec() override
     {
         input_injection_thread = std::thread([this]() -> void
         {

=== modified file 'tests/acceptance-tests/test_test_framework.cpp'
--- tests/acceptance-tests/test_test_framework.cpp	2012-12-20 09:00:28 +0000
+++ tests/acceptance-tests/test_test_framework.cpp	2013-04-10 16:59:23 +0000
@@ -35,7 +35,7 @@
 {
     struct Server : TestingServerConfiguration
     {
-        void exec(mir::DisplayServer* )
+        void exec()
         {
             using namespace testing;
             FAIL() << "Proving a test can fail";

=== modified file 'tests/behavior-tests/session_management_context.cpp'
--- tests/behavior-tests/session_management_context.cpp	2013-03-29 16:51:35 +0000
+++ tests/behavior-tests/session_management_context.cpp	2013-04-10 16:59:23 +0000
@@ -30,7 +30,7 @@
 #include "mir/graphics/display.h"
 #include "mir/default_server_configuration.h"
 
-#include "src/server/shell/surface.h"
+#include "mir/shell/surface.h"
 
 #include "mir_test_doubles/stub_surface_builder.h"
 #include "mir_test/fake_shared.h"

=== modified file 'tests/integration-tests/graphics/gbm/test_buffer_integration.cpp'
--- tests/integration-tests/graphics/gbm/test_buffer_integration.cpp	2013-03-13 08:09:52 +0000
+++ tests/integration-tests/graphics/gbm/test_buffer_integration.cpp	2013-04-10 16:59:23 +0000
@@ -99,6 +99,11 @@
     {
         return std::shared_ptr<mg::PlatformIPCPackage>();
     }
+
+    EGLNativeDisplayType shell_egl_display()
+    {
+        return static_cast<EGLNativeDisplayType>(0);
+    }
 };
 
 class GBMBufferIntegration : public ::testing::Test

=== modified file 'tests/integration-tests/test_display_info.cpp'
--- tests/integration-tests/test_display_info.cpp	2013-03-22 10:34:16 +0000
+++ tests/integration-tests/test_display_info.cpp	2013-04-10 16:59:23 +0000
@@ -103,7 +103,10 @@
     {
         return std::make_shared<mg::PlatformIPCPackage>();
     }
-
+    EGLNativeDisplayType shell_egl_display()
+    {
+        return static_cast<EGLNativeDisplayType>(0);
+    }
 };
 
 void connection_callback(MirConnection* connection, void* context)

=== modified file 'tests/integration-tests/test_drm_auth_magic.cpp'
--- tests/integration-tests/test_drm_auth_magic.cpp	2013-03-22 10:34:16 +0000
+++ tests/integration-tests/test_drm_auth_magic.cpp	2013-04-10 16:59:23 +0000
@@ -80,6 +80,11 @@
         return std::make_shared<mg::PlatformIPCPackage>();
     }
 
+    EGLNativeDisplayType shell_egl_display()
+    {
+        return static_cast<EGLNativeDisplayType>(0);
+    }
+
     MOCK_METHOD1(drm_auth_magic, void(unsigned int));
 };
 

=== modified file 'tests/integration-tests/test_surfaceloop.cpp'
--- tests/integration-tests/test_surfaceloop.cpp	2013-04-08 04:03:40 +0000
+++ tests/integration-tests/test_surfaceloop.cpp	2013-04-10 16:59:23 +0000
@@ -346,6 +346,11 @@
         {
             return std::make_shared<mg::PlatformIPCPackage>();
         }
+
+        EGLNativeDisplayType shell_egl_display()
+        {
+            return static_cast<EGLNativeDisplayType>(0);
+        }
     };
 
     std::shared_ptr<mg::Platform> the_graphics_platform()
@@ -473,6 +478,11 @@
         {
             return std::make_shared<mg::PlatformIPCPackage>();
         }
+
+        EGLNativeDisplayType shell_egl_display()
+        {
+            return (EGLNativeDisplayType) 0;
+        }
     };
 
     std::shared_ptr<mg::Platform> the_graphics_platform()

=== modified file 'tests/mir_test_framework/testing_process_manager.cpp'
--- tests/mir_test_framework/testing_process_manager.cpp	2013-03-28 12:38:23 +0000
+++ tests/mir_test_framework/testing_process_manager.cpp	2013-04-10 16:59:23 +0000
@@ -64,7 +64,7 @@
         // We're in the server process, so create a display server
         SCOPED_TRACE("Server");
 
-        mir::run_mir(config, [&](mir::DisplayServer& server) { config.exec(&server); });
+        mir::run_mir(config, [&](mir::DisplayServer&) { config.exec(); });
 
         config.on_exit();
     }

=== modified file 'tests/mir_test_framework/testing_server_options.cpp'
--- tests/mir_test_framework/testing_server_options.cpp	2013-03-29 16:51:35 +0000
+++ tests/mir_test_framework/testing_server_options.cpp	2013-04-10 16:59:23 +0000
@@ -105,6 +105,11 @@
     {
         return std::make_shared<mg::PlatformIPCPackage>();
     }
+    
+    EGLNativeDisplayType shell_egl_display()
+    {
+        return (EGLNativeDisplayType) 0;
+    }
 };
 
 class StubRenderer : public mg::Renderer
@@ -186,7 +191,7 @@
             });
 }
 
-void mtf::TestingServerConfiguration::exec(DisplayServer* )
+void mtf::TestingServerConfiguration::exec()
 {
 }
 

=== modified file 'tests/unit-tests/client/gbm/test_gbm_client_platform.cpp'
--- tests/unit-tests/client/gbm/test_gbm_client_platform.cpp	2013-03-01 18:50:18 +0000
+++ tests/unit-tests/client/gbm/test_gbm_client_platform.cpp	2013-04-10 16:59:23 +0000
@@ -21,6 +21,8 @@
 #include "mir_test_doubles/mock_client_context.h"
 #include "mir_test_doubles/mock_client_surface.h"
 
+#include "mir_toolkit/mesa/native_display.h"
+
 #include <gtest/gtest.h>
 
 namespace mcl = mir::client;
@@ -44,12 +46,12 @@
     mcl::NativeClientPlatformFactory factory;
     auto platform = factory.create_client_platform(&context);
 
-    EGLNativeDisplayType nd;
+    MirMesaEGLNativeDisplay* nd;
     {
         std::shared_ptr<EGLNativeDisplayType> native_display = platform->create_egl_native_display();
 
-        nd = *native_display;
-        EXPECT_TRUE(mir_egl_native_display_is_valid(nd));
+        nd = reinterpret_cast<MirMesaEGLNativeDisplay*>(*native_display);
+        EXPECT_TRUE(mir_egl_mesa_display_is_valid(nd));
     }
-    EXPECT_FALSE(mir_egl_native_display_is_valid(nd));
+    EXPECT_FALSE(mir_egl_mesa_display_is_valid(nd));
 }

=== modified file 'tests/unit-tests/frontend/test_session_mediator.cpp'
--- tests/unit-tests/frontend/test_session_mediator.cpp	2013-03-29 16:51:35 +0000
+++ tests/unit-tests/frontend/test_session_mediator.cpp	2013-04-10 16:59:23 +0000
@@ -35,7 +35,7 @@
 #include "mir_test_doubles/stub_surface_builder.h"
 #include "mir_test/fake_shared.h"
 
-#include "src/server/shell/surface.h"
+#include "mir/shell/surface.h"
 
 #include <gtest/gtest.h>
 #include <gmock/gmock.h>
@@ -121,6 +121,11 @@
     {
         return std::make_shared<mg::PlatformIPCPackage>();
     }
+    
+    EGLNativeDisplayType shell_egl_display()
+    {
+        return static_cast<EGLNativeDisplayType>(0);
+    }
 };
 
 }

=== modified file 'tests/unit-tests/frontend/test_session_mediator_android.cpp'
--- tests/unit-tests/frontend/test_session_mediator_android.cpp	2013-03-21 03:32:59 +0000
+++ tests/unit-tests/frontend/test_session_mediator_android.cpp	2013-04-10 16:59:23 +0000
@@ -78,6 +78,11 @@
     {
         return std::make_shared<mg::PlatformIPCPackage>();
     }
+
+    EGLNativeDisplayType shell_egl_display()
+    {
+        return static_cast<EGLNativeDisplayType>(0);
+    }
 };
 
 }

=== modified file 'tests/unit-tests/frontend/test_session_mediator_gbm.cpp'
--- tests/unit-tests/frontend/test_session_mediator_gbm.cpp	2013-03-21 03:32:59 +0000
+++ tests/unit-tests/frontend/test_session_mediator_gbm.cpp	2013-04-10 16:59:23 +0000
@@ -82,6 +82,11 @@
         return std::make_shared<mg::PlatformIPCPackage>();
     }
 
+    EGLNativeDisplayType shell_egl_display()
+    {
+        return static_cast<EGLNativeDisplayType>(0);
+    }
+
     MOCK_METHOD1(drm_auth_magic, void(drm_magic_t));
 };
 

=== modified file 'tests/unit-tests/graphics/CMakeLists.txt'
--- tests/unit-tests/graphics/CMakeLists.txt	2013-03-13 04:54:15 +0000
+++ tests/unit-tests/graphics/CMakeLists.txt	2013-04-10 16:59:23 +0000
@@ -11,7 +11,7 @@
 
 if (MIR_PLATFORM STREQUAL "gbm")
 add_subdirectory(gbm/)
+add_subdirectory(egl)
 endif()
 
 set(UNIT_TEST_SOURCES ${UNIT_TEST_SOURCES} PARENT_SCOPE)
-

=== added directory 'tests/unit-tests/graphics/egl'
=== added file 'tests/unit-tests/graphics/egl/CMakeLists.txt'
--- tests/unit-tests/graphics/egl/CMakeLists.txt	1970-01-01 00:00:00 +0000
+++ tests/unit-tests/graphics/egl/CMakeLists.txt	2013-04-10 16:59:23 +0000
@@ -0,0 +1,6 @@
+list(APPEND UNIT_TEST_SOURCES
+    ${CMAKE_CURRENT_SOURCE_DIR}/test_mirserver_mesa_egl_native_display.cpp
+)
+
+set(UNIT_TEST_SOURCES ${UNIT_TEST_SOURCES} PARENT_SCOPE)
+

=== added file 'tests/unit-tests/graphics/egl/test_mirserver_mesa_egl_native_display.cpp'
--- tests/unit-tests/graphics/egl/test_mirserver_mesa_egl_native_display.cpp	1970-01-01 00:00:00 +0000
+++ tests/unit-tests/graphics/egl/test_mirserver_mesa_egl_native_display.cpp	2013-04-10 16:59:23 +0000
@@ -0,0 +1,207 @@
+/*
+ * Copyright © 2013 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authored by: Robert Carr <[e-mail]>
+ */
+#include "mir/graphics/egl/mesa_native_display.h"
+#include "mir/graphics/platform.h"
+#include "mir/graphics/platform_ipc_package.h"
+#include "mir/compositor/buffer_ipc_package.h"
+
+#include "mir_toolkit/c_types.h"
+#include "mir_toolkit/mesa/native_display.h"
+
+#include "mir_test/fake_shared.h"
+#include "mir_test_doubles/stub_surface_builder.h"
+#include "mir_test_doubles/mock_surface.h"
+#include "mir_test_doubles/mock_buffer.h"
+
+#include <gtest/gtest.h>
+#include <gmock/gmock.h>
+
+namespace mg = mir::graphics;
+namespace mgeglm = mg::egl::mesa;
+namespace mc = mir::compositor;
+namespace msh = mir::shell;
+namespace geom = mir::geometry;
+namespace mt = mir::test;
+namespace mtd = mt::doubles;
+
+namespace
+{
+
+struct MockGraphicsPlatform : public mg::Platform
+{
+    MOCK_METHOD1(create_buffer_allocator, std::shared_ptr<mc::GraphicBufferAllocator>(std::shared_ptr<mg::BufferInitializer> const&));
+    MOCK_METHOD0(create_display, std::shared_ptr<mg::Display>());
+    MOCK_METHOD0(get_ipc_package, std::shared_ptr<mg::PlatformIPCPackage>());
+    MOCK_METHOD0(shell_egl_display, EGLNativeDisplayType());
+};
+
+struct MirServerMesaEGLNativeDisplaySetup : public testing::Test
+{
+    MirServerMesaEGLNativeDisplaySetup()
+    {
+        using namespace ::testing;
+
+        test_platform_package.ipc_data = {1, 2};
+        test_platform_package.ipc_fds = {2, 3};
+        
+        ON_CALL(mock_graphics_platform, get_ipc_package())
+            .WillByDefault(Return(mt::fake_shared(test_platform_package)));
+    }
+
+    // Test dependencies
+    MockGraphicsPlatform mock_graphics_platform;
+
+    // Useful stub data
+    mg::PlatformIPCPackage test_platform_package;
+};
+
+MATCHER_P(PackageMatches, package, "")
+{
+    if (arg.data_items != static_cast<int>(package.ipc_data.size()))
+        return false;
+    for (uint i = 0; i < package.ipc_data.size(); i++)
+    {
+        if (arg.data[i] != package.ipc_data[i]) return false;
+    }
+    if (arg.fd_items != static_cast<int>(package.ipc_fds.size()))
+        return false;
+    for (uint i = 0; i < package.ipc_fds.size(); i++)
+    {
+        if (arg.fd[i] != package.ipc_fds[i]) return false;
+    }
+    return true;
+}
+
+MATCHER_P(StrideMatches, package, "")
+{
+    if (arg.stride != package.stride)
+    {
+        return false;
+    }
+    return true;
+}
+
+MATCHER_P(ParametersHaveSize, size, "")
+{
+    if (static_cast<uint32_t>(arg.width) != size.width.as_uint32_t())
+        return false;
+    if (static_cast<uint32_t>(arg.height) != size.height.as_uint32_t())
+        return false;
+    return true;
+}
+
+}
+
+TEST_F(MirServerMesaEGLNativeDisplaySetup, display_get_platform_is_cached_platform_package)
+{
+    using namespace ::testing;
+    
+    EXPECT_CALL(mock_graphics_platform, get_ipc_package()).Times(1);
+
+    auto display = mgeglm::create_native_display(mt::fake_shared(mock_graphics_platform));
+    
+    MirPlatformPackage package;
+    memset(&package, 0, sizeof(MirPlatformPackage));
+    display->display_get_platform(display.get(), &package);
+    EXPECT_THAT(package, PackageMatches(test_platform_package));
+
+    MirPlatformPackage requeried_package;
+    // The package is cached to allow the package creator to control lifespan of fd members
+    // and so this should not trigger another call to get_ipc_package()
+    display->display_get_platform(display.get(), &requeried_package);
+    
+    EXPECT_FALSE(memcmp(&requeried_package.data, &package.data, sizeof(package.data[0])*package.data_items));
+    EXPECT_FALSE(memcmp(&requeried_package.fd, &package.fd, sizeof(package.fd[0])*package.fd_items));
+}
+
+TEST_F(MirServerMesaEGLNativeDisplaySetup, surface_get_current_buffer)
+{
+    using namespace ::testing;
+    
+    mtd::MockSurface surface(std::make_shared<mtd::StubSurfaceBuilder>());
+    auto buffer = std::make_shared<mtd::MockBuffer>(geom::Size(), geom::Stride(), geom::PixelFormat());
+    
+    mc::BufferIPCPackage test_buffer_package;
+    
+    test_buffer_package.ipc_data = {1, 2};
+    test_buffer_package.ipc_fds = {2, 3};
+    test_buffer_package.stride = 77;
+
+    auto display = mgeglm::create_native_display(mt::fake_shared(mock_graphics_platform));
+    
+    EXPECT_CALL(surface, client_buffer()).Times(1)
+        .WillOnce(Return(buffer));
+    EXPECT_CALL(*buffer, get_ipc_package()).Times(1)
+        .WillOnce(Return(mt::fake_shared(test_buffer_package)));
+    
+    MirBufferPackage buffer_package;
+    memset(&buffer_package, 0, sizeof(MirBufferPackage));
+    display->surface_get_current_buffer(
+        display.get(), static_cast<MirEGLNativeWindowType>(&surface), &buffer_package);
+    EXPECT_THAT(buffer_package, AllOf(PackageMatches(test_buffer_package), StrideMatches(test_buffer_package)));
+}
+
+TEST_F(MirServerMesaEGLNativeDisplaySetup, surface_advance_buffer)
+{
+    using namespace ::testing;
+    
+    mtd::MockSurface surface(std::make_shared<mtd::StubSurfaceBuilder>());
+    
+    auto display = mgeglm::create_native_display(mt::fake_shared(mock_graphics_platform));
+    
+    EXPECT_CALL(surface, advance_client_buffer()).Times(1);
+    
+    display->surface_advance_buffer(
+        display.get(), static_cast<MirEGLNativeWindowType>(&surface));
+}
+
+TEST(MirServerMesaEGLNativeDisplayInvariants, pixel_format_formats_are_castable)
+{
+    EXPECT_EQ(static_cast<MirPixelFormat>(geom::PixelFormat::invalid), mir_pixel_format_invalid);
+    EXPECT_EQ(static_cast<MirPixelFormat>(geom::PixelFormat::abgr_8888), mir_pixel_format_abgr_8888);
+    EXPECT_EQ(static_cast<MirPixelFormat>(geom::PixelFormat::xbgr_8888), mir_pixel_format_xbgr_8888);
+    EXPECT_EQ(static_cast<MirPixelFormat>(geom::PixelFormat::argb_8888), mir_pixel_format_argb_8888);
+    EXPECT_EQ(static_cast<MirPixelFormat>(geom::PixelFormat::xrgb_8888), mir_pixel_format_xrgb_8888);
+    EXPECT_EQ(static_cast<MirPixelFormat>(geom::PixelFormat::bgr_888), mir_pixel_format_bgr_888);
+}
+
+TEST_F(MirServerMesaEGLNativeDisplaySetup, surface_get_parameters)
+{
+    using namespace ::testing;
+    
+    geom::Size const test_surface_size = geom::Size{geom::Width{17},
+                                                    geom::Height{29}};
+    geom::PixelFormat const test_pixel_format = geom::PixelFormat::xrgb_8888;
+
+    auto display = mgeglm::create_native_display(mt::fake_shared(mock_graphics_platform));
+
+    mtd::MockSurface surface(std::make_shared<mtd::StubSurfaceBuilder>());
+    EXPECT_CALL(surface, size()).Times(AtLeast(1)).WillRepeatedly(Return(test_surface_size));
+    EXPECT_CALL(surface, pixel_format()).Times(AtLeast(1)).WillRepeatedly(Return(test_pixel_format));
+    
+    MirSurfaceParameters parameters;
+    memset(&parameters, 0, sizeof(MirSurfaceParameters));
+    display->surface_get_parameters(
+        display.get(), static_cast<MirEGLNativeWindowType>(&surface), &parameters);
+
+    EXPECT_THAT(parameters, ParametersHaveSize(test_surface_size));
+    EXPECT_EQ(parameters.pixel_format, static_cast<MirPixelFormat>(geom::PixelFormat::xrgb_8888));
+
+    // TODO: What to do about buffer usage besides hardware? ~racarr
+    EXPECT_EQ(parameters.buffer_usage, mir_buffer_usage_hardware);
+}

=== modified file 'tests/unit-tests/shell/test_application_session.cpp'
--- tests/unit-tests/shell/test_application_session.cpp	2013-03-29 16:51:35 +0000
+++ tests/unit-tests/shell/test_application_session.cpp	2013-04-10 16:59:23 +0000
@@ -25,7 +25,7 @@
 #include "mir_test_doubles/stub_surface_builder.h"
 #include "mir_test_doubles/stub_surface.h"
 
-#include "src/server/shell/surface.h"
+#include "mir/shell/surface.h"
 
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>

=== modified file 'tests/unit-tests/shell/test_session_manager.cpp'
--- tests/unit-tests/shell/test_session_manager.cpp	2013-03-29 16:51:35 +0000
+++ tests/unit-tests/shell/test_session_manager.cpp	2013-04-10 16:59:23 +0000
@@ -32,7 +32,7 @@
 #include "mir_test_doubles/null_buffer_bundle.h"
 #include "mir_test_doubles/stub_surface_builder.h"
 
-#include "src/server/shell/surface.h"
+#include "mir/shell/surface.h"
 
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>

=== modified file 'tests/unit-tests/shell/test_surface.cpp'
--- tests/unit-tests/shell/test_surface.cpp	2013-03-29 17:03:04 +0000
+++ tests/unit-tests/shell/test_surface.cpp	2013-04-10 16:59:23 +0000
@@ -16,7 +16,7 @@
  * Authored by: Alan Griffiths <[e-mail]>
  */
 
-#include "src/server/shell/surface.h"
+#include "mir/shell/surface.h"
 #include "mir/surfaces/surface.h"
 #include "mir/frontend/surface_creation_parameters.h"
 #include "mir/surfaces/surface_stack_model.h"