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

Revision history for this message
Gerry Boland (gerboland) wrote :

> 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

« Back to merge proposal