Code review comment for lp:~andreas-pokorny/mir/allow-transparent-server-buffers

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

Don't add header files to the sources.

~~~~

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

Naming convention: availableFormats => available_formats

~~~~

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

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

~~~~

88 + for(auto const& f : availableFormats )

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

~~~~

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

~~~~

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

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