Merge lp:~mir-team/mir/client-api-deprecation into lp:mir

[Alberto Aguirre]

DO NOT MERGE. At least not yet....

These are the client side APIs to deprecate as discussed during the sprint.

[Alberto Aguirre (albaguirre)]

Alexandros pointed out that the getter APIs (such as mir_surface_get_state, mir_surface_get_type, etc) are just as racy as receiving the corresponding events, therefore should be left in.

[Mir CI Bot (mir-ci-bot)]

FAILED: Continuous integration, rev:3788
https://mir-jenkins.ubuntu.com/job/mir-ci/2056/
Executed test runs:
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-mir/2643/console
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/2706
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/2698
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/2698
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/2698
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=yakkety/2672/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial+overlay/2672/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=yakkety/2672/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=android,release=vivid+overlay/2672/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=android,release=vivid+overlay/2672/console
    FAILURE: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial+overlay/2672/console

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

review: Needs Fixing (continuous-integration)

[Brandon Schaefer (brandontschaefer)]

lgtm

review: Approve

[Kevin DuBois (kdub)]

Looks like what we all worked on together, lgtm to me.

I'm ok with merging, as long as some items like:
+ * \deprecated Use "get_platform" extension
get captured in todo's in a document tracking the road to 1.0. (did we have a doc like that?)

Also, might be worth planning how the transition will work... eg, assuming no server breaks are needed for other reasons, something like this makes sense to me:
0.25 gets published
0.26 has all final header versions along with deprecations
0.27 removes deprecations/is the client api/abi break

[Brandon Schaefer (brandontschaefer)]

So we remember as well, need to go through the event api

[Alberto Aguirre (albaguirre)]

On mir_buffer_stream_set_scale and related, what is the alternative?

Are we still waiting on the outcome on this? https://docs.google.com/document/d/1Se4kbUHZNbgB9e3lrr3EQnVnQgSgFThyiBJ0MDgkZ0c/edit#heading=h.bo242kvnuqub

[Alan Griffiths (alan-griffiths)]

> Looks like what we all worked on together, lgtm to me.
>
> I'm ok with merging, as long as some items like:
> + * \deprecated Use "get_platform" extension
> get captured in todo's in a document tracking the road to 1.0. (did we have a
> doc like that?)
>
> Also, might be worth planning how the transition will work... eg, assuming no
> server breaks are needed for other reasons, something like this makes sense to
> me:
> 0.25 gets published
> 0.26 has all final header versions along with deprecations
> 0.27 removes deprecations/is the client api/abi break

Surely that's 1.0?!

[Daniel van Vugt (vanvugt)]

Possibly don't deprecate 'get' functions and suggest events are an alternative. While libmirclient keeps such values cached, the most efficient and most convenient way to get at them is to 'get' from that cache. That avoids every toolkit having to re-implement the same cache for its getters.

Above the toolkit level, apps will sometimes want to get_some_attribute_of(surface) where they are not the surface owner and also don't see its events. So at the app level there is always a use case for needing getter functions in toolkits. And I don't think it's sensible to duplicate all the effort in every toolkit.

review: Needs Fixing

[Daniel van Vugt (vanvugt)]

... unless on a case by case basis it can be shown that deprecating a getter makes it possible to completely remove caching of event values from libmirclient. You still have the duplication issue, but at least libmirclient would become smaller.

[Daniel van Vugt (vanvugt)]

Regardless of the above comments, I would like to reiterate that not everyone wants to delete functions right now. I don't strongly endorse any deletions, and as we saw last week Gerry is also concerned. So whoever wants to delete client functions should also own all the corresponding downstream changes.

There are also changes unrelated to deprecation in this branch, so those could be proposed separately with less contention...

[Daniel van Vugt (vanvugt)]

Actually I'm experiencing the (lack of) getter problem right now and having to implement caching in my app for some attributes that don't have getters (visibility and surface output refresh rate).

Maybe what we need instead of getters is: e = mir_surface_get_last_event(surf, type)
That way all getters can indeed be replaced by the event query functions, but also each toolkit/app then doesn't have to duplicate too much code to write a getter for their own API.

[Alberto Aguirre (albaguirre)]

> Regardless of the above comments, I would like to reiterate that not everyone
> wants to delete functions right now.
That's why we will deprecate things first before deleting in 1.0

> I don't strongly endorse any deletions,
> and as we saw last week Gerry is also concerned.

Just confirmed with Gerry that deprecating first is ok as long as corresponding new apis that cover similar functionality are in place so the transition can be made.

>So whoever wants to delete
> client functions should also own all the corresponding downstream changes.
>
We have historically done so, I don't see why this time would be any different.

[Andreas Pokorny (andreas-pokorny)]

Hm from gdk/gtks perspective those removals are not problematic... I think some of the comments could point to the attribute change events..

review: Approve

[Gerry Boland (gerboland)]

> Regardless of the above comments, I would like to reiterate that not everyone
> wants to delete functions right now. I don't strongly endorse any deletions,
> and as we saw last week Gerry is also concerned. So whoever wants to delete
> client functions should also own all the corresponding downstream changes.

To clarify my perspective, I don't mind deprecations as long as I've had time to react to them - which for me means there is a release where the old api is deprecated but not removed, and the new API ready for use. As long as new api is a superset of the old api and gives me similar behaviour, I am not too concerned. If I find that is not the case, then we at least will have time to figure it out.

[Andreas Pokorny (andreas-pokorny)]

^ But will take a closer look with the current gtk+ ..

review: Abstain

[Daniel van Vugt (vanvugt)]

OK, I abstain on the whole deprecation bit.

Just want to see the new bool return value and chrome changes proposed separately. They're unrelated to deprecation really. And the first one requires further logic changes anyway:
  bool mir_buffer_stream_get_graphics_region

review: Needs Fixing

[Daniel van Vugt (vanvugt)]

review: Abstain

[Daniel van Vugt (vanvugt)]

review: Needs Fixing

[Daniel van Vugt (vanvugt)]

Is this branch now superseded by Brandon's?
https://code.launchpad.net/~brandontschaefer/mir/first-round-deprecation/+merge/310024

[Cemil Azizoglu (cemil-azizoglu)]

79 + * \note TODO: Cleanup remove callback?

Yes we should.
----------------------

+ *
139 + * \note May need deprecation - Need a consistent API for high DPI scaling/destination size
140 + *
141 * \param [in] buffer_stream The buffer stream
142 * \param [in] scale The scale
143 - * \return A handle that can be passed to mir_wait_for
144 + * \return A handle that can be passed to mir_wait_for
145 */
146 MirWaitHandle *mir_buffer_stream_set_scale(MirBufferStream* buffer_stream, float scale);

At least the wait handle needs to be done away with.
----------------------

479 + * \note There is no implicit buffer stream creation. See mir_xxx to attach content.

You mean the "default" buffer stream.

Also, it should read "mir_surface_spec_add_render_surface() to attach content".
----------------------

488 + * \deprecated No replacement. This property is now part of the backing content
...

503 + * \deprecated No replacement. This property is now part of the backing content

More precisely, there is no default stream backing the surface.
----------------------

[Brandon Schaefer (brandontschaefer)]

This branch is a TODO pretty much of the talk we had during the sprint. Soo more of a working/branch of document i support.

[Daniel van Vugt (vanvugt)]

Sounds like status should not be "Needs review" then.

Look to reviewing this one instead:
https://code.launchpad.net/~brandontschaefer/mir/first-round-deprecation/+merge/310024

[Daniel van Vugt (vanvugt)]

Alright, firstly Brandon's branch mentioned above replaced this one.

Secondly, he/we are in the process of proposing the various changes separately as smaller proposals, thus replacing Brandon's first-round-deprecation.

review: Disapprove

[Daniel van Vugt (vanvugt)]

I'm going to reject this because it's just a notepad. We can still refer to the notes even after it's rejected.

Unmerged revisions

3788. By Alberto Aguirre on 2016-10-26

Start deprecation of client side APIs

=== modified file 'include/client/mir_toolkit/debug/surface.h'
--- include/client/mir_toolkit/debug/surface.h	2015-02-22 07:46:25 +0000
+++ include/client/mir_toolkit/debug/surface.h	2016-10-26 15:43:25 +0000
@@ -36,16 +36,23 @@
 
 /**
  * Get the ID of the surface's current buffer (only useful for debug purposes)
+ *
+ * \deprecated Not useful anymore
+ *
  *   \pre                         The surface is valid
  *   \param   [in] surface        The surface
  *   \return                      The internal buffer ID of the surface's current buffer.
  *                                This is the buffer that is currently being drawn to,
  *                                and would be returned by mir_surface_get_current_buffer.
  */
+[[deprecated]]
 uint32_t mir_debug_surface_current_buffer_id(MirSurface *surface);
 
 /**
  * Get the screen coordinates corresponding to a pair of surface coordinates
+ *
+ * \deprecated Use the "debug_surface_coords" extension instead
+ *
  * \pre                               The surface is valid
  * \param   [in] surface              The surface
  * \param   [in] x, y                 Surface coordinates to map to screen coordinates
@@ -59,6 +66,7 @@
  *          This call will only be interesting for automated testing, where both the client
  *          and shell state is known and constrained.
  */
+[[deprecated]]
 bool mir_debug_surface_coords_to_screen(MirSurface* surface,
                                         int x, int y,
                                         int* screen_x, int* screen_y);

=== modified file 'include/client/mir_toolkit/mir_buffer_stream.h'
--- include/client/mir_toolkit/mir_buffer_stream.h	2016-10-12 06:03:15 +0000
+++ include/client/mir_toolkit/mir_buffer_stream.h	2016-10-26 15:43:25 +0000
@@ -52,6 +52,8 @@
 /**
  * Create a new buffer stream. 
  *
+ * \deprecated See mir_render_surface_create_buffer_stream
+ *
  * For example, the resulting buffer stream may be used
  * with mir_cursor_configuration_from_buffer_stream, 
  * in order to post images to the system cursor.
@@ -70,6 +72,7 @@
  *
  * \return                    A handle that can be supplied to mir_wait_for
  */
+[[deprecated]]
 MirWaitHandle* mir_connection_create_buffer_stream(MirConnection *connection,
     int width, int height,
     MirPixelFormat format,
@@ -83,6 +86,8 @@
  * mir_cursor_configuration_from_buffer_stream in order to post images 
  * to the system cursor.
  *
+ * \deprecated See mir_render_surface_create_buffer_stream_sync
+ *
  * \param [in] connection       A valid connection
  * \param [in] width            Requested buffer width
  * \param [in] height           Requested buffer height
@@ -93,6 +98,7 @@
  * \return                      The new buffer stream. This is guaranteed non-null, 
  *                              but may be invalid in the case of error.
  */
+[[deprecated]]
 MirBufferStream* mir_connection_create_buffer_stream_sync(MirConnection *connection,
     int width, int height,
     MirPixelFormat format,
@@ -100,6 +106,9 @@
 
 /**
  * Release the supplied stream and any associated buffer. The returned wait
+ *
+ * \note TODO: Cleanup remove callback?
+ *
  * handle remains valid until the connection to the server is released.
  *   \warning callback could be called from another thread. You must do any
  *            locking appropriate to protect your data accessed in the
@@ -140,17 +149,19 @@
  *   \return                  One of mir_platform_type_android or 
  *                            mir_platform_type_gbm
  */
-/// @cond
-__attribute__ ((deprecated))
-/// @endcond
+[[deprecated]]
 MirPlatformType mir_buffer_stream_get_platform_type(MirBufferStream *stream);
 
 /**
  * Retrieve the current buffer in "raw" representation.
+ *
+ * \deprecated Replaced by a platform specific extension
+ *
  *   \pre                         The buffer stream is valid
  *   \param [in]  buffer_stream   The buffer stream
  *   \param [out] buffer_package  Structure to be populated
  */
+[[deprecated]]
 void mir_buffer_stream_get_current_buffer(MirBufferStream *buffer_stream,
     MirNativeBuffer **buffer_package);
 
@@ -181,35 +192,47 @@
 
 /**
  * Retrieve a buffer stream's graphics region
+ *
  *   \warning Depending on platform, this can map the graphics buffer each
  *            time its called. The region remains mapped until
  *            mir_buffer_stream_swap_buffers().
  *   \pre                          The buffer stream is valid
  *   \param [in] buffer_stream     The buffer stream
  *   \param [out] graphics_region  Structure to be populated
+ *   \return                       True if the region is valid, false otherwise.
  */
-void mir_buffer_stream_get_graphics_region(
+bool mir_buffer_stream_get_graphics_region(
     MirBufferStream *buffer_stream,
     MirGraphicsRegion *graphics_region);
 
 /**
  * Retrieve a window type which may be used by EGL.
+ *
+ * \deprecated No replacement, use MirRenderSurface * directly with EGL
+ *
  *   \param [in] buffer_stream The buffer stream
  *   \return                   An EGLNativeWindowType that the client can use
  */
+[[deprecated]]
 MirEGLNativeWindowType mir_buffer_stream_get_egl_native_window(MirBufferStream *buffer_stream);
 
 /**
  * Set the scale associated with all buffers in the stream
+ *
+ * \note May need deprecation - Need a consistent API for high DPI scaling/destination size
+ *
  * \param [in] buffer_stream The buffer stream
  * \param [in] scale         The scale
- * \return                  A handle that can be passed to mir_wait_for
+ * \return                   A handle that can be passed to mir_wait_for
  */
 MirWaitHandle *mir_buffer_stream_set_scale(MirBufferStream* buffer_stream, float scale);
 
 /**
  * Set the scale as in mir_buffer_stream_set_scale(), but also wait for the
  * operation to complete.
+ *
+ * \note May need deprecation - Need a consistent API for high DPI scaling/destination size
+ *
  * \param [in] buffer_stream The buffer stream
  * \param [in] scale         The scale
  */

=== modified file 'include/client/mir_toolkit/mir_connection.h'
--- include/client/mir_toolkit/mir_connection.h	2016-10-12 06:03:15 +0000
+++ include/client/mir_toolkit/mir_connection.h	2016-10-26 15:43:25 +0000
@@ -51,7 +51,7 @@
     char const *server,
     char const *app_name,
     mir_connected_callback callback,
-    void *context);
+    void *context); 
 
 /**
  * Perform a mir_connect() but also wait for and return the result.
@@ -89,20 +89,26 @@
 /**
  * Query platform-specific data and/or file descriptors that are required to
  * initialize GL/EGL features.
+ *
+ * \deprecated Use "get_platform" extension
+ *
  *   \param [in]  connection        The connection
  *   \param [out] platform_package  Structure to be populated
  */
+[[deprecated]]
 void mir_connection_get_platform(MirConnection *connection, MirPlatformPackage *platform_package);
 
 /**
  * Query graphics platform module.
  *
+ * \deprecated Use "get_graphics_module" extension
  * \note The char pointers in MirModuleProperties are owned by the connection and should not be
  * freed. They remain valid until the connection is released.
  *
  *   \param [in]  connection    The connection
  *   \param [out] properties    Structure to be populated
  */
+[[deprecated]]
 void mir_connection_get_graphics_module(MirConnection *connection, MirModuleProperties *properties);
 
 /**
@@ -146,13 +152,14 @@
 /**
  * Query the display
  *
- *   \deprecated  Use mir_connection_create_display_configuration() instead.
+ *   \deprecated  See mir_connection_create_display_configuration()
  *
  *   \warning return value must be destroyed via mir_display_config_destroy()
  *   \warning may return null if connection is invalid
  *   \param [in]  connection        The connection
  *   \return                        structure that describes the display configuration
  */
+[[deprecated("Use mir_connection_create_display_configuration")]]
 MirDisplayConfiguration* mir_connection_create_display_config(MirConnection *connection);
 
 /**
@@ -189,6 +196,8 @@
 /**
  * Apply the display configuration
  *
+ * \deprecated Replace with mir_connection_apply_display_configuration(...)
+ *
  * The display configuration is applied to this connection only (per-connection
  * configuration) and is invalidated when a hardware change occurs. Clients should
  * register a callback with mir_connection_set_display_config_change_callback()
@@ -199,11 +208,14 @@
  *   \param [in] display_configuration  The display_configuration to apply
  *   \return                            A handle that can be passed to mir_wait_for
  */
+[[deprecated]]
 MirWaitHandle* mir_connection_apply_display_config(MirConnection *connection, MirDisplayConfiguration* display_configuration);
 
 /**
  * Set the base display configuration
  *
+ * \deprecated Use mir_connection_preview_base_display_configuration/mir_connection_confirm_base_display_configuration
+ *
  * The base display configuration is the configuration the server applies when
  * there is no active per-connection configuration.
  *
@@ -222,6 +234,7 @@
  *   \param [in] display_configuration  The display_configuration to set as base
  *   \return                            A handle that can be passed to mir_wait_for
  */
+[[deprecated]]
 MirWaitHandle* mir_connection_set_base_display_config(
     MirConnection* connection,
     MirDisplayConfiguration const* display_configuration);
@@ -298,6 +311,9 @@
 
 /**
  * Get a display type that can be used for OpenGL ES 2.0 acceleration.
+ *
+ * \deprecated No replacement. Use MirConnection * directly with EGL
+ *
  *   \param [in] connection  The connection
  *   \return                 An EGLNativeDisplayType that the client can use
  */
@@ -306,11 +322,15 @@
 /**
  * Get the exact MirPixelFormat to use in creating a surface for a chosen
  * EGLConfig.
+ *
+ * \deprecated No replacement. Use EGL configs directly.
+ *
  *   \param [in] connection  The connection
  *   \param [in] egldisplay  The EGLDisplay for the given config
  *   \param [in] eglconfig   The EGLConfig you have chosen to use
  *   \return                 The MirPixelFormat to use in surface creation
  */
+[[deprecated]]
 MirPixelFormat mir_connection_get_egl_pixel_format(
     MirConnection *connection, void *egldisplay, void *eglconfig);
 
@@ -336,6 +356,8 @@
 /**
  * Perform a platform specific operation.
  *
+ * \deprecated Use the platform extensions API instead
+ *
  * The MirPlatformMessage used for the request needs to remain valid
  * until this operation finishes.
  *
@@ -345,6 +367,7 @@
  * \param [in,out] context User data passed to the callback function
  * \return                 A handle that can be passed to mir_wait_for
  */
+[[deprecated]]
 MirWaitHandle* mir_connection_platform_operation(
     MirConnection* connection,
     MirPlatformMessage const* request,

=== modified file 'include/client/mir_toolkit/mir_cursor_configuration.h'
--- include/client/mir_toolkit/mir_cursor_configuration.h	2016-09-08 02:58:56 +0000
+++ include/client/mir_toolkit/mir_cursor_configuration.h	2016-10-26 15:43:25 +0000
@@ -57,6 +57,9 @@
  * Returns a new cursor configuration tied to a given buffer stream.
  * If the configuration is successfully applied buffers from the stream will be used 
  * to fill the system cursor.
+ *
+ * \note Replace MirBufferStream with MirRenderSurface
+ *
  *    \param [in] stream      The buffer stream
  *    \param [in] hotspot_x The x-coordinate to use as the cursor's hotspot.
  *    \param [in] hotspot_y The y-coordinate to use as the cursor's hotspot.

=== modified file 'include/client/mir_toolkit/mir_display_configuration.h'
--- include/client/mir_toolkit/mir_display_configuration.h	2016-10-20 15:07:39 +0000
+++ include/client/mir_toolkit/mir_display_configuration.h	2016-10-26 15:43:25 +0000
@@ -50,6 +50,8 @@
  * Get the maximum possible number of simultaneously active outputs this system
  * supports.
  *
+ *  No replacement. API is not useful.
+ *
  * \note There may be restrictions on the configuration required to achieve this
  * many active outputs. Typically the achievable number of simultaneously active
  * outputs is lower than this number.
@@ -58,6 +60,7 @@
  * \returns      The maximum number of simultaneously active outputs
  *               supportable at this time.
  */
+[[deprecated]]
 int mir_display_config_get_max_simultaneous_outputs(
     MirDisplayConfig const* config);
 
@@ -263,9 +266,12 @@
 /**
  * Get the textual name of an output type.
  *
+ * \deprecated See mir_output_type_name
+ *
  * \param [in]  type  The MirDisplayOutputType to describe.
  * \returns           The name of the output type.
  */
+[[deprecated]]
 char const* mir_display_output_type_name(MirDisplayOutputType type);
 
 /**

=== modified file 'include/client/mir_toolkit/mir_platform_message.h'
--- include/client/mir_toolkit/mir_platform_message.h	2014-12-10 11:02:04 +0000
+++ include/client/mir_toolkit/mir_platform_message.h	2016-10-26 15:43:25 +0000
@@ -48,16 +48,22 @@
  * Each call to mir_platform_message_create() should be matched by
  * a call to mir_platform_message_release() to avoid memory leaks.
  *
+ * \deprecated Use platform extensions API
+ *
  *   \param [in] opcode    The platform message opcode
  *   \return               The created MirPlatformMessage
  */
+[[deprecated]]
 MirPlatformMessage* mir_platform_message_create(unsigned int opcode);
 
 /**
  * Release a platform message.
  *
+ * \deprecated Use platform extensions API
+ *
  *   \param [in] message   The MirPlatformMessage
  */
+[[deprecated]]
 void mir_platform_message_release(MirPlatformMessage const* message);
 
 /**
@@ -65,10 +71,13 @@
  *
  * The data is copied into the message.
  *
+ * \deprecated Use platform extensions API
+ *
  *   \param [in] message    The MirPlatformMessage
  *   \param [in] data       Pointer to the data
  *   \param [in] data_size  The size of the data in bytes
  */
+[[deprecated]]
 void mir_platform_message_set_data(MirPlatformMessage* message, void const* data, size_t data_size);
 
 /**
@@ -82,18 +91,24 @@
  * is released. The caller is responsible for closing the fds when the message
  * doesn't need them anymore (see also mir_platform_message_get_fds()).
  *
+ * \deprecated Use platform extensions API
+ *
  *   \param [in] message   The MirPlatformMessage
  *   \param [in] fds       Pointer to the array of fds
  *   \param [in] num_fds   The number of fds
  */
+[[deprecated]]
 void mir_platform_message_set_fds(MirPlatformMessage* message, int const* fds, size_t num_fds);
 
 /**
  * Get the opcode of a message.
  *
+ * \deprecated Use platform extensions API
+ *
  *   \param [in] message   The MirPlatformMessage
  *   \return               The opcode
  */
+[[deprecated]]
 unsigned int mir_platform_message_get_opcode(MirPlatformMessage const* message);
 
 /**
@@ -103,9 +118,11 @@
  * valid only as long as the message is valid and mir_platform_set_data() is
  * not called. You must not change or free the returned data array.
  *
+ * \deprecated Use platform extensions API
  *   \param [in] message   The MirPlatformMessage
  *   \return               The data
  */
+[[deprecated]]
 MirPlatformMessageData mir_platform_message_get_data(MirPlatformMessage const* message);
 
 /**
@@ -119,9 +136,11 @@
  * message is released. Users are responsible for getting and closing the
  * fds to avoid leaks.
  *
+ * \deprecated Use platform extensions API
  *   \param [in] message   The MirPlatformMessage
  *   \return               The fds
  */
+[[deprecated]]
 MirPlatformMessageFds mir_platform_message_get_fds(MirPlatformMessage const* message);
 
 #ifdef __cplusplus

=== modified file 'include/client/mir_toolkit/mir_screencast.h'
--- include/client/mir_toolkit/mir_screencast.h	2016-07-18 07:38:38 +0000
+++ include/client/mir_toolkit/mir_screencast.h	2016-10-26 15:43:25 +0000
@@ -89,10 +89,13 @@
 /**
  * Set the requested number of buffers to use.
  *
+ * \deprecated See mir_screencast_capture_to_buffer
+ *
  * \param [in] spec     Specification to mutate
  * \param [in] nbuffers The number of buffers to allocate for screencasting
  *
  */
+[[deprecated]]
 void mir_screencast_spec_set_number_of_buffers(MirScreencastSpec* spec, unsigned int nbuffers);
 
 /**
@@ -135,11 +138,14 @@
  *
  * A screencast allows clients to read the contents of the screen.
  *
+ * \deprecated See mir_screencast_create_sync
+ *
  *   \warning This request may be denied.
  *   \param [in] connection  The connection
  *   \param [in] parameters  The screencast parameters
  *   \return                 The resulting screencast
  */
+[[deprecated]]
 MirScreencast* mir_connection_create_screencast_sync(
     MirConnection* connection,
     MirScreencastParameters* parameters);
@@ -155,8 +161,11 @@
  * Retrieve the MirBufferStream associated with a screencast 
  * (to advance buffers, obtain EGLNativeWindowType, etc...)
  * 
+ * \deprecated See mir_screencast_capture_to_buffer
+ *
  *   \param[in] screencast The screencast
  */
+[[deprecated]]
 MirBufferStream* mir_screencast_get_buffer_stream(MirScreencast* screencast);
 
 #ifdef __cplusplus

=== modified file 'include/client/mir_toolkit/mir_surface.h'
--- include/client/mir_toolkit/mir_surface.h	2016-10-12 06:03:15 +0000
+++ include/client/mir_toolkit/mir_surface.h	2016-10-26 15:43:25 +0000
@@ -232,6 +232,7 @@
 /**
  * Create a surface from a given specification
  *
+ * \note There is no implicit buffer stream creation. See mir_xxx to attach content.
  *
  * \param [in] requested_specification  Specification of the attributes for the created surface
  * \param [in] callback                 Callback function to be invoked when creation is complete
@@ -411,6 +412,9 @@
 
 /**
  * Set the requested pixel format.
+ *
+ * \deprecated No replacement. This property is now part of the backing content
+ *
  * \param [in] spec     Specification to mutate
  * \param [in] format   Requested pixel format
  *
@@ -418,10 +422,14 @@
  *          If the server is unable to create a surface with this pixel format at
  *          the point mir_surface_create() is called it will instead return an invalid surface.
  */
+[[deprecated]]
 void mir_surface_spec_set_pixel_format(MirSurfaceSpec* spec, MirPixelFormat format);
 
 /**
  * Set the requested buffer usage.
+ *
+ * \deprecated No replacement. This property is now part of the backing content
+ *
  * \param [in] spec     Specification to mutate
  * \param [in] usage    Requested buffer usage
  *
@@ -429,13 +437,15 @@
  *          If the server is unable to create a surface with this buffer usage at
  *          the point mir_surface_create() is called it will instead return an invalid surface.
  */
+[[deprecated]]
 void mir_surface_spec_set_buffer_usage(MirSurfaceSpec* spec, MirBufferUsage usage);
 
 /**
  * \param [in] spec         Specification to mutate
  * \param [in] output_id    ID of output to place surface on. From MirDisplayOutput.output_id
  *
- * \note    If this call returns %true then a valid surface returned from mir_surface_create() is
+ * \note   Update comment below
+ * \note    If this call returns %t then a valid surface returned from mir_surface_create() is
  *          guaranteed to be fullscreen on the specified output. An invalid surface is returned
  *          if the server unable to, or policy prevents it from, honouring this request.
  */
@@ -501,6 +511,8 @@
  * but is in the list will be associated with the surface.
  * Streams set a displacement from the top-left corner of the surface.
  * 
+ * \deprecated Replaced by mir_surface_set_render_surfaces
+ *
  * \warning disassociating streams from the surface will not release() them.
  * \warning It is wiser to arrange the streams within the bounds of the
  *          surface the spec is applied to. Shells can define their own
@@ -510,6 +522,7 @@
  * \param [in] streams     An array of non-null streams info.
  * \param [in] num_streams The number of elements in the streams array.
  */
+[[deprecated]]
 void mir_surface_spec_set_streams(MirSurfaceSpec* spec,
                                   MirBufferStreamInfo* streams,
                                   unsigned int num_streams);
@@ -600,6 +613,9 @@
 
 /**
  * Set the event handler to be called when events arrive for a surface.
+ *
+ * \deprecated Use mir_surface_spec_set_event_handler instead.
+ *
  *   \warning event_handler could be called from another thread. You must do
  *            any locking appropriate to protect your data accessed in the
  *            callback. There is also a chance that different events will be
@@ -609,6 +625,7 @@
  *   \param [in] callback       The callback function
  *   \param [in] context        Additional argument to be passed to callback
  */
+[[deprecated("Use mir_surface_spec_set_event_handler")]]
 void mir_surface_set_event_handler(MirSurface *surface,
                                    mir_surface_event_callback callback,
                                    void* context);
@@ -626,6 +643,7 @@
  *            be removed, and this function will return NULL.
  *   \param[in] surface The surface
  */
+[[deprecated]]
 MirBufferStream* mir_surface_get_buffer_stream(MirSurface *surface);
 
 /**
@@ -649,10 +667,14 @@
 
 /**
  * Get a surface's parameters.
+ *
+ * \deprecated MirSurfaceParameters is obsolete.
+ *
  *   \pre                     The surface is valid
  *   \param [in] surface      The surface
  *   \param [out] parameters  Structure to be populated
  */
+[[deprecated]]
 void mir_surface_get_parameters(MirSurface *surface, MirSurfaceParameters *parameters);
 
 /**
@@ -681,29 +703,44 @@
 
 /**
  * Get the type (purpose) of a surface.
+ *
+ * \deprecated ? Mostly a convenience function. Look for MirSurfaceType events instead?
+ *
  *   \param [in] surface  The surface to query
  *   \return              The type of the surface
  */
+[[deprecated]]
 MirSurfaceType mir_surface_get_type(MirSurface *surface);
 
 /**
  * Change the state of a surface.
+ *
+ * \deprecated Use mir_surface_apply_spec instead
+ *
  *   \param [in] surface  The surface to operate on
  *   \param [in] state    The new state of the surface
  *   \return              A wait handle that can be passed to mir_wait_for
  */
+[[deprecated("See mir_surface_apply_spec")]]
 MirWaitHandle* mir_surface_set_state(MirSurface *surface,
                                      MirSurfaceState state);
 
 /**
  * Get the current state of a surface.
+ *
+ * \deprecated ? Mostly a convenience function. Look for MirSurfaceState events instead?
+ *
  *   \param [in] surface  The surface to query
  *   \return              The state of the surface
  */
+[[deprecated]]
 MirSurfaceState mir_surface_get_state(MirSurface *surface);
 
 /**
  * Set the swapinterval for the default stream.
+ *
+ * \deprecated No replacement. This is not a property of the surface anymore.
+ *
  *   \warning EGL users should use eglSwapInterval directly.
  *   \warning Only swapinterval of 0 or 1 is supported.
  *   \warning If the surface was created with, or modified to have a
@@ -716,27 +753,39 @@
  *   \return              A wait handle that can be passed to mir_wait_for,
  *                        or NULL if the interval could not be supported
  */
+[[deprecated]]
 MirWaitHandle* mir_surface_set_swapinterval(MirSurface* surface, int interval);
 
 /**
  * Query the swapinterval that the surface is operating with.
  * The default interval is 1.
+ *
+ * \deprecated No replacement. This is not a property of the surface anymore.
+ *
  *   \param [in] surface  The surface to operate on
  *   \return              The swapinterval value that the client is operating with.
  *                        Returns -1 if surface is invalid, or if the default stream
  *                        was removed by use of mir_surface_spec_set_streams().
  */
+[[deprecated]]
 int mir_surface_get_swapinterval(MirSurface* surface);
 
 /**
  * Query the DPI value of the surface (dots per inch). This will vary depending
  * on the physical display configuration and where the surface is within it.
+ *
+ * \deprecated ? Mostly a convenience function - Use dpi events instead?
+ *
  *   \return  The DPI of the surface, or zero if unknown.
  */
+[[deprecated]]
 int mir_surface_get_dpi(MirSurface* surface);
     
 /**
  * Query the focus state for a surface.
+ *
+ * \deprecated
+ *
  *   \param [in] surface The surface to operate on
  *   \return             The focus state of said surface
  */
@@ -744,6 +793,9 @@
 
 /**
  * Query the visibility state for a surface.
+ *
+ * \deprecated
+ *
  *   \param [in] surface The surface to operate on
  *   \return             The visibility state of said surface
  */
@@ -752,6 +804,9 @@
 /**
  * Choose the cursor state for a surface: whether a cursor is shown, 
  * and which cursor if so.
+ *
+ * \deprecated Use mir_surface_apply_spec instead
+ *
  *    \param [in] surface    The surface to operate on
  *    \param [in] parameters The configuration parameters obtained
  *                           from mir_cursor* family of functions.
@@ -759,30 +814,43 @@
  *                           or NULL if parameters is invalid.
  *
  */
+[[deprecated("See mir_surface_apply_spec")]]
 MirWaitHandle* mir_surface_configure_cursor(MirSurface *surface, MirCursorConfiguration const* parameters);
 
 /**
  * Get the orientation of a surface.
+ *
+ * \deprecated ? Mostly a convenience function - Use dpi events instead?
+ *
  *   \param [in] surface  The surface to query
  *   \return              The orientation of the surface
  */
+[[deprecated]]
 MirOrientation mir_surface_get_orientation(MirSurface *surface);
 
 /**
  * Request to set the preferred orientations of a surface.
  * The request may be rejected by the server; to check wait on the
  * result and check the applied value using mir_surface_get_preferred_orientation
+ *
+ * \deprecated Use mir_surface_apply_spec instead
+ *
  *   \param [in] surface     The surface to operate on
  *   \param [in] orientation The preferred orientation modes
  *   \return                 A wait handle that can be passed to mir_wait_for
  */
+[[deprecated("See mir_surface_apply_spec")]]
 MirWaitHandle* mir_surface_set_preferred_orientation(MirSurface *surface, MirOrientationMode orientation);
 
 /**
  * Get the preferred orientation modes of a surface.
+ *
+ * \deprecated No alternative as client should already know what the preferred orientation the surface is set to.
+ *
  *   \param [in] surface  The surface to query
  *   \return              The preferred orientation modes
  */
+[[deprecated]]
 MirOrientationMode mir_surface_get_preferred_orientation(MirSurface *surface);
 
 /**

=== modified file 'include/core/mir_toolkit/common.h'
--- include/core/mir_toolkit/common.h	2016-10-20 15:05:56 +0000
+++ include/core/mir_toolkit/common.h	2016-10-26 15:43:25 +0000
@@ -349,6 +349,7 @@
 {
     mir_shell_chrome_normal,
     mir_shell_chrome_low,
+    mir_shell_chrome_none
 } MirShellChrome;
 
 /**