Code review comment for lp:~andreas-pokorny/mir/add-pixel-format-to-display-configuration

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?