Code review comment for lp:~gerboland/qtmir/qmirserver-hides-mirserver

> > In src/platforms/mirserver/display.cpp:
> >
> > """
> > -Display::Display(const QSharedPointer<MirServer> &server, QObject *parent)
> > - : QObject(parent)
> > - , m_mirServer(server)
> > +Display::Display(const QSharedPointer<MirServer> &server)
> > {
> > - std::shared_ptr<mir::graphics::DisplayConfiguration> displayConfig =
> > m_mirServer->the_display()->configuration();
> > + std::shared_ptr<mir::graphics::DisplayConfiguration> displayConfig =
> > server->the_display()->configuration();
> > """
> >
> > I think you could go further and pass the display config already in the
> > constructor instead of the whole server since that's the only thing Display
> is
> > interested in after all.
> >
> > -------------------------
> >
> > In src/platforms/mirserver/miropenglcontext.cpp:
> >
> > """
> > MirOpenGLContext::MirOpenGLContext(const QSharedPointer<MirServer> &server,
> > const QSurfaceFormat &format)
> > - : m_mirServer(server)
> > #if GL_DEBUG
> > - , m_logger(new QOpenGLDebugLogger(this))
> > + : m_logger(new QOpenGLDebugLogger(this))
> > #endif
> > {
> > - std::shared_ptr<mir::graphics::Display> display =
> > m_mirServer->the_display();
> > + std::shared_ptr<mir::graphics::Display> display =
> server->the_display();
> > """
> >
> > Likewise here.
>
>
> I had a reason for not doing that at the time, but I can't recall it now. Will
> do

I thought of the reason. It would require me to include loads of Mir internal headers in MirServerIntegration. Since qtmir::Display wants mir::graphics::DisplayConfiguration, I would need to include headers for mg::Display & mg::DisplayConfiguration.

As the multimonitor work will refactor those classes quite a bit, I decided to just keep this approach knowing that I'll improve it later.