Mir

Merge lp:~andreas-pokorny/mir/add-pixel-format-to-display-configuration into lp:mir

Proposed by Andreas Pokorny on 2014-01-02
Status: Merged
Approved by: Andreas Pokorny on 2014-01-10
Approved revision: 1314
Merged at revision: 1328
Proposed branch: lp:~andreas-pokorny/mir/add-pixel-format-to-display-configuration
Merge into: lp:mir
Prerequisite: lp:~andreas-pokorny/mir/pixel-format-utils
Diff against target: 1838 lines (+481/-326)
40 files modified
debian/control (+2/-2)
debian/libmirclient5.install (+1/-1)
examples/demo-shell/window_manager.cpp (+2/-1)
examples/server_configuration.cpp (+8/-4)
include/platform/mir/graphics/display_configuration.h (+5/-4)
include/shared/mir_toolkit/client_types.h (+1/-1)
include/test/mir_test_doubles/null_display_configuration.h (+7/-7)
include/test/mir_test_doubles/stub_display_configuration.h (+6/-5)
src/client/CMakeLists.txt (+1/-1)
src/client/display_configuration.cpp (+1/-1)
src/platform/graphics/android/android_display_configuration.cpp (+9/-3)
src/platform/graphics/android/android_display_configuration.h (+4/-3)
src/platform/graphics/mesa/real_kms_display_configuration.cpp (+21/-5)
src/platform/graphics/mesa/real_kms_display_configuration.h (+4/-4)
src/server/frontend/protobuf_buffer_packer.cpp (+1/-1)
src/server/frontend/session_mediator.cpp (+4/-1)
src/server/graphics/default_display_configuration_policy.cpp (+53/-14)
src/server/graphics/nested/nested_display.cpp (+42/-57)
src/server/graphics/nested/nested_display.h (+6/-5)
src/server/graphics/nested/nested_display_configuration.cpp (+24/-5)
src/server/graphics/nested/nested_display_configuration.h (+3/-3)
src/server/graphics/nested/nested_output.cpp (+3/-2)
src/server/graphics/nested/nested_output.h (+2/-1)
src/server/graphics/nested/nested_platform.cpp (+2/-2)
src/server/graphics/nested/nested_platform.h (+5/-5)
src/server/graphics/offscreen/display_configuration.cpp (+4/-2)
src/server/graphics/offscreen/display_configuration.h (+4/-4)
src/server/scene/mediating_display_changer.cpp (+1/-0)
tests/acceptance-tests/test_client_library.cpp (+1/-1)
tests/mir_test/display_config_matchers.cpp (+5/-5)
tests/mir_test_framework/stubbed_server_configuration.cpp (+1/-1)
tests/unit-tests/frontend/test_session_mediator.cpp (+7/-3)
tests/unit-tests/graphics/android/test_android_fb.cpp (+22/-22)
tests/unit-tests/graphics/mesa/test_cursor.cpp (+7/-7)
tests/unit-tests/graphics/mesa/test_display_configuration.cpp (+7/-7)
tests/unit-tests/graphics/mesa/test_display_multi_monitor.cpp (+16/-6)
tests/unit-tests/graphics/mesa/test_overlapping_output_grouping.cpp (+2/-2)
tests/unit-tests/graphics/nested/test_nested_display_configuration.cpp (+27/-11)
tests/unit-tests/graphics/test_default_display_configuration_policy.cpp (+159/-116)
tests/unit-tests/graphics/test_display_configuration.cpp (+1/-1)
To merge this branch: bzr merge lp:~andreas-pokorny/mir/add-pixel-format-to-display-configuration
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve on 2014-01-10
Daniel van Vugt Approve on 2014-01-10
Alan Griffiths Abstain on 2014-01-09
Robert Carr (community) 2014-01-02 Approve on 2014-01-02
Review via email: mp+200294@code.launchpad.net

This proposal supersedes a proposal from 2014-01-02.

Commit message

Adds a mir pixel format parameter to DisplayConfiguration::configure_output

So within DisplayConfigurationPolicy the format can be specified for each output.
The pixel format parameter is still ignored by android and also mesa to some degree,
which only validates it. It is supported by nested display. The default configuration
policy implementation tries to select an opaque format though.

Description of the change

Adds a mir pixel format parameter to DisplayConfiguration::configure_output

So within DisplayConfigurationPolicy the format can be specified for each output.
The pixel format parameter is still ignored by android and also mesa to some degree,
which only validates it. It is supported by nested display. The default configuration
policy implementation tries to select an opaque format though.

To post a comment you must log in.
Robert Carr (robertcarr) wrote :

LGTM. A few small comments:

1. It's a shame that all these updates to configure_output are making the diff noisy. It's not entirely clear to me why the PixelFormat needs to go through the configure_output call besides the deficiencies of the interface (no other way from frontend to the nested surfaces). Conceptually it doesn't seem to match. You aren't creating that here though.

2.
"+using namespace mir::graphics;"
253 +namespace mesa
etc...

Unless there has been a recent change Mir style is not to do this and use the aliases. Maybe we should be open to changing this. I'm not fond of using namespace, but could be happy to enclose implementations in namespace scopes in C++ files.

3. This is another thing that you aren't causing, as there seems to be a lot of inconsistency in this area of the code anyway...there are some indentation "errors" though

+ virtual void configure_output(DisplayConfigurationOutputId id, bool used, geometry::Point top_left,
+ size_t mode_index, MirPixelFormat format, MirPowerMode power_mode) = 0;

Typically mir style is instead:
+ virtual void configure_output(DisplayConfigurationOutputId id, bool used, geometry::Point top_left,
+ size_t mode_index, MirPixelFormat format, MirPowerMode power_mode) = 0;

That is to say just follow the 4 space indentation rather than align. Obviously this code isn't respecting that to start though, and I have a feeling it might be my fault so no blocking there ;)

I'll approve but can someone else comment on the namespacing before we top approve?

review: Approve
Andreas Pokorny (andreas-pokorny) wrote :

1) My first attempt introduced a separate interface that would only serve the purpose of configuring the pixel format iirc. That one got reject in favour of adding a parameter to configure_output

2) wrt: +using mir::graphics it was mg = mir::graphics initially and duflu indicated that we should avoid that style, and suggested "using". In the other file that I touched after that review I changed it to namespace mir { namespace ... { } } as a better compromise. We should discuss that topic.

3) I am still playing around with getting my vim cinoptions into the right shape. I thought I saw that alignment of the second line parameters in the mir style guide. http://unity.ubuntu.com/mir/cppguide/index.html?showone=Function_Declarations_and_Definitions#Function_Declarations_and_Definitions
But then again I was also told that those examples are wrong :)

Daniel van Vugt (vanvugt) wrote :

Re:
    namespace mg=mir::graphics;
    mg::Something
Namespace aliases are cryptic in the least. We should aim to do better. I suggest a good alternative/compromise is:
    using namespace mir;
    graphics::Something
Although if it's the only "Something" used by the source file then it's even better to reduce syntactic coupling by:
    using mir::graphics::Something;
    Something
I also accept "using namespace mir::graphics" :)

Daniel van Vugt (vanvugt) wrote :

(1) I agree in real display systems (non-nested) the pixel format is not something you choose. What you choose is a colour depth (which may or may not fully dictate the pixel format), and from that you can query what the physical pixel format is. That's if you even need it at all. In the case of OpenGL for example, you never need to know the physical display pixel format. I'm wondering if should make it more obvious that the MirPixelFormat parameter is only meaningful to nested displays?

My key concern is that although two graphics systems might look similar, one may only support RGB and the next might only support BGR. And a third might only support 16-bit colour (like i8xx GPUs). So specifying a desired pixel format is very dangerous to portability _if_ you require that the format be honored. It's not yet clear to me if we are using it in an unsafe way though.

(2) Namespaces: See previous comment

review: Needs Information
Andreas Pokorny (andreas-pokorny) wrote :

> (1) I agree in real display systems (non-nested) the pixel format is not
> something you choose. What you choose is a colour depth (which may or may not
> fully dictate the pixel format), and from that you can query what the physical
> pixel format is. That's if you even need it at all. In the case of OpenGL for
> example, you never need to know the physical display pixel format. I'm
> wondering if should make it more obvious that the MirPixelFormat parameter is
> only meaningful to nested displays?
>
> My key concern is that although two graphics systems might look similar, one
> may only support RGB and the next might only support BGR. And a third might
> only support 16-bit colour (like i8xx GPUs). So specifying a desired pixel
> format is very dangerous to portability _if_ you require that the format be
> honored. It's not yet clear to me if we are using it in an unsafe way though.

I think neither for nested nor for the real display outputs specifying a pixel format makes
sense. What is shown on the display changes dynamically based on the content provided.
So if there is content provided by a client that can be scanned out directly the
"current pixel format" at the output as an up-front configuration makes no sense.
If we can somehow leverage more hardware composting support, we might have opaque and
 non opaque RGB buffers and YUV buffers at the same time - and no notion of "current pixel format"
on an output.

As soon as we do that (=hwc) for real displays we will also do that for the nested case.
Then we should also rework NestedDisplay and NestedOutput. Those classes should only create
buffers when the nested shell request buffers for rendering.

But right now the graphical structure (as i understand them) is:
 * there is a mir server that creates a frame buffer
 * the frame buffer contains the rendering result of the shell that draws all its visible clients
and that recursively for all levels of nested servers. Bypass is just an exception through the system.

With the hwc support we discussed in December it may become:
 * there is a mir server
 * the shell decides which set of buffers define the rendering result of the server
and that recursively for all levels of nested servers. Bypass does not exist as a special case.

At that point the "current pixel format" should no longer be part of the Output Configuration.

> (2) Namespaces: See previous comment

What is bad about to opening namespace scope inside source files of the class to be implemented?

Daniel van Vugt (vanvugt) wrote :

(1) OK I don't care to argue about pixel format passing any more. If nesting needs it right now, then leave it. But I'd like to think we will eventually revisit it for a nicer solution.

(2) Nothing is bad about "using namespace" in most cpp files. In fact, it's very good to reduce the syntactic coupling and improve readability. There are some cases where it could be dangerous, but they're relatively rare.

Daniel van Vugt (vanvugt) :
review: Abstain
Daniel van Vugt (vanvugt) wrote :

(1) I know I said I would drop the issue, but now I notice this, my original concerns seem justified. We all know that EGL won't and can't honour the exact format. But passing it as a parameter could give easily the reader a false impression that we expect the given pixel format to be honoured.
680 +EGLConfig mgn::detail::EGLDisplayHandle::choose_windowed_es_config(MirPixelFormat format) const
681 {
682 + EGLint const nested_egl_config_attribs[] =
683 + {
684 + EGL_SURFACE_TYPE, EGL_WINDOW_BIT,
685 + EGL_RED_SIZE, mg::red_channel_depth(format),
686 + EGL_GREEN_SIZE, mg::green_channel_depth(format),
687 + EGL_BLUE_SIZE, mg::blue_channel_depth(format),
688 + EGL_ALPHA_SIZE, mg::alpha_channel_depth(format),
689 + EGL_RENDERABLE_TYPE, EGL_OPENGL_ES2_BIT,
690 + EGL_NONE
691 + };

(2) Back on the subject of namespaces... Although I do endorse "using namespace", introducing it in existing source files can and does inflate the diff unnecessarily (like in real_kms_display_configuration.cpp). I'm not saying change it back, but just beware in future.

(4) The comment doesn't match the code:
63 /** The index in the 'pixel_format' vector of the current output pixel format. */
64 - size_t current_format_index;
65 + MirPixelFormat current_format;

(5) Unnecessary change in indentation:
88 - std::shared_ptr<DisplayConfigurationPolicy> const& initial_conf_policy) = 0;
89 + std::shared_ptr<DisplayConfigurationPolicy> const& initial_conf_policy) = 0;

(6) This is a client ABI break, so we need to bump MIRCLIENT_ABI and debian/* info:
101 - uint32_t current_output_format;
102 + MirPixelFormat current_output_format;

(7) Don't you mean "mode_index >= modes.size()" ?
521 + if (mode_index > modes.size())
522 + return 0;

(8) Please avoid the keyword "inline". The compiler is a better judge of that. Inlining unnecessarily just leads to binary bloat.

review: Needs Fixing
Alan Griffiths (alan-griffiths) wrote :

As this seems to be the MP for an argument about namespaces I'll state my reason for a (mild) preference for introducing short aliases as done by the majority of existing code:

Assume we have a header containing "namespace example { <declaration> }"

Then in an implementation we want to provide a definition.

We can do this many ways:

1. "namespace example { <definition> }"
2. "example::<definition>"
3. "namespace eg = example; eg::<definition>"

What we can't do (as it defines a different entity in global namespace) is:

4. "using namespace example; <definition>"

In the past I've seen projects trip over the "definition is an implicit declaration" trap due to mismatched names in header and implementation. (The trap is that there is no compile-time error as nothing tells the compiler that the declaration and definition are intended to reference the same thing.)

In order to get an early (compile-time) error I prefer style 2 or 3 in these cases. That approach is largely followed in the Mir codebase.

I am aware that we often have a definition in the header (typically a class def) and the equivalent of 4 works for defining member functions. My preference is for a consistent approach over a more complex rule depending upon whether the header has a definition or a declaration or whether we are defining member functions or functions.

Andreas Pokorny (andreas-pokorny) wrote :

> (1) I know I said I would drop the issue, but now I notice this, my original
> concerns seem justified. We all know that EGL won't and can't honour the exact
> format. But passing it as a parameter could give easily the reader a false
> impression that we expect the given pixel format to be honoured.
> 680 +EGLConfig
> mgn::detail::EGLDisplayHandle::choose_windowed_es_config(MirPixelFormat
> format) const
> 681 {
> 682 + EGLint const nested_egl_config_attribs[] =
> 683 + {
> 684 + EGL_SURFACE_TYPE, EGL_WINDOW_BIT,
> 685 + EGL_RED_SIZE, mg::red_channel_depth(format),
> 686 + EGL_GREEN_SIZE, mg::green_channel_depth(format),
> 687 + EGL_BLUE_SIZE, mg::blue_channel_depth(format),
> 688 + EGL_ALPHA_SIZE, mg::alpha_channel_depth(format),
> 689 + EGL_RENDERABLE_TYPE, EGL_OPENGL_ES2_BIT,
> 690 + EGL_NONE
> 691 + };

What is your suggestion here? Make a cosmetic change and split the pixel format into four components somewhere higher in the call hierarchy? Rework the frame buffer allocation responsibility entirely? But still at some point someone requests a certain format and EGL tried to find one or more that match. The relevant piece here is that EGL_ALPHA_SIZE is zero in the right cases and non-zero in the other cases.

>
> (2) Back on the subject of namespaces... Although I do endorse "using
> namespace", introducing it in existing source files can and does inflate the
> diff unnecessarily (like in real_kms_display_configuration.cpp). I'm not
> saying change it back, but just beware in future.

That was my reaction to your complaint on mg = mir::graphics; + excessive Boy Scouting. I cannot second using using :) in most cases, but I had the impression that we should abandon the acronym-alias style - hence removed it from a file I touched afterwards.

> (4)-(8)

aye - thanks for paying so much attention!

Daniel van Vugt (vanvugt) wrote :

(1) I don't have a suggestion for something better right now, so won't block on it. Just a concern.

(2) Now you're aware of our varying views, use your own judgement. Namespace style won't usually be a blocker, so long as you're sensible (which you are).

(4)-(8) Need fixing.

(9) You have a prerequisite on a branch which is not up for review ("Work in progress"). This means some of the diff is hidden and not visible on this page. Please ensure prerequisite branches are up for review first.

review: Needs Fixing
1311. By Andreas Pokorny on 2014-01-08

Android fixes

1312. By Andreas Pokorny on 2014-01-08

merged undoing of PixelFormat wrapper

Alan Griffiths (alan-griffiths) wrote :

466 +MirPixelFormat select_format(MirPixelFormat format, std::vector<MirPixelFormat> const& formats)
467 +{
468 + if (formats.empty())
469 + return mir_pixel_format_invalid;
470 +
471 + if (!mg::contains_alpha(format))
472 + return format;
473 +
474 + auto opaque_pos = std::find_if_not(formats.begin(), formats.end(), mg::contains_alpha);
475 +
476 + if (opaque_pos == formats.end())
477 + return format;
478 +
479 + return *opaque_pos;
480 +}

From the prototype I would expect that select_format() would return a format that exists in formats (or possibly fail).

What is the intent of this function? The logic seems weird - is it right?

Is failure indicated by returning 468 mir_pixel_format_invalid as in ll468-9? (If failure is an option I'd expect an exception, if not a "sensible" default.)

If format doesn't contain an alpha component it will return format regardless of whether format is in formats (unless formats is empty per check on l468).

If format does contain alpha as then it it actually uses the contents of formats to decide which format to return: It either returns an entry in formats (as I initially expected - although it doesn't prioritize what I read as the requested format) or it returns format where I expected a failure.

So the return is one of mir_pixel_format_invalid, format, or the first alpha format from formats.

review: Needs Information
Andreas Pokorny (andreas-pokorny) wrote :

> 466 +MirPixelFormat select_format(MirPixelFormat format,
> std::vector<MirPixelFormat> const& formats)
> 467 +{
> 468 + if (formats.empty())
> 469 + return mir_pixel_format_invalid;
> 470 +
> 471 + if (!mg::contains_alpha(format))
> 472 + return format;
> 473 +
> 474 + auto opaque_pos = std::find_if_not(formats.begin(), formats.end(),
> mg::contains_alpha);
> 475 +
> 476 + if (opaque_pos == formats.end())
> 477 + return format;
> 478 +
> 479 + return *opaque_pos;
> 480 +}
>
> From the prototype I would expect that select_format() would return a format
> that exists in formats (or possibly fail).
>
> What is the intent of this function? The logic seems weird - is it right?

The idea was that default display configuration policy should try to select an opaque format where possible.
There might be multiple opaque formats, and one might be already configured - so the default policy does not
touch it.
The idea was that a decorated policy selected certain opaque format on purpose.

> Is failure indicated by returning 468 mir_pixel_format_invalid as in ll468-9?
> (If failure is an option I'd expect an exception, if not a "sensible"
> default.)

Yes, that is failure and in some graphics platforms that yields an exception (mesa, nested).
Here I was not sure how the class should behave, since all but nested ignore the value of the
configured output format.

> If format doesn't contain an alpha component it will return format regardless
> of whether format is in formats (unless formats is empty per check on l468).

Yes thats an error case that I did not handle. (It is handled in nested and mesa platform though - with an exception)

> If format does contain alpha as then it it actually uses the contents of
> formats to decide which format to return: It either returns an entry in
> formats (as I initially expected - although it doesn't prioritize what I read
> as the requested format) or it returns format where I expected a failure.

Hm if there is no opaque format in the vector it takes the preconfigured one - blindly assuming that it is part of the vector. Which would then be a format with alpha channel.

> So the return is one of mir_pixel_format_invalid, format, or the first alpha
> format from formats.

no the first non-alpha format - it uses find_if_not

Daniel van Vugt (vanvugt) wrote :

(6) There's a real danger of external projects compiling successfully with this change, and then crashing with out-of-bounds array accesses:
127 - uint32_t current_output_format;
128 + MirPixelFormat current_output_format;
because we kept the same name and both are int-compatible but have different meanings. To remove the risk of such regressions, I suggest renaming fields; something like "current_format".

review: Needs Fixing
Alan Griffiths (alan-griffiths) wrote :

> The idea was that default display configuration policy should try to select an
> opaque format where possible.
...
> Hm if there is no opaque format in the vector it takes the preconfigured one -
> blindly assuming that it is part of the vector. Which would then be a format
> with alpha channel.
>
> > So the return is one of mir_pixel_format_invalid, format, or the first alpha
> > format from formats.
>
> no the first non-alpha format - it uses find_if_not

I'd find it easier to follow like this:

MirPixelFormat select_opaque_format(MirPixelFormat format, std::vector<MirPixelFormat> const& formats)
{
    auto const format_in_formats = formats.end() != std::find(formats.begin(), formats.end(), format);

    if (!mg::contains_alpha(format) && format_in_formats)
        return format;

    // format is either unavailable or transparent
    auto const first_opaque = std::find_if_not(formats.begin(), formats.end(), mg::contains_alpha);

    if (first_opaque != formats.end())
        return *first_opaque;

    // only tranparent options - allow choice if available
    if (format_in_formats)
        return format;

    if (formats.size())
        return formats.at(0);

    return mir_pixel_format_invalid;
}

review: Abstain
1313. By Andreas Pokorny on 2014-01-09

more reasonable opaque format selection, and turned chnage into a real API change for the client_types

Andreas Pokorny (andreas-pokorny) wrote :

yes a lot better.

Daniel van Vugt (vanvugt) :
review: Abstain
Daniel van Vugt (vanvugt) wrote :

(9) Duplicate alias definition:
799 +namespace mgn = mg::nested;
800 namespace mgn = mir::graphics::nested;
But not important.

review: Approve
1314. By Andreas Pokorny on 2014-01-10

duplicate namespaces aliases removed

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/control'
2--- debian/control 2014-01-10 05:07:43 +0000
3+++ debian/control 2014-01-10 10:21:28 +0000
4@@ -128,7 +128,7 @@
5 .
6 Contains header files required to build Mir servers.
7
8-Package: libmirclient4
9+Package: libmirclient5
10 Section: libs
11 Architecture: i386 amd64 armhf arm64
12 Multi-Arch: same
13@@ -146,7 +146,7 @@
14 Architecture: i386 amd64 armhf arm64
15 Multi-Arch: same
16 Pre-Depends: ${misc:Pre-Depends}
17-Depends: libmirclient4 (= ${binary:Version}),
18+Depends: libmirclient5 (= ${binary:Version}),
19 libmirprotobuf-dev (= ${binary:Version}),
20 mircommon-dev (= ${binary:Version}),
21 ${misc:Depends},
22
23=== renamed file 'debian/libmirclient4.install' => 'debian/libmirclient5.install'
24--- debian/libmirclient4.install 2013-10-21 17:49:01 +0000
25+++ debian/libmirclient5.install 2014-01-10 10:21:28 +0000
26@@ -1,1 +1,1 @@
27-usr/lib/*/libmirclient.so.4
28+usr/lib/*/libmirclient.so.5
29
30=== modified file 'examples/demo-shell/window_manager.cpp'
31--- examples/demo-shell/window_manager.cpp 2014-01-10 03:23:02 +0000
32+++ examples/demo-shell/window_manager.cpp 2014-01-10 10:21:28 +0000
33@@ -151,8 +151,9 @@
34 power_mode = mir_power_mode_off;
35
36 conf->configure_output(output.id, output.used,
37- output.top_left,
38+ output.top_left,
39 output.current_mode_index,
40+ output.current_format,
41 power_mode);
42 });
43 display_off = !display_off;
44
45=== modified file 'examples/server_configuration.cpp'
46--- examples/server_configuration.cpp 2013-10-21 10:40:19 +0000
47+++ examples/server_configuration.cpp 2014-01-10 10:21:28 +0000
48@@ -61,14 +61,16 @@
49 available_outputs_for_card[conf_output.card_id] > 0)
50 {
51 conf.configure_output(conf_output.id, true, geom::Point{max_x, 0},
52- preferred_mode_index, mir_power_mode_on);
53+ preferred_mode_index, conf_output.current_format,
54+ mir_power_mode_on);
55 max_x += conf_output.modes[preferred_mode_index].size.width.as_int();
56 --available_outputs_for_card[conf_output.card_id];
57 }
58 else
59 {
60 conf.configure_output(conf_output.id, false, conf_output.top_left,
61- conf_output.current_mode_index, mir_power_mode_on);
62+ conf_output.current_mode_index, conf_output.current_format,
63+ mir_power_mode_on);
64 }
65 });
66 }
67@@ -88,13 +90,15 @@
68 if (!done && conf_output.connected && conf_output.modes.size() > 0)
69 {
70 conf.configure_output(conf_output.id, true, geom::Point{0, 0},
71- preferred_mode_index, mir_power_mode_on);
72+ preferred_mode_index, conf_output.current_format,
73+ mir_power_mode_on);
74 done = true;
75 }
76 else
77 {
78 conf.configure_output(conf_output.id, false, conf_output.top_left,
79- conf_output.current_mode_index, mir_power_mode_on);
80+ conf_output.current_mode_index, conf_output.current_format,
81+ mir_power_mode_on);
82 }
83 });
84 }
85
86=== modified file 'include/platform/mir/graphics/display_configuration.h'
87--- include/platform/mir/graphics/display_configuration.h 2013-12-18 02:19:19 +0000
88+++ include/platform/mir/graphics/display_configuration.h 2014-01-10 10:21:28 +0000
89@@ -22,6 +22,7 @@
90 #include "mir/int_wrapper.h"
91 #include "mir/geometry/size.h"
92 #include "mir/geometry/point.h"
93+#include "mir/graphics/pixel_format_utils.h"
94 #include "mir_toolkit/common.h"
95
96 #include <functional>
97@@ -103,8 +104,8 @@
98 geometry::Point top_left;
99 /** The index in the 'modes' vector of the current output mode. */
100 size_t current_mode_index;
101- /** The index in the 'pixel_format' vector of the current output pixel format. */
102- size_t current_format_index;
103+ /** The current output pixel format. A matching entry should be found in the 'pixel_formats' vector*/
104+ MirPixelFormat current_format;
105 /** Current power mode **/
106 MirPowerMode power_mode;
107 };
108@@ -135,8 +136,8 @@
109 virtual void for_each_output(std::function<void(DisplayConfigurationOutput const&)> f) const = 0;
110
111 /** Configures an output. */
112- virtual void configure_output(DisplayConfigurationOutputId id, bool used,
113- geometry::Point top_left, size_t mode_index, MirPowerMode power_mode) = 0;
114+ virtual void configure_output(DisplayConfigurationOutputId id, bool used, geometry::Point top_left,
115+ size_t mode_index, MirPixelFormat format, MirPowerMode power_mode) = 0;
116
117 protected:
118 DisplayConfiguration() = default;
119
120=== modified file 'include/shared/mir_toolkit/client_types.h'
121--- include/shared/mir_toolkit/client_types.h 2013-12-18 02:19:19 +0000
122+++ include/shared/mir_toolkit/client_types.h 2014-01-10 10:21:28 +0000
123@@ -230,7 +230,7 @@
124
125 uint32_t num_output_formats;
126 MirPixelFormat* output_formats;
127- uint32_t current_output_format;
128+ MirPixelFormat current_format;
129
130 uint32_t card_id;
131 uint32_t output_id;
132
133=== modified file 'include/test/mir_test_doubles/null_display_configuration.h'
134--- include/test/mir_test_doubles/null_display_configuration.h 2013-09-12 21:36:55 +0000
135+++ include/test/mir_test_doubles/null_display_configuration.h 2014-01-10 10:21:28 +0000
136@@ -29,13 +29,13 @@
137 {
138 class NullDisplayConfiguration : public graphics::DisplayConfiguration
139 {
140- void for_each_card(std::function<void(graphics::DisplayConfigurationCard const&)>) const
141- {
142- }
143- void for_each_output(std::function<void(graphics::DisplayConfigurationOutput const&)>) const
144- {
145- }
146- void configure_output(graphics::DisplayConfigurationOutputId, bool, geometry::Point, size_t, MirPowerMode) override
147+ void for_each_card(std::function<void(graphics::DisplayConfigurationCard const&)>) const override
148+ {
149+ }
150+ void for_each_output(std::function<void(graphics::DisplayConfigurationOutput const&)>) const override
151+ {
152+ }
153+ void configure_output(graphics::DisplayConfigurationOutputId, bool, geometry::Point, size_t, MirPixelFormat, MirPowerMode) override
154 {
155 }
156 };
157
158=== modified file 'include/test/mir_test_doubles/stub_display_configuration.h'
159--- include/test/mir_test_doubles/stub_display_configuration.h 2013-12-18 02:19:19 +0000
160+++ include/test/mir_test_doubles/stub_display_configuration.h 2014-01-10 10:21:28 +0000
161@@ -79,7 +79,7 @@
162 ((i % 2) == 0),
163 ((i % 2) == 1),
164 top_left,
165- mode_index, 1u,
166+ mode_index, pfs[0],
167 mir_power_mode_off
168 };
169
170@@ -107,7 +107,8 @@
171 graphics::DisplayConfigurationOutputType::vga,
172 std::vector<MirPixelFormat>{mir_pixel_format_abgr_8888},
173 {{rect.size, 60.0}},
174- 0, geometry::Size{}, true, true, rect.top_left, 0, 0, mir_power_mode_on
175+ 0, geometry::Size{}, true, true, rect.top_left, 0,
176+ mir_pixel_format_abgr_8888, mir_power_mode_on
177 };
178
179 outputs.push_back(output);
180@@ -122,13 +123,13 @@
181 cards.push_back(card);
182 }
183
184- void for_each_card(std::function<void(graphics::DisplayConfigurationCard const&)> f) const
185+ void for_each_card(std::function<void(graphics::DisplayConfigurationCard const&)> f) const override
186 {
187 for (auto const& card : cards)
188 f(card);
189 }
190
191- void for_each_output(std::function<void(graphics::DisplayConfigurationOutput const&)> f) const
192+ void for_each_output(std::function<void(graphics::DisplayConfigurationOutput const&)> f) const override
193 {
194 for (auto& disp : outputs)
195 {
196@@ -136,7 +137,7 @@
197 }
198 }
199
200- void configure_output(graphics::DisplayConfigurationOutputId, bool, geometry::Point, size_t, MirPowerMode)
201+ void configure_output(graphics::DisplayConfigurationOutputId, bool, geometry::Point, size_t, MirPixelFormat, MirPowerMode) override
202 {
203 }
204
205
206=== modified file 'src/client/CMakeLists.txt'
207--- src/client/CMakeLists.txt 2013-12-20 18:08:18 +0000
208+++ src/client/CMakeLists.txt 2014-01-10 10:21:28 +0000
209@@ -69,7 +69,7 @@
210 ${CLIENT_SOURCES}
211 )
212
213-set(MIRCLIENT_ABI 4)
214+set(MIRCLIENT_ABI 5)
215
216 set_target_properties(
217 mirclient
218
219=== modified file 'src/client/display_configuration.cpp'
220--- src/client/display_configuration.cpp 2013-12-18 02:19:19 +0000
221+++ src/client/display_configuration.cpp 2014-01-10 10:21:28 +0000
222@@ -88,7 +88,7 @@
223 {
224 output.output_formats[i] = static_cast<MirPixelFormat>(msg.pixel_format(i));
225 }
226- output.current_output_format = msg.current_format();
227+ output.current_format = static_cast<MirPixelFormat>(msg.current_format());
228
229 output.position_x = msg.position_x();
230 output.position_y = msg.position_y();
231
232=== modified file 'src/platform/graphics/android/android_display_configuration.cpp'
233--- src/platform/graphics/android/android_display_configuration.cpp 2013-12-18 02:19:19 +0000
234+++ src/platform/graphics/android/android_display_configuration.cpp 2014-01-10 10:21:28 +0000
235@@ -24,14 +24,20 @@
236 : configuration{mg::DisplayConfigurationOutputId{1},
237 mg::DisplayConfigurationCardId{0},
238 mg::DisplayConfigurationOutputType::lvds,
239- {mir_pixel_format_abgr_8888},
240+ {
241+ mir_pixel_format_abgr_8888,
242+ mir_pixel_format_bgr_888,
243+ mir_pixel_format_xbgr_8888
244+ },
245 {mg::DisplayConfigurationMode{display_size,0.0f}},
246 0,
247 geom::Size{0,0},
248 true,
249 true,
250 geom::Point{0,0},
251- 0, 0, mir_power_mode_on},
252+ 0,
253+ mir_pixel_format_abgr_8888,
254+ mir_power_mode_on},
255 card{mg::DisplayConfigurationCardId{0}, 1}
256 {
257 }
258@@ -62,7 +68,7 @@
259 f(configuration);
260 }
261
262-void mga::AndroidDisplayConfiguration::configure_output(mg::DisplayConfigurationOutputId, bool, geom::Point, size_t, MirPowerMode power_mode)
263+void mga::AndroidDisplayConfiguration::configure_output(mg::DisplayConfigurationOutputId, bool, geom::Point, size_t, MirPixelFormat, MirPowerMode power_mode)
264 {
265 configuration.power_mode = power_mode;
266 }
267
268=== modified file 'src/platform/graphics/android/android_display_configuration.h'
269--- src/platform/graphics/android/android_display_configuration.h 2013-12-18 02:19:19 +0000
270+++ src/platform/graphics/android/android_display_configuration.h 2014-01-10 10:21:28 +0000
271@@ -35,9 +35,10 @@
272
273 virtual ~AndroidDisplayConfiguration() = default;
274
275- void for_each_card(std::function<void(DisplayConfigurationCard const&)> f) const;
276- void for_each_output(std::function<void(DisplayConfigurationOutput const&)> f) const;
277- void configure_output(DisplayConfigurationOutputId, bool, geometry::Point, size_t, MirPowerMode power_mode);
278+ void for_each_card(std::function<void(DisplayConfigurationCard const&)> f) const override;
279+ void for_each_output(std::function<void(DisplayConfigurationOutput const&)> f) const override;
280+ void configure_output(DisplayConfigurationOutputId id, bool used, geometry::Point top_left,
281+ size_t mode_index, MirPixelFormat format, MirPowerMode power_mode) override;
282
283 private:
284 DisplayConfigurationOutput configuration;
285
286=== modified file 'src/platform/graphics/mesa/real_kms_display_configuration.cpp'
287--- src/platform/graphics/mesa/real_kms_display_configuration.cpp 2013-12-18 02:19:19 +0000
288+++ src/platform/graphics/mesa/real_kms_display_configuration.cpp 2014-01-10 10:21:28 +0000
289@@ -18,6 +18,7 @@
290
291 #include "real_kms_display_configuration.h"
292 #include "drm_mode_resources.h"
293+#include "mir/graphics/pixel_format_utils.h"
294
295 #include <cmath>
296 #include <limits>
297@@ -65,6 +66,11 @@
298 return static_cast<mg::DisplayConfigurationOutputType>(connector_type);
299 }
300
301+bool format_available_in_pixel_formats(MirPixelFormat format, mg::DisplayConfigurationOutput const& output)
302+{
303+ return output.pixel_formats.end() != find(output.pixel_formats.begin(), output.pixel_formats.end(), format);
304+}
305+
306 }
307
308 mgm::RealKMSDisplayConfiguration::RealKMSDisplayConfiguration(int drm_fd)
309@@ -109,7 +115,7 @@
310 void mgm::RealKMSDisplayConfiguration::configure_output(
311 DisplayConfigurationOutputId id, bool used,
312 geometry::Point top_left, size_t mode_index,
313- MirPowerMode power_mode)
314+ MirPixelFormat format, MirPowerMode power_mode)
315 {
316 auto iter = find_output_with_id(id);
317
318@@ -120,9 +126,16 @@
319 if (used && mode_index >= output.modes.size())
320 BOOST_THROW_EXCEPTION(std::runtime_error("Invalid mode_index for used output"));
321
322+ if (used && !valid_pixel_format(format))
323+ BOOST_THROW_EXCEPTION(std::runtime_error("Invalid format for used output"));
324+
325+ if (used && !format_available_in_pixel_formats(format, output))
326+ BOOST_THROW_EXCEPTION(std::runtime_error("Format not available for used output"));
327+
328 output.used = used;
329 output.top_left = top_left;
330 output.current_mode_index = mode_index;
331+ output.current_format = format;
332 output.power_mode = power_mode;
333 }
334 else
335@@ -164,7 +177,7 @@
336 DRMModeResources resources{drm_fd};
337
338 size_t max_outputs = std::min(resources.num_crtcs(), resources.num_connectors());
339- card = {mg::DisplayConfigurationCardId{0}, max_outputs};
340+ card = {DisplayConfigurationCardId{0}, max_outputs};
341
342 resources.for_each_connector([&](DRMModeConnectorUPtr connector)
343 {
344@@ -224,7 +237,8 @@
345 {
346 outputs.push_back({id, card_id, type, formats, modes, preferred_mode_index,
347 physical_size, connected, false, geom::Point(),
348- current_mode_index, 0u, mir_power_mode_on});
349+ current_mode_index, mir_pixel_format_xrgb_8888,
350+ mir_power_mode_on});
351 }
352 else
353 {
354@@ -235,11 +249,12 @@
355 output.physical_size_mm = physical_size;
356 output.connected = connected;
357 output.current_mode_index = current_mode_index;
358+ output.current_format = mir_pixel_format_xrgb_8888;
359 }
360 }
361
362 std::vector<mg::DisplayConfigurationOutput>::iterator
363-mgm::RealKMSDisplayConfiguration::find_output_with_id(DisplayConfigurationOutputId id)
364+mgm::RealKMSDisplayConfiguration::find_output_with_id(mg::DisplayConfigurationOutputId id)
365 {
366 return std::find_if(outputs.begin(), outputs.end(),
367 [id](DisplayConfigurationOutput const& output)
368@@ -249,7 +264,7 @@
369 }
370
371 std::vector<mg::DisplayConfigurationOutput>::const_iterator
372-mgm::RealKMSDisplayConfiguration::find_output_with_id(DisplayConfigurationOutputId id) const
373+mgm::RealKMSDisplayConfiguration::find_output_with_id(mg::DisplayConfigurationOutputId id) const
374 {
375 return std::find_if(outputs.begin(), outputs.end(),
376 [id](DisplayConfigurationOutput const& output)
377@@ -257,3 +272,4 @@
378 return output.id == id;
379 });
380 }
381+
382
383=== modified file 'src/platform/graphics/mesa/real_kms_display_configuration.h'
384--- src/platform/graphics/mesa/real_kms_display_configuration.h 2013-12-18 02:19:19 +0000
385+++ src/platform/graphics/mesa/real_kms_display_configuration.h 2014-01-10 10:21:28 +0000
386@@ -37,10 +37,10 @@
387 RealKMSDisplayConfiguration(RealKMSDisplayConfiguration const& conf);
388 RealKMSDisplayConfiguration& operator=(RealKMSDisplayConfiguration const& conf);
389
390- void for_each_card(std::function<void(DisplayConfigurationCard const&)> f) const;
391- void for_each_output(std::function<void(DisplayConfigurationOutput const&)> f) const;
392- void configure_output(DisplayConfigurationOutputId id, bool used,
393- geometry::Point top_left, size_t mode_index, MirPowerMode power_mode);
394+ void for_each_card(std::function<void(DisplayConfigurationCard const&)> f) const override;
395+ void for_each_output(std::function<void(DisplayConfigurationOutput const&)> f) const override;
396+ void configure_output(DisplayConfigurationOutputId id, bool used, geometry::Point top_left,
397+ size_t mode_index, MirPixelFormat format, MirPowerMode power_mode) override;
398
399 uint32_t get_kms_connector_id(DisplayConfigurationOutputId id) const;
400 size_t get_kms_mode_index(DisplayConfigurationOutputId id, size_t conf_mode_index) const;
401
402=== modified file 'src/server/frontend/protobuf_buffer_packer.cpp'
403--- src/server/frontend/protobuf_buffer_packer.cpp 2013-12-18 02:19:19 +0000
404+++ src/server/frontend/protobuf_buffer_packer.cpp 2014-01-10 10:21:28 +0000
405@@ -64,7 +64,7 @@
406 protobuf_output.set_position_x(display_output.top_left.x.as_uint32_t());
407 protobuf_output.set_position_y(display_output.top_left.y.as_uint32_t());
408 protobuf_output.set_current_mode(display_output.current_mode_index);
409- protobuf_output.set_current_format(display_output.current_format_index);
410+ protobuf_output.set_current_format(static_cast<uint32_t>(display_output.current_format));
411 protobuf_output.set_power_mode(static_cast<uint32_t>(display_output.power_mode));
412 }
413
414
415=== modified file 'src/server/frontend/session_mediator.cpp'
416--- src/server/frontend/session_mediator.cpp 2013-12-18 02:19:19 +0000
417+++ src/server/frontend/session_mediator.cpp 2014-01-10 10:21:28 +0000
418@@ -34,6 +34,7 @@
419 #include "mir/graphics/platform.h"
420 #include "mir/frontend/display_changer.h"
421 #include "mir/graphics/display_configuration.h"
422+#include "mir/graphics/pixel_format_utils.h"
423 #include "mir/graphics/platform_ipc_package.h"
424 #include "mir/frontend/client_constants.h"
425 #include "mir/frontend/event_sink.h"
426@@ -321,7 +322,9 @@
427 mg::DisplayConfigurationOutputId output_id{static_cast<int>(output.output_id())};
428 config->configure_output(output_id, output.used(),
429 geom::Point{output.position_x(), output.position_y()},
430- output.current_mode(), static_cast<MirPowerMode>(output.power_mode()));
431+ output.current_mode(),
432+ static_cast<MirPixelFormat>(output.current_format()),
433+ static_cast<MirPowerMode>(output.power_mode()));
434 }
435
436 display_changer->configure(session, config);
437
438=== modified file 'src/server/graphics/default_display_configuration_policy.cpp'
439--- src/server/graphics/default_display_configuration_policy.cpp 2013-12-18 02:19:19 +0000
440+++ src/server/graphics/default_display_configuration_policy.cpp 2014-01-10 10:21:28 +0000
441@@ -18,12 +18,52 @@
442
443 #include "default_display_configuration_policy.h"
444 #include "mir/graphics/display_configuration.h"
445+#include "mir/graphics/pixel_format_utils.h"
446
447 #include <unordered_map>
448+#include <algorithm>
449
450 namespace mg = mir::graphics;
451 namespace geom = mir::geometry;
452
453+namespace
454+{
455+size_t select_mode_index(size_t mode_index, std::vector<mg::DisplayConfigurationMode> const & modes)
456+{
457+ if (modes.empty())
458+ return std::numeric_limits<size_t>::max();
459+
460+ if (mode_index >= modes.size())
461+ return 0;
462+
463+ return mode_index;
464+}
465+
466+MirPixelFormat select_opaque_format(MirPixelFormat format, std::vector<MirPixelFormat> const& formats)
467+{
468+ auto const format_in_formats = formats.end() != std::find(formats.begin(), formats.end(), format);
469+
470+ if (!mg::contains_alpha(format) && format_in_formats)
471+ return format;
472+
473+ // format is either unavailable or transparent
474+ auto const first_opaque = std::find_if_not(formats.begin(), formats.end(), mg::contains_alpha);
475+
476+ if (first_opaque != formats.end())
477+ return *first_opaque;
478+
479+ // only tranparent options - allow choice if available
480+ if (format_in_formats)
481+ return format;
482+
483+ if (formats.size())
484+ return formats.at(0);
485+
486+ return mir_pixel_format_invalid;
487+}
488+
489+}
490+
491 void mg::DefaultDisplayConfigurationPolicy::apply_to(DisplayConfiguration& conf)
492 {
493 static MirPowerMode const default_power_state = mir_power_mode_on;
494@@ -38,23 +78,22 @@
495 conf.for_each_output(
496 [&](DisplayConfigurationOutput const& conf_output)
497 {
498- if (conf_output.connected && conf_output.modes.size() > 0 &&
499- available_outputs_for_card[conf_output.card_id] > 0)
500- {
501- size_t preferred_mode_index{conf_output.preferred_mode_index};
502- if (preferred_mode_index > conf_output.modes.size())
503- preferred_mode_index = 0;
504-
505- conf.configure_output(conf_output.id, true, geom::Point(),
506- preferred_mode_index, default_power_state);
507-
508- --available_outputs_for_card[conf_output.card_id];
509- }
510- else
511+ if (!conf_output.connected || conf_output.modes.empty() ||
512+ available_outputs_for_card[conf_output.card_id] == 0)
513 {
514 conf.configure_output(conf_output.id, false, conf_output.top_left,
515- conf_output.current_mode_index, default_power_state);
516+ conf_output.current_mode_index, conf_output.current_format,
517+ default_power_state);
518+ return;
519 }
520+
521+ size_t preferred_mode_index{select_mode_index(conf_output.preferred_mode_index, conf_output.modes)};
522+ MirPixelFormat format{select_opaque_format(conf_output.current_format, conf_output.pixel_formats)};
523+
524+ conf.configure_output(conf_output.id, true, geom::Point(), preferred_mode_index,
525+ format, default_power_state);
526+
527+ --available_outputs_for_card[conf_output.card_id];
528 });
529 }
530
531
532=== modified file 'src/server/graphics/nested/nested_display.cpp'
533--- src/server/graphics/nested/nested_display.cpp 2013-12-20 05:06:28 +0000
534+++ src/server/graphics/nested/nested_display.cpp 2014-01-10 10:21:28 +0000
535@@ -22,8 +22,10 @@
536 #include "mir_api_wrappers.h"
537
538 #include "mir/geometry/rectangle.h"
539+#include "mir/graphics/pixel_format_utils.h"
540 #include "mir/graphics/gl_context.h"
541 #include "mir/graphics/surfaceless_egl_context.h"
542+#include "mir/graphics/display_configuration_policy.h"
543 #include "host_connection.h"
544
545 #include <boost/throw_exception.hpp>
546@@ -34,45 +36,8 @@
547 namespace mgnw = mir::graphics::nested::mir_api_wrappers;
548 namespace geom = mir::geometry;
549
550-namespace
551-{
552-
553-MirPixelFormat find_opaque_surface_format(MirConnection* connection)
554-{
555- static unsigned const max_formats = 32;
556- MirPixelFormat formats[max_formats];
557- unsigned int valid_formats;
558-
559- mir_connection_get_available_surface_formats(connection, formats,
560- max_formats, &valid_formats);
561-
562- // Find an opaque surface format
563- for (auto f = formats; f != formats+valid_formats; ++f)
564- {
565- if (*f == mir_pixel_format_xbgr_8888 ||
566- *f == mir_pixel_format_xrgb_8888)
567- {
568- return *f;
569- }
570- }
571-
572- BOOST_THROW_EXCEPTION(
573- std::runtime_error("Nested Mir failed to find an opaque surface format"));
574-}
575-
576-}
577-
578-EGLint const mgn::detail::nested_egl_config_attribs[] = {
579- EGL_SURFACE_TYPE, EGL_WINDOW_BIT,
580- EGL_RED_SIZE, 8,
581- EGL_GREEN_SIZE, 8,
582- EGL_BLUE_SIZE, 8,
583- EGL_ALPHA_SIZE, 0,
584- EGL_RENDERABLE_TYPE, EGL_OPENGL_ES2_BIT,
585- EGL_NONE
586-};
587-
588-EGLint const mgn::detail::nested_egl_context_attribs[] = {
589+EGLint const mgn::detail::nested_egl_context_attribs[] =
590+{
591 EGL_CONTEXT_CLIENT_VERSION, 2,
592 EGL_NONE
593 };
594@@ -93,6 +58,8 @@
595 }
596
597 mgn::detail::EGLDisplayHandle::EGLDisplayHandle(MirConnection* connection)
598+ : egl_display(EGL_NO_DISPLAY),
599+ egl_context_(EGL_NO_CONTEXT)
600 {
601 auto const native_display = (EGLNativeDisplayType) mir_connection_get_egl_native_display(connection);
602 egl_display = eglGetDisplay(native_display);
603@@ -100,7 +67,7 @@
604 BOOST_THROW_EXCEPTION(std::runtime_error("Nested Mir Display Error: Failed to fetch EGL display."));
605 }
606
607-void mgn::detail::EGLDisplayHandle::initialize()
608+void mgn::detail::EGLDisplayHandle::initialize(MirPixelFormat format)
609 {
610 int major;
611 int minor;
612@@ -110,17 +77,28 @@
613 BOOST_THROW_EXCEPTION(std::runtime_error("Nested Mir Display Error: Failed to initialize EGL."));
614 }
615
616- egl_context_ = eglCreateContext(egl_display, choose_config(detail::nested_egl_config_attribs), EGL_NO_CONTEXT, detail::nested_egl_context_attribs);
617+ egl_context_ = eglCreateContext(egl_display, choose_windowed_es_config(format), EGL_NO_CONTEXT, detail::nested_egl_context_attribs);
618+
619 if (egl_context_ == EGL_NO_CONTEXT)
620 BOOST_THROW_EXCEPTION(std::runtime_error("Failed to create shared EGL context"));
621 }
622
623-EGLConfig mgn::detail::EGLDisplayHandle::choose_config(const EGLint attrib_list[]) const
624+EGLConfig mgn::detail::EGLDisplayHandle::choose_windowed_es_config(MirPixelFormat format) const
625 {
626+ EGLint const nested_egl_config_attribs[] =
627+ {
628+ EGL_SURFACE_TYPE, EGL_WINDOW_BIT,
629+ EGL_RED_SIZE, mg::red_channel_depth(format),
630+ EGL_GREEN_SIZE, mg::green_channel_depth(format),
631+ EGL_BLUE_SIZE, mg::blue_channel_depth(format),
632+ EGL_ALPHA_SIZE, mg::alpha_channel_depth(format),
633+ EGL_RENDERABLE_TYPE, EGL_OPENGL_ES2_BIT,
634+ EGL_NONE
635+ };
636 EGLConfig result;
637 int n;
638
639- int res = eglChooseConfig(egl_display, attrib_list, &result, 1, &n);
640+ int res = eglChooseConfig(egl_display, nested_egl_config_attribs, &result, 1, &n);
641 if ((res != EGL_TRUE) || (n != 1))
642 BOOST_THROW_EXCEPTION(std::runtime_error("Nested Mir Display Error: Failed to choose EGL configuration."));
643
644@@ -149,17 +127,18 @@
645 mgn::NestedDisplay::NestedDisplay(
646 std::shared_ptr<HostConnection> const& connection,
647 std::shared_ptr<input::EventFilter> const& event_handler,
648- std::shared_ptr<mg::DisplayReport> const& display_report) :
649+ std::shared_ptr<mg::DisplayReport> const& display_report,
650+ std::shared_ptr<mg::DisplayConfigurationPolicy> const& initial_conf_policy) :
651 connection{connection},
652 event_handler{event_handler},
653 display_report{display_report},
654 egl_display{*connection},
655- egl_pixel_format{find_opaque_surface_format(*connection)},
656 outputs{}
657 {
658- egl_display.initialize();
659- eglMakeCurrent(egl_display, EGL_NO_SURFACE, EGL_NO_SURFACE, egl_display.egl_context());
660- configure(*configuration());
661+
662+ std::shared_ptr<DisplayConfiguration> conf(configuration());
663+ initial_conf_policy->apply_to(*conf);
664+ configure(*conf);
665 }
666
667 mgn::NestedDisplay::~NestedDisplay() noexcept
668@@ -178,6 +157,14 @@
669 return std::make_shared<NestedDisplayConfiguration>(mir_connection_create_display_config(*connection));
670 }
671
672+void mgn::NestedDisplay::complete_display_initialization(MirPixelFormat format)
673+{
674+ if (egl_display.egl_context() != EGL_NO_CONTEXT) return;
675+
676+ egl_display.initialize(format);
677+ eglMakeCurrent(egl_display, EGL_NO_SURFACE, EGL_NO_SURFACE, egl_display.egl_context());
678+}
679+
680 void mgn::NestedDisplay::configure(mg::DisplayConfiguration const& configuration)
681 {
682 decltype(outputs) result;
683@@ -193,13 +180,16 @@
684 geometry::Rectangle const area{output.top_left, output.modes[output.current_mode_index].size};
685
686 auto const& egl_display_mode = output.modes[output.current_mode_index];
687+ auto const& egl_config_format = output.current_format;
688+
689+ complete_display_initialization(egl_config_format);
690
691 MirSurfaceParameters const request_params =
692 {
693 "Mir nested display",
694 egl_display_mode.size.width.as_int(),
695 egl_display_mode.size.height.as_int(),
696- egl_pixel_format,
697+ egl_config_format,
698 mir_buffer_usage_hardware,
699 static_cast<uint32_t>(output.id.as_value())
700 };
701@@ -213,7 +203,8 @@
702 egl_display,
703 mir_surface,
704 area,
705- event_handler);
706+ event_handler,
707+ output.current_format);
708 }
709 });
710
711@@ -274,13 +265,7 @@
712 return std::weak_ptr<Cursor>();
713 }
714
715-namespace
716-{
717-}
718-
719 std::unique_ptr<mg::GLContext> mgn::NestedDisplay::create_gl_context()
720 {
721- return std::unique_ptr<mg::GLContext>{new SurfacelessEGLContext(egl_display,
722- detail::nested_egl_config_attribs,
723- EGL_NO_CONTEXT)};
724+ return std::unique_ptr<mg::GLContext>{new SurfacelessEGLContext(egl_display, EGL_NO_CONTEXT)};
725 }
726
727=== modified file 'src/server/graphics/nested/nested_display.h'
728--- src/server/graphics/nested/nested_display.h 2013-12-18 02:19:19 +0000
729+++ src/server/graphics/nested/nested_display.h 2014-01-10 10:21:28 +0000
730@@ -42,6 +42,7 @@
731 {
732 class DisplayReport;
733 class DisplayBuffer;
734+class DisplayConfigurationPolicy;
735
736 namespace nested
737 {
738@@ -67,8 +68,8 @@
739 explicit EGLDisplayHandle(MirConnection* connection);
740 ~EGLDisplayHandle() noexcept;
741
742- void initialize();
743- EGLConfig choose_config(const EGLint attrib_list[]) const;
744+ void initialize(MirPixelFormat format);
745+ EGLConfig choose_windowed_es_config(MirPixelFormat format) const;
746 EGLNativeWindowType native_window(EGLConfig egl_config, MirSurface* mir_surface) const;
747 EGLContext egl_context() const;
748 operator EGLDisplay() const { return egl_display; }
749@@ -83,7 +84,6 @@
750
751 class NestedOutput;
752
753-extern EGLint const nested_egl_config_attribs[];
754 extern EGLint const nested_egl_context_attribs[];
755 }
756
757@@ -95,7 +95,8 @@
758 NestedDisplay(
759 std::shared_ptr<HostConnection> const& connection,
760 std::shared_ptr<input::EventFilter> const& event_handler,
761- std::shared_ptr<DisplayReport> const& display_report);
762+ std::shared_ptr<DisplayReport> const& display_report,
763+ std::shared_ptr<DisplayConfigurationPolicy> const& conf_policy);
764
765 ~NestedDisplay() noexcept;
766
767@@ -124,11 +125,11 @@
768 std::shared_ptr<input::EventFilter> const event_handler;
769 std::shared_ptr<DisplayReport> const display_report;
770 detail::EGLDisplayHandle egl_display;
771- MirPixelFormat const egl_pixel_format;
772
773 std::mutex outputs_mutex;
774 std::unordered_map<DisplayConfigurationOutputId, std::shared_ptr<detail::NestedOutput>> outputs;
775 DisplayConfigurationChangeHandler my_conf_change_handler;
776+ void complete_display_initialization(MirPixelFormat format);
777 };
778
779 }
780
781=== modified file 'src/server/graphics/nested/nested_display_configuration.cpp'
782--- src/server/graphics/nested/nested_display_configuration.cpp 2013-12-18 02:19:19 +0000
783+++ src/server/graphics/nested/nested_display_configuration.cpp 2014-01-10 10:21:28 +0000
784@@ -18,15 +18,26 @@
785
786 #include "nested_display_configuration.h"
787
788-namespace mg = mir::graphics;
789-namespace mgn = mg::nested;
790+#include "mir/graphics/pixel_format_utils.h"
791
792 #include <boost/throw_exception.hpp>
793
794 #include <stdexcept>
795 #include <algorithm>
796
797-namespace mgn = mir::graphics::nested;
798+
799+namespace mg = mir::graphics;
800+namespace mgn = mg::nested;
801+
802+namespace
803+{
804+bool format_valid_for_output(MirDisplayOutput const& output, MirPixelFormat format)
805+{
806+ MirPixelFormat * end = output.output_formats + output.num_output_formats;
807+ return end != std::find(output.output_formats, end, format);
808+}
809+
810+}
811
812 mgn::NestedDisplayConfiguration::NestedDisplayConfiguration(MirDisplayConfiguration* connection) :
813 display_config{connection}
814@@ -77,7 +88,7 @@
815 !!mir_output.used,
816 geometry::Point{mir_output.position_x, mir_output.position_y},
817 mir_output.current_mode,
818- mir_output.current_output_format,
819+ mir_output.current_format,
820 mir_output.power_mode
821 };
822
823@@ -86,7 +97,7 @@
824 }
825
826 void mgn::NestedDisplayConfiguration::configure_output(DisplayConfigurationOutputId id, bool used,
827- geometry::Point top_left, size_t mode_index, MirPowerMode power_mode)
828+ geometry::Point top_left, size_t mode_index, MirPixelFormat format, MirPowerMode power_mode)
829 {
830 for (auto mir_output = display_config->outputs;
831 mir_output != display_config->outputs+display_config->num_outputs;
832@@ -97,13 +108,21 @@
833 if (used && mode_index >= mir_output->num_modes)
834 BOOST_THROW_EXCEPTION(std::runtime_error("Invalid mode_index for used output"));
835
836+ if (used && !mg::valid_pixel_format(format))
837+ BOOST_THROW_EXCEPTION(std::runtime_error("Invalid format for used output"));
838+
839+ if (used && !format_valid_for_output(*mir_output, format))
840+ BOOST_THROW_EXCEPTION(std::runtime_error("Format not available for used output"));
841+
842 mir_output->used = used;
843 mir_output->position_x = top_left.x.as_uint32_t();
844 mir_output->position_y = top_left.y.as_uint32_t();
845 mir_output->current_mode = mode_index;
846+ mir_output->current_format = format;
847 mir_output->power_mode = static_cast<MirPowerMode>(power_mode);
848 return;
849 }
850 }
851 BOOST_THROW_EXCEPTION(std::runtime_error("Trying to configure invalid output"));
852 }
853+
854
855=== modified file 'src/server/graphics/nested/nested_display_configuration.h'
856--- src/server/graphics/nested/nested_display_configuration.h 2013-09-12 21:36:55 +0000
857+++ src/server/graphics/nested/nested_display_configuration.h 2014-01-10 10:21:28 +0000
858@@ -34,11 +34,11 @@
859 explicit NestedDisplayConfiguration(MirDisplayConfiguration* display_config);
860 virtual ~NestedDisplayConfiguration() noexcept;
861
862- void for_each_card(std::function<void(DisplayConfigurationCard const&)>) const;
863- void for_each_output(std::function<void(DisplayConfigurationOutput const&)>) const;
864+ void for_each_card(std::function<void(DisplayConfigurationCard const&)>) const override;
865+ void for_each_output(std::function<void(DisplayConfigurationOutput const&)>) const override;
866
867 void configure_output(DisplayConfigurationOutputId id, bool used, geometry::Point top_left, size_t mode_index,
868- MirPowerMode power_mode);
869+ MirPixelFormat format, MirPowerMode power_mode) override;
870
871 operator MirDisplayConfiguration*() const { return display_config; }
872 private:
873
874=== modified file 'src/server/graphics/nested/nested_output.cpp'
875--- src/server/graphics/nested/nested_output.cpp 2013-09-23 13:37:44 +0000
876+++ src/server/graphics/nested/nested_output.cpp 2014-01-10 10:21:28 +0000
877@@ -41,10 +41,11 @@
878 EGLDisplayHandle const& egl_display,
879 MirSurface* mir_surface,
880 geometry::Rectangle const& area,
881- std::shared_ptr<input::EventFilter> const& event_handler) :
882+ std::shared_ptr<input::EventFilter> const& event_handler,
883+ MirPixelFormat preferred_format) :
884 egl_display(egl_display),
885 mir_surface{mir_surface},
886- egl_config{egl_display.choose_config(nested_egl_config_attribs)},
887+ egl_config{egl_display.choose_windowed_es_config(preferred_format)},
888 egl_context{egl_display, eglCreateContext(egl_display, egl_config, egl_display.egl_context(), nested_egl_context_attribs)},
889 area{area.top_left, area.size},
890 event_handler{event_handler},
891
892=== modified file 'src/server/graphics/nested/nested_output.h'
893--- src/server/graphics/nested/nested_output.h 2013-09-23 13:37:44 +0000
894+++ src/server/graphics/nested/nested_output.h 2014-01-10 10:21:28 +0000
895@@ -54,7 +54,8 @@
896 EGLDisplayHandle const& egl_display,
897 MirSurface* mir_surface,
898 geometry::Rectangle const& area,
899- std::shared_ptr<input::EventFilter> const& event_handler);
900+ std::shared_ptr<input::EventFilter> const& event_handler,
901+ MirPixelFormat preferred_format);
902
903 ~NestedOutput() noexcept;
904
905
906=== modified file 'src/server/graphics/nested/nested_platform.cpp'
907--- src/server/graphics/nested/nested_platform.cpp 2013-12-18 02:19:19 +0000
908+++ src/server/graphics/nested/nested_platform.cpp 2014-01-10 10:21:28 +0000
909@@ -113,9 +113,9 @@
910 return native_platform->create_buffer_allocator(buffer_initializer);
911 }
912
913-std::shared_ptr<mg::Display> mgn::NestedPlatform::create_display(std::shared_ptr<mg::DisplayConfigurationPolicy> const& /*initial_conf_policy*/)
914+std::shared_ptr<mg::Display> mgn::NestedPlatform::create_display(std::shared_ptr<mg::DisplayConfigurationPolicy> const& conf_policy)
915 {
916- return std::make_shared<mgn::NestedDisplay>(connection, event_handler, display_report);
917+ return std::make_shared<mgn::NestedDisplay>(connection, event_handler, display_report, conf_policy);
918 }
919
920 std::shared_ptr<mg::PlatformIPCPackage> mgn::NestedPlatform::get_ipc_package()
921
922=== modified file 'src/server/graphics/nested/nested_platform.h'
923--- src/server/graphics/nested/nested_platform.h 2013-12-18 02:19:19 +0000
924+++ src/server/graphics/nested/nested_platform.h 2014-01-10 10:21:28 +0000
925@@ -42,12 +42,12 @@
926
927 ~NestedPlatform() noexcept;
928 std::shared_ptr<GraphicBufferAllocator> create_buffer_allocator(
929- std::shared_ptr<BufferInitializer> const& buffer_initializer);
930+ std::shared_ptr<BufferInitializer> const& buffer_initializer) override;
931 std::shared_ptr<Display> create_display(
932- std::shared_ptr<DisplayConfigurationPolicy> const& initial_conf_policy);
933- std::shared_ptr<PlatformIPCPackage> get_ipc_package();
934- std::shared_ptr<InternalClient> create_internal_client();
935- void fill_ipc_package(BufferIPCPacker* packer, Buffer const* Buffer) const;
936+ std::shared_ptr<DisplayConfigurationPolicy> const& initial_conf_policy) override;
937+ std::shared_ptr<PlatformIPCPackage> get_ipc_package() override;
938+ std::shared_ptr<InternalClient> create_internal_client() override;
939+ void fill_ipc_package(BufferIPCPacker* packer, Buffer const* Buffer) const override;
940 EGLNativeDisplayType egl_native_display() const;
941
942 private:
943
944=== modified file 'src/server/graphics/offscreen/display_configuration.cpp'
945--- src/server/graphics/offscreen/display_configuration.cpp 2013-12-18 02:19:19 +0000
946+++ src/server/graphics/offscreen/display_configuration.cpp 2014-01-10 10:21:28 +0000
947@@ -31,7 +31,9 @@
948 true,
949 true,
950 geom::Point{0,0},
951- 0, 0, mir_power_mode_on},
952+ 0,
953+ mir_pixel_format_xrgb_8888,
954+ mir_power_mode_on},
955 card{mg::DisplayConfigurationCardId{0}, 1}
956 {
957 }
958@@ -67,6 +69,6 @@
959 }
960
961 void mgo::DisplayConfiguration::configure_output(
962- mg::DisplayConfigurationOutputId, bool, geom::Point, size_t, MirPowerMode)
963+ mg::DisplayConfigurationOutputId, bool, geom::Point, size_t, MirPixelFormat, MirPowerMode)
964 {
965 }
966
967=== modified file 'src/server/graphics/offscreen/display_configuration.h'
968--- src/server/graphics/offscreen/display_configuration.h 2013-11-12 17:23:53 +0000
969+++ src/server/graphics/offscreen/display_configuration.h 2014-01-10 10:21:28 +0000
970@@ -33,10 +33,10 @@
971 DisplayConfiguration(DisplayConfiguration const& other);
972 DisplayConfiguration& operator=(DisplayConfiguration const& other);
973
974- void for_each_card(std::function<void(DisplayConfigurationCard const&)> f) const;
975- void for_each_output(std::function<void(DisplayConfigurationOutput const&)> f) const;
976- void configure_output(DisplayConfigurationOutputId, bool, geometry::Point,
977- size_t, MirPowerMode power_mode);
978+ void for_each_card(std::function<void(DisplayConfigurationCard const&)> f) const override;
979+ void for_each_output(std::function<void(DisplayConfigurationOutput const&)> f) const override;
980+ virtual void configure_output(DisplayConfigurationOutputId id, bool used, geometry::Point top_left,
981+ size_t mode_index, MirPixelFormat format, MirPowerMode power_mode) override;
982
983 private:
984 DisplayConfigurationOutput output;
985
986=== modified file 'src/server/scene/mediating_display_changer.cpp'
987--- src/server/scene/mediating_display_changer.cpp 2013-12-18 02:19:19 +0000
988+++ src/server/scene/mediating_display_changer.cpp 2014-01-10 10:21:28 +0000
989@@ -136,6 +136,7 @@
990 conf->configure_output(output.id, output.used,
991 output.top_left,
992 output.current_mode_index,
993+ output.current_format,
994 mir_power_mode_on);
995 }
996 });
997
998=== modified file 'tests/acceptance-tests/test_client_library.cpp'
999--- tests/acceptance-tests/test_client_library.cpp 2013-12-18 02:19:19 +0000
1000+++ tests/acceptance-tests/test_client_library.cpp 2014-01-10 10:21:28 +0000
1001@@ -885,7 +885,7 @@
1002 MirDisplayOutput* disp = &configuration->outputs[i];
1003 ASSERT_NE(nullptr, disp);
1004 EXPECT_GE(disp->num_modes, disp->current_mode);
1005- EXPECT_GE(disp->num_output_formats, disp->current_output_format);
1006+ EXPECT_GE(disp->num_output_formats, disp->current_format);
1007 }
1008
1009 mir_display_config_destroy(configuration);
1010
1011=== modified file 'tests/mir_test/display_config_matchers.cpp'
1012--- tests/mir_test/display_config_matchers.cpp 2013-12-18 02:19:19 +0000
1013+++ tests/mir_test/display_config_matchers.cpp 2014-01-10 10:21:28 +0000
1014@@ -67,7 +67,7 @@
1015 geom::Point{protobuf_output.position_x(),
1016 protobuf_output.position_y()},
1017 protobuf_output.current_mode(),
1018- protobuf_output.current_format(),
1019+ static_cast<MirPixelFormat>(protobuf_output.current_format()),
1020 static_cast<MirPowerMode>(protobuf_output.power_mode())
1021 };
1022
1023@@ -132,7 +132,7 @@
1024 geom::Point{client_output.position_x,
1025 client_output.position_y},
1026 client_output.current_mode,
1027- client_output.current_output_format,
1028+ client_output.current_format,
1029 static_cast<MirPowerMode>(client_output.power_mode)
1030 };
1031
1032@@ -162,20 +162,20 @@
1033 }
1034 }
1035
1036- void for_each_card(std::function<void(mg::DisplayConfigurationCard const&)> f) const
1037+ void for_each_card(std::function<void(mg::DisplayConfigurationCard const&)> f) const override
1038 {
1039 for (auto const& card : cards)
1040 f(card);
1041 }
1042
1043- void for_each_output(std::function<void(mg::DisplayConfigurationOutput const&)> f) const
1044+ void for_each_output(std::function<void(mg::DisplayConfigurationOutput const&)> f) const override
1045 {
1046 for (auto const& output : outputs)
1047 f(output);
1048 }
1049
1050 void configure_output(mg::DisplayConfigurationOutputId, bool,
1051- geom::Point, size_t, MirPowerMode)
1052+ geom::Point, size_t, MirPixelFormat, MirPowerMode) override
1053 {
1054 }
1055
1056
1057=== modified file 'tests/mir_test_framework/stubbed_server_configuration.cpp'
1058--- tests/mir_test_framework/stubbed_server_configuration.cpp 2014-01-07 03:30:47 +0000
1059+++ tests/mir_test_framework/stubbed_server_configuration.cpp 2014-01-10 10:21:28 +0000
1060@@ -132,7 +132,7 @@
1061 mg::DisplayConfigurationCardId{0},
1062 mg::DisplayConfigurationOutputType::vga,
1063 std::vector<MirPixelFormat>{mir_pixel_format_abgr_8888},
1064- modes, 0, geom::Size{}, true, true, geom::Point{0,0}, 0, 0, mir_power_mode_on};
1065+ modes, 0, geom::Size{}, true, true, geom::Point{0,0}, 0, mir_pixel_format_abgr_8888, mir_power_mode_on};
1066
1067 f(dummy_output_config);
1068 }
1069
1070=== modified file 'tests/unit-tests/frontend/test_session_mediator.cpp'
1071--- tests/unit-tests/frontend/test_session_mediator.cpp 2013-12-18 02:19:19 +0000
1072+++ tests/unit-tests/frontend/test_session_mediator.cpp 2014-01-10 10:21:28 +0000
1073@@ -83,7 +83,7 @@
1074 {
1075 MOCK_CONST_METHOD1(for_each_card, void(std::function<void(mg::DisplayConfigurationCard const&)>));
1076 MOCK_CONST_METHOD1(for_each_output, void(std::function<void(mg::DisplayConfigurationOutput const&)>));
1077- MOCK_METHOD5(configure_output, void(mg::DisplayConfigurationOutputId, bool, geom::Point, size_t, MirPowerMode));
1078+ MOCK_METHOD6(configure_output, void(mg::DisplayConfigurationOutputId, bool, geom::Point, size_t, MirPixelFormat, MirPowerMode));
1079 };
1080
1081 }
1082@@ -555,6 +555,8 @@
1083 bool used0 = false, used1 = true;
1084 geom::Point pt0{44,22}, pt1{3,2};
1085 size_t mode_index0 = 1, mode_index1 = 3;
1086+ MirPixelFormat format0{mir_pixel_format_invalid};
1087+ MirPixelFormat format1{mir_pixel_format_argb_8888};
1088 mg::DisplayConfigurationOutputId id0{6}, id1{3};
1089
1090 NiceMock<MockConfig> mock_display_config;
1091@@ -568,9 +570,9 @@
1092 EXPECT_CALL(*mock_display_selector, active_configuration())
1093 .InSequence(seq)
1094 .WillOnce(Return(mt::fake_shared(mock_display_config)));
1095- EXPECT_CALL(mock_display_config, configure_output(id0, used0, pt0, mode_index0, mir_power_mode_on))
1096+ EXPECT_CALL(mock_display_config, configure_output(id0, used0, pt0, mode_index0, format0, mir_power_mode_on))
1097 .InSequence(seq);
1098- EXPECT_CALL(mock_display_config, configure_output(id1, used1, pt1, mode_index1, mir_power_mode_off))
1099+ EXPECT_CALL(mock_display_config, configure_output(id1, used1, pt1, mode_index1, format1, mir_power_mode_off))
1100 .InSequence(seq);
1101 EXPECT_CALL(*mock_display_selector, configure(_,_))
1102 .InSequence(seq);
1103@@ -593,6 +595,7 @@
1104 disp0->set_position_x(pt0.x.as_uint32_t());
1105 disp0->set_position_y(pt0.y.as_uint32_t());
1106 disp0->set_current_mode(mode_index0);
1107+ disp0->set_current_format(format0);
1108 disp0->set_power_mode(static_cast<uint32_t>(mir_power_mode_on));
1109
1110 auto disp1 = configuration.add_display_output();
1111@@ -601,6 +604,7 @@
1112 disp1->set_position_x(pt1.x.as_uint32_t());
1113 disp1->set_position_y(pt1.y.as_uint32_t());
1114 disp1->set_current_mode(mode_index1);
1115+ disp1->set_current_format(format1);
1116 disp1->set_power_mode(static_cast<uint32_t>(mir_power_mode_off));
1117
1118 session_mediator.configure_display(nullptr, &configuration,
1119
1120=== modified file 'tests/unit-tests/graphics/android/test_android_fb.cpp'
1121--- tests/unit-tests/graphics/android/test_android_fb.cpp 2013-12-18 02:19:19 +0000
1122+++ tests/unit-tests/graphics/android/test_android_fb.cpp 2014-01-10 10:21:28 +0000
1123@@ -256,28 +256,28 @@
1124 configuration->for_each_output([&](mg::DisplayConfigurationOutput const& output)
1125 {
1126 configuration->configure_output(
1127- output.id, output.used, output.top_left, output.current_mode_index, mir_power_mode_on);
1128- });
1129- display.configure(*configuration);
1130-
1131- configuration->for_each_output([&](mg::DisplayConfigurationOutput const& output)
1132- {
1133- configuration->configure_output(
1134- output.id, output.used, output.top_left, output.current_mode_index, mir_power_mode_standby);
1135- });
1136- display.configure(*configuration);
1137-
1138- configuration->for_each_output([&](mg::DisplayConfigurationOutput const& output)
1139- {
1140- configuration->configure_output(
1141- output.id, output.used, output.top_left, output.current_mode_index, mir_power_mode_off);
1142- });
1143- display.configure(*configuration);
1144-
1145- configuration->for_each_output([&](mg::DisplayConfigurationOutput const& output)
1146- {
1147- configuration->configure_output(
1148- output.id, output.used, output.top_left, output.current_mode_index, mir_power_mode_suspend);
1149+ output.id, output.used, output.top_left, output.current_mode_index, output.current_format, mir_power_mode_on);
1150+ });
1151+ display.configure(*configuration);
1152+
1153+ configuration->for_each_output([&](mg::DisplayConfigurationOutput const& output)
1154+ {
1155+ configuration->configure_output(
1156+ output.id, output.used, output.top_left, output.current_mode_index, output.current_format, mir_power_mode_standby);
1157+ });
1158+ display.configure(*configuration);
1159+
1160+ configuration->for_each_output([&](mg::DisplayConfigurationOutput const& output)
1161+ {
1162+ configuration->configure_output(
1163+ output.id, output.used, output.top_left, output.current_mode_index, output.current_format, mir_power_mode_off);
1164+ });
1165+ display.configure(*configuration);
1166+
1167+ configuration->for_each_output([&](mg::DisplayConfigurationOutput const& output)
1168+ {
1169+ configuration->configure_output(
1170+ output.id, output.used, output.top_left, output.current_mode_index, output.current_format, mir_power_mode_suspend);
1171 });
1172 display.configure(*configuration);
1173 }
1174
1175=== modified file 'tests/unit-tests/graphics/mesa/test_cursor.cpp'
1176--- tests/unit-tests/graphics/mesa/test_cursor.cpp 2013-12-18 02:19:19 +0000
1177+++ tests/unit-tests/graphics/mesa/test_cursor.cpp 2014-01-10 10:21:28 +0000
1178@@ -105,7 +105,7 @@
1179 true,
1180 geom::Point{0, 0},
1181 1,
1182- 0,
1183+ mir_pixel_format_invalid,
1184 mir_power_mode_on
1185 });
1186 outputs.push_back(
1187@@ -124,33 +124,33 @@
1188 true,
1189 geom::Point{100, 50},
1190 0,
1191- 0,
1192+ mir_pixel_format_invalid,
1193 mir_power_mode_on
1194 });
1195 }
1196
1197- void for_each_card(std::function<void(mg::DisplayConfigurationCard const&)> f) const
1198+ void for_each_card(std::function<void(mg::DisplayConfigurationCard const&)> f) const override
1199 {
1200 f({card_id, outputs.size()});
1201 }
1202
1203- void for_each_output(std::function<void(mg::DisplayConfigurationOutput const&)> f) const
1204+ void for_each_output(std::function<void(mg::DisplayConfigurationOutput const&)> f) const override
1205 {
1206 for (auto const& output : outputs)
1207 f(output);
1208 }
1209
1210 void configure_output(mg::DisplayConfigurationOutputId, bool,
1211- geom::Point, size_t, MirPowerMode)
1212+ geom::Point, size_t, MirPixelFormat, MirPowerMode) override
1213 {
1214 }
1215
1216- uint32_t get_kms_connector_id(mg::DisplayConfigurationOutputId id) const
1217+ uint32_t get_kms_connector_id(mg::DisplayConfigurationOutputId id) const override
1218 {
1219 return id.as_value();
1220 }
1221
1222- size_t get_kms_mode_index(mg::DisplayConfigurationOutputId, size_t conf_mode_index) const
1223+ size_t get_kms_mode_index(mg::DisplayConfigurationOutputId, size_t conf_mode_index) const override
1224 {
1225 return conf_mode_index;
1226 }
1227
1228=== modified file 'tests/unit-tests/graphics/mesa/test_display_configuration.cpp'
1229--- tests/unit-tests/graphics/mesa/test_display_configuration.cpp 2013-12-18 02:19:19 +0000
1230+++ tests/unit-tests/graphics/mesa/test_display_configuration.cpp 2014-01-10 10:21:28 +0000
1231@@ -197,7 +197,7 @@
1232 true,
1233 geom::Point(),
1234 1,
1235- 0,
1236+ mir_pixel_format_invalid,
1237 mir_power_mode_on
1238 },
1239 {
1240@@ -212,7 +212,7 @@
1241 false,
1242 geom::Point(),
1243 std::numeric_limits<size_t>::max(),
1244- std::numeric_limits<size_t>::max(),
1245+ mir_pixel_format_invalid,
1246 mir_power_mode_on
1247 },
1248 {
1249@@ -227,7 +227,7 @@
1250 false,
1251 geom::Point(),
1252 std::numeric_limits<size_t>::max(),
1253- std::numeric_limits<size_t>::max(),
1254+ mir_pixel_format_invalid,
1255 mir_power_mode_on
1256 }
1257 };
1258@@ -381,7 +381,7 @@
1259 true,
1260 geom::Point(),
1261 1,
1262- 0,
1263+ mir_pixel_format_invalid,
1264 mir_power_mode_on
1265 },
1266 {
1267@@ -396,7 +396,7 @@
1268 false,
1269 geom::Point(),
1270 std::numeric_limits<size_t>::max(),
1271- std::numeric_limits<size_t>::max(),
1272+ mir_pixel_format_invalid,
1273 mir_power_mode_on
1274 },
1275 };
1276@@ -415,7 +415,7 @@
1277 true,
1278 geom::Point(),
1279 std::numeric_limits<size_t>::max(),
1280- std::numeric_limits<size_t>::max(),
1281+ mir_pixel_format_invalid,
1282 mir_power_mode_on
1283 },
1284 {
1285@@ -430,7 +430,7 @@
1286 false,
1287 geom::Point(),
1288 1,
1289- 0,
1290+ mir_pixel_format_invalid,
1291 mir_power_mode_on
1292 },
1293 };
1294
1295=== modified file 'tests/unit-tests/graphics/mesa/test_display_multi_monitor.cpp'
1296--- tests/unit-tests/graphics/mesa/test_display_multi_monitor.cpp 2013-12-18 02:19:19 +0000
1297+++ tests/unit-tests/graphics/mesa/test_display_multi_monitor.cpp 2014-01-10 10:21:28 +0000
1298@@ -57,12 +57,16 @@
1299 if (conf_output.connected && conf_output.modes.size() > 0)
1300 {
1301 conf.configure_output(conf_output.id, true, geom::Point{0, 0},
1302- conf_output.preferred_mode_index, mir_power_mode_on);
1303+ conf_output.preferred_mode_index,
1304+ conf_output.current_format,
1305+ mir_power_mode_on);
1306 }
1307 else
1308 {
1309 conf.configure_output(conf_output.id, false, conf_output.top_left,
1310- conf_output.current_mode_index, mir_power_mode_on);
1311+ conf_output.current_mode_index,
1312+ conf_output.current_format,
1313+ mir_power_mode_on);
1314 }
1315 });
1316 }
1317@@ -81,13 +85,17 @@
1318 if (conf_output.connected && conf_output.modes.size() > 0)
1319 {
1320 conf.configure_output(conf_output.id, true, geom::Point{max_x, 0},
1321- conf_output.preferred_mode_index, mir_power_mode_on);
1322+ conf_output.preferred_mode_index,
1323+ conf_output.current_format,
1324+ mir_power_mode_on);
1325 max_x += conf_output.modes[conf_output.preferred_mode_index].size.width.as_int();
1326 }
1327 else
1328 {
1329 conf.configure_output(conf_output.id, false, conf_output.top_left,
1330- conf_output.current_mode_index, mir_power_mode_on);
1331+ conf_output.current_mode_index,
1332+ conf_output.current_format,
1333+ mir_power_mode_on);
1334 }
1335 });
1336 }
1337@@ -472,7 +480,8 @@
1338 [&](mg::DisplayConfigurationOutput const& conf_output)
1339 {
1340 conf->configure_output(conf_output.id, false, conf_output.top_left,
1341- conf_output.preferred_mode_index, mir_power_mode_on);
1342+ conf_output.preferred_mode_index, mir_pixel_format_xrgb_8888,
1343+ mir_power_mode_on);
1344 });
1345
1346 display->configure(*conf);
1347@@ -508,7 +517,8 @@
1348 [&](mg::DisplayConfigurationOutput const& conf_output)
1349 {
1350 conf->configure_output(conf_output.id, false, conf_output.top_left,
1351- conf_output.preferred_mode_index, mir_power_mode_on);
1352+ conf_output.preferred_mode_index, mir_pixel_format_xrgb_8888,
1353+ mir_power_mode_on);
1354 });
1355
1356 display->configure(*conf);
1357
1358=== modified file 'tests/unit-tests/graphics/mesa/test_overlapping_output_grouping.cpp'
1359--- tests/unit-tests/graphics/mesa/test_overlapping_output_grouping.cpp 2013-12-18 02:19:19 +0000
1360+++ tests/unit-tests/graphics/mesa/test_overlapping_output_grouping.cpp 2014-01-10 10:21:28 +0000
1361@@ -79,7 +79,7 @@
1362 used,
1363 rect.top_left,
1364 i - 1,
1365- 0,
1366+ mir_pixel_format_invalid,
1367 mir_power_mode_on
1368 };
1369
1370@@ -89,7 +89,7 @@
1371 }
1372
1373 void configure_output(mg::DisplayConfigurationOutputId, bool,
1374- geom::Point, size_t, MirPowerMode)
1375+ geom::Point, size_t, MirPixelFormat, MirPowerMode) override
1376 {
1377 }
1378
1379
1380=== modified file 'tests/unit-tests/graphics/nested/test_nested_display_configuration.cpp'
1381--- tests/unit-tests/graphics/nested/test_nested_display_configuration.cpp 2013-09-12 21:36:55 +0000
1382+++ tests/unit-tests/graphics/nested/test_nested_display_configuration.cpp 2014-01-10 10:21:28 +0000
1383@@ -36,7 +36,7 @@
1384 uint32_t const default_current_mode = 0;
1385 uint32_t const default_num_output_formats = 0;
1386 MirPixelFormat* const default_output_formats = nullptr;
1387-uint32_t const default_current_output_format = 0;
1388+MirPixelFormat const default_current_output_format = mir_pixel_format_abgr_8888;
1389 uint32_t const default_card_id = 1;
1390 uint32_t const second_card_id = 2;
1391 uint32_t const default_output_id = 0;
1392@@ -84,7 +84,7 @@
1393 output->current_mode = 0;
1394 output->num_output_formats = NoOfFormats;
1395 output->output_formats = format_tmp;
1396- output->current_output_format = 0;
1397+ output->current_format = NoOfFormats > 0 ? formats[0] : mir_pixel_format_invalid;
1398 }
1399
1400 template<int NoOfOutputs, int NoOfModes, int NoOfFormats>
1401@@ -257,7 +257,7 @@
1402 mgn::NestedDisplayConfiguration config(build_trivial_configuration());
1403
1404 config.configure_output(mg::DisplayConfigurationOutputId(default_output_id), true,
1405- top_left, default_current_mode, mir_power_mode_on);
1406+ top_left, default_current_mode, default_current_output_format, mir_power_mode_on);
1407
1408 MockOutputVisitor ov;
1409 EXPECT_CALL(ov, f(_)).Times(Exactly(1));
1410@@ -268,6 +268,7 @@
1411 EXPECT_EQ(true, output.used);
1412 EXPECT_EQ(top_left, output.top_left);
1413 EXPECT_EQ(0, output.current_mode_index);
1414+ EXPECT_EQ(default_current_output_format, output.current_format);
1415 });
1416 }
1417
1418@@ -278,11 +279,25 @@
1419 mgn::NestedDisplayConfiguration config(build_trivial_configuration());
1420
1421 EXPECT_THROW(
1422- {config.configure_output(mg::DisplayConfigurationOutputId(default_output_id), true, top_left, -1, mir_power_mode_on);},
1423- std::runtime_error);
1424-
1425- EXPECT_THROW(
1426- {config.configure_output(mg::DisplayConfigurationOutputId(default_output_id), true, top_left, too_big_mode_index, mir_power_mode_on);},
1427+ {config.configure_output(mg::DisplayConfigurationOutputId(default_output_id), true, top_left, -1, default_current_output_format, mir_power_mode_on);},
1428+ std::runtime_error);
1429+
1430+ EXPECT_THROW(
1431+ {config.configure_output(mg::DisplayConfigurationOutputId(default_output_id), true, top_left, too_big_mode_index, default_current_output_format, mir_power_mode_on);},
1432+ std::runtime_error);
1433+}
1434+
1435+TEST_F(NestedDisplayConfiguration, configure_output_rejects_invalid_format)
1436+{
1437+ geom::Point const top_left{10,20};
1438+ mgn::NestedDisplayConfiguration config(build_trivial_configuration());
1439+
1440+ EXPECT_THROW(
1441+ {config.configure_output(mg::DisplayConfigurationOutputId(default_output_id), true, top_left, default_current_mode, mir_pixel_format_invalid, mir_power_mode_on);},
1442+ std::runtime_error);
1443+
1444+ EXPECT_THROW(
1445+ {config.configure_output(mg::DisplayConfigurationOutputId(default_output_id), true, top_left, default_current_mode, mir_pixel_formats, mir_power_mode_on);},
1446 std::runtime_error);
1447 }
1448
1449@@ -292,11 +307,11 @@
1450 mgn::NestedDisplayConfiguration config(build_trivial_configuration());
1451
1452 EXPECT_THROW(
1453- {config.configure_output(mg::DisplayConfigurationOutputId(default_output_id+1), true, top_left, default_current_mode, mir_power_mode_on);},
1454+ {config.configure_output(mg::DisplayConfigurationOutputId(default_output_id+1), true, top_left, default_current_mode, default_current_output_format, mir_power_mode_on);},
1455 std::runtime_error);
1456
1457 EXPECT_THROW(
1458- {config.configure_output(mg::DisplayConfigurationOutputId(default_output_id-1), true, top_left, default_current_mode, mir_power_mode_on);},
1459+ {config.configure_output(mg::DisplayConfigurationOutputId(default_output_id-1), true, top_left, default_current_mode, default_current_output_format, mir_power_mode_on);},
1460 std::runtime_error);
1461 }
1462
1463@@ -326,7 +341,7 @@
1464 geom::Point const top_left{100,200};
1465 mgn::NestedDisplayConfiguration config(build_non_trivial_configuration());
1466
1467- config.configure_output(id, true, top_left, 1, mir_power_mode_on);
1468+ config.configure_output(id, true, top_left, 1, mir_pixel_format_argb_8888, mir_power_mode_on);
1469
1470 MockOutputVisitor ov;
1471 EXPECT_CALL(ov, f(_)).Times(Exactly(3));
1472@@ -338,6 +353,7 @@
1473 EXPECT_EQ(true, output.used);
1474 EXPECT_EQ(top_left, output.top_left);
1475 EXPECT_EQ(1, output.current_mode_index);
1476+ EXPECT_EQ(mir_pixel_format_argb_8888, output.current_format);
1477 }
1478 });
1479 }
1480
1481=== modified file 'tests/unit-tests/graphics/test_default_display_configuration_policy.cpp'
1482--- tests/unit-tests/graphics/test_default_display_configuration_policy.cpp 2013-12-18 02:19:19 +0000
1483+++ tests/unit-tests/graphics/test_default_display_configuration_policy.cpp 2014-01-10 10:21:28 +0000
1484@@ -22,149 +22,158 @@
1485 #include <gtest/gtest.h>
1486 #include <gmock/gmock.h>
1487
1488-namespace mg = mir::graphics;
1489-namespace geom = mir::geometry;
1490+using namespace mir::graphics;
1491+using namespace mir::geometry;
1492
1493 namespace
1494 {
1495
1496-class MockDisplayConfiguration : public mg::DisplayConfiguration
1497+class MockDisplayConfiguration : public DisplayConfiguration
1498 {
1499 public:
1500- MockDisplayConfiguration(size_t max_simultaneous_outputs)
1501- : card_id{1}, max_simultaneous_outputs{max_simultaneous_outputs}
1502+ MockDisplayConfiguration(MockDisplayConfiguration && m)
1503+ : max_simultaneous_outputs{m.max_simultaneous_outputs},
1504+ outputs{std::move(m.outputs)}
1505 {
1506- /* Connected with modes */
1507- outputs.push_back(
1508- {
1509- mg::DisplayConfigurationOutputId{10},
1510- card_id,
1511- mg::DisplayConfigurationOutputType::vga,
1512- {
1513- mir_pixel_format_abgr_8888
1514- },
1515- {
1516- {geom::Size{123, 111}, 59.9},
1517- {geom::Size{123, 111}, 59.9},
1518- {geom::Size{123, 111}, 59.9}
1519- },
1520- 2,
1521- geom::Size{324, 642},
1522- true,
1523- false,
1524- geom::Point{geom::X{123}, geom::Y{343}},
1525- 1,
1526- 0,
1527- mir_power_mode_on
1528- });
1529- /* Connected without modes */
1530- outputs.push_back(
1531- {
1532- mg::DisplayConfigurationOutputId{11},
1533- card_id,
1534- mg::DisplayConfigurationOutputType::vga,
1535- {},
1536- {},
1537- std::numeric_limits<size_t>::max(),
1538- geom::Size{566, 111},
1539- true,
1540- false,
1541- geom::Point(),
1542- std::numeric_limits<size_t>::max(),
1543- std::numeric_limits<size_t>::max(),
1544- mir_power_mode_on
1545- });
1546- /* Connected with a single mode */
1547- outputs.push_back(
1548- {
1549- mg::DisplayConfigurationOutputId{12},
1550- card_id,
1551- mg::DisplayConfigurationOutputType::vga,
1552- {
1553- mir_pixel_format_abgr_8888
1554- },
1555- {
1556- {geom::Size{523, 555}, 60.0},
1557- },
1558- 0,
1559- geom::Size{324, 642},
1560- true,
1561- false,
1562- geom::Point(),
1563- 0,
1564- 0,
1565- mir_power_mode_on
1566- });
1567- /* Not connected */
1568- outputs.push_back(
1569- {
1570- mg::DisplayConfigurationOutputId{13},
1571- card_id,
1572- mg::DisplayConfigurationOutputType::vga,
1573- {
1574- mir_pixel_format_abgr_8888
1575- },
1576- {},
1577- 0,
1578- geom::Size{324, 642},
1579- false,
1580- false,
1581- geom::Point(),
1582- 1,
1583- 0,
1584- mir_power_mode_on
1585- });
1586+ }
1587
1588+ MockDisplayConfiguration(size_t max_simultaneous_outputs, std::vector<DisplayConfigurationOutput> && config)
1589+ : max_simultaneous_outputs{max_simultaneous_outputs},
1590+ outputs{config}
1591+ {
1592 if (max_simultaneous_outputs == max_simultaneous_outputs_all)
1593 max_simultaneous_outputs = outputs.size();
1594 }
1595
1596- MockDisplayConfiguration()
1597- : MockDisplayConfiguration{max_simultaneous_outputs_all}
1598- {
1599- }
1600-
1601- void for_each_card(std::function<void(mg::DisplayConfigurationCard const&)> f) const
1602- {
1603- f({card_id, max_simultaneous_outputs});
1604- }
1605-
1606- void for_each_output(std::function<void(mg::DisplayConfigurationOutput const&)> f) const
1607+ MockDisplayConfiguration(std::vector<DisplayConfigurationOutput> && config)
1608+ : MockDisplayConfiguration(max_simultaneous_outputs_all, std::move(config))
1609+ {
1610+ }
1611+
1612+ void for_each_card(std::function<void(DisplayConfigurationCard const&)> f) const
1613+ {
1614+ f({DisplayConfigurationCardId{1}, max_simultaneous_outputs});
1615+ }
1616+
1617+ void for_each_output(std::function<void(DisplayConfigurationOutput const&)> f) const
1618 {
1619 for (auto const& output : outputs)
1620 f(output);
1621 }
1622
1623- MOCK_METHOD5(configure_output, void(mg::DisplayConfigurationOutputId, bool,
1624- geom::Point, size_t, MirPowerMode));
1625+ MOCK_METHOD6(configure_output, void(DisplayConfigurationOutputId, bool,
1626+ Point, size_t, MirPixelFormat, MirPowerMode));
1627
1628+ static const size_t max_simultaneous_outputs_all{std::numeric_limits<size_t>::max()};
1629 private:
1630- static const size_t max_simultaneous_outputs_all{std::numeric_limits<size_t>::max()};
1631- mg::DisplayConfigurationCardId const card_id;
1632 size_t max_simultaneous_outputs;
1633- std::vector<mg::DisplayConfigurationOutput> outputs;
1634+ std::vector<DisplayConfigurationOutput> outputs;
1635 };
1636
1637 }
1638
1639+DisplayConfigurationOutput default_output(DisplayConfigurationOutputId id)
1640+{
1641+ return { id, DisplayConfigurationCardId{1},
1642+ DisplayConfigurationOutputType::vga,
1643+ {mir_pixel_format_abgr_8888},
1644+ { {Size{523, 555}, 60.0} },
1645+ 0,
1646+ Size{324, 642},
1647+ true,
1648+ false,
1649+ Point{X{123}, Y{343}},
1650+ 0,
1651+ mir_pixel_format_abgr_8888,
1652+ mir_power_mode_on
1653+ };
1654+}
1655+
1656+DisplayConfigurationOutput connected_with_modes()
1657+{
1658+ DisplayConfigurationOutput output = default_output(DisplayConfigurationOutputId{10}) ;
1659+ output.modes =
1660+ {
1661+ {Size{123, 111}, 59.9},
1662+ {Size{123, 111}, 59.9},
1663+ {Size{123, 111}, 59.9}
1664+ };
1665+ output.preferred_mode_index = 2;
1666+ output.current_mode_index = 1;
1667+ return output;
1668+}
1669+DisplayConfigurationOutput connected_without_modes()
1670+{
1671+ DisplayConfigurationOutput output = default_output(DisplayConfigurationOutputId{11});
1672+ output.pixel_formats = {};
1673+ output.modes = {};
1674+ output.current_format = mir_pixel_format_invalid;
1675+ output.current_mode_index = std::numeric_limits<size_t>::max();
1676+ return output;
1677+}
1678+
1679+DisplayConfigurationOutput connected_with_single_mode()
1680+{
1681+ return default_output(DisplayConfigurationOutputId{12});
1682+}
1683+
1684+DisplayConfigurationOutput not_connected()
1685+{
1686+ DisplayConfigurationOutput output = default_output(DisplayConfigurationOutputId{13});
1687+ output.connected = false;
1688+ output.current_mode_index = 1;
1689+ return output;
1690+}
1691+
1692+DisplayConfigurationOutput connected_with_rgba_and_xrgb()
1693+{
1694+ DisplayConfigurationOutput output = default_output(DisplayConfigurationOutputId{14});
1695+ output.pixel_formats = {mir_pixel_format_argb_8888, mir_pixel_format_xrgb_8888};
1696+ return output;
1697+}
1698+
1699+DisplayConfigurationOutput connected_with_xrgb_bgr()
1700+{
1701+ DisplayConfigurationOutput output = default_output(DisplayConfigurationOutputId{15});
1702+ output.pixel_formats = {mir_pixel_format_xrgb_8888, mir_pixel_format_bgr_888};
1703+ output.current_format = mir_pixel_format_bgr_888;
1704+ return output;
1705+}
1706+
1707+MockDisplayConfiguration create_default_configuration(size_t max_outputs = MockDisplayConfiguration::max_simultaneous_outputs_all)
1708+{
1709+ return MockDisplayConfiguration
1710+ {
1711+ max_outputs,
1712+ {
1713+ connected_with_modes(),
1714+ connected_without_modes(),
1715+ connected_with_single_mode(),
1716+ not_connected(),
1717+ }
1718+ };
1719+}
1720+
1721 TEST(DefaultDisplayConfigurationPolicyTest, uses_all_connected_valid_outputs)
1722 {
1723 using namespace ::testing;
1724
1725- mg::DefaultDisplayConfigurationPolicy policy;
1726- MockDisplayConfiguration conf;
1727+ DefaultDisplayConfigurationPolicy policy;
1728+ MockDisplayConfiguration conf{create_default_configuration()};
1729
1730- conf.for_each_output([&conf](mg::DisplayConfigurationOutput const& output)
1731+ conf.for_each_output([&conf](DisplayConfigurationOutput const& output)
1732 {
1733 if (output.connected && output.modes.size() > 0)
1734 {
1735- EXPECT_CALL(conf, configure_output(output.id, true, geom::Point(),
1736- output.preferred_mode_index, _));
1737+ EXPECT_CALL(conf, configure_output(output.id, true, Point(),
1738+ output.preferred_mode_index,
1739+ _, _));
1740 }
1741 else
1742 {
1743 EXPECT_CALL(conf, configure_output(output.id, false, output.top_left,
1744- output.current_mode_index, _));
1745+ output.current_mode_index,
1746+ _, _));
1747 }
1748 });
1749
1750@@ -175,12 +184,12 @@
1751 {
1752 using namespace ::testing;
1753
1754- mg::DefaultDisplayConfigurationPolicy policy;
1755- MockDisplayConfiguration conf;
1756+ DefaultDisplayConfigurationPolicy policy;
1757+ MockDisplayConfiguration conf{create_default_configuration()};
1758
1759- conf.for_each_output([&conf](mg::DisplayConfigurationOutput const& output)
1760+ conf.for_each_output([&conf](DisplayConfigurationOutput const& output)
1761 {
1762- EXPECT_CALL(conf, configure_output(output.id, _, _, _, mir_power_mode_on));
1763+ EXPECT_CALL(conf, configure_output(output.id, _, _, _, _, mir_power_mode_on));
1764 });
1765
1766 policy.apply_to(conf);
1767@@ -191,20 +200,54 @@
1768 using namespace ::testing;
1769
1770 size_t const max_simultaneous_outputs{1};
1771- mg::DefaultDisplayConfigurationPolicy policy;
1772- MockDisplayConfiguration conf{max_simultaneous_outputs};
1773+ DefaultDisplayConfigurationPolicy policy;
1774+ MockDisplayConfiguration conf{create_default_configuration(max_simultaneous_outputs)};
1775
1776 size_t output_count{0};
1777- conf.for_each_output([&output_count](mg::DisplayConfigurationOutput const&)
1778+ conf.for_each_output([&output_count](DisplayConfigurationOutput const&)
1779 {
1780 ++output_count;
1781 });
1782
1783- EXPECT_CALL(conf, configure_output(_, true, _, _, _))
1784+ EXPECT_CALL(conf, configure_output(_, true, _, _, _, _))
1785 .Times(AtMost(max_simultaneous_outputs));
1786
1787- EXPECT_CALL(conf, configure_output(_, false, _, _, _))
1788+ EXPECT_CALL(conf, configure_output(_, false, _, _, _, _))
1789 .Times(AtLeast(output_count - max_simultaneous_outputs));
1790
1791 policy.apply_to(conf);
1792 }
1793+
1794+TEST(DefaultDisplayConfigurationPolicyTest, prefer_opaque_over_alpha)
1795+{
1796+ using namespace ::testing;
1797+
1798+ DefaultDisplayConfigurationPolicy policy;
1799+ MockDisplayConfiguration pick_xrgb{ { connected_with_rgba_and_xrgb() } };
1800+
1801+ EXPECT_CALL(pick_xrgb, configure_output(_, true, _, _, mir_pixel_format_xrgb_8888, _));
1802+ policy.apply_to(pick_xrgb);
1803+}
1804+
1805+TEST(DefaultDisplayConfigurationPolicyTest, preserve_opaque_selection)
1806+{
1807+ using namespace ::testing;
1808+
1809+ DefaultDisplayConfigurationPolicy policy;
1810+ MockDisplayConfiguration keep_bgr{ { connected_with_xrgb_bgr() } };
1811+
1812+ EXPECT_CALL(keep_bgr, configure_output(_, true, _, _, mir_pixel_format_bgr_888, _));
1813+ policy.apply_to(keep_bgr);
1814+}
1815+
1816+TEST(DefaultDisplayConfigurationPolicyTest, accept_transparency_when_only_option)
1817+{
1818+ using namespace ::testing;
1819+
1820+ DefaultDisplayConfigurationPolicy policy;
1821+ MockDisplayConfiguration pick_rgba{ { default_output(DisplayConfigurationOutputId{15}) } };
1822+
1823+ EXPECT_CALL(pick_rgba, configure_output(_, true, _, _, mir_pixel_format_abgr_8888, _));
1824+ policy.apply_to(pick_rgba);
1825+}
1826+
1827
1828=== modified file 'tests/unit-tests/graphics/test_display_configuration.cpp'
1829--- tests/unit-tests/graphics/test_display_configuration.cpp 2013-12-18 02:19:19 +0000
1830+++ tests/unit-tests/graphics/test_display_configuration.cpp 2014-01-10 10:21:28 +0000
1831@@ -45,7 +45,7 @@
1832 true,
1833 geom::Point(),
1834 2,
1835- 0,
1836+ mir_pixel_format_abgr_8888,
1837 mir_power_mode_on
1838 };
1839

Subscribers

People subscribed via source and target branches