Merge lp:~andreas-pokorny/mir/allow-transparent-server-buffers into lp:mir

[Andreas Pokorny]

Adds an interface in ServerConfiguration to select the MirPixelFormat of the buffers(s) the server allocates.
So far this is only limited to nested servers.

I think I should also put that into the offscreen platform, which currently defaults to RGBA.
Not sure for mesa and android.

And of course I would like to have hints on the interface ..

[Alan Griffiths (alan-griffiths)]

5 add_executable(mir_demo_server_shell
6 demo_shell.cpp
7 + fullscreen_placement_strategy.h
8 fullscreen_placement_strategy.cpp
9 + translucent_outputs.h
10 + translucent_outputs.cpp
11 + window_manager.h
12 window_manager.cpp
13 + ../server_configuration.h
14 ../server_configuration.cpp
15 )

Don't add header files to the sources.

~~~~

86 +MirPixelFormat TranslucentOutputs::get_pixel_format(pixel_format_array const& availableFormats)

Naming convention: availableFormats => available_formats

~~~~

196 + typedef std::vector<MirPixelFormat> pixel_format_array;

Naming convention: pixel_format_array => PixelFormatArray (PixelFormats is probably better)

~~~~

88 + for(auto const& f : availableFormats )

Whitespace. Vis:
    for (auto const& f : availableFormats)

~~~~

It seems odd to be adding OutputConfiguration as a parameter to create_display() when it is only relevant to NestedPlatform. Why not make it a dependency of NestedPlatform and supply it to the constructor?

~~~~

198 + virtual MirPixelFormat get_pixel_format(pixel_format_array const& availableFormats) = 0;

"get_" lacks semantic content. Perhaps "choose_"?

review: Needs Fixing

[Alan Griffiths (alan-griffiths)]

Seeing OutputConfiguration passed around next to DisplayConfigurationPolicy makes me wonder if there is a missing abstraction that covers both.

[Andreas Pokorny (andreas-pokorny)]

I think my curly braces placement is wrong too? In:

88 + for (auto const& f : availableFormats )
89 + {
90 + if (f == mir_pixel_format_abgr_8888 ||
91 + f == mir_pixel_format_argb_8888)
92 + {

Yes didnt like that passing around either, but I thought a pixel format configuration would also be reasonable to some degree for non nested platforms. We could turn that into a list of DisplayConfigurationPolicies (for convenience) and extend the DisplayConfiguration interface to be able to also affect the preferred PixelFormat on the Output?

I added those header files there since IDEs that support cmake see those header files that are part of the project, and shows them, and cmake can separate between things to compile and other stuff.

[Alexandros Frantzis (afrantzis)]

Choosing the pixel format of each output's framebuffer is part of the display (re)configuration process, so I think that it conceptually belongs in DisplayConfigurationPolicy. However, I don't think that any changes to the policy interface are actually needed. We only need to allow the policy to set the format, by extending DisplayConfiguration::configure_output() to accept a 'format_index' parameter (note that DisplayConfigurationOutput already contains a 'pixel_formats' and 'current_format_index' fields).

review: Needs Fixing

[Daniel van Vugt (vanvugt)]

pixel_format_utils.cpp could eventually be replaced by the proposal in bug 1236254. But they're not conflicting ideas so that's OK.

I haven't followed this through yet, but please read the existing discussion --> bug 1256702
The important point is to ensure that translucency is optional, _never_ used on a real shell, and only used in temporary things like greeters.

[Daniel van Vugt (vanvugt)]

1. Wrapping: Please wrap lines earlier, ideally before column 80. The human brain loses track of the next line to read otherwise...
49 +CascadedDisplayConfigurationPolicy::CascadedDisplayConfigurationPolicy(std::shared_ptr<graphics::DisplayConfigurationPolicy> const& l,
50 + std::shared_ptr<graphics::DisplayConfigurationPolicy> const& r)
Although the class name seems excessively long too. Not sure what to do about that.

2. I think guard macros are meant to follow the namespace?.. So:
   MIR_DEMO_SHELL_CASCADED_DISPLAY_CONFIGURATION_POLICY_H
becomes:
   MIR_EXAMPLES_CASCADED_DISPLAY_CONFIGURATION_POLICY_H_

3. It's possibly too easy to enable translucency. That's an unusual problem, but it's absolutely critical to performance that no shell implementer accidentally uses translucency. It should be clearly marked as "for greeter use only", somehow.

4. Per #3, please remove ALL translucency support from demo-shell. We don't want people copying demo-shell thinking it's a good idea to use translucency on shells when it will cripple performance. Perhaps create a "demo-greeter" based on examples/basic_server.cpp instead?

5. "if(" should have a space; "if ("

6. Please remove spaces: "mg::red_channel_depth( format )"

7. Half of this diff is not yours. It seems to have come from raof's "surfaceless" branch. Please resubmit this branch listing that one as the prerequisite. Then his work won't appear in your MP diff, and it will be half the size to review.

8. Tests for your new logic (other than pixel format utils); is it just
 tests/unit-tests/graphics/nested/test_nested_display_configuration.cpp
?

This isn't an exhaustive review, but I've written enough for now :)

review: Needs Fixing

[Andreas Pokorny (andreas-pokorny)]

1. ok.. just that coding guideline state 120 and there was already lots of code in that length

4. Should it have any specific functionality other than picking a different pixel format?

8. Behavior change so far is only working inside nested - the other platforms ignore the policies more or less. Mesa needs fixing if now somebody tries to create a non nesting shell with alpha channel it will not get a buffer.

[Daniel van Vugt (vanvugt)]

Lines 49/50 are 134 characters wide, so still too long ;)

[Alan Griffiths (alan-griffiths)]

Do we actually need CascadedDisplayConfigurationPolicy? Or could TranslucentOutputs simply decorate the default policy?

~~~~

49 +CascadedDisplayConfigurationPolicy::CascadedDisplayConfigurationPolicy(std::shared_ptr<graphics::DisplayConfigurationPolicy> const& l,
50 + std::shared_ptr<graphics::DisplayConfigurationPolicy> const& r)

vs

102 + CascadedDisplayConfigurationPolicy(std::shared_ptr<graphics::DisplayConfigurationPolicy> const& left,
103 + std::shared_ptr<graphics::DisplayConfigurationPolicy> const& right);

The naming should be consistent between declaration and definition.
http://unity.ubuntu.com/mir/cppguide/index.html?showone=Function_Declarations_and_Definitions#Function_Declarations_and_Definitions

Also, I don't understand the significance of left/right (or the alternative l/r) - are these names meaningful? What I can see in the code is better represented as "first/second".

~~~~

106 + std::shared_ptr<DisplayConfigurationPolicy> left, right;

These dependencies should be const.

Also, while it isn't in the style guide, we have evolved a strong tendency towards single declarations in a statement. Vis:

std::shared_ptr<DisplayConfigurationPolicy> const left;
std::shared_ptr<DisplayConfigurationPolicy> const right;

~~~~

438 - void configure_output(DisplayConfigurationOutputId, bool, geometry::Point, size_t, MirPowerMode power_mode);
439 + void configure_output(DisplayConfigurationOutputId, bool, geometry::Point, size_t, size_t, MirPowerMode power_mode);

OK, not introduced by this MP, but the declaration doesn't:

1. Name the parameters
2. mention "virtual" or "override"

review: Needs Fixing

[Andreas Pokorny (andreas-pokorny)]

wrt: l vs left r vs right - I am still prone to my previous style guide and compiler setup which included -Wshadow.
Despite the fact I prefer mirs, I constantly fall back in old patterns.

[lp:~andreas-pokorny/mir/allow-transparent-server-buffers]

1287. By Andreas Pokorny on 2013-12-18

formating issues fixed

1288. By Andreas Pokorny on 2013-12-19

code style and formating improved

1289. By Andreas Pokorny on 2013-12-19

Moving Translucent option out of demo shell

1290. By Andreas Pokorny on 2013-12-19

Adding Basic Server Copy which always selects a transucent outpu

Currently named greeter - but does not greet reasonable yet.

1291. By Andreas Pokorny on 2013-12-19

Override and variable nameing missing

1292. By Andreas Pokorny on 2013-12-19

* Preparing for release 0.1.3
* No-change rebuild for ust.

merging cross compile fixes

[Alan Griffiths (alan-griffiths)]

286 + std::shared_ptr<graphics::DisplayConfigurationPolicy> base_policy;

Should be const.

~~~~

344 +#include <cstdint>
345 +#include "mir_toolkit/common.h"

http://unity.ubuntu.com/mir/cppguide/index.html?showone=Names_and_Order_of_Includes#Names_and_Order_of_Includes

~~~~

93 +
94 + using mir::examples::ServerConfiguration::the_options;

Not used

~~~~

82 + void launch_client()
83 + {
84 + if (the_options()->is_set(launch_child_opt))
85 + {
86 + char buffer[128] = {0};
87 + sprintf(buffer, "fd://%d", the_connector()->client_socket_fd());
88 + setenv("MIR_SOCKET", buffer, 1);
89 + auto ignore = system((the_options()->get(launch_child_opt, "") + "&").c_str());
90 + (void)ignore;
91 + }

This code looks familiar - maybe it should be in a common base class? (mir::examples::ServerConfiguration?)

I'm not sure how useful the "greeter" is as an example server. Certainly, if it exists, there ought to be more documentation on how to get it to do anything interesting.

review: Needs Fixing

[lp:~andreas-pokorny/mir/allow-transparent-server-buffers]

1293. By Andreas Pokorny on 2013-12-19

WIP rework of render_surfaces demo - dropped greeter

1294. By Andreas Pokorny on 2013-12-20

Merging surfacesless branch with latest improvement

1295. By Andreas Pokorny on 2013-12-20

Reworked the example - integrated it into render surfaces

1296. By Andreas Pokorny on 2013-12-20

Merge from devel to make auto merge from CI conflict free

+ cleanups in the MP, dropping unnecessary bits and whitespace noise

1297. By Andreas Pokorny on 2013-12-20

uninitialized member variable

1298. By Andreas Pokorny on 2013-12-20

duplicated parameter removed, const added to SelectPixelFormat

1299. By Andreas Pokorny on 2013-12-25

Ensure that opaque pixel formats are preferred over translucent in the default policy

Unmerged revisions

1299. By Andreas Pokorny on 2013-12-25

Ensure that opaque pixel formats are preferred over translucent in the default policy

1298. By Andreas Pokorny on 2013-12-20

duplicated parameter removed, const added to SelectPixelFormat

1297. By Andreas Pokorny on 2013-12-20

uninitialized member variable

1296. By Andreas Pokorny on 2013-12-20

Merge from devel to make auto merge from CI conflict free

+ cleanups in the MP, dropping unnecessary bits and whitespace noise

1295. By Andreas Pokorny on 2013-12-20

Reworked the example - integrated it into render surfaces

1294. By Andreas Pokorny on 2013-12-20

Merging surfacesless branch with latest improvement

1293. By Andreas Pokorny on 2013-12-19

WIP rework of render_surfaces demo - dropped greeter

1292. By Andreas Pokorny on 2013-12-19

* Preparing for release 0.1.3
* No-change rebuild for ust.

merging cross compile fixes

1291. By Andreas Pokorny on 2013-12-19

Override and variable nameing missing

1290. By Andreas Pokorny on 2013-12-19

Adding Basic Server Copy which always selects a transucent outpu

Currently named greeter - but does not greet reasonable yet.

=== modified file 'examples/CMakeLists.txt'
--- examples/CMakeLists.txt	2013-12-18 02:19:19 +0000
+++ examples/CMakeLists.txt	2013-12-20 13:46:31 +0000
@@ -109,6 +109,7 @@
     buffer_render_target.cpp
     image_renderer.cpp
     server_configuration.cpp
+    select_pixel_format.cpp
 )
 add_executable(mir_demo_standalone_render_surfaces ${RENDER_SURFACES_SOURCES})
 target_link_libraries(mir_demo_standalone_render_surfaces
@@ -157,4 +158,3 @@
 install(TARGETS mir_demo_server_basic
   RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
 )
-

=== modified file 'examples/basic_server.cpp'
--- examples/basic_server.cpp	2013-09-25 11:56:15 +0000
+++ examples/basic_server.cpp	2013-12-20 13:46:31 +0000
@@ -52,8 +52,6 @@
             (void)ignore;
         }
     }
-
-    using mir::examples::ServerConfiguration::the_options;
 };
 }
 

=== modified file 'examples/demo-shell/window_manager.cpp'
--- examples/demo-shell/window_manager.cpp	2013-12-18 02:19:19 +0000
+++ examples/demo-shell/window_manager.cpp	2013-12-20 13:46:31 +0000
@@ -153,6 +153,7 @@
                 conf->configure_output(output.id, output.used,
                                        output.top_left, 
                                        output.current_mode_index,
+                                       output.current_format_index,
                                        power_mode);
             });
             display_off = !display_off;

=== modified file 'examples/render_surfaces.cpp'
--- examples/render_surfaces.cpp	2013-12-18 02:19:19 +0000
+++ examples/render_surfaces.cpp	2013-12-20 13:46:31 +0000
@@ -24,6 +24,7 @@
 #include "mir/geometry/size.h"
 #include "mir/geometry/rectangles.h"
 #include "mir/graphics/buffer_initializer.h"
+#include "mir/graphics/pixel_format_utils.h"
 #include "mir/graphics/cursor.h"
 #include "mir/graphics/display.h"
 #include "mir/graphics/display_buffer.h"
@@ -36,13 +37,15 @@
 #include "buffer_render_target.h"
 #include "image_renderer.h"
 #include "server_configuration.h"
+#include "select_pixel_format.h"
 
-#include <thread>
+#include <algorithm>
 #include <atomic>
 #include <chrono>
 #include <csignal>
 #include <iostream>
 #include <sstream>
+#include <thread>
 #include <vector>
 
 #include <glm/gtc/matrix_transform.hpp>
@@ -136,6 +139,7 @@
 
 char const* const surfaces_to_render = "surfaces-to-render";
 char const* const display_cursor     = "display-cursor";
+char const* const translucent = "translucent";
 
 ///\internal [StopWatch_tag]
 // tracks elapsed time - for animation.
@@ -260,6 +264,10 @@
         add_options()
             (surfaces_to_render, po::value<int>(),  "Number of surfaces to render"
                                                     " [int:default=5]")
+            (surfaces_to_render, po::value<int>(),  "Number of surfaces to render"
+                                                    " [int:default=5]")
+            (translucent, po::bool_switch()->default_value(false), 
+             "Enable translucent frame buffer")
             (display_cursor, po::value<bool>(), "Display test cursor. (If input is "
                                                 "disabled it gets animated.) "
                                                 "[bool:default=false]");
@@ -306,6 +314,18 @@
         return std::make_shared<RenderResourcesBufferInitializer>();
     }
     ///\internal [RenderResourcesBufferInitializer_tag]
+   
+    std::shared_ptr<mg::DisplayConfigurationPolicy> the_display_configuration_policy() override
+    {
+        return display_configuration_policy(
+            [this]() -> std::shared_ptr<mg::DisplayConfigurationPolicy>
+            {
+                return std::make_shared<me::SelectPixelFormat>(
+                    me::ServerConfiguration::the_display_configuration_policy(), 
+                    the_options()->get(translucent,false));
+            }
+            );
+    }
 
     ///\internal [RenderSurfacesDisplayBufferCompositor_tag]
     // Decorate the DefaultDisplayBufferCompositor in order to move surfaces.
@@ -334,7 +354,7 @@
                     stop_watch.restart();
                 }
 
-                glClearColor(0.0, 1.0, 0.0, 1.0);
+                glClearColor(0.0, 1.0, 0.0, 0.0);
                 db_compositor->composite();
 
                 for (auto& m : moveables)

=== added file 'examples/select_pixel_format.cpp'
--- examples/select_pixel_format.cpp	1970-01-01 00:00:00 +0000
+++ examples/select_pixel_format.cpp	2013-12-20 13:46:31 +0000
@@ -0,0 +1,69 @@
+/*
+ * 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: Andreas Pokorny <[e-mail]>
+ */
+
+#include "select_pixel_format.h"
+
+#include "mir/graphics/display_configuration.h"
+#include "mir/graphics/pixel_format_utils.h"
+
+#include <stdexcept>
+#include <algorithm>
+
+
+namespace mir
+{
+namespace examples
+{
+
+SelectPixelFormat::SelectPixelFormat(std::shared_ptr<DisplayConfigurationPolicy> const& base_policy,
+                                     bool with_alpha)
+    : base_policy(base_policy),
+    with_alpha(with_alpha)
+{}
+
+void SelectPixelFormat::apply_to(graphics::DisplayConfiguration & conf)
+{
+    base_policy->apply_to(conf);
+    conf.for_each_output(
+        [&](graphics::DisplayConfigurationOutput const& conf_output)
+        {
+            if (!conf_output.connected || !conf_output.used) return;
+
+            auto const& pos = find_if(conf_output.pixel_formats.begin(),
+                                      conf_output.pixel_formats.end(), 
+                                      [&](MirPixelFormat format) -> bool
+                                          {
+                                              return graphics::contains_alpha(format) == with_alpha;
+                                          }
+                                     );
+
+            // keep the default setings if nothing was found
+            if (pos == conf_output.pixel_formats.end())
+                return;
+
+            conf.configure_output(conf_output.id, true, conf_output.top_left,
+                                  conf_output.current_mode_index,
+                                  std::distance(conf_output.pixel_formats.begin(), pos),
+                                  conf_output.power_mode
+                                 );
+        });
+}
+
+}
+}
+

=== added file 'examples/select_pixel_format.h'
--- examples/select_pixel_format.h	1970-01-01 00:00:00 +0000
+++ examples/select_pixel_format.h	2013-12-20 13:46:31 +0000
@@ -0,0 +1,52 @@
+/*
+ * 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: Andreas Pokorny <[e-mail]>
+ */
+
+
+#ifndef MIR_EXAMPLES_SELECT_PIXEL_FORMAT_H
+#define MIR_EXAMPLES_SELECT_PIXEL_FORMAT_H
+
+#include "mir/graphics/display_configuration_policy.h"
+
+#include <boost/shared_ptr.hpp>
+
+
+namespace mir
+{
+namespace examples
+{
+
+/**
+ * \brief Example of a DisplayConfigurationPolicy that tries to find
+ * an opaque or transparent pixel format, or falls back to the default
+ * if not found.
+ */
+class SelectPixelFormat : public graphics::DisplayConfigurationPolicy
+{
+public:
+    SelectPixelFormat(std::shared_ptr<graphics::DisplayConfigurationPolicy> const& base_policy,
+                      bool with_alpha);
+    virtual void apply_to(graphics::DisplayConfiguration& conf);
+private:
+    std::shared_ptr<graphics::DisplayConfigurationPolicy> const base_policy;
+    bool with_alpha;
+};
+
+}
+}
+
+#endif

=== modified file 'examples/server_configuration.cpp'
--- examples/server_configuration.cpp	2013-10-21 10:40:19 +0000
+++ examples/server_configuration.cpp	2013-12-20 13:46:31 +0000
@@ -61,14 +61,16 @@
                     available_outputs_for_card[conf_output.card_id] > 0)
                 {
                     conf.configure_output(conf_output.id, true, geom::Point{max_x, 0},
-                                          preferred_mode_index, mir_power_mode_on);
+                                          preferred_mode_index, conf_output.current_format_index,
+                                          mir_power_mode_on);
                     max_x += conf_output.modes[preferred_mode_index].size.width.as_int();
                     --available_outputs_for_card[conf_output.card_id];
                 }
                 else
                 {
                     conf.configure_output(conf_output.id, false, conf_output.top_left,
-                                          conf_output.current_mode_index, mir_power_mode_on);
+                                          conf_output.current_mode_index, conf_output.current_format_index,
+                                          mir_power_mode_on);
                 }
             });
     }
@@ -88,13 +90,15 @@
                 if (!done && conf_output.connected && conf_output.modes.size() > 0)
                 {
                     conf.configure_output(conf_output.id, true, geom::Point{0, 0},
-                                          preferred_mode_index, mir_power_mode_on);
+                                          preferred_mode_index, conf_output.current_format_index,
+                                          mir_power_mode_on);
                     done = true;
                 }
                 else
                 {
                     conf.configure_output(conf_output.id, false, conf_output.top_left,
-                                          conf_output.current_mode_index, mir_power_mode_on);
+                                          conf_output.current_mode_index, conf_output.current_format_index,
+                                          mir_power_mode_on);
                 }
             });
     }

=== modified file 'include/platform/mir/graphics/display_configuration.h'
--- include/platform/mir/graphics/display_configuration.h	2013-12-18 02:19:19 +0000
+++ include/platform/mir/graphics/display_configuration.h	2013-12-20 13:46:31 +0000
@@ -135,8 +135,8 @@
     virtual void for_each_output(std::function<void(DisplayConfigurationOutput const&)> f) const = 0;
 
     /** Configures an output. */
-    virtual void configure_output(DisplayConfigurationOutputId id, bool used,
-                                  geometry::Point top_left, size_t mode_index, MirPowerMode power_mode) = 0;
+    virtual void configure_output(DisplayConfigurationOutputId id, bool used, geometry::Point top_left, 
+                                  size_t mode_index, size_t format_index, MirPowerMode power_mode) = 0;
 
 protected:
     DisplayConfiguration() = default;

=== modified file 'include/platform/mir/graphics/platform.h'
--- include/platform/mir/graphics/platform.h	2013-12-18 02:19:19 +0000
+++ include/platform/mir/graphics/platform.h	2013-12-20 13:46:31 +0000
@@ -81,7 +81,7 @@
      * Creates the display subsystem.
      */
     virtual std::shared_ptr<Display> create_display(
-        std::shared_ptr<DisplayConfigurationPolicy> const& initial_conf_policy) = 0;
+            std::shared_ptr<DisplayConfigurationPolicy> const& initial_conf_policy) = 0;
 
     /**
      * Gets the IPC package for the platform.

=== added file 'include/server/mir/graphics/pixel_format_utils.h'
--- include/server/mir/graphics/pixel_format_utils.h	1970-01-01 00:00:00 +0000
+++ include/server/mir/graphics/pixel_format_utils.h	2013-12-20 13:46:31 +0000
@@ -0,0 +1,46 @@
+/*
+ * 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: Andreas Pokorny <[e-mail]>
+ */
+
+#include "mir_toolkit/common.h"
+
+#include <cstdint>
+
+namespace mir
+{
+namespace graphics
+{
+
+/*!
+ * \name MirPixelFormat utility functions
+ *
+ * A set of functions to query details of MirPixelFormat
+ * TODO improve this through https://bugs.launchpad.net/mir/+bug/1236254 
+ * \{
+ */
+bool contains_alpha(MirPixelFormat format);
+int32_t red_channel_depth(MirPixelFormat format);
+int32_t blue_channel_depth(MirPixelFormat format);
+int32_t green_channel_depth(MirPixelFormat format);
+int32_t alpha_channel_depth(MirPixelFormat format);
+/*!
+ * \}
+ */
+
+
+}
+}

=== modified file 'include/test/mir_test_doubles/null_display_configuration.h'
--- include/test/mir_test_doubles/null_display_configuration.h	2013-09-12 21:36:55 +0000
+++ include/test/mir_test_doubles/null_display_configuration.h	2013-12-20 13:46:31 +0000
@@ -35,7 +35,7 @@
     void for_each_output(std::function<void(graphics::DisplayConfigurationOutput const&)>) const
     {
     }
-    void configure_output(graphics::DisplayConfigurationOutputId, bool, geometry::Point, size_t, MirPowerMode) override
+    void configure_output(graphics::DisplayConfigurationOutputId, bool, geometry::Point, size_t, size_t, MirPowerMode) override
     {
     }
 };

=== modified file 'include/test/mir_test_doubles/stub_display_configuration.h'
--- include/test/mir_test_doubles/stub_display_configuration.h	2013-12-18 02:19:19 +0000
+++ include/test/mir_test_doubles/stub_display_configuration.h	2013-12-20 13:46:31 +0000
@@ -136,7 +136,7 @@
         }
     }
 
-    void configure_output(graphics::DisplayConfigurationOutputId, bool, geometry::Point, size_t, MirPowerMode)
+    void configure_output(graphics::DisplayConfigurationOutputId, bool, geometry::Point, size_t, size_t, MirPowerMode)
     {
     }
 

=== modified file 'src/platform/graphics/android/android_display_configuration.cpp'
--- src/platform/graphics/android/android_display_configuration.cpp	2013-12-18 02:19:19 +0000
+++ src/platform/graphics/android/android_display_configuration.cpp	2013-12-20 13:46:31 +0000
@@ -62,7 +62,7 @@
     f(configuration);
 }
 
-void mga::AndroidDisplayConfiguration::configure_output(mg::DisplayConfigurationOutputId, bool, geom::Point, size_t, MirPowerMode power_mode)
+void mga::AndroidDisplayConfiguration::configure_output(mg::DisplayConfigurationOutputId, bool, geom::Point, size_t, size_t, MirPowerMode power_mode)
 {
     configuration.power_mode = power_mode;
 }

=== modified file 'src/platform/graphics/android/android_display_configuration.h'
--- src/platform/graphics/android/android_display_configuration.h	2013-12-18 02:19:19 +0000
+++ src/platform/graphics/android/android_display_configuration.h	2013-12-20 13:46:31 +0000
@@ -37,7 +37,7 @@
 
     void for_each_card(std::function<void(DisplayConfigurationCard const&)> f) const;
     void for_each_output(std::function<void(DisplayConfigurationOutput const&)> f) const;
-    void configure_output(DisplayConfigurationOutputId, bool, geometry::Point, size_t, MirPowerMode power_mode);
+    void configure_output(DisplayConfigurationOutputId, bool, geometry::Point, size_t, size_t, MirPowerMode power_mode);
 
 private:
     DisplayConfigurationOutput configuration;

=== modified file 'src/platform/graphics/mesa/real_kms_display_configuration.cpp'
--- src/platform/graphics/mesa/real_kms_display_configuration.cpp	2013-12-18 02:19:19 +0000
+++ src/platform/graphics/mesa/real_kms_display_configuration.cpp	2013-12-20 13:46:31 +0000
@@ -109,7 +109,7 @@
 void mgm::RealKMSDisplayConfiguration::configure_output(
     DisplayConfigurationOutputId id, bool used,
     geometry::Point top_left, size_t mode_index,
-    MirPowerMode power_mode)
+    size_t format_index, MirPowerMode power_mode)
 {
     auto iter = find_output_with_id(id);
 
@@ -120,9 +120,13 @@
         if (used && mode_index >= output.modes.size())
             BOOST_THROW_EXCEPTION(std::runtime_error("Invalid mode_index for used output"));
 
+        if (used && format_index >= output.pixel_formats.size())
+            BOOST_THROW_EXCEPTION(std::runtime_error("Invalid format_index for used output"));
+
         output.used = used;
         output.top_left = top_left;
         output.current_mode_index = mode_index;
+        output.current_format_index = format_index;
         output.power_mode = power_mode;
     }
     else

=== modified file 'src/platform/graphics/mesa/real_kms_display_configuration.h'
--- src/platform/graphics/mesa/real_kms_display_configuration.h	2013-12-18 02:19:19 +0000
+++ src/platform/graphics/mesa/real_kms_display_configuration.h	2013-12-20 13:46:31 +0000
@@ -39,8 +39,8 @@
 
     void for_each_card(std::function<void(DisplayConfigurationCard const&)> f) const;
     void for_each_output(std::function<void(DisplayConfigurationOutput const&)> f) const;
-    void configure_output(DisplayConfigurationOutputId id, bool used,
-                          geometry::Point top_left, size_t mode_index, MirPowerMode power_mode);
+    void configure_output(DisplayConfigurationOutputId id, bool used, geometry::Point top_left, 
+                          size_t mode_index, size_t foramt_index, MirPowerMode power_mode);
 
     uint32_t get_kms_connector_id(DisplayConfigurationOutputId id) const;
     size_t get_kms_mode_index(DisplayConfigurationOutputId id, size_t conf_mode_index) const;

=== modified file 'src/server/frontend/session_mediator.cpp'
--- src/server/frontend/session_mediator.cpp	2013-12-18 02:19:19 +0000
+++ src/server/frontend/session_mediator.cpp	2013-12-20 13:46:31 +0000
@@ -321,7 +321,8 @@
             mg::DisplayConfigurationOutputId output_id{static_cast<int>(output.output_id())};
             config->configure_output(output_id, output.used(),
                                      geom::Point{output.position_x(), output.position_y()},
-                                     output.current_mode(), static_cast<MirPowerMode>(output.power_mode()));
+                                     output.current_mode(), output.current_format(), 
+                                     static_cast<MirPowerMode>(output.power_mode()));
         }
 
         display_changer->configure(session, config);

=== modified file 'src/server/graphics/CMakeLists.txt'
--- src/server/graphics/CMakeLists.txt	2013-12-20 05:06:28 +0000
+++ src/server/graphics/CMakeLists.txt	2013-12-20 13:46:31 +0000
@@ -5,6 +5,7 @@
 
   default_configuration.cpp
   default_display_configuration_policy.cpp
+  pixel_format_utils.cpp
   gl_extensions_base.cpp
   surfaceless_egl_context.cpp
 )

=== modified file 'src/server/graphics/default_display_configuration_policy.cpp'
--- src/server/graphics/default_display_configuration_policy.cpp	2013-12-18 02:19:19 +0000
+++ src/server/graphics/default_display_configuration_policy.cpp	2013-12-20 13:46:31 +0000
@@ -45,15 +45,16 @@
                 if (preferred_mode_index > conf_output.modes.size())
                     preferred_mode_index = 0;
 
-                conf.configure_output(conf_output.id, true, geom::Point(),
-                                      preferred_mode_index, default_power_state);
+                conf.configure_output(conf_output.id, true, geom::Point(), preferred_mode_index, 
+                                      conf_output.current_format_index, default_power_state);
 
                 --available_outputs_for_card[conf_output.card_id];
             }
             else
             {
                 conf.configure_output(conf_output.id, false, conf_output.top_left,
-                                      conf_output.current_mode_index, default_power_state);
+                                      conf_output.current_mode_index, conf_output.current_format_index,
+                                      default_power_state);
             }
         });
 }

=== modified file 'src/server/graphics/nested/nested_display.cpp'
--- src/server/graphics/nested/nested_display.cpp	2013-12-20 05:06:28 +0000
+++ src/server/graphics/nested/nested_display.cpp	2013-12-20 13:46:31 +0000
@@ -22,8 +22,10 @@
 #include "mir_api_wrappers.h"
 
 #include "mir/geometry/rectangle.h"
+#include "mir/graphics/pixel_format_utils.h"
 #include "mir/graphics/gl_context.h"
 #include "mir/graphics/surfaceless_egl_context.h"
+#include "mir/graphics/display_configuration_policy.h"
 #include "host_connection.h"
 
 #include <boost/throw_exception.hpp>
@@ -34,45 +36,8 @@
 namespace mgnw = mir::graphics::nested::mir_api_wrappers;
 namespace geom = mir::geometry;
 
-namespace
-{
-
-MirPixelFormat find_opaque_surface_format(MirConnection* connection)
-{
-    static unsigned const max_formats = 32;
-    MirPixelFormat formats[max_formats];
-    unsigned int valid_formats;
-
-    mir_connection_get_available_surface_formats(connection, formats,
-                                                 max_formats, &valid_formats);
-
-    // Find an opaque surface format
-    for (auto f = formats; f != formats+valid_formats; ++f)
-    {
-        if (*f == mir_pixel_format_xbgr_8888 ||
-            *f == mir_pixel_format_xrgb_8888)
-        {
-            return *f;
-        }
-    }
-
-    BOOST_THROW_EXCEPTION(
-        std::runtime_error("Nested Mir failed to find an opaque surface format"));
-}
-
-}
-
-EGLint const mgn::detail::nested_egl_config_attribs[] = {
-    EGL_SURFACE_TYPE, EGL_WINDOW_BIT,
-    EGL_RED_SIZE, 8,
-    EGL_GREEN_SIZE, 8,
-    EGL_BLUE_SIZE, 8,
-    EGL_ALPHA_SIZE, 0,
-    EGL_RENDERABLE_TYPE, EGL_OPENGL_ES2_BIT,
-    EGL_NONE
-};
-
-EGLint const mgn::detail::nested_egl_context_attribs[] = {
+EGLint const mgn::detail::nested_egl_context_attribs[] =
+{
     EGL_CONTEXT_CLIENT_VERSION, 2,
     EGL_NONE
 };
@@ -100,7 +65,7 @@
         BOOST_THROW_EXCEPTION(std::runtime_error("Nested Mir Display Error: Failed to fetch EGL display."));
 }
 
-void mgn::detail::EGLDisplayHandle::initialize()
+void mgn::detail::EGLDisplayHandle::initialize(MirPixelFormat format)
 {
     int major;
     int minor;
@@ -110,17 +75,28 @@
         BOOST_THROW_EXCEPTION(std::runtime_error("Nested Mir Display Error: Failed to initialize EGL."));
     }
 
-    egl_context_ = eglCreateContext(egl_display, choose_config(detail::nested_egl_config_attribs), EGL_NO_CONTEXT, detail::nested_egl_context_attribs);
+    egl_context_ = eglCreateContext(egl_display, choose_windowed_es_config(format), EGL_NO_CONTEXT, detail::nested_egl_context_attribs);
+
     if (egl_context_ == EGL_NO_CONTEXT)
         BOOST_THROW_EXCEPTION(std::runtime_error("Failed to create shared EGL context"));
 }
 
-EGLConfig mgn::detail::EGLDisplayHandle::choose_config(const EGLint attrib_list[]) const
+EGLConfig mgn::detail::EGLDisplayHandle::choose_windowed_es_config(MirPixelFormat format) const
 {
+    EGLint const nested_egl_config_attribs[] =
+    {
+        EGL_SURFACE_TYPE, EGL_WINDOW_BIT,
+        EGL_RED_SIZE, mg::red_channel_depth(format),
+        EGL_GREEN_SIZE, mg::green_channel_depth(format),
+        EGL_BLUE_SIZE, mg::blue_channel_depth(format),
+        EGL_ALPHA_SIZE, mg::alpha_channel_depth(format),
+        EGL_RENDERABLE_TYPE, EGL_OPENGL_ES2_BIT,
+        EGL_NONE
+    };
     EGLConfig result;
     int n;
 
-    int res = eglChooseConfig(egl_display, attrib_list, &result, 1, &n);
+    int res = eglChooseConfig(egl_display, nested_egl_config_attribs, &result, 1, &n);
     if ((res != EGL_TRUE) || (n != 1))
         BOOST_THROW_EXCEPTION(std::runtime_error("Nested Mir Display Error: Failed to choose EGL configuration."));
 
@@ -149,17 +125,18 @@
 mgn::NestedDisplay::NestedDisplay(
     std::shared_ptr<HostConnection> const& connection,
     std::shared_ptr<input::EventFilter> const& event_handler,
-    std::shared_ptr<mg::DisplayReport> const& display_report) :
+    std::shared_ptr<mg::DisplayReport> const& display_report,
+    std::shared_ptr<mg::DisplayConfigurationPolicy> const& initial_conf_policy) :
     connection{connection},
     event_handler{event_handler},
     display_report{display_report},
     egl_display{*connection},
-    egl_pixel_format{find_opaque_surface_format(*connection)},
     outputs{}
 {
-    egl_display.initialize();
-    eglMakeCurrent(egl_display, EGL_NO_SURFACE, EGL_NO_SURFACE, egl_display.egl_context());
-    configure(*configuration());
+
+    std::shared_ptr<DisplayConfiguration> conf(configuration());
+    initial_conf_policy->apply_to(*conf);
+    configure(*conf);
 }
 
 mgn::NestedDisplay::~NestedDisplay() noexcept
@@ -178,6 +155,14 @@
     return std::make_shared<NestedDisplayConfiguration>(mir_connection_create_display_config(*connection));
 }
 
+void mgn::NestedDisplay::complete_display_initialization(MirPixelFormat format)
+{
+    if (egl_display.egl_context() != EGL_NO_CONTEXT)  return;
+
+    egl_display.initialize(format);
+    eglMakeCurrent(egl_display, EGL_NO_SURFACE, EGL_NO_SURFACE, egl_display.egl_context());
+}
+
 void mgn::NestedDisplay::configure(mg::DisplayConfiguration const& configuration)
 {
     decltype(outputs) result;
@@ -193,13 +178,16 @@
                 geometry::Rectangle const area{output.top_left, output.modes[output.current_mode_index].size};
 
                 auto const& egl_display_mode = output.modes[output.current_mode_index];
+                auto const& egl_config_format = output.pixel_formats[output.current_format_index];
+
+                complete_display_initialization(egl_config_format);
 
                 MirSurfaceParameters const request_params =
                     {
                         "Mir nested display",
                         egl_display_mode.size.width.as_int(),
                         egl_display_mode.size.height.as_int(),
-                        egl_pixel_format,
+                        egl_config_format,
                         mir_buffer_usage_hardware,
                         static_cast<uint32_t>(output.id.as_value())
                     };
@@ -213,7 +201,8 @@
                     egl_display,
                     mir_surface,
                     area,
-                    event_handler);
+                    event_handler,
+                    output.pixel_formats[output.current_format_index]);
             }
         });
 
@@ -274,13 +263,7 @@
     return std::weak_ptr<Cursor>();
 }
 
-namespace
-{
-}
-
 std::unique_ptr<mg::GLContext> mgn::NestedDisplay::create_gl_context()
 {
-    return std::unique_ptr<mg::GLContext>{new SurfacelessEGLContext(egl_display,
-                                                                    detail::nested_egl_config_attribs,
-                                                                    EGL_NO_CONTEXT)};
+    return std::unique_ptr<mg::GLContext>{new SurfacelessEGLContext(egl_display, EGL_NO_CONTEXT)};
 }

=== modified file 'src/server/graphics/nested/nested_display.h'
--- src/server/graphics/nested/nested_display.h	2013-12-18 02:19:19 +0000
+++ src/server/graphics/nested/nested_display.h	2013-12-20 13:46:31 +0000
@@ -42,6 +42,7 @@
 {
 class DisplayReport;
 class DisplayBuffer;
+class DisplayConfigurationPolicy;
 
 namespace nested
 {
@@ -67,8 +68,8 @@
     explicit EGLDisplayHandle(MirConnection* connection);
     ~EGLDisplayHandle() noexcept;
 
-    void initialize();
-    EGLConfig choose_config(const EGLint attrib_list[]) const;
+    void initialize(MirPixelFormat format);
+    EGLConfig choose_windowed_es_config (MirPixelFormat format) const;
     EGLNativeWindowType native_window(EGLConfig egl_config, MirSurface* mir_surface) const;
     EGLContext egl_context() const;
     operator EGLDisplay() const { return egl_display; }
@@ -83,7 +84,6 @@
 
 class NestedOutput;
 
-extern EGLint const nested_egl_config_attribs[];
 extern EGLint const nested_egl_context_attribs[];
 }
 
@@ -95,7 +95,8 @@
     NestedDisplay(
         std::shared_ptr<HostConnection> const& connection,
         std::shared_ptr<input::EventFilter> const& event_handler,
-        std::shared_ptr<DisplayReport> const& display_report);
+        std::shared_ptr<DisplayReport> const& display_report,
+        std::shared_ptr<DisplayConfigurationPolicy> const& conf_policy);
 
     ~NestedDisplay() noexcept;
 
@@ -124,11 +125,11 @@
     std::shared_ptr<input::EventFilter> const event_handler;
     std::shared_ptr<DisplayReport> const display_report;
     detail::EGLDisplayHandle egl_display;
-    MirPixelFormat const egl_pixel_format;
 
     std::mutex outputs_mutex;
     std::unordered_map<DisplayConfigurationOutputId, std::shared_ptr<detail::NestedOutput>> outputs;
     DisplayConfigurationChangeHandler my_conf_change_handler;
+    void complete_display_initialization( MirPixelFormat format );
 };
 
 }

=== modified file 'src/server/graphics/nested/nested_display_configuration.cpp'
--- src/server/graphics/nested/nested_display_configuration.cpp	2013-12-18 02:19:19 +0000
+++ src/server/graphics/nested/nested_display_configuration.cpp	2013-12-20 13:46:31 +0000
@@ -86,7 +86,7 @@
 }
 
 void mgn::NestedDisplayConfiguration::configure_output(DisplayConfigurationOutputId id, bool used,
-    geometry::Point top_left, size_t mode_index, MirPowerMode power_mode)
+    geometry::Point top_left, size_t mode_index, size_t format_index, MirPowerMode power_mode)
 {
     for (auto mir_output = display_config->outputs;
         mir_output != display_config->outputs+display_config->num_outputs;
@@ -97,10 +97,14 @@
             if (used && mode_index >= mir_output->num_modes)
                 BOOST_THROW_EXCEPTION(std::runtime_error("Invalid mode_index for used output"));
 
+            if (used && format_index >= mir_output->num_output_formats)
+                BOOST_THROW_EXCEPTION(std::runtime_error("Invalid format_index for used output"));
+
             mir_output->used = used;
             mir_output->position_x = top_left.x.as_uint32_t();
             mir_output->position_y = top_left.y.as_uint32_t();
             mir_output->current_mode = mode_index;
+            mir_output->current_output_format = format_index;
             mir_output->power_mode = static_cast<MirPowerMode>(power_mode);
             return;
         }

=== modified file 'src/server/graphics/nested/nested_display_configuration.h'
--- src/server/graphics/nested/nested_display_configuration.h	2013-09-12 21:36:55 +0000
+++ src/server/graphics/nested/nested_display_configuration.h	2013-12-20 13:46:31 +0000
@@ -38,7 +38,7 @@
     void for_each_output(std::function<void(DisplayConfigurationOutput const&)>) const;
 
     void configure_output(DisplayConfigurationOutputId id, bool used, geometry::Point top_left, size_t mode_index,
-                          MirPowerMode power_mode);
+                          size_t format_index, MirPowerMode power_mode);
 
     operator MirDisplayConfiguration*() const { return display_config; }
 private:

=== modified file 'src/server/graphics/nested/nested_output.cpp'
--- src/server/graphics/nested/nested_output.cpp	2013-09-23 13:37:44 +0000
+++ src/server/graphics/nested/nested_output.cpp	2013-12-20 13:46:31 +0000
@@ -41,10 +41,11 @@
     EGLDisplayHandle const& egl_display,
     MirSurface* mir_surface,
     geometry::Rectangle const& area,
-    std::shared_ptr<input::EventFilter> const& event_handler) :
+    std::shared_ptr<input::EventFilter> const& event_handler,
+    MirPixelFormat preferred_format) :
     egl_display(egl_display),
     mir_surface{mir_surface},
-    egl_config{egl_display.choose_config(nested_egl_config_attribs)},
+    egl_config{egl_display.choose_windowed_es_config(preferred_format)},
     egl_context{egl_display, eglCreateContext(egl_display, egl_config, egl_display.egl_context(), nested_egl_context_attribs)},
     area{area.top_left, area.size},
     event_handler{event_handler},

=== modified file 'src/server/graphics/nested/nested_output.h'
--- src/server/graphics/nested/nested_output.h	2013-09-23 13:37:44 +0000
+++ src/server/graphics/nested/nested_output.h	2013-12-20 13:46:31 +0000
@@ -54,7 +54,8 @@
         EGLDisplayHandle const& egl_display,
         MirSurface* mir_surface,
         geometry::Rectangle const& area,
-        std::shared_ptr<input::EventFilter> const& event_handler);
+        std::shared_ptr<input::EventFilter> const& event_handler,
+        MirPixelFormat preferred_format);
 
     ~NestedOutput() noexcept;
 

=== modified file 'src/server/graphics/nested/nested_platform.cpp'
--- src/server/graphics/nested/nested_platform.cpp	2013-12-18 02:19:19 +0000
+++ src/server/graphics/nested/nested_platform.cpp	2013-12-20 13:46:31 +0000
@@ -113,9 +113,9 @@
     return native_platform->create_buffer_allocator(buffer_initializer);
 }
 
-std::shared_ptr<mg::Display> mgn::NestedPlatform::create_display(std::shared_ptr<mg::DisplayConfigurationPolicy> const& /*initial_conf_policy*/)
+std::shared_ptr<mg::Display> mgn::NestedPlatform::create_display(std::shared_ptr<mg::DisplayConfigurationPolicy> const& conf_policy)
 {
-    return std::make_shared<mgn::NestedDisplay>(connection, event_handler, display_report);
+    return std::make_shared<mgn::NestedDisplay>(connection, event_handler, display_report, conf_policy);
 }
 
 std::shared_ptr<mg::PlatformIPCPackage> mgn::NestedPlatform::get_ipc_package()

=== modified file 'src/server/graphics/nested/nested_platform.h'
--- src/server/graphics/nested/nested_platform.h	2013-12-18 02:19:19 +0000
+++ src/server/graphics/nested/nested_platform.h	2013-12-20 13:46:31 +0000
@@ -42,12 +42,12 @@
 
     ~NestedPlatform() noexcept;
     std::shared_ptr<GraphicBufferAllocator> create_buffer_allocator(
-            std::shared_ptr<BufferInitializer> const& buffer_initializer);
+            std::shared_ptr<BufferInitializer> const& buffer_initializer) override;
     std::shared_ptr<Display> create_display(
-            std::shared_ptr<DisplayConfigurationPolicy> const& initial_conf_policy);
-    std::shared_ptr<PlatformIPCPackage> get_ipc_package();
-    std::shared_ptr<InternalClient> create_internal_client();
-    void fill_ipc_package(BufferIPCPacker* packer, Buffer const* Buffer) const;
+            std::shared_ptr<DisplayConfigurationPolicy> const& initial_conf_policy) override;
+    std::shared_ptr<PlatformIPCPackage> get_ipc_package() override;
+    std::shared_ptr<InternalClient> create_internal_client() override;
+    void fill_ipc_package(BufferIPCPacker* packer, Buffer const* Buffer) const override;
     EGLNativeDisplayType egl_native_display() const;
 
 private:

=== modified file 'src/server/graphics/offscreen/display_configuration.cpp'
--- src/server/graphics/offscreen/display_configuration.cpp	2013-12-18 02:19:19 +0000
+++ src/server/graphics/offscreen/display_configuration.cpp	2013-12-20 13:46:31 +0000
@@ -67,6 +67,6 @@
 }
 
 void mgo::DisplayConfiguration::configure_output(
-    mg::DisplayConfigurationOutputId, bool, geom::Point, size_t, MirPowerMode)
+    mg::DisplayConfigurationOutputId, bool, geom::Point, size_t, size_t, MirPowerMode)
 {
 }

=== modified file 'src/server/graphics/offscreen/display_configuration.h'
--- src/server/graphics/offscreen/display_configuration.h	2013-11-12 17:23:53 +0000
+++ src/server/graphics/offscreen/display_configuration.h	2013-12-20 13:46:31 +0000
@@ -33,10 +33,10 @@
     DisplayConfiguration(DisplayConfiguration const& other);
     DisplayConfiguration& operator=(DisplayConfiguration const& other);
 
-    void for_each_card(std::function<void(DisplayConfigurationCard const&)> f) const;
-    void for_each_output(std::function<void(DisplayConfigurationOutput const&)> f) const;
-    void configure_output(DisplayConfigurationOutputId, bool, geometry::Point,
-                          size_t, MirPowerMode power_mode);
+    void for_each_card(std::function<void(DisplayConfigurationCard const&)> f) const override;
+    void for_each_output(std::function<void(DisplayConfigurationOutput const&)> f) const override;
+    virtual void configure_output(DisplayConfigurationOutputId id, bool used, geometry::Point top_left, 
+                                  size_t mode_index, size_t format_index, MirPowerMode power_mode) override;
 
 private:
     DisplayConfigurationOutput output;

=== added file 'src/server/graphics/pixel_format_utils.cpp'
--- src/server/graphics/pixel_format_utils.cpp	1970-01-01 00:00:00 +0000
+++ src/server/graphics/pixel_format_utils.cpp	2013-12-20 13:46:31 +0000
@@ -0,0 +1,65 @@
+/*
+ * 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: Andreas Pokorny <[e-mail]>
+ */
+
+#include "mir/graphics/pixel_format_utils.h"
+
+namespace mir
+{
+namespace graphics
+{
+
+bool contains_alpha(MirPixelFormat format)
+{
+    return (format == mir_pixel_format_abgr_8888 ||
+            format == mir_pixel_format_argb_8888);
+}
+
+int32_t red_channel_depth(MirPixelFormat format)
+{
+    if (mir_pixel_format_invalid < format &&
+        format < mir_pixel_formats)
+        return 8;
+    return 0;
+}
+
+int32_t blue_channel_depth(MirPixelFormat format)
+{
+    if (mir_pixel_format_invalid < format &&
+        format < mir_pixel_formats)
+        return 8;
+    return 0;
+}
+
+int32_t green_channel_depth( MirPixelFormat format )
+{
+    if (mir_pixel_format_invalid < format &&
+        format < mir_pixel_formats)
+        return 8;
+    return 0;
+}
+
+int32_t alpha_channel_depth( MirPixelFormat format )
+{
+    if (format == mir_pixel_format_abgr_8888 ||
+        format == mir_pixel_format_argb_8888)
+        return 8;
+    return 0;
+}
+
+}
+}

=== modified file 'src/server/scene/mediating_display_changer.cpp'
--- src/server/scene/mediating_display_changer.cpp	2013-12-18 02:19:19 +0000
+++ src/server/scene/mediating_display_changer.cpp	2013-12-20 13:46:31 +0000
@@ -136,6 +136,7 @@
             conf->configure_output(output.id, output.used,
                                    output.top_left,
                                    output.current_mode_index,
+                                   output.current_format_index,
                                    mir_power_mode_on);
         }
     });

=== modified file 'tests/mir_test/display_config_matchers.cpp'
--- tests/mir_test/display_config_matchers.cpp	2013-12-18 02:19:19 +0000
+++ tests/mir_test/display_config_matchers.cpp	2013-12-20 13:46:31 +0000
@@ -175,7 +175,7 @@
     }
 
     void configure_output(mg::DisplayConfigurationOutputId, bool,
-                          geom::Point, size_t, MirPowerMode)
+                          geom::Point, size_t, size_t, MirPowerMode)
     {
     }
 

=== modified file 'tests/unit-tests/frontend/test_session_mediator.cpp'
--- tests/unit-tests/frontend/test_session_mediator.cpp	2013-12-18 02:19:19 +0000
+++ tests/unit-tests/frontend/test_session_mediator.cpp	2013-12-20 13:46:31 +0000
@@ -83,7 +83,7 @@
 {
     MOCK_CONST_METHOD1(for_each_card, void(std::function<void(mg::DisplayConfigurationCard const&)>));
     MOCK_CONST_METHOD1(for_each_output, void(std::function<void(mg::DisplayConfigurationOutput const&)>));
-    MOCK_METHOD5(configure_output, void(mg::DisplayConfigurationOutputId, bool, geom::Point, size_t, MirPowerMode));
+    MOCK_METHOD6(configure_output, void(mg::DisplayConfigurationOutputId, bool, geom::Point, size_t, size_t, MirPowerMode));
 };
 
 }
@@ -555,6 +555,7 @@
     bool used0 = false, used1 = true;
     geom::Point pt0{44,22}, pt1{3,2};
     size_t mode_index0 = 1, mode_index1 = 3;
+    size_t format_index0 = 2, format_index1 = 0;
     mg::DisplayConfigurationOutputId id0{6}, id1{3};
 
     NiceMock<MockConfig> mock_display_config;
@@ -568,9 +569,9 @@
     EXPECT_CALL(*mock_display_selector, active_configuration())
         .InSequence(seq)
         .WillOnce(Return(mt::fake_shared(mock_display_config)));
-    EXPECT_CALL(mock_display_config, configure_output(id0, used0, pt0, mode_index0,  mir_power_mode_on))
+    EXPECT_CALL(mock_display_config, configure_output(id0, used0, pt0, mode_index0, format_index0, mir_power_mode_on))
         .InSequence(seq);
-    EXPECT_CALL(mock_display_config, configure_output(id1, used1, pt1, mode_index1, mir_power_mode_off))
+    EXPECT_CALL(mock_display_config, configure_output(id1, used1, pt1, mode_index1, format_index1, mir_power_mode_off))
         .InSequence(seq);
     EXPECT_CALL(*mock_display_selector, configure(_,_))
         .InSequence(seq);
@@ -593,6 +594,7 @@
     disp0->set_position_x(pt0.x.as_uint32_t());
     disp0->set_position_y(pt0.y.as_uint32_t());
     disp0->set_current_mode(mode_index0);
+    disp0->set_current_format(format_index0);
     disp0->set_power_mode(static_cast<uint32_t>(mir_power_mode_on));
 
     auto disp1 = configuration.add_display_output();
@@ -601,6 +603,7 @@
     disp1->set_position_x(pt1.x.as_uint32_t());
     disp1->set_position_y(pt1.y.as_uint32_t());
     disp1->set_current_mode(mode_index1);
+    disp1->set_current_format(format_index1);
     disp1->set_power_mode(static_cast<uint32_t>(mir_power_mode_off));
 
     session_mediator.configure_display(nullptr, &configuration,

=== modified file 'tests/unit-tests/graphics/CMakeLists.txt'
--- tests/unit-tests/graphics/CMakeLists.txt	2013-12-20 05:06:28 +0000
+++ tests/unit-tests/graphics/CMakeLists.txt	2013-12-20 13:46:31 +0000
@@ -6,6 +6,7 @@
     ${CMAKE_CURRENT_SOURCE_DIR}/test_default_display_configuration_policy.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/test_buffer_id.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/test_buffer_properties.cpp
+    ${CMAKE_CURRENT_SOURCE_DIR}/test_pixel_format_utils.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/test_surfaceless_egl_context.cpp
 )
 

=== modified file 'tests/unit-tests/graphics/android/test_android_fb.cpp'
--- tests/unit-tests/graphics/android/test_android_fb.cpp	2013-12-18 02:19:19 +0000
+++ tests/unit-tests/graphics/android/test_android_fb.cpp	2013-12-20 13:46:31 +0000
@@ -256,28 +256,28 @@
     configuration->for_each_output([&](mg::DisplayConfigurationOutput const& output)
     {
         configuration->configure_output(
-            output.id, output.used, output.top_left, output.current_mode_index, mir_power_mode_on);
-    });
-    display.configure(*configuration);
-
-    configuration->for_each_output([&](mg::DisplayConfigurationOutput const& output)
-    {
-        configuration->configure_output(
-            output.id, output.used, output.top_left, output.current_mode_index, mir_power_mode_standby);
-    });
-    display.configure(*configuration);
-
-    configuration->for_each_output([&](mg::DisplayConfigurationOutput const& output)
-    {
-        configuration->configure_output(
-            output.id, output.used, output.top_left, output.current_mode_index, mir_power_mode_off);
-    });
-    display.configure(*configuration);
-
-    configuration->for_each_output([&](mg::DisplayConfigurationOutput const& output)
-    {
-        configuration->configure_output(
-            output.id, output.used, output.top_left, output.current_mode_index, mir_power_mode_suspend);
+            output.id, output.used, output.top_left, output.current_mode_index, output.current_format_index, mir_power_mode_on);
+    });
+    display.configure(*configuration);
+
+    configuration->for_each_output([&](mg::DisplayConfigurationOutput const& output)
+    {
+        configuration->configure_output(
+            output.id, output.used, output.top_left, output.current_mode_index, output.current_format_index, mir_power_mode_standby);
+    });
+    display.configure(*configuration);
+
+    configuration->for_each_output([&](mg::DisplayConfigurationOutput const& output)
+    {
+        configuration->configure_output(
+            output.id, output.used, output.top_left, output.current_mode_index, output.current_format_index, mir_power_mode_off);
+    });
+    display.configure(*configuration);
+
+    configuration->for_each_output([&](mg::DisplayConfigurationOutput const& output)
+    {
+        configuration->configure_output(
+            output.id, output.used, output.top_left, output.current_mode_index, output.current_format_index, mir_power_mode_suspend);
     });
     display.configure(*configuration);
 }

=== modified file 'tests/unit-tests/graphics/mesa/test_cursor.cpp'
--- tests/unit-tests/graphics/mesa/test_cursor.cpp	2013-12-18 02:19:19 +0000
+++ tests/unit-tests/graphics/mesa/test_cursor.cpp	2013-12-20 13:46:31 +0000
@@ -141,7 +141,7 @@
     }
 
     void configure_output(mg::DisplayConfigurationOutputId, bool,
-                          geom::Point, size_t, MirPowerMode)
+                          geom::Point, size_t, size_t, MirPowerMode)
     {
     }
 

=== modified file 'tests/unit-tests/graphics/mesa/test_display_multi_monitor.cpp'
--- tests/unit-tests/graphics/mesa/test_display_multi_monitor.cpp	2013-12-18 02:19:19 +0000
+++ tests/unit-tests/graphics/mesa/test_display_multi_monitor.cpp	2013-12-20 13:46:31 +0000
@@ -57,12 +57,16 @@
                 if (conf_output.connected && conf_output.modes.size() > 0)
                 {
                     conf.configure_output(conf_output.id, true, geom::Point{0, 0},
-                                          conf_output.preferred_mode_index, mir_power_mode_on);
+                                          conf_output.preferred_mode_index,
+                                          conf_output.current_format_index,
+                                          mir_power_mode_on);
                 }
                 else
                 {
                     conf.configure_output(conf_output.id, false, conf_output.top_left,
-                                          conf_output.current_mode_index, mir_power_mode_on);
+                                          conf_output.current_mode_index,
+                                          conf_output.current_format_index,
+                                          mir_power_mode_on);
                 }
             });
     }
@@ -81,13 +85,17 @@
                 if (conf_output.connected && conf_output.modes.size() > 0)
                 {
                     conf.configure_output(conf_output.id, true, geom::Point{max_x, 0},
-                                          conf_output.preferred_mode_index, mir_power_mode_on);
+                                          conf_output.preferred_mode_index,
+                                          conf_output.current_format_index,
+                                          mir_power_mode_on);
                     max_x += conf_output.modes[conf_output.preferred_mode_index].size.width.as_int();
                 }
                 else
                 {
                     conf.configure_output(conf_output.id, false, conf_output.top_left,
-                                          conf_output.current_mode_index, mir_power_mode_on);
+                                          conf_output.current_mode_index,
+                                          conf_output.current_format_index,
+                                          mir_power_mode_on);
                 }
             });
     }
@@ -472,7 +480,8 @@
         [&](mg::DisplayConfigurationOutput const& conf_output)
         {
             conf->configure_output(conf_output.id, false, conf_output.top_left,
-                                   conf_output.preferred_mode_index, mir_power_mode_on);
+                                   conf_output.preferred_mode_index, 0,
+                                   mir_power_mode_on);
         });
 
     display->configure(*conf);
@@ -508,7 +517,8 @@
         [&](mg::DisplayConfigurationOutput const& conf_output)
         {
             conf->configure_output(conf_output.id, false, conf_output.top_left,
-                                   conf_output.preferred_mode_index, mir_power_mode_on);
+                                   conf_output.preferred_mode_index, 0,
+                                   mir_power_mode_on);
         });
 
     display->configure(*conf);

=== modified file 'tests/unit-tests/graphics/mesa/test_overlapping_output_grouping.cpp'
--- tests/unit-tests/graphics/mesa/test_overlapping_output_grouping.cpp	2013-12-18 02:19:19 +0000
+++ tests/unit-tests/graphics/mesa/test_overlapping_output_grouping.cpp	2013-12-20 13:46:31 +0000
@@ -89,7 +89,7 @@
     }
 
     void configure_output(mg::DisplayConfigurationOutputId, bool,
-                          geom::Point, size_t, MirPowerMode)
+                          geom::Point, size_t, size_t, MirPowerMode)
     {
     }
 

=== modified file 'tests/unit-tests/graphics/nested/test_nested_display_configuration.cpp'
--- tests/unit-tests/graphics/nested/test_nested_display_configuration.cpp	2013-09-12 21:36:55 +0000
+++ tests/unit-tests/graphics/nested/test_nested_display_configuration.cpp	2013-12-20 13:46:31 +0000
@@ -257,7 +257,7 @@
     mgn::NestedDisplayConfiguration config(build_trivial_configuration());
 
     config.configure_output(mg::DisplayConfigurationOutputId(default_output_id), true,
-        top_left, default_current_mode, mir_power_mode_on);
+        top_left, default_current_mode, default_current_output_format, mir_power_mode_on);
 
     MockOutputVisitor ov;
     EXPECT_CALL(ov, f(_)).Times(Exactly(1));
@@ -268,6 +268,7 @@
             EXPECT_EQ(true, output.used);
             EXPECT_EQ(top_left, output.top_left);
             EXPECT_EQ(0, output.current_mode_index);
+            EXPECT_EQ(0, output.current_format_index);
         });
 }
 
@@ -278,11 +279,26 @@
     mgn::NestedDisplayConfiguration config(build_trivial_configuration());
 
     EXPECT_THROW(
-        {config.configure_output(mg::DisplayConfigurationOutputId(default_output_id), true, top_left, -1, mir_power_mode_on);},
-        std::runtime_error);
-
-    EXPECT_THROW(
-        {config.configure_output(mg::DisplayConfigurationOutputId(default_output_id), true, top_left, too_big_mode_index, mir_power_mode_on);},
+        {config.configure_output(mg::DisplayConfigurationOutputId(default_output_id), true, top_left, -1, default_current_output_format, mir_power_mode_on);},
+        std::runtime_error);
+
+    EXPECT_THROW(
+        {config.configure_output(mg::DisplayConfigurationOutputId(default_output_id), true, top_left, too_big_mode_index, default_current_output_format, mir_power_mode_on);},
+        std::runtime_error);
+}
+
+TEST_F(NestedDisplayConfiguration, configure_output_rejects_invalid_format)
+{
+    geom::Point const top_left{10,20};
+    size_t const too_big_format_index = 1;
+    mgn::NestedDisplayConfiguration config(build_trivial_configuration());
+
+    EXPECT_THROW(
+        {config.configure_output(mg::DisplayConfigurationOutputId(default_output_id), true, top_left, default_current_mode, -1, mir_power_mode_on);},
+        std::runtime_error);
+
+    EXPECT_THROW(
+        {config.configure_output(mg::DisplayConfigurationOutputId(default_output_id), true, top_left, default_current_mode, too_big_format_index, mir_power_mode_on);},
         std::runtime_error);
 }
 
@@ -292,11 +308,11 @@
     mgn::NestedDisplayConfiguration config(build_trivial_configuration());
 
     EXPECT_THROW(
-        {config.configure_output(mg::DisplayConfigurationOutputId(default_output_id+1), true, top_left, default_current_mode, mir_power_mode_on);},
+        {config.configure_output(mg::DisplayConfigurationOutputId(default_output_id+1), true, top_left, default_current_mode, default_current_output_format, mir_power_mode_on);},
         std::runtime_error);
 
     EXPECT_THROW(
-        {config.configure_output(mg::DisplayConfigurationOutputId(default_output_id-1), true, top_left, default_current_mode, mir_power_mode_on);},
+        {config.configure_output(mg::DisplayConfigurationOutputId(default_output_id-1), true, top_left, default_current_mode, default_current_output_format, mir_power_mode_on);},
         std::runtime_error);
 }
 
@@ -326,7 +342,7 @@
     geom::Point const top_left{100,200};
     mgn::NestedDisplayConfiguration config(build_non_trivial_configuration());
 
-    config.configure_output(id, true, top_left, 1, mir_power_mode_on);
+    config.configure_output(id, true, top_left, 1, 2, mir_power_mode_on);
 
     MockOutputVisitor ov;
     EXPECT_CALL(ov, f(_)).Times(Exactly(3));
@@ -338,6 +354,7 @@
                 EXPECT_EQ(true, output.used);
                 EXPECT_EQ(top_left, output.top_left);
                 EXPECT_EQ(1, output.current_mode_index);
+                EXPECT_EQ(2, output.current_format_index);
             }
         });
 }

=== modified file 'tests/unit-tests/graphics/test_default_display_configuration_policy.cpp'
--- tests/unit-tests/graphics/test_default_display_configuration_policy.cpp	2013-12-18 02:19:19 +0000
+++ tests/unit-tests/graphics/test_default_display_configuration_policy.cpp	2013-12-20 13:46:31 +0000
@@ -135,8 +135,8 @@
             f(output);
     }
 
-    MOCK_METHOD5(configure_output, void(mg::DisplayConfigurationOutputId, bool,
-                                        geom::Point, size_t, MirPowerMode));
+    MOCK_METHOD6(configure_output, void(mg::DisplayConfigurationOutputId, bool,
+                                        geom::Point, size_t, size_t, MirPowerMode));
 
 private:
     static const size_t max_simultaneous_outputs_all{std::numeric_limits<size_t>::max()};
@@ -159,12 +159,14 @@
         if (output.connected && output.modes.size() > 0)
         {
             EXPECT_CALL(conf, configure_output(output.id, true, geom::Point(),
-                                               output.preferred_mode_index, _));
+                                               output.preferred_mode_index, 
+                                               output.current_format_index, _));
         }
         else
         {
             EXPECT_CALL(conf, configure_output(output.id, false, output.top_left,
-                                               output.current_mode_index, _));
+                                               output.current_mode_index,
+                                               output.current_format_index, _));
         }
     });
 
@@ -180,7 +182,7 @@
 
     conf.for_each_output([&conf](mg::DisplayConfigurationOutput const& output)
     {
-        EXPECT_CALL(conf, configure_output(output.id, _, _, _, mir_power_mode_on));
+        EXPECT_CALL(conf, configure_output(output.id, _, _, _, _, mir_power_mode_on));
     });
 
     policy.apply_to(conf);
@@ -200,10 +202,10 @@
         ++output_count;
     });
 
-    EXPECT_CALL(conf, configure_output(_, true, _, _, _))
+    EXPECT_CALL(conf, configure_output(_, true, _, _, _, _))
         .Times(AtMost(max_simultaneous_outputs));
 
-    EXPECT_CALL(conf, configure_output(_, false, _, _, _))
+    EXPECT_CALL(conf, configure_output(_, false, _, _, _, _))
         .Times(AtLeast(output_count - max_simultaneous_outputs));
 
     policy.apply_to(conf);

=== added file 'tests/unit-tests/graphics/test_pixel_format_utils.cpp'
--- tests/unit-tests/graphics/test_pixel_format_utils.cpp	1970-01-01 00:00:00 +0000
+++ tests/unit-tests/graphics/test_pixel_format_utils.cpp	2013-12-20 13:46:31 +0000
@@ -0,0 +1,79 @@
+/*
+ * 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: Andreas Pokorny <[e-mail]>
+ */
+
+#include "mir_toolkit/common.h"
+#include "mir/graphics/pixel_format_utils.h"
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+namespace mg = mir::graphics;
+TEST(MirPixelformat,contains_alpha)
+{
+    EXPECT_FALSE(mg::contains_alpha(mir_pixel_format_xbgr_8888));
+    EXPECT_FALSE(mg::contains_alpha(mir_pixel_format_bgr_888));
+    EXPECT_FALSE(mg::contains_alpha(mir_pixel_format_xrgb_8888));
+    EXPECT_FALSE(mg::contains_alpha(mir_pixel_format_xbgr_8888));
+    EXPECT_TRUE(mg::contains_alpha(mir_pixel_format_argb_8888));
+    EXPECT_TRUE(mg::contains_alpha(mir_pixel_format_abgr_8888));
+    EXPECT_FALSE(mg::contains_alpha(mir_pixel_format_invalid));
+} 
+
+TEST(MirPixelformat,red_channel_depths)
+{
+    EXPECT_EQ(8, mg::red_channel_depth(mir_pixel_format_xbgr_8888));
+    EXPECT_EQ(8, mg::red_channel_depth(mir_pixel_format_bgr_888));
+    EXPECT_EQ(8, mg::red_channel_depth(mir_pixel_format_xrgb_8888));
+    EXPECT_EQ(8, mg::red_channel_depth(mir_pixel_format_xbgr_8888));
+    EXPECT_EQ(8, mg::red_channel_depth(mir_pixel_format_argb_8888));
+    EXPECT_EQ(8, mg::red_channel_depth(mir_pixel_format_abgr_8888));
+    EXPECT_EQ(0, mg::red_channel_depth(mir_pixel_format_invalid));
+}
+
+TEST(MirPixelformat,blue_channel_depths)
+{
+    EXPECT_EQ(8, mg::blue_channel_depth(mir_pixel_format_xbgr_8888));
+    EXPECT_EQ(8, mg::blue_channel_depth(mir_pixel_format_bgr_888));
+    EXPECT_EQ(8, mg::blue_channel_depth(mir_pixel_format_xrgb_8888));
+    EXPECT_EQ(8, mg::blue_channel_depth(mir_pixel_format_xbgr_8888));
+    EXPECT_EQ(8, mg::blue_channel_depth(mir_pixel_format_argb_8888));
+    EXPECT_EQ(8, mg::blue_channel_depth(mir_pixel_format_abgr_8888));
+    EXPECT_EQ(0, mg::red_channel_depth(mir_pixel_format_invalid));
+}
+
+TEST(MirPixelformat,green_channel_depths)
+{
+    EXPECT_EQ(8, mg::green_channel_depth(mir_pixel_format_xbgr_8888));
+    EXPECT_EQ(8, mg::green_channel_depth(mir_pixel_format_bgr_888));
+    EXPECT_EQ(8, mg::green_channel_depth(mir_pixel_format_xrgb_8888));
+    EXPECT_EQ(8, mg::green_channel_depth(mir_pixel_format_xbgr_8888));
+    EXPECT_EQ(8, mg::green_channel_depth(mir_pixel_format_argb_8888));
+    EXPECT_EQ(8, mg::green_channel_depth(mir_pixel_format_abgr_8888));
+    EXPECT_EQ(0, mg::red_channel_depth(mir_pixel_format_invalid));
+}
+
+
+TEST(MirPixelformat,alpha_channel_depths)
+{
+    EXPECT_EQ(0, mg::alpha_channel_depth(mir_pixel_format_xbgr_8888));
+    EXPECT_EQ(0, mg::alpha_channel_depth(mir_pixel_format_bgr_888));
+    EXPECT_EQ(0, mg::alpha_channel_depth(mir_pixel_format_xrgb_8888));
+    EXPECT_EQ(0, mg::alpha_channel_depth(mir_pixel_format_xbgr_8888));
+    EXPECT_EQ(8, mg::alpha_channel_depth(mir_pixel_format_argb_8888));
+    EXPECT_EQ(8, mg::alpha_channel_depth(mir_pixel_format_abgr_8888));
+    EXPECT_EQ(0, mg::red_channel_depth(mir_pixel_format_invalid));
+}