Merge lp:~alan-griffiths/miral/tidy-SurfaceObserver-part1 into lp:miral

Proposed by Alan Griffiths on 2016-10-27
Status: Rejected
Rejected by: Alan Griffiths on 2016-10-31
Proposed branch: lp:~alan-griffiths/miral/tidy-SurfaceObserver-part1
Merge into: lp:miral
Diff against target: 152 lines (+0/-92)
3 files modified
miral-qt/src/modules/Unity/Application/mirsurface.cpp (+0/-1)
miral-qt/src/platforms/mirserver/surfaceobserver.cpp (+0/-75)
miral-qt/src/platforms/mirserver/surfaceobserver.h (+0/-16)
To merge this branch: bzr merge lp:~alan-griffiths/miral/tidy-SurfaceObserver-part1
Reviewer Review Type Date Requested Status
Daniel d'Andrada (community) 2016-10-27 Needs Information on 2016-10-28
Review via email: mp+309496@code.launchpad.net

Commit message

[miral-qt] Tidy up dead and purposeless code in SurfaceObserver

To post a comment you must log in.
422. By Alan Griffiths on 2016-10-27

Delete pointless code

Gerry Boland (gerboland) wrote :

-void SurfaceObserver::notifySurfaceModifications(const mir::shell::SurfaceSpecification &modifications)
I'm only happy to proceed with this removal when we have the miral-based equivalent to this ready to land. Otherwise it will upset Daniels work integrating miral-qt with unity8.

Alan Griffiths (alan-griffiths) wrote :

> -void SurfaceObserver::notifySurfaceModifications(const
> mir::shell::SurfaceSpecification &modifications)
> I'm only happy to proceed with this removal when we have the miral-based
> equivalent to this ready to land. Otherwise it will upset Daniels work
> integrating miral-qt with unity8.

It is called by Daniel's code? Where's that?

Daniel d'Andrada (dandrader) wrote :

In qtmir, SurfaceObserver::notifySurfaceModifications is called by src/platforms/mirserver/mirwindowmanager.cpp

There's no mirwindowmanager.cpp in miral-qt anymore, so this code is already broken here.

Question now is when did it get removed and what replaces it.

review: Needs Information
Alan Griffiths (alan-griffiths) wrote :

> In qtmir, SurfaceObserver::notifySurfaceModifications is called by
> src/platforms/mirserver/mirwindowmanager.cpp
>
> There's no mirwindowmanager.cpp in miral-qt anymore, so this code is already
> broken here.
>
> Question now is when did it get removed and what replaces it.

Not sure when it got removed, but it looks like these notifications should be generated from WindowManagementPolicy::handle_modify_window() invoking WindowModelNotifier.

In any case, I think the problem is pre-existing and shouldn't block this MP. I'll take the action of creating new wiring to Q_EMIT the signals this generated.

Daniel d'Andrada (dandrader) wrote :

On 31/10/2016 08:07, Alan Griffiths wrote:
>> In qtmir, SurfaceObserver::notifySurfaceModifications is called by
>> src/platforms/mirserver/mirwindowmanager.cpp
>>
>> There's no mirwindowmanager.cpp in miral-qt anymore, so this code is already
>> broken here.
>>
>> Question now is when did it get removed and what replaces it.
> Not sure when it got removed, but it looks like these notifications should be generated from WindowManagementPolicy::handle_modify_window() invoking WindowModelNotifier.
>
> In any case, I think the problem is pre-existing and shouldn't block this MP. I'll take the action of creating new wiring to Q_EMIT the signals this generated.

Indeed. Thanks!

Unmerged revisions

422. By Alan Griffiths on 2016-10-27

Delete pointless code

421. By Alan Griffiths on 2016-10-27

Delete dead code

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'miral-qt/src/modules/Unity/Application/mirsurface.cpp'
2--- miral-qt/src/modules/Unity/Application/mirsurface.cpp 2016-10-12 15:49:08 +0000
3+++ miral-qt/src/modules/Unity/Application/mirsurface.cpp 2016-10-27 16:26:37 +0000
4@@ -89,7 +89,6 @@
5 {
6 DEBUG_MSG << "()";
7
8- SurfaceObserver::registerObserverForSurface(m_surfaceObserver.get(), m_surface.get());
9 m_surface->add_observer(m_surfaceObserver);
10
11 //m_shellChrome = creationHints.shellChrome; TODO - where will this come from now?
12
13=== modified file 'miral-qt/src/platforms/mirserver/surfaceobserver.cpp'
14--- miral-qt/src/platforms/mirserver/surfaceobserver.cpp 2016-09-23 15:56:53 +0000
15+++ miral-qt/src/platforms/mirserver/surfaceobserver.cpp 2016-10-27 16:26:37 +0000
16@@ -29,25 +29,6 @@
17 #include <mir/shell/surface_specification.h>
18
19
20-namespace {
21-
22-QRect calculateBoundingRect(const std::vector<mir::geometry::Rectangle> &rectVector)
23-{
24- QRect boundingRect;
25- for (auto mirRect : rectVector) {
26- boundingRect |= QRect(mirRect.top_left.x.as_int(),
27- mirRect.top_left.y.as_int(),
28- mirRect.size.width.as_int(),
29- mirRect.size.height.as_int());
30- }
31- return boundingRect;
32-}
33-
34-} // anonymous namespace
35-
36-QHash<const mir::scene::Surface*, SurfaceObserver*> SurfaceObserver::m_surfaceToObserverMap;
37-QMutex SurfaceObserver::mutex;
38-
39 SurfaceObserver::SurfaceObserver()
40 : m_listener(nullptr)
41 , m_framesPosted(false)
42@@ -99,15 +80,6 @@
43
44 SurfaceObserver::~SurfaceObserver()
45 {
46- QMutexLocker locker(&mutex);
47- QMutableHashIterator<const mir::scene::Surface*, SurfaceObserver*> i(m_surfaceToObserverMap);
48- while (i.hasNext()) {
49- i.next();
50- if (i.value() == this) {
51- i.remove();
52- return;
53- }
54- }
55 }
56
57 void SurfaceObserver::setListener(QObject *listener)
58@@ -154,53 +126,6 @@
59 Q_EMIT resized(QSize(size.width.as_int(), size.height.as_int()));
60 }
61
62-void SurfaceObserver::notifySurfaceModifications(const mir::shell::SurfaceSpecification &modifications)
63-{
64- if (modifications.min_width.is_set()) {
65- Q_EMIT minimumWidthChanged(modifications.min_width.value().as_int());
66- }
67- if (modifications.min_height.is_set()) {
68- Q_EMIT minimumHeightChanged(modifications.min_height.value().as_int());
69- }
70- if (modifications.max_width.is_set()) {
71- Q_EMIT maximumWidthChanged(modifications.max_width.value().as_int());
72- }
73- if (modifications.max_height.is_set()) {
74- Q_EMIT maximumHeightChanged(modifications.max_height.value().as_int());
75- }
76- if (modifications.width_inc.is_set()) {
77- Q_EMIT widthIncrementChanged(modifications.width_inc.value().as_int());
78- }
79- if (modifications.height_inc.is_set()) {
80- Q_EMIT heightIncrementChanged(modifications.height_inc.value().as_int());
81- }
82- if (modifications.shell_chrome.is_set()) {
83- Q_EMIT shellChromeChanged(modifications.shell_chrome.value());
84- }
85- if (modifications.input_shape.is_set()) {
86- QRect qRect = calculateBoundingRect(modifications.input_shape.value());
87- Q_EMIT inputBoundsChanged(qRect);
88- }
89- if (modifications.confine_pointer.is_set()) {
90- Q_EMIT confinesMousePointerChanged(modifications.confine_pointer.value() == mir_pointer_confined_to_surface);
91- }
92-}
93-
94-SurfaceObserver *SurfaceObserver::observerForSurface(const mir::scene::Surface *surface)
95-{
96- if (m_surfaceToObserverMap.contains(surface)) {
97- return m_surfaceToObserverMap.value(surface);
98- } else {
99- return nullptr;
100- }
101-}
102-
103-void SurfaceObserver::registerObserverForSurface(SurfaceObserver *observer, const mir::scene::Surface *surface)
104-{
105- QMutexLocker locker(&mutex);
106- m_surfaceToObserverMap[surface] = observer;
107-}
108-
109 void SurfaceObserver::cursor_image_set_to(const mir::graphics::CursorImage &cursorImage)
110 {
111 QCursor qcursor = createQCursorFromMirCursorImage(cursorImage);
112
113=== modified file 'miral-qt/src/platforms/mirserver/surfaceobserver.h'
114--- miral-qt/src/platforms/mirserver/surfaceobserver.h 2016-09-23 15:56:53 +0000
115+++ miral-qt/src/platforms/mirserver/surfaceobserver.h 2016-10-27 16:26:37 +0000
116@@ -27,15 +27,6 @@
117 #include <mir/scene/surface_observer.h>
118 #include <mir/version.h>
119
120-namespace mir {
121- namespace scene {
122- class Surface;
123- }
124- namespace shell {
125- struct SurfaceSpecification;
126- }
127-}
128-
129 class SurfaceObserver : public QObject, public mir::scene::SurfaceObserver
130 {
131 Q_OBJECT
132@@ -69,12 +60,6 @@
133 void placed_relative(mir::geometry::Rectangle const& placement) override;
134 #endif
135
136- void notifySurfaceModifications(const mir::shell::SurfaceSpecification&);
137-
138- static SurfaceObserver *observerForSurface(const mir::scene::Surface *surface);
139- static void registerObserverForSurface(SurfaceObserver *observer, const mir::scene::Surface *surface);
140- static QMutex mutex;
141-
142 Q_SIGNALS:
143 void attributeChanged(const MirSurfaceAttrib attribute, const int value);
144 void framesPosted();
145@@ -98,7 +83,6 @@
146 QObject *m_listener;
147 bool m_framesPosted;
148 QMap<QByteArray, Qt::CursorShape> m_cursorNameToShape;
149- static QHash<const mir::scene::Surface*, SurfaceObserver*> m_surfaceToObserverMap;
150 };
151
152 #endif

Subscribers

People subscribed via source and target branches