Mir

Merge lp:~alan-griffiths/mir/mir_create_window_spec into lp:mir

Proposed by Alan Griffiths on 2017-05-31
Status: Rejected
Rejected by: Alan Griffiths on 2017-06-06
Proposed branch: lp:~alan-griffiths/mir/mir_create_window_spec
Merge into: lp:mir
Diff against target: 224 lines (+55/-46)
5 files modified
examples/flicker.c (+16/-24)
examples/progressbar.c (+10/-19)
include/client/mir_toolkit/mir_window.h (+15/-3)
src/client/mir_surface_api.cpp (+13/-0)
src/client/symbols.map (+1/-0)
To merge this branch: bzr merge lp:~alan-griffiths/mir/mir_create_window_spec
Reviewer Review Type Date Requested Status
Alan Griffiths Disapprove on 2017-06-06
Brandon Schaefer (community) 2017-05-31 Approve on 2017-06-05
Mir CI Bot continuous-integration Approve on 2017-05-31
Review via email: mp+324878@code.launchpad.net

Commit message

Add mir_window_create_spec() to make resizing Render Surfaces simpler to code.

To post a comment you must log in.
Mir CI Bot (mir-ci-bot) wrote :

PASSED: Continuous integration, rev:4183
https://mir-jenkins.ubuntu.com/job/mir-ci/3425/
Executed test runs:
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-mir/4667
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-0-fetch/4800
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=artful/4790
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/4790
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-1-sourcepkg/release=zesty/4790
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=artful/4703
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=artful/4703/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4703
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=clang,platform=mesa,release=zesty/4703/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=artful/4703
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=artful/4703/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/4703
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=xenial/4703/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4703
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=amd64,compiler=gcc,platform=mesa,release=zesty/4703/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=artful/4703
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=artful/4703/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=zesty/4703
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=cross-armhf,compiler=gcc,platform=mesa,release=zesty/4703/artifact/output/*zip*/output.zip
    SUCCESS: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/4703
        deb: https://mir-jenkins.ubuntu.com/job/build-2-binpkg-mir/arch=i386,compiler=gcc,platform=mesa,release=xenial/4703/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://mir-jenkins.ubuntu.com/job/mir-ci/3425/rebuild

review: Approve (continuous-integration)
Brandon Schaefer (brandontschaefer) wrote :

Only nit i see is the pointer is on the right "Type *<var>" vs left but lgtm

review: Approve
Alan Griffiths (alan-griffiths) wrote :

I've thought of an alternative, possibly more general, approach. We should chose after that's proposed.

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

> I've thought of an alternative, possibly more general, approach. We should
> chose after that's proposed.

lp:~alan-griffiths/mir/mir_window_get_connection/+merge/325143

Alan Griffiths (alan-griffiths) wrote :

Actually, I'm sure the other approach is better.

review: Disapprove

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'examples/flicker.c'
2--- examples/flicker.c 2017-05-19 13:49:04 +0000
3+++ examples/flicker.c 2017-05-31 17:11:43 +0000
4@@ -101,15 +101,9 @@
5 }
6 }
7
8-typedef struct Context
9-{
10- MirConnection *connection;
11- MirRenderSurface* surface;
12-} Context;
13-
14 static void handle_event(MirWindow* window, MirEvent const* ev, void* context_)
15 {
16- Context* context = (Context*)context_;
17+ MirRenderSurface* surface = (MirRenderSurface*)context_;
18
19 switch (mir_event_get_type(ev))
20 {
21@@ -119,9 +113,9 @@
22 int const new_width = mir_resize_event_get_width(resize);
23 int const new_height = mir_resize_event_get_height(resize);
24
25- mir_render_surface_set_size(context->surface, new_width, new_height);
26- MirWindowSpec* spec = mir_create_window_spec(context->connection);
27- mir_window_spec_add_render_surface(spec, context->surface, new_width, new_height, 0, 0);
28+ mir_render_surface_set_size(surface, new_width, new_height);
29+ MirWindowSpec* spec = mir_window_create_spec(window);
30+ mir_window_spec_add_render_surface(spec, surface, new_width, new_height, 0, 0);
31 mir_window_apply_spec(window, spec);
32 mir_window_spec_release(spec);
33 break;
34@@ -177,30 +171,28 @@
35 }
36 }
37
38- Context context = { 0, 0 };
39-
40 puts("Starting");
41
42- context.connection = mir_connect_sync(socket_file, __FILE__);
43- assert(context.connection != NULL);
44- assert(mir_connection_is_valid(context.connection));
45- assert(strcmp(mir_connection_get_error_message(context.connection), "") == 0);
46+ MirConnection *const connection = mir_connect_sync(socket_file, __FILE__);
47+ assert(connection != NULL);
48+ assert(mir_connection_is_valid(connection));
49+ assert(strcmp(mir_connection_get_error_message(connection), "") == 0);
50 puts("Connected");
51
52 unsigned int const num_formats = 32;
53 MirPixelFormat pixel_formats[num_formats];
54 unsigned int valid_formats;
55- mir_connection_get_available_surface_formats(context.connection, pixel_formats, num_formats, &valid_formats);
56+ mir_connection_get_available_surface_formats(connection, pixel_formats, num_formats, &valid_formats);
57 MirPixelFormat pixel_format = find_8888_format(pixel_formats, valid_formats);
58
59- context.surface = mir_connection_create_render_surface_sync(context.connection, width, height);
60+ MirRenderSurface *const surface = mir_connection_create_render_surface_sync(connection, width, height);
61 puts("Surface created");
62
63- MirWindowSpec *spec = mir_create_normal_window_spec(context.connection, width, height);
64+ MirWindowSpec *spec = mir_create_normal_window_spec(connection, width, height);
65 assert(spec != NULL);
66 mir_window_spec_set_name(spec, __FILE__);
67- mir_window_spec_add_render_surface(spec, context.surface, width, height, 0, 0);
68- mir_window_spec_set_event_handler(spec, handle_event, &context);
69+ mir_window_spec_add_render_surface(spec, surface, width, height, 0, 0);
70+ mir_window_spec_set_event_handler(spec, handle_event, surface);
71
72 window = mir_create_window_sync(spec);
73 mir_window_spec_release(spec);
74@@ -210,7 +202,7 @@
75 assert(strcmp(mir_window_get_error_message(window), "") == 0);
76 puts("Window created");
77
78- MirBufferStream* bs = mir_render_surface_get_buffer_stream(context.surface, width, height, pixel_format);
79+ MirBufferStream* bs = mir_render_surface_get_buffer_stream(surface, width, height, pixel_format);
80 mir_buffer_stream_set_swapinterval(bs, swapinterval);
81
82 uint32_t pattern[2] = {0};
83@@ -234,10 +226,10 @@
84 mir_window_release_sync(window);
85 puts("Window released");
86
87- mir_render_surface_release(context.surface);
88+ mir_render_surface_release(surface);
89 puts("Surface released");
90
91- mir_connection_release(context.connection);
92+ mir_connection_release(connection);
93 puts("Connection released");
94
95 return 0;
96
97=== modified file 'examples/progressbar.c'
98--- examples/progressbar.c 2017-05-19 15:11:18 +0000
99+++ examples/progressbar.c 2017-05-31 17:11:43 +0000
100@@ -52,15 +52,9 @@
101 }
102 }
103
104-typedef struct
105-{
106- MirConnection *conn;
107- MirRenderSurface* surface;
108-} Surface;
109-
110-static void handle_event(MirWindow* window, MirEvent const* ev, void* context_)
111-{
112- Surface* context = (Surface*)context_;
113+static void handle_event(MirWindow* window, MirEvent const* ev, void* context)
114+{
115+ MirRenderSurface* surface = (MirRenderSurface*)context;
116
117 switch (mir_event_get_type(ev))
118 {
119@@ -70,9 +64,9 @@
120 int const new_width = mir_resize_event_get_width(resize);
121 int const new_height = mir_resize_event_get_height(resize);
122
123- mir_render_surface_set_size(context->surface, new_width, new_height);
124- MirWindowSpec* spec = mir_create_window_spec(context->conn);
125- mir_window_spec_add_render_surface(spec, context->surface, new_width, new_height, 0, 0);
126+ mir_render_surface_set_size(surface, new_width, new_height);
127+ MirWindowSpec* spec = mir_window_create_spec(window);
128+ mir_window_spec_add_render_surface(spec, surface, new_width, new_height, 0, 0);
129 mir_window_apply_spec(window, spec);
130 mir_window_spec_release(spec);
131 break;
132@@ -286,16 +280,13 @@
133 int width = 500;
134 int height = 500;
135
136- Surface surface;
137-
138- surface.conn = conn;
139- surface.surface = mir_connection_create_render_surface_sync(conn, width, height);
140- MirBufferStream *bs = mir_render_surface_get_buffer_stream(surface.surface, width, height, pixel_format);
141+ MirRenderSurface *const surface = mir_connection_create_render_surface_sync(conn, width, height);
142+ MirBufferStream *bs = mir_render_surface_get_buffer_stream(surface, width, height, pixel_format);
143
144 MirWindowSpec *spec = mir_create_normal_window_spec(conn, width, height);
145
146- mir_window_spec_add_render_surface(spec, surface.surface, width, height, 0, 0);
147- mir_window_spec_set_event_handler(spec, &handle_event, &surface);
148+ mir_window_spec_add_render_surface(spec, surface, width, height, 0, 0);
149+ mir_window_spec_set_event_handler(spec, &handle_event, surface);
150
151 if (spec == NULL)
152 {
153
154=== modified file 'include/client/mir_toolkit/mir_window.h'
155--- include/client/mir_toolkit/mir_window.h 2017-05-25 08:58:03 +0000
156+++ include/client/mir_toolkit/mir_window.h 2017-05-31 17:11:43 +0000
157@@ -219,9 +219,8 @@
158 * Create a window specification.
159 * This can be used with mir_create_window() to create a window or with
160 * mir_window_apply_spec() to change an existing window.
161- * \remark For use with mir_create_window() at least the type, width, height,
162- * format and buffer_usage must be set. (And for types requiring a parent that
163- * too must be set.)
164+ * \remark For use with mir_create_window() at least the type, width and height
165+ * must be set. (And for types requiring a parent that too must be set.)
166 *
167 * \param [in] connection a valid mir connection
168 * \return A handle that can ultimately be passed to
169@@ -230,6 +229,19 @@
170 MirWindowSpec* mir_create_window_spec(MirConnection* connection);
171
172 /**
173+ * Create a window specification.
174+ * This can be used with mir_create_window() to create a window or with
175+ * mir_window_apply_spec() to change an existing window.
176+ * \remark For use with mir_create_window() at least the type, width and height
177+ * must be set. (And for types requiring a parent that too must be set.)
178+ *
179+ * \param [in] window a valid MirWindow
180+ * \return A handle that can ultimately be passed to
181+ * mir_create_window() or mir_window_apply_spec()
182+ */
183+MirWindowSpec* mir_window_create_spec(MirWindow* window);
184+
185+/**
186 * Set the requested parent.
187 *
188 * \param [in] spec Specification to mutate
189
190=== modified file 'src/client/mir_surface_api.cpp'
191--- src/client/mir_surface_api.cpp 2017-05-08 03:04:26 +0000
192+++ src/client/mir_surface_api.cpp 2017-05-31 17:11:43 +0000
193@@ -184,6 +184,19 @@
194 std::abort(); // If we just failed to allocate a MirWindowSpec returning isn't safe
195 }
196
197+MirWindowSpec* mir_window_create_spec(MirWindow* window)
198+try
199+{
200+ mir::require(mir_window_is_valid(window));
201+
202+ return mir_create_window_spec(window->connection());
203+}
204+catch (std::exception const& ex)
205+{
206+ MIR_LOG_UNCAUGHT_EXCEPTION(ex);
207+ std::abort(); // If we just failed to allocate a MirWindowSpec returning isn't safe
208+}
209+
210 void mir_window_spec_set_parent(MirWindowSpec* spec, MirWindow* parent)
211 try
212 {
213
214=== modified file 'src/client/symbols.map'
215--- src/client/symbols.map 2017-05-25 08:58:03 +0000
216+++ src/client/symbols.map 2017-05-31 17:11:43 +0000
217@@ -607,6 +607,7 @@
218 mir_output_has_custom_logical_size;
219 mir_output_set_logical_size;
220 mir_output_get_logical_width;
221+ mir_window_create_spec;
222 mir_window_request_user_move;
223 mir_window_request_user_resize;
224 mir_touchscreen_config_get_mapping_mode;

Subscribers

People subscribed via source and target branches