QtMir

Merge lp:~mir-team/qtmir/compatibility-with-mir-API-changes into lp:qtmir

  1. compatibility-with-mir-API-changes
  2. Merge into trunk
Proposed by Alan Griffiths
Status: Rejected
Rejected by: Alan Griffiths
Proposed branch: lp:~mir-team/qtmir/compatibility-with-mir-API-changes
Merge into: lp:qtmir
Diff against target: 259 lines (+49/-20)
13 files modified
CMakeLists.txt (+2/-1)
debian/control (+2/-2)
src/modules/Unity/Application/CMakeLists.txt (+0/-1)
src/modules/Unity/Application/application_manager.cpp (+7/-4)
src/platforms/mirserver/CMakeLists.txt (+1/-1)
src/platforms/mirserver/displaywindow.cpp (+16/-2)
src/platforms/mirserver/displaywindow.h (+3/-1)
src/platforms/mirserver/mirserver.h (+0/-1)
src/platforms/mirserver/mirserverintegration.cpp (+11/-4)
src/platforms/mirserver/mirshell.cpp (+3/-1)
src/platforms/mirserver/surfaceobserver.h (+1/-0)
tests/modules/common/mock_surface.h (+2/-1)
tests/modules/common/stub_scene_surface.h (+1/-1)
To merge this branch: bzr merge lp:~mir-team/qtmir/compatibility-with-mir-API-changes
Reviewer Review Type Date Requested Status
Gerry Boland (community) code Needs Fixing
PS Jenkins bot (community) continuous-integration Needs Fixing
Review via email: mp+252589@code.launchpad.net

Commit message

Compatibility with recent API changes in Mir development

Description of the change

Compatibility with recent API changes in Mir development

To post a comment you must log in.
Revision history for this message
Gerry Boland (gerboland) wrote :

- g++-4.9:native,
+# g++-4.9:native,
oopsie :)

- m_displayBuffer->flip();
+ m_displayGroup->post();
so this exposes a QtMir architecture problem now, as DisplayWindow is supposed to wrap a mg::DisplayBuffer. We use Qt's multithreaded renderer, where each DisplayWindow is rendered to relatively independently, and post() called also individually.

But in multimonitor case where a DisplaySyncGroup contains 2 DisplayBuffers, one post() call will submit both mg::DisplayBuffers for flipping, which can happen before the other DisplayWindow has been rendered to, causing visual artifacts. I suggest a quick TODO/FIXME, just to point this out.

Rest looks ok

review: Needs Fixing
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:326
http://jenkins.qa.ubuntu.com/job/qtmir-ci/234/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-amd64-ci/85/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/85/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/qtmir-ci/234/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:327
http://jenkins.qa.ubuntu.com/job/qtmir-ci/235/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-amd64-ci/86/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/86/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/qtmir-ci/235/rebuild

review: Needs Fixing (continuous-integration)
lp:~mir-team/qtmir/compatibility-with-mir-API-changes updated
273. By Daniel van Vugt

Merge latest trunk. Now identical.

274. By Daniel van Vugt

Merge branch lp:~mir-team/qtmir/compatibility-with-mir-API-changes

275. By Daniel van Vugt

Link to libmirclient too. That's where all mir_* client (and input
functions) now correctly reside.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:331
http://jenkins.qa.ubuntu.com/job/qtmir-ci/242/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-amd64-ci/93/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/qtmir-vivid-armhf-ci/93/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/qtmir-ci/242/rebuild

review: Needs Fixing (continuous-integration)
lp:~mir-team/qtmir/compatibility-with-mir-API-changes updated
276. By Daniel van Vugt

Migrate to MirEvent 2.1 API, coming in Mir 0.13

277. By Daniel van Vugt

Update mock headers so they can build again, following racarr's
cursor BufferStream changes.

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

<pedantic>
+ [&view_area](mir::graphics::DisplaySyncGroup& group) {
please use "&group" - i.e. reference qualifier attached to the variable name

+ if (!first_group) first_group = &group;
+ if (!first_buffer) first_buffer = &buffer;
Qt coding style prefers:
if (bool)
    operation();
or
if (bool) {
    operation();
}

for_each_display_sync_group([&](mg::DisplaySyncGroup& group)
"&group" again

+ void keymap_changed(xkb_rule_names const&) override {}
"const xkb_rule_names &" please. Same for set_keymap.
</pedantic>

review: Needs Fixing (code)
lp:~mir-team/qtmir/compatibility-with-mir-API-changes updated
278. By Alan Griffiths

merge lp:~mir-team/qtmir/compatibility-with-mir-API-changes

279. By Alan Griffiths

layout fixes

280. By Alan Griffiths

Use current event API

Unmerged revisions

282. By Alan Griffiths

merge lp:~mir-team/qtmir/devel-mir-next-update/

281. By Alan Griffiths

merge lp:~alan-griffiths/qtmir/spike-using-WindowManager

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'CMakeLists.txt'
2--- CMakeLists.txt 2015-03-18 10:12:16 +0000
3+++ CMakeLists.txt 2015-03-25 14:59:05 +0000
4@@ -66,7 +66,8 @@
5 message(SEND_ERROR "protoc executable not found! Missing protobuf-compiler package?")
6 endif()
7
8-pkg_check_modules(MIRSERVER mirserver>=0.11 REQUIRED)
9+pkg_check_modules(MIRSERVER mirserver>=0.13 REQUIRED)
10+pkg_check_modules(MIRCLIENT mirclient>=0.13 REQUIRED)
11
12 pkg_check_modules(GLIB glib-2.0 REQUIRED)
13 pkg_check_modules(PROCESS_CPP process-cpp REQUIRED)
14
15=== modified file 'debian/control'
16--- debian/control 2015-03-04 17:08:47 +0000
17+++ debian/control 2015-03-25 14:59:05 +0000
18@@ -17,9 +17,9 @@
19 libgles2-mesa-dev,
20 libglib2.0-dev,
21 liblttng-ust-dev,
22- libmirclient-dev (>= 0.6.0),
23+ libmirclient-dev (>= 0.13.0),
24 libmircommon-dev,
25- libmirserver-dev (>= 0.11.0),
26+ libmirserver-dev (>= 0.13.0),
27 libmtdev-dev,
28 libprocess-cpp-dev,
29 libqtdbusmock1-dev (>= 0.2),
30
31=== modified file 'src/modules/Unity/Application/CMakeLists.txt'
32--- src/modules/Unity/Application/CMakeLists.txt 2015-01-08 12:37:08 +0000
33+++ src/modules/Unity/Application/CMakeLists.txt 2015-03-25 14:59:05 +0000
34@@ -2,7 +2,6 @@
35 ${GLIB_INCLUDE_DIRS}
36 ${GIO_INCLUDE_DIRS}
37 ${GIO_UNIX_INCLUDE_DIRS}
38- ${MIRCOMMON_INCLUDE_DIRS}
39 ${MIRSERVER_INCLUDE_DIRS}
40 ${PROCESS_CPP_INCLUDE_DIRS}
41 ${UBUNTU_PLATFORM_API_INCLUDE_DIRS}
42
43=== modified file 'src/modules/Unity/Application/application_manager.cpp'
44--- src/modules/Unity/Application/application_manager.cpp 2015-02-25 11:27:56 +0000
45+++ src/modules/Unity/Application/application_manager.cpp 2015-03-25 14:59:05 +0000
46@@ -67,11 +67,14 @@
47 const int tabletModeMinimimWithGU = 100;
48
49 // Obtain display size
50+ //TODO: should use mir::graphics::Display::configuration
51 mir::geometry::Rectangles view_area;
52- mirServer->the_display()->for_each_display_buffer(
53- [&view_area](const mir::graphics::DisplayBuffer & db)
54- {
55- view_area.add(db.view_area());
56+ mirServer->the_display()->for_each_display_sync_group(
57+ [&view_area](mir::graphics::DisplaySyncGroup& group) {
58+ group.for_each_display_buffer(
59+ [&view_area](const mir::graphics::DisplayBuffer & db) {
60+ view_area.add(db.view_area());
61+ });
62 });
63
64 // Get current Grid Unit value
65
66=== modified file 'src/platforms/mirserver/CMakeLists.txt'
67--- src/platforms/mirserver/CMakeLists.txt 2015-01-28 14:25:36 +0000
68+++ src/platforms/mirserver/CMakeLists.txt 2015-03-25 14:59:05 +0000
69@@ -18,7 +18,6 @@
70 endforeach(item ${Qt5Gui_PRIVATE_INCLUDE_DIRS})
71
72 include_directories(
73- ${MIRCOMMON_INCLUDE_DIRS}
74 ${MIRSERVER_INCLUDE_DIRS}
75
76 ${URL_DISPATCHER_INCLUDE_DIRS}
77@@ -69,6 +68,7 @@
78 qpa-mirserver
79
80 ${MIRSERVER_LDFLAGS}
81+ ${MIRCLIENT_LDFLAGS}
82 ${URL_DISPATCHER_LDFLAGS}
83 ${PROTOBUF_LDFLAGS}
84 ${EGL_LDFLAGS}
85
86=== modified file 'src/platforms/mirserver/displaywindow.cpp'
87--- src/platforms/mirserver/displaywindow.cpp 2015-01-07 19:12:45 +0000
88+++ src/platforms/mirserver/displaywindow.cpp 2015-03-25 14:59:05 +0000
89@@ -36,10 +36,14 @@
90 return ++id;
91 }
92
93-DisplayWindow::DisplayWindow(QWindow *window, mir::graphics::DisplayBuffer *displayBuffer)
94+DisplayWindow::DisplayWindow(
95+ QWindow *window,
96+ mir::graphics::DisplaySyncGroup *displayGroup,
97+ mir::graphics::DisplayBuffer *displayBuffer)
98 : QObject(nullptr), QPlatformWindow(window)
99 , m_isExposed(true)
100 , m_winId(newWId())
101+ , m_displayGroup(displayGroup)
102 , m_displayBuffer(displayBuffer)
103 {
104 qDebug() << "DisplayWindow::DisplayWindow";
105@@ -101,7 +105,17 @@
106 void DisplayWindow::swapBuffers()
107 {
108 m_displayBuffer->gl_swap_buffers();
109- m_displayBuffer->flip();
110+
111+ // FIXME this exposes a QtMir architecture problem now, as DisplayWindow
112+ // is supposed to wrap a mg::DisplayBuffer. We use Qt's multithreaded
113+ // renderer, where each DisplayWindow is rendered to relatively
114+ // independently, and post() called also individually.
115+ //
116+ // But in multimonitor case where a DisplaySyncGroup contains 2
117+ // DisplayBuffers, one post() call will submit both
118+ // mg::DisplayBuffers for flipping, which can happen before the other
119+ // DisplayWindow has been rendered to, causing visual artifacts
120+ m_displayGroup->post();
121 }
122
123 void DisplayWindow::makeCurrent()
124
125=== modified file 'src/platforms/mirserver/displaywindow.h'
126--- src/platforms/mirserver/displaywindow.h 2014-08-21 07:06:33 +0000
127+++ src/platforms/mirserver/displaywindow.h 2015-03-25 14:59:05 +0000
128@@ -21,6 +21,7 @@
129
130 #include <qpa/qplatformwindow.h>
131
132+#include <mir/graphics/display.h>
133 #include <mir/graphics/display_buffer.h>
134
135 #include <QObject>
136@@ -32,7 +33,7 @@
137 {
138 Q_OBJECT
139 public:
140- explicit DisplayWindow(QWindow *window, mir::graphics::DisplayBuffer*);
141+ explicit DisplayWindow(QWindow *window, mir::graphics::DisplaySyncGroup*, mir::graphics::DisplayBuffer*);
142
143 QRect geometry() const override;
144 void setGeometry(const QRect &rect) override;
145@@ -50,6 +51,7 @@
146 private:
147 bool m_isExposed;
148 WId m_winId;
149+ mir::graphics::DisplaySyncGroup *m_displayGroup;
150 mir::graphics::DisplayBuffer *m_displayBuffer;
151 };
152
153
154=== modified file 'src/platforms/mirserver/mirserver.h'
155--- src/platforms/mirserver/mirserver.h 2015-02-02 12:05:00 +0000
156+++ src/platforms/mirserver/mirserver.h 2015-03-25 14:59:05 +0000
157@@ -51,7 +51,6 @@
158 using mir::Server::the_prompt_session_manager;
159 using mir::Server::the_session_authorizer;
160 using mir::Server::the_session_listener;
161- using mir::Server::the_surface_configurator;
162
163 /* qt specific */
164 // getters
165
166=== modified file 'src/platforms/mirserver/mirserverintegration.cpp'
167--- src/platforms/mirserver/mirserverintegration.cpp 2014-12-11 16:55:37 +0000
168+++ src/platforms/mirserver/mirserverintegration.cpp 2015-03-25 14:59:05 +0000
169@@ -135,12 +135,19 @@
170
171 DisplayWindow* displayWindow = nullptr;
172
173- m_mirServer->the_display()->for_each_display_buffer(
174- [&](mg::DisplayBuffer& buffer) {
175- // FIXME(gerry) this will go very bad for >1 display buffer
176- displayWindow = new DisplayWindow(window, &buffer);
177+ mg::DisplayBuffer* first_buffer{nullptr};
178+ mg::DisplaySyncGroup* first_group{nullptr};
179+ m_mirServer->the_display()->for_each_display_sync_group([&](mg::DisplaySyncGroup& group) {
180+ if (!first_group) first_group = &group;
181+ group.for_each_display_buffer([&](mg::DisplayBuffer& buffer) {
182+ if (!first_buffer) first_buffer = &buffer;
183+ });
184 });
185
186+ // FIXME(gerry) this will go very bad for >1 display buffer
187+ if (first_group && first_buffer)
188+ displayWindow = new DisplayWindow(window, first_group, first_buffer);
189+
190 if (!displayWindow)
191 return nullptr;
192
193
194=== modified file 'src/platforms/mirserver/mirshell.cpp'
195--- src/platforms/mirserver/mirshell.cpp 2015-02-04 17:17:53 +0000
196+++ src/platforms/mirserver/mirshell.cpp 2015-03-25 14:59:05 +0000
197@@ -22,6 +22,7 @@
198 #include <mir/scene/session.h>
199 #include <mir/scene/surface_creation_parameters.h>
200 #include <mir/shell/display_layout.h>
201+#include <mir/shell/null_window_manager.h>
202
203 namespace ms = mir::scene;
204 using mir::shell::AbstractShell;
205@@ -32,7 +33,8 @@
206 const std::shared_ptr<mir::scene::SessionCoordinator> &sessionCoordinator,
207 const std::shared_ptr<mir::scene::PromptSessionManager> &promptSessionManager,
208 const std::shared_ptr<mir::shell::DisplayLayout> &displayLayout) :
209- AbstractShell(inputTargeter, surfaceCoordinator, sessionCoordinator, promptSessionManager),
210+ AbstractShell(inputTargeter, surfaceCoordinator, sessionCoordinator, promptSessionManager,
211+ [](mir::shell::FocusController*) { return std::make_shared<mir::shell::NullWindowManager>(); }),
212 m_displayLayout{displayLayout}
213 {
214 qCDebug(QTMIR_MIR_MESSAGES) << "MirShell::MirShell";
215
216=== modified file 'src/platforms/mirserver/surfaceobserver.h'
217--- src/platforms/mirserver/surfaceobserver.h 2015-01-14 08:24:29 +0000
218+++ src/platforms/mirserver/surfaceobserver.h 2015-03-25 14:59:05 +0000
219@@ -42,6 +42,7 @@
220 void cursor_image_set_to(mir::graphics::CursorImage const&) override {}
221 void orientation_set_to(MirOrientation) override {}
222 void client_surface_close_requested() override {}
223+ void keymap_changed(xkb_rule_names const&) override {}
224
225 Q_SIGNALS:
226 void framesPosted();
227
228=== modified file 'tests/modules/common/mock_surface.h'
229--- tests/modules/common/mock_surface.h 2015-02-03 16:52:55 +0000
230+++ tests/modules/common/mock_surface.h 2015-03-25 14:59:05 +0000
231@@ -66,6 +66,7 @@
232 MOCK_METHOD1(set_reception_mode, void(input::InputReceptionMode mode));
233 MOCK_METHOD0(request_client_surface_close, void());
234 MOCK_CONST_METHOD1(buffers_ready_for_compositor, int(void const*));
235+ void set_keymap(xkb_rule_names const&) override {}
236
237 // from mir::input::surface
238 MOCK_CONST_METHOD1(input_area_contains, bool(geometry::Point const& point));
239@@ -78,7 +79,7 @@
240 MOCK_CONST_METHOD0(supports_input, bool());
241 MOCK_CONST_METHOD0(client_input_fd, int());
242 MOCK_METHOD2(configure, int(MirSurfaceAttrib attrib, int value));
243- MOCK_METHOD1(query, int(MirSurfaceAttrib attrib));
244+ MOCK_CONST_METHOD1(query, int(MirSurfaceAttrib attrib));
245
246 // from mir::scene::SurfaceBufferAccess
247 MOCK_METHOD1(with_most_recent_buffer_do, void(std::function<void(graphics::Buffer&)> const& exec));
248
249=== modified file 'tests/modules/common/stub_scene_surface.h'
250--- tests/modules/common/stub_scene_surface.h 2014-08-13 11:59:23 +0000
251+++ tests/modules/common/stub_scene_surface.h 2015-03-25 14:59:05 +0000
252@@ -96,7 +96,7 @@
253 bool supports_input() const override { return true;}
254 int client_input_fd() const override { return fd;}
255 int configure(MirSurfaceAttrib, int) override { return 0; }
256- int query(MirSurfaceAttrib) override { return 0; }
257+ int query(MirSurfaceAttrib) const override { return 0; }
258 void with_most_recent_buffer_do(std::function<void(graphics::Buffer&)> const& ) override {}
259 };
260

Subscribers

People subscribed via source and target branches