Merge lp:~kdub/mir/gbm-cleanup into lp:~mir-team/mir/trunk

[Kevin DuBois]

add hooks for eglSwapInterval in the gbm platform. Split the mesa EGLMesaNativeDisplay into an EGLMesaNativeDisplay and an EGLMesaNativeSurface

we need some hooks in the driver to get eglSwapInterval(dpy,0); to work on the gbm platform (android already has these hooks). Since its disruptive to bump the driver, I did a cleanup of the interface that mesa uses to get buffers from us; we now have both an EGLNativeDisplayType and an EGLNativeSurface type, and the upstream version of the drivers we maintain is now (probably) upstreamable.

the patch for mesa has landed to (https://github.com/RAOF/mesa.git), we should bump the package in the archives to the latest version at about the same time this lands. I'll coordinate with RAOF to minimize disruption.

[Kevin DuBois (kdub)]

if you navigate over to github, you'll see the platform driver cleanup removes "mir_client_library.h", so the driver doesn't have to worry about any ipc stuff.

also, @l46, note that gbm is 'inprocess egl/internal client' agnostic. (the driver doesn't care if ipc is involved or not, as long as it gets buffers)

the rough edges, (which hopefully are ok) are
1) Since mir_toolkit/mesa/native_display.h has some #defines, this is because this file must be C-compatible for mesa.
2) gbm_client_platform.cpp has some awkwardness, this is what android does and is a pre-existing issue with this interface for acquiring a native window type on the client side.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:728
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~kdub/mir/gbm-cleanup/+merge/169306/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/mir-ci/726/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/903
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/785
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-raring-amd64-ci/211
        deb: http://jenkins.qa.ubuntu.com/job/mir-raring-amd64-ci/211/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=quantal,flavor=amd64/415

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

review: Needs Fixing (continuous-integration)

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:729
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~kdub/mir/gbm-cleanup/+merge/169306/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/mir-ci/729/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/906
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/788
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-raring-amd64-ci/214
        deb: http://jenkins.qa.ubuntu.com/job/mir-raring-amd64-ci/214/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=quantal,flavor=amd64/417

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

review: Needs Fixing (continuous-integration)

[Daniel van Vugt (vanvugt)]

+1 for any progress toward eglSwapInterval(dpy,0) support. Although I assume we'll only ever need support for a zero interval in clients. The server will probably always use interval 1... ?

[Kevin DuBois (kdub)]

> +1 for any progress toward eglSwapInterval(dpy,0) support. Although I assume
> we'll only ever need support for a zero interval in clients. The server will
> probably always use interval 1... ?
right, thats the plan!

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:729
http://jenkins.qa.ubuntu.com/job/mir-ci/741/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-android-raring-i386-build/922
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-clang-raring-amd64-build/804
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-raring-amd64-ci/226
        deb: http://jenkins.qa.ubuntu.com/job/mir-raring-amd64-ci/226/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/mir-vm-ci-build/./distribution=quantal,flavor=amd64/427

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

review: Approve (continuous-integration)

[Alan Griffiths (alan-griffiths)]

Some of the TODO comments are overly terse, but generally seems to result in cleaner code.

review: Approve

[Alexandros Frantzis (afrantzis)]

Looks good.

Non-blocking nits:

495 + MirBufferPackage* package)
515 + MirSurfaceParameters* parameters)

Wrong alignment.

588 + static int set_swapinterval_static(MirMesaEGLNativeSurface* surface, int interval);
589 + static int advance_buffer_static(MirMesaEGLNativeSurface* surface,
590 + MirBufferPackage* package);
591 +
592 + static int get_parameters_static(MirMesaEGLNativeSurface* surface,
593 + MirSurfaceParameters* parameters);

These could be private.

review: Approve

[Kevin DuBois (kdub)]

since this is changing our header that mesa depends upon, changing mir version to 0.0.4

[Chris Halse Rogers (raof)]

Superceded by https://code.launchpad.net/~raof/mir/gbm-cleanup-extra/+merge/169972 which has a couple of fixups to the packaging.

=== modified file 'debian/changelog'
--- debian/changelog	2013-05-15 00:37:34 +0000
+++ debian/changelog	2013-06-18 01:07:24 +0000
@@ -1,3 +1,9 @@
+mir (0.0.4-0ubuntu1) raring; urgency=low
+
+  * change mesa header to clean up the file and add swapinterval 0 hook
+
+ -- Kevin DuBois <[e-mail]>  Mon, 17 Jun 2013 18:02:32 -0700
+
 mir (0.0.3-0ubuntu1) raring; urgency=low
 
   [ Bryce Harrington ]

=== modified file 'include/shared/mir_toolkit/mesa/native_display.h'
--- include/shared/mir_toolkit/mesa/native_display.h	2013-05-08 18:57:03 +0000
+++ include/shared/mir_toolkit/mesa/native_display.h	2013-06-18 01:07:24 +0000
@@ -21,6 +21,9 @@
 #include "mir_toolkit/mir_native_buffer.h"
 #include "mir_toolkit/client_types.h"
 
+#define MIR_MESA_TRUE 1
+#define MIR_MESA_FALSE 0
+
 #ifdef __cplusplus
 /**
  * \addtogroup mir_toolkit
@@ -31,31 +34,24 @@
 #endif
 
 typedef struct MirMesaEGLNativeDisplay MirMesaEGLNativeDisplay;
+typedef struct MirMesaEGLNativeSurface MirMesaEGLNativeSurface;
 
 struct MirMesaEGLNativeDisplay
 {
-    void (*display_get_platform)(MirMesaEGLNativeDisplay* display,
+    int (*display_get_platform)(MirMesaEGLNativeDisplay* display,
                                  MirPlatformPackage* package);
-    void (*surface_get_current_buffer)(MirMesaEGLNativeDisplay* display,
-                                       MirEGLNativeWindowType surface,
-                                       MirBufferPackage* buffer_package);
-    void (*surface_get_parameters)(MirMesaEGLNativeDisplay* display,
-                                   MirEGLNativeWindowType surface,
+    void *context;
+};
+
+struct MirMesaEGLNativeSurface
+{
+    int (*surface_set_swapinterval)(MirMesaEGLNativeSurface* surface, int interval);
+    int (*surface_advance_buffer)(MirMesaEGLNativeSurface* surface,
+                                   MirBufferPackage* buffer_package);
+    int (*surface_get_parameters)(MirMesaEGLNativeSurface* surface,
                                    MirSurfaceParameters* surface_parameters);
-    void (*surface_advance_buffer)(MirMesaEGLNativeDisplay* display,
-                                   MirEGLNativeWindowType surface);
-
-    void *context;
 };
 
-typedef enum mir_display_type
-{
-    MIR_DISPLAY_TYPE_CLIENT,
-    MIR_DISPLAY_TYPE_SERVER_INTERNAL
-} mir_display_type;
-
-mir_display_type mir_get_display_type(MirMesaEGLNativeDisplay* display);
-
 int mir_egl_mesa_display_is_valid(MirMesaEGLNativeDisplay* display);
 int mir_server_internal_display_is_valid(MirMesaEGLNativeDisplay* display);
 

=== modified file 'src/client/CMakeLists.txt'
--- src/client/CMakeLists.txt	2013-06-03 12:15:45 +0000
+++ src/client/CMakeLists.txt	2013-06-18 01:07:24 +0000
@@ -54,6 +54,7 @@
   gbm/gbm_client_buffer_factory.cpp
   gbm/gbm_client_buffer.cpp
   gbm/mesa_native_display_container.cpp
+  gbm/gbm_native_surface.cpp
   ${CLIENT_SOURCES}
 )
 endif()

=== modified file 'src/client/gbm/gbm_client_platform.cpp'
--- src/client/gbm/gbm_client_platform.cpp	2013-05-03 21:59:56 +0000
+++ src/client/gbm/gbm_client_platform.cpp	2013-06-18 01:07:24 +0000
@@ -21,6 +21,7 @@
 #include "gbm_client_buffer_factory.h"
 #include "mesa_native_display_container.h"
 #include "drm_fd_handler.h"
+#include "gbm_native_surface.h"
 #include "../mir_connection.h"
 #include "../client_buffer_factory.h"
 #include "../native_client_platform_factory.h"
@@ -129,11 +130,32 @@
     return std::make_shared<mclg::GBMClientBufferFactory>(drm_fd_handler);
 }
 
+namespace
+{
+struct NativeWindowDeleter
+{
+    NativeWindowDeleter(mclg::GBMNativeSurface* window)
+     : window(window) {}
+
+    void operator()(EGLNativeWindowType* type)
+    {
+        delete type;
+        delete window;
+    }
+
+private:
+    mclg::GBMNativeSurface* window;
+};
+}
+
 std::shared_ptr<EGLNativeWindowType> mclg::GBMClientPlatform::create_egl_native_window(ClientSurface* client_surface)
 {
-    auto window_type = std::make_shared<EGLNativeWindowType>();
-    *window_type = reinterpret_cast<EGLNativeWindowType>(client_surface);
-    return window_type;
+    //TODO: this is awkward on both android and gbm...
+    auto gbm_window = new GBMNativeSurface(*client_surface);
+    auto egl_native_window = new EGLNativeWindowType;
+    *egl_native_window = gbm_window; 
+    NativeWindowDeleter deleter(gbm_window);
+    return std::shared_ptr<EGLNativeWindowType>(egl_native_window, deleter);
 }
 
 std::shared_ptr<EGLNativeDisplayType> mclg::GBMClientPlatform::create_egl_native_display()

=== added file 'src/client/gbm/gbm_native_surface.cpp'
--- src/client/gbm/gbm_native_surface.cpp	1970-01-01 00:00:00 +0000
+++ src/client/gbm/gbm_native_surface.cpp	2013-06-18 01:07:24 +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 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 Lesser 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: Kevin DuBois <[e-mail]>
+ */
+
+#include <cstring>
+#include "../client_buffer.h"
+#include "gbm_native_surface.h"
+
+namespace mclg=mir::client::gbm;
+
+namespace
+{
+static int advance_buffer_static(MirMesaEGLNativeSurface* surface,
+                                  MirBufferPackage* buffer_package)
+{
+    auto s = static_cast<mclg::GBMNativeSurface*>(surface);
+    return s->advance_buffer(buffer_package);
+}
+
+static int get_parameters_static(MirMesaEGLNativeSurface* surface,
+                                  MirSurfaceParameters* surface_parameters)
+{
+    auto s = static_cast<mclg::GBMNativeSurface*>(surface);
+    return s->get_parameters(surface_parameters);
+}
+
+static int set_swapinterval_static(MirMesaEGLNativeSurface* surface, int interval)
+{
+    //TODO
+    (void) surface;
+    (void) interval;
+    return MIR_MESA_TRUE;
+} 
+
+static void buffer_advanced_callback(MirSurface*  /* surface */,
+                                     void*  /* context */)
+{
+}
+}
+
+mclg::GBMNativeSurface::GBMNativeSurface(ClientSurface& surface)
+    : surface(surface)
+{
+    surface_advance_buffer = advance_buffer_static; 
+    surface_get_parameters = get_parameters_static; 
+    surface_set_swapinterval = set_swapinterval_static; 
+}
+
+int mclg::GBMNativeSurface::advance_buffer(MirBufferPackage* buffer_package)
+{
+    mir_wait_for(surface.next_buffer(buffer_advanced_callback, NULL));
+    auto buffer = surface.get_current_buffer();
+
+    auto buffer_to_driver = buffer->native_buffer_handle();
+    memcpy(buffer_package, buffer_to_driver.get(), sizeof(MirBufferPackage));
+    return MIR_MESA_TRUE;
+}
+
+int mclg::GBMNativeSurface::get_parameters(MirSurfaceParameters* surface_parameters)
+{
+    auto params = surface.get_parameters();
+    memcpy(surface_parameters, &params, sizeof(MirSurfaceParameters));
+    return MIR_MESA_TRUE;
+}

=== added file 'src/client/gbm/gbm_native_surface.h'
--- src/client/gbm/gbm_native_surface.h	1970-01-01 00:00:00 +0000
+++ src/client/gbm/gbm_native_surface.h	2013-06-18 01:07:24 +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 Lesser 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: Kevin DuBois <[e-mail]>
+ */
+
+#include "mir_toolkit/mesa/native_display.h"
+#include "../mir_client_surface.h"
+
+namespace mir
+{
+namespace client
+{
+namespace gbm
+{
+
+class GBMNativeSurface : public MirMesaEGLNativeSurface
+{
+public:
+    explicit GBMNativeSurface(ClientSurface&);
+
+    int advance_buffer(MirBufferPackage* buffer_package);
+    int get_parameters(MirSurfaceParameters* surface_parameters);
+private:
+    ClientSurface& surface;
+};
+
+}
+}
+}

=== modified file 'src/client/gbm/mesa_native_display_container.cpp'
--- src/client/gbm/mesa_native_display_container.cpp	2013-06-06 08:36:17 +0000
+++ src/client/gbm/mesa_native_display_container.cpp	2013-06-18 01:07:24 +0000
@@ -33,42 +33,12 @@
 extern "C"
 {
 
-static void gbm_egl_display_get_platform(MirMesaEGLNativeDisplay* display,
+static int gbm_egl_display_get_platform(MirMesaEGLNativeDisplay* display,
                                          MirPlatformPackage* package)
 {
     auto connection = static_cast<MirConnection*>(display->context);
-
     mir_connection_get_platform(connection, package);
-}
-
-static void gbm_egl_surface_get_current_buffer(MirMesaEGLNativeDisplay* /* display */,
-                                               MirEGLNativeWindowType surface,
-                                               MirBufferPackage* buffer_package)
-{
-    MirSurface* ms = static_cast<MirSurface*>(surface);
-    MirBufferPackage * current_package;
-    mir_surface_get_current_buffer(ms, &current_package);
-    memcpy(buffer_package, current_package, sizeof(MirBufferPackage));
-}
-
-static void buffer_advanced_callback(MirSurface*  /* surface */,
-                                     void*  /* context */)
-{
-}
-
-static void gbm_egl_surface_advance_buffer(MirMesaEGLNativeDisplay* /* display */,
-                                           MirEGLNativeWindowType surface)
-{
-    MirSurface* ms = static_cast<MirSurface*>(surface);
-    mir_wait_for(mir_surface_swap_buffers(ms, buffer_advanced_callback, nullptr));
-}
-
-static void gbm_egl_surface_get_parameters(MirMesaEGLNativeDisplay* /* display */,
-                                           MirEGLNativeWindowType surface,
-                                           MirSurfaceParameters* surface_parameters)
-{
-    MirSurface* ms = static_cast<MirSurface*>(surface);
-    mir_surface_get_parameters(ms,  surface_parameters);
+    return MIR_MESA_TRUE;
 }
 
 int mir_egl_mesa_display_is_valid(MirMesaEGLNativeDisplay* display)
@@ -99,8 +69,7 @@
     }
 }
 
-bool
-mclg::MesaNativeDisplayContainer::validate(MirEGLNativeDisplayType display) const
+bool mclg::MesaNativeDisplayContainer::validate(MirEGLNativeDisplayType display) const
 {
     std::lock_guard<std::mutex> lg(guard);
     return (valid_displays.find(display) != valid_displays.end());
@@ -111,9 +80,6 @@
 {
     MirMesaEGLNativeDisplay* display = new MirMesaEGLNativeDisplay();
     display->display_get_platform = gbm_egl_display_get_platform;
-    display->surface_get_current_buffer = gbm_egl_surface_get_current_buffer;
-    display->surface_advance_buffer = gbm_egl_surface_advance_buffer;
-    display->surface_get_parameters = gbm_egl_surface_get_parameters;
     display->context = connection;
 
     std::lock_guard<std::mutex> lg(guard);
@@ -123,8 +89,7 @@
     return egl_display;
 }
 
-void
-mclg::MesaNativeDisplayContainer::release(MirEGLNativeDisplayType display)
+void mclg::MesaNativeDisplayContainer::release(MirEGLNativeDisplayType display)
 {
     std::lock_guard<std::mutex> lg(guard);
 

=== modified file 'src/server/graphics/gbm/CMakeLists.txt'
--- src/server/graphics/gbm/CMakeLists.txt	2013-05-23 13:01:05 +0000
+++ src/server/graphics/gbm/CMakeLists.txt	2013-06-18 01:07:24 +0000
@@ -27,6 +27,7 @@
   kms_page_flipper.cpp
   linux_virtual_terminal.cpp
   internal_native_display.cpp
+  internal_native_surface.cpp
   internal_client.cpp
   drm_close_threadsafe.cpp
 )

=== modified file 'src/server/graphics/gbm/internal_client.cpp'
--- src/server/graphics/gbm/internal_client.cpp	2013-05-14 16:45:25 +0000
+++ src/server/graphics/gbm/internal_client.cpp	2013-06-18 01:07:24 +0000
@@ -34,5 +34,10 @@
 
 EGLNativeWindowType mgg::InternalClient::egl_native_window(std::shared_ptr<mf::Surface> const& surface)
 {
-    return reinterpret_cast<EGLNativeWindowType>(surface.get());
+    if (!client_window)
+    {
+        client_window = std::make_shared<mgg::InternalNativeSurface>(surface);
+    }
+
+    return client_window.get();
 }

=== modified file 'src/server/graphics/gbm/internal_client.h'
--- src/server/graphics/gbm/internal_client.h	2013-05-14 16:45:25 +0000
+++ src/server/graphics/gbm/internal_client.h	2013-06-18 01:07:24 +0000
@@ -21,6 +21,7 @@
 
 #include "mir/graphics/internal_client.h"
 #include "internal_native_display.h"
+#include "internal_native_surface.h"
 #include <memory>
 
 namespace mir
@@ -44,6 +45,7 @@
 
 private:
     std::shared_ptr<MirMesaEGLNativeDisplay> const native_display;
+    std::shared_ptr<MirMesaEGLNativeSurface> client_window;
 };
 
 }

=== modified file 'src/server/graphics/gbm/internal_native_display.cpp'
--- src/server/graphics/gbm/internal_native_display.cpp	2013-05-14 22:14:13 +0000
+++ src/server/graphics/gbm/internal_native_display.cpp	2013-06-18 01:07:24 +0000
@@ -38,23 +38,9 @@
 {
     context = this;
     this->display_get_platform = &InternalNativeDisplay::native_display_get_platform;
-    this->surface_get_current_buffer = &InternalNativeDisplay::native_display_surface_get_current_buffer;
-    this->surface_get_parameters = &InternalNativeDisplay::native_display_surface_get_parameters;
-    this->surface_advance_buffer = &InternalNativeDisplay::native_display_surface_advance_buffer;
-}
-
-void mgg::InternalNativeDisplay::native_display_surface_get_current_buffer(MirMesaEGLNativeDisplay*, 
-                                                      MirEGLNativeWindowType surface,
-                                                      MirBufferPackage* package)
-{
-        auto mir_surface = static_cast<mf::Surface*>(surface);
-
-        auto buffer = mir_surface->client_buffer();
-        auto buffer_package = buffer->native_buffer_handle();
-        memcpy(package, buffer_package.get(), sizeof(MirBufferPackage));
-}
-
-void mgg::InternalNativeDisplay::native_display_get_platform(MirMesaEGLNativeDisplay* display, MirPlatformPackage* package)
+}
+
+int mgg::InternalNativeDisplay::native_display_get_platform(MirMesaEGLNativeDisplay* display, MirPlatformPackage* package)
 {
     auto native_disp = static_cast<InternalNativeDisplay*>(display);
     package->data_items = native_disp->platform_package->ipc_data.size();
@@ -68,23 +54,5 @@
         package->fd[i] = native_disp->platform_package->ipc_fds[i];
     }
 
-}
-
-void mgg::InternalNativeDisplay::native_display_surface_get_parameters(MirMesaEGLNativeDisplay*, 
-                                                  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;
-}
-
-void mgg::InternalNativeDisplay::native_display_surface_advance_buffer(MirMesaEGLNativeDisplay*, 
-                                                  MirEGLNativeWindowType surface)
-{
-    auto mir_surface = static_cast<mf::Surface*>(surface);
-    mir_surface->advance_client_buffer();
+    return MIR_MESA_TRUE;
 }

=== modified file 'src/server/graphics/gbm/internal_native_display.h'
--- src/server/graphics/gbm/internal_native_display.h	2013-05-09 20:58:30 +0000
+++ src/server/graphics/gbm/internal_native_display.h	2013-06-18 01:07:24 +0000
@@ -1,4 +1,3 @@
-
 /*
  * Copyright © 2013 Canonical Ltd.
  *
@@ -37,16 +36,9 @@
 public:
     InternalNativeDisplay(std::shared_ptr<PlatformIPCPackage> const& platform_package);
 
-    static void native_display_get_platform(MirMesaEGLNativeDisplay* display, MirPlatformPackage* package);
-    static void native_display_surface_get_current_buffer(MirMesaEGLNativeDisplay* /* display */, 
-                                                          MirEGLNativeWindowType surface,
-                                                          MirBufferPackage* package);
-    static void native_display_surface_get_parameters(MirMesaEGLNativeDisplay* /* display  */, 
-                                                      MirEGLNativeWindowType surface,
-                                                      MirSurfaceParameters* parameters);
-    static void native_display_surface_advance_buffer(MirMesaEGLNativeDisplay* /* display */, 
-                                                      MirEGLNativeWindowType surface);
 private:
+    static int native_display_get_platform(MirMesaEGLNativeDisplay* display, MirPlatformPackage* package);
+
     std::shared_ptr<PlatformIPCPackage> platform_package;
 };
 

=== added file 'src/server/graphics/gbm/internal_native_surface.cpp'
--- src/server/graphics/gbm/internal_native_surface.cpp	1970-01-01 00:00:00 +0000
+++ src/server/graphics/gbm/internal_native_surface.cpp	2013-06-18 01:07:24 +0000
@@ -0,0 +1,77 @@
+/*
+ * 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: Kevin DuBois <[e-mail]>
+ */
+
+#include "mir/frontend/surface.h"
+#include "mir/compositor/buffer.h"
+#include "internal_native_surface.h"
+#include <cstring>
+namespace mgg = mir::graphics::gbm;
+
+mgg::InternalNativeSurface::InternalNativeSurface(std::shared_ptr<frontend::Surface> const& surface)
+    : surface(surface)
+{
+    surface_advance_buffer = advance_buffer_static;
+    surface_get_parameters = get_parameters_static;
+    surface_set_swapinterval = set_swapinterval_static; 
+}
+
+int mgg::InternalNativeSurface::advance_buffer_static(
+    MirMesaEGLNativeSurface* surface, MirBufferPackage* package)
+{
+    auto native_surface = static_cast<mgg::InternalNativeSurface*>(surface);
+    return native_surface->advance_buffer(package);
+}
+
+int mgg::InternalNativeSurface::advance_buffer(MirBufferPackage* package)
+{
+    /* TODO: kdub has a cleanup branch for the surface interface that will make this less ugly */
+    current_buffer.reset();
+    surface->advance_client_buffer();
+    current_buffer = surface->client_buffer();
+
+    auto buffer_package = current_buffer->native_buffer_handle();
+    memcpy(package, buffer_package.get(), sizeof(MirBufferPackage));
+
+    return MIR_MESA_TRUE;
+}
+
+int mgg::InternalNativeSurface::get_parameters_static(
+    MirMesaEGLNativeSurface* surface, MirSurfaceParameters* parameters)
+{
+    auto native_surface = static_cast<mgg::InternalNativeSurface*>(surface);
+    return native_surface->get_parameters(parameters);
+}
+
+int mgg::InternalNativeSurface::get_parameters(MirSurfaceParameters* parameters)
+{
+    auto size = surface->size();
+    parameters->width = size.width.as_uint32_t();
+    parameters->height = size.height.as_uint32_t();
+    parameters->pixel_format = static_cast<MirPixelFormat>(surface->pixel_format());
+    parameters->buffer_usage = mir_buffer_usage_hardware;
+
+    return MIR_MESA_TRUE;
+}
+
+int mgg::InternalNativeSurface::set_swapinterval_static(MirMesaEGLNativeSurface* surface, int interval)
+{
+    //TODO:
+    (void) surface;
+    (void) interval;
+    return MIR_MESA_TRUE;
+}

=== added file 'src/server/graphics/gbm/internal_native_surface.h'
--- src/server/graphics/gbm/internal_native_surface.h	1970-01-01 00:00:00 +0000
+++ src/server/graphics/gbm/internal_native_surface.h	2013-06-18 01:07:24 +0000
@@ -0,0 +1,62 @@
+/*
+ * 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: Kevin DuBois <[e-mail]>
+ */
+
+#ifndef MIR_GRAPHICS_GBM_INTERNAL_NATIVE_SURFACE_H_
+#define MIR_GRAPHICS_GBM_INTERNAL_NATIVE_SURFACE_H_
+
+#include "mir_toolkit/mesa/native_display.h"
+#include <memory>
+
+namespace mir
+{
+namespace frontend
+{
+class Surface;
+}
+namespace compositor
+{
+class Buffer;
+}
+namespace graphics
+{
+namespace gbm
+{
+
+class InternalNativeSurface : public MirMesaEGLNativeSurface
+{
+public:
+    InternalNativeSurface(std::shared_ptr<frontend::Surface> const& surface);
+
+    int advance_buffer(MirBufferPackage* package);
+    int get_parameters(MirSurfaceParameters* parameters);
+
+private:
+    static int set_swapinterval_static(MirMesaEGLNativeSurface* surface, int interval);
+    static int advance_buffer_static(MirMesaEGLNativeSurface* surface,
+                                      MirBufferPackage* package);
+    static int get_parameters_static(MirMesaEGLNativeSurface* surface,
+                                      MirSurfaceParameters* parameters);
+
+    std::shared_ptr<frontend::Surface> surface;
+    std::shared_ptr<compositor::Buffer> current_buffer;
+};
+
+}
+}
+}
+#endif /* MIR_GRAPHICS_GBM_INTERNAL_NATIVE_DISPLAY_H_ */

=== modified file 'tests/unit-tests/client/gbm/CMakeLists.txt'
--- tests/unit-tests/client/gbm/CMakeLists.txt	2013-03-24 23:33:49 +0000
+++ tests/unit-tests/client/gbm/CMakeLists.txt	2013-06-18 01:07:24 +0000
@@ -19,6 +19,7 @@
   ${CMAKE_CURRENT_SOURCE_DIR}/test_gbm_client_buffer.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test_gbm_client_platform.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test_mesa_native_display_container.cpp
+  ${CMAKE_CURRENT_SOURCE_DIR}/test_native_surface.cpp
 )
 
 set(UNIT_TEST_SOURCES ${UNIT_TEST_SOURCES} PARENT_SCOPE)

=== modified file 'tests/unit-tests/client/gbm/test_gbm_client_platform.cpp'
--- tests/unit-tests/client/gbm/test_gbm_client_platform.cpp	2013-05-08 17:52:49 +0000
+++ tests/unit-tests/client/gbm/test_gbm_client_platform.cpp	2013-06-18 01:07:24 +0000
@@ -29,17 +29,6 @@
 namespace mt = mir::test;
 namespace mtd = mir::test::doubles;
 
-TEST(GBMClientPlatformTest, egl_native_window_is_client_surface)
-{
-    mtd::MockClientContext context;
-    mcl::NativeClientPlatformFactory factory;
-    mtd::MockClientSurface surface;
-    auto platform = factory.create_client_platform(&context);
-    auto mock_client_surface = std::make_shared<mtd::MockClientSurface>();
-    auto native_window = platform->create_egl_native_window(&surface);
-    EXPECT_EQ(reinterpret_cast<EGLNativeWindowType>(&surface), *native_window);
-}
-
 /* TODO: mir_egl_mesa_display_is_valid is a bit fragile because libmirserver and libmirclient both have very
  *       different implementations and both have symbols for it. If the linking order of the test changes,
  *       specifically, if mir_egl_mesa_display_is_valid resolves into libmirserver, then this test will break. 

=== added file 'tests/unit-tests/client/gbm/test_native_surface.cpp'
--- tests/unit-tests/client/gbm/test_native_surface.cpp	1970-01-01 00:00:00 +0000
+++ tests/unit-tests/client/gbm/test_native_surface.cpp	2013-06-18 01:07:24 +0000
@@ -0,0 +1,114 @@
+/*
+ * 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: Kevin DuBois <[e-mail]>
+ */
+
+#include "src/client/gbm/gbm_native_surface.h"
+#include "src/client/client_buffer.h"
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+namespace mcl=mir::client;
+namespace mcl=mir::client;
+namespace mclg=mir::client::gbm;
+namespace geom=mir::geometry;
+
+namespace
+{
+
+struct MockClientBuffer : public mcl::ClientBuffer
+{
+    MockClientBuffer()
+    {
+        ON_CALL(*this, native_buffer_handle())
+            .WillByDefault(testing::Return(std::make_shared<MirBufferPackage>()));
+    }
+    ~MockClientBuffer() noexcept {}
+
+    MOCK_METHOD0(secure_for_cpu_write, std::shared_ptr<mcl::MemoryRegion>());
+    MOCK_CONST_METHOD0(size, geom::Size());
+    MOCK_CONST_METHOD0(stride, geom::Stride());
+    MOCK_CONST_METHOD0(pixel_format, geom::PixelFormat());
+
+    MOCK_CONST_METHOD0(age, uint32_t());
+    MOCK_METHOD0(mark_as_submitted, void());
+    MOCK_METHOD0(increment_age, void());
+
+    MOCK_CONST_METHOD0(native_buffer_handle, std::shared_ptr<MirNativeBuffer>());
+};
+
+struct MockMirSurface : public mcl::ClientSurface
+{
+    MockMirSurface(MirSurfaceParameters params)
+     : params(params)
+    {
+        using namespace testing;
+        ON_CALL(*this, get_parameters())
+            .WillByDefault(Return(params));
+        ON_CALL(*this, get_current_buffer())
+            .WillByDefault(Return(
+                std::make_shared<NiceMock<MockClientBuffer>>()));
+    }
+
+    MOCK_CONST_METHOD0(get_parameters, MirSurfaceParameters());
+    MOCK_METHOD0(get_current_buffer, std::shared_ptr<mcl::ClientBuffer>());
+    MOCK_METHOD2(next_buffer, MirWaitHandle*(mir_surface_lifecycle_callback callback, void * context));
+
+    MirSurfaceParameters params;
+};
+}
+
+class GBMInterpreterTest : public ::testing::Test
+{
+public:
+    virtual void SetUp()
+    {
+        using namespace testing;
+        surf_params.width = 530;
+        surf_params.height = 715;
+        surf_params.pixel_format = mir_pixel_format_abgr_8888;
+
+        buffer = std::make_shared<MirBufferPackage>();
+    }
+
+    std::shared_ptr<MirBufferPackage> buffer;
+    MirSurfaceParameters surf_params;
+};
+
+TEST_F(GBMInterpreterTest, basic_parameters)
+{
+    testing::NiceMock<MockMirSurface> mock_surface{surf_params};
+    mclg::GBMNativeSurface interpreter(mock_surface);
+
+    MirSurfaceParameters params;
+    interpreter.surface_get_parameters(&interpreter, &params);
+}
+
+TEST_F(GBMInterpreterTest, basic_advance)
+{
+    using namespace testing;
+    testing::NiceMock<MockMirSurface> mock_surface{surf_params};
+
+    mclg::GBMNativeSurface interpreter(mock_surface);
+
+    EXPECT_CALL(mock_surface, next_buffer(_,_))
+        .Times(1);
+    EXPECT_CALL(mock_surface, get_current_buffer())
+        .Times(1);
+
+    MirBufferPackage buffer_package;
+    interpreter.surface_advance_buffer(&interpreter, &buffer_package);
+}

=== modified file 'tests/unit-tests/graphics/android/test_internal_client_interpreter.cpp'
--- tests/unit-tests/graphics/android/test_internal_client_interpreter.cpp	2013-05-09 18:26:45 +0000
+++ tests/unit-tests/graphics/android/test_internal_client_interpreter.cpp	2013-06-18 01:07:24 +0000
@@ -147,3 +147,9 @@
     auto rc_format = interpreter.driver_requests_info(NATIVE_WINDOW_FORMAT);
     EXPECT_EQ(HAL_PIXEL_FORMAT_RGBA_8888, rc_format); 
 }
+
+TEST_F(InternalClientWindow, window_holds_buffer_resource_for_driver)
+{
+    mga::InternalClientWindow interpreter(mock_surface, mock_cache);
+
+}

=== modified file 'tests/unit-tests/graphics/gbm/CMakeLists.txt'
--- tests/unit-tests/graphics/gbm/CMakeLists.txt	2013-05-08 17:26:17 +0000
+++ tests/unit-tests/graphics/gbm/CMakeLists.txt	2013-06-18 01:07:24 +0000
@@ -10,6 +10,7 @@
   ${CMAKE_CURRENT_SOURCE_DIR}/test_kms_page_flipper.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test_linux_virtual_terminal.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/test_internal_native_display.cpp
+  ${CMAKE_CURRENT_SOURCE_DIR}/test_internal_native_surface.cpp
 )
 
 set(UNIT_TEST_SOURCES ${UNIT_TEST_SOURCES} PARENT_SCOPE)

=== modified file 'tests/unit-tests/graphics/gbm/test_internal_client.cpp'
--- tests/unit-tests/graphics/gbm/test_internal_client.cpp	2013-05-14 16:45:25 +0000
+++ tests/unit-tests/graphics/gbm/test_internal_client.cpp	2013-06-18 01:07:24 +0000
@@ -19,6 +19,7 @@
 #include "mir/frontend/surface.h"
 #include "mir_toolkit/mesa/native_display.h"
 #include "src/server/graphics/gbm/internal_client.h"
+#include "src/server/graphics/gbm/internal_native_surface.h"
 #include "mir_test_doubles/stub_platform.h"
 #include "mir_test_doubles/stub_surface.h"
 
@@ -29,15 +30,24 @@
 namespace mgg=mir::graphics::gbm;
 namespace mtd=mir::test::doubles;
 
-TEST(InternalClient, native_display)
+TEST(InternalClient, native_display_sanity)
 {
-    auto stub_window = std::make_shared<mtd::StubSurface>();
     auto stub_display = std::make_shared<MirMesaEGLNativeDisplay>();
     mgg::InternalClient client(stub_display);
 
     auto native_display = client.egl_native_display();
-    auto native_window = client.egl_native_window(stub_window);
-
     EXPECT_EQ(reinterpret_cast<EGLNativeDisplayType>(stub_display.get()), native_display);
-    EXPECT_EQ(reinterpret_cast<EGLNativeWindowType>(stub_window.get()), native_window);
+}
+
+TEST(InternalClient, native_surface_sanity)
+{
+    auto stub_display = std::make_shared<MirMesaEGLNativeDisplay>();
+    mgg::InternalClient client(stub_display);
+
+    auto stub_window = std::make_shared<mtd::StubSurface>();
+    auto native_window = static_cast<mgg::InternalNativeSurface*>(client.egl_native_window(stub_window));
+
+    ASSERT_NE(nullptr, native_window->surface_advance_buffer);
+    ASSERT_NE(nullptr, native_window->surface_get_parameters);
+    ASSERT_NE(nullptr, native_window->surface_set_swapinterval);
 }

=== modified file 'tests/unit-tests/graphics/gbm/test_internal_native_display.cpp'
--- tests/unit-tests/graphics/gbm/test_internal_native_display.cpp	2013-05-14 22:27:33 +0000
+++ tests/unit-tests/graphics/gbm/test_internal_native_display.cpp	2013-06-18 01:07:24 +0000
@@ -43,10 +43,6 @@
 
 struct InternalNativeDisplay : public testing::Test
 {
-    InternalNativeDisplay()
-    {
-    }
-
     void SetUp()
     {
         using namespace ::testing;
@@ -75,24 +71,6 @@
     }
     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(InternalNativeDisplay, display_get_platform_matches_construction_platform)
@@ -107,84 +85,6 @@
     EXPECT_THAT(test_package, PackageMatches(platform_package));
 }
 
-TEST_F(InternalNativeDisplay, 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());
-    
-    auto test_buffer_package = std::make_shared<MirBufferPackage>();
-    
-    test_buffer_package->data_items = 2;
-    test_buffer_package->data[0] = 1;
-    test_buffer_package->data[1] = 2;
-    test_buffer_package->fd_items = 2;
-    test_buffer_package->fd[0] = 3;
-    test_buffer_package->fd[1] = 4;
-    test_buffer_package->stride = 77;
-
-    mgg::InternalNativeDisplay native_display(platform_package); 
-    
-    EXPECT_CALL(*buffer, native_buffer_handle())
-        .WillOnce(Return(test_buffer_package)); 
-    EXPECT_CALL(surface, client_buffer()).Times(1)
-        .WillOnce(Return(buffer));
- 
-    MirBufferPackage buffer_package;
-    memset(&buffer_package, 0, sizeof(MirBufferPackage));
-    native_display.surface_get_current_buffer(
-        &native_display, static_cast<MirEGLNativeWindowType>(&surface), &buffer_package);
-
-    EXPECT_EQ(test_buffer_package->data_items, buffer_package.data_items);
-    EXPECT_EQ(test_buffer_package->data[0], buffer_package.data[0]);
-    EXPECT_EQ(test_buffer_package->data[1], buffer_package.data[1]);
-    EXPECT_EQ(test_buffer_package->fd_items, buffer_package.fd_items);
-    EXPECT_EQ(test_buffer_package->fd[0], buffer_package.fd[0]);
-    EXPECT_EQ(test_buffer_package->fd[1], buffer_package.fd[1]);
-    EXPECT_EQ(test_buffer_package->stride, buffer_package.stride);
-}
-
-TEST_F(InternalNativeDisplay, surface_advance_buffer)
-{
-    using namespace ::testing;
-    
-    mtd::MockSurface surface(std::make_shared<mtd::StubSurfaceBuilder>());
-    
-    mgg::InternalNativeDisplay native_display(platform_package); 
-    
-    EXPECT_CALL(surface, advance_client_buffer()).Times(1);
-    
-    native_display.surface_advance_buffer(
-        &native_display, static_cast<MirEGLNativeWindowType>(&surface));
-}
-
-TEST_F(InternalNativeDisplay, 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;
-
-    mgg::InternalNativeDisplay native_display(platform_package); 
-
-    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));
-    native_display.surface_get_parameters(
-        &native_display, 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);
-}
-
 TEST(MirServerMesaEGLNativeDisplayInvariants, pixel_format_formats_are_castable)
 {
     EXPECT_EQ(static_cast<MirPixelFormat>(geom::PixelFormat::invalid), mir_pixel_format_invalid);

=== added file 'tests/unit-tests/graphics/gbm/test_internal_native_surface.cpp'
--- tests/unit-tests/graphics/gbm/test_internal_native_surface.cpp	1970-01-01 00:00:00 +0000
+++ tests/unit-tests/graphics/gbm/test_internal_native_surface.cpp	2013-06-18 01:07:24 +0000
@@ -0,0 +1,170 @@
+/*
+ * 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: Kevin DuBois <[e-mail]>
+ */
+
+#include "src/server/graphics/gbm/internal_native_surface.h"
+#include "mir/graphics/platform.h"
+#include "mir/graphics/platform_ipc_package.h"
+#include "mir/frontend/surface.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 mgg = mir::graphics::gbm;
+namespace mc = mir::compositor;
+namespace msh = mir::shell;
+namespace geom = mir::geometry;
+namespace mt = mir::test;
+namespace mtd = mt::doubles;
+
+namespace
+{
+
+
+class MockFrontendSurface : public mir::frontend::Surface
+{
+public:
+    MOCK_METHOD0(destroy, void());
+    MOCK_METHOD0(force_requests_to_complete, void());
+    MOCK_CONST_METHOD0(size, geom::Size());
+    MOCK_CONST_METHOD0(pixel_format, geom::PixelFormat());
+    MOCK_CONST_METHOD0(client_buffer, std::shared_ptr<mc::Buffer>());
+    MOCK_METHOD0(advance_client_buffer, void());
+    MOCK_CONST_METHOD0(supports_input, bool());
+    MOCK_CONST_METHOD0(client_input_fd, int());
+    MOCK_METHOD2(configure, int(MirSurfaceAttrib, int));
+};
+
+struct InternalNativeSurface : public testing::Test
+{
+    void SetUp()
+    {
+        using namespace ::testing;
+        mock_surface = std::make_shared<MockFrontendSurface>();
+    }
+
+    std::shared_ptr<MockFrontendSurface> mock_surface;
+};
+
+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(InternalNativeSurface, surface_advance_buffer_packaging)
+{
+    using namespace ::testing;
+    
+    auto buffer = std::make_shared<mtd::MockBuffer>();
+    
+    auto test_buffer_package = std::make_shared<MirBufferPackage>();    
+    test_buffer_package->data_items = 2;
+    test_buffer_package->data[0] = 1;
+    test_buffer_package->data[1] = 2;
+    test_buffer_package->fd_items = 2;
+    test_buffer_package->fd[0] = 3;
+    test_buffer_package->fd[1] = 4;
+    test_buffer_package->stride = 77;
+
+    mgg::InternalNativeSurface native_surface(mock_surface); 
+    
+    EXPECT_CALL(*buffer, native_buffer_handle())
+        .WillOnce(Return(test_buffer_package)); 
+    EXPECT_CALL(*mock_surface, client_buffer())
+        .Times(1)
+        .WillOnce(Return(buffer));
+ 
+    MirBufferPackage buffer_package;
+    memset(&buffer_package, 0, sizeof(MirBufferPackage));
+    native_surface.surface_advance_buffer(&native_surface, &buffer_package);
+
+    EXPECT_EQ(test_buffer_package->data_items, buffer_package.data_items);
+    EXPECT_EQ(test_buffer_package->data[0], buffer_package.data[0]);
+    EXPECT_EQ(test_buffer_package->data[1], buffer_package.data[1]);
+    EXPECT_EQ(test_buffer_package->fd_items, buffer_package.fd_items);
+    EXPECT_EQ(test_buffer_package->fd[0], buffer_package.fd[0]);
+    EXPECT_EQ(test_buffer_package->fd[1], buffer_package.fd[1]);
+    EXPECT_EQ(test_buffer_package->stride, buffer_package.stride);
+}
+
+TEST_F(InternalNativeSurface, surface_advance_buffer_secures_resource)
+{
+    using namespace ::testing; 
+    mgg::InternalNativeSurface native_surface(mock_surface); 
+    auto stub_buffer1 = std::make_shared<mtd::MockBuffer>();
+    auto stub_buffer2 = std::make_shared<mtd::MockBuffer>();
+
+    auto test_buffer_package = std::make_shared<MirBufferPackage>();    
+    EXPECT_CALL(*stub_buffer1, native_buffer_handle())
+        .WillOnce(Return(test_buffer_package)); 
+    EXPECT_CALL(*stub_buffer2, native_buffer_handle())
+        .WillOnce(Return(test_buffer_package)); 
+    EXPECT_CALL(*mock_surface, client_buffer())
+        .Times(2)
+        .WillOnce(Return(stub_buffer1))
+        .WillOnce(Return(stub_buffer2));
+
+    auto use_count_1 = stub_buffer1.use_count(); 
+    auto use_count_2 = stub_buffer2.use_count();
+    
+    MirBufferPackage buffer_package;
+    native_surface.surface_advance_buffer(&native_surface, &buffer_package);
+    EXPECT_EQ(use_count_1 + 1, stub_buffer1.use_count());
+ 
+    native_surface.surface_advance_buffer(&native_surface, &buffer_package);
+    EXPECT_EQ(use_count_1, stub_buffer1.use_count());
+    EXPECT_EQ(use_count_2 + 1, stub_buffer2.use_count());
+}
+
+TEST_F(InternalNativeSurface, 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;
+    EXPECT_CALL(*mock_surface, size())
+        .Times(1)
+        .WillOnce(Return(test_surface_size));
+    EXPECT_CALL(*mock_surface, pixel_format())
+        .Times(1)
+        .WillOnce(Return(test_pixel_format));
+
+    mgg::InternalNativeSurface native_surface(mock_surface); 
+    
+    MirSurfaceParameters parameters;
+    memset(&parameters, 0, sizeof(MirSurfaceParameters));
+    native_surface.surface_get_parameters(&native_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);
+}