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

> (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!