Code review comment for lp:~dandrader/qtmir/mousePointer

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

Ok ignore my relativeMovement thing. I'll not object to the usual terminology.

However I fear I need to ask you to rebase on top of
https://code.launchpad.net/~gerboland/qtmir/multimonitor/+merge/269906
as there's a few conflicts, and the concept of multiple qwindows does make this more complex.

+ // We will draw our own cursor.
+ add_init_callback([this](){ the_cursor()->hide(); });
This isn't great, as the mir cursor object is still being created. Can we replace Mir's implementation with our own one? -- not a blocker on this MR, can consider this later.

++ src/platforms/mirserver/mirsingleton.cpp
+qtmir::Mir::~Mir()
+{
+ m_instance = nullptr;
+}
You're not deleting what you potentially "new"ed. QScopedPointer helps prevent such accidents...

+++ src/platforms/mirserver/mirsingleton.h
+private:
+ Mir();
maybe http://doc.qt.io/qt-5/qobject.html#Q_DISABLE_COPY

Using this Mir singleton to save the cursorName will do fine for now, but I'm wary of it being a dumping ground for lots of little things.

review: Needs Fixing

« Back to merge proposal