[PATCH] Bug fix client apps because of output change
Wang, Quanxian
quanxian.wang at intel.com
Mon Mar 10 01:23:45 PDT 2014
Thanks Pq. Comments below.
>-----Original Message-----
>From: wayland-devel-bounces at lists.freedesktop.org
>[mailto:wayland-devel-bounces at lists.freedesktop.org] On Behalf Of Pekka
>Paalanen
>Sent: Monday, March 10, 2014 3:47 PM
>To: Wang, Quanxian
>Cc: wayland-devel at lists.freedesktop.org
>Subject: Re: [PATCH] Bug fix client apps because of output change
>
>Hi,
>
>overall this looks fine now, but I still haven't tested it. I'm moving on to nitpick on
>the minor details. ;-)
>
>The patch title (topic line) is usually prefixed with the component the patch is
>affecting. Looking at the clients/desktop-shell.c history, the appropriate prefix
>would be "desktop-shell:". Also, this patch does not affect any other clients.
>
>On Thu, 6 Mar 2014 18:31:14 +0800
>Quanxian Wang <quanxian.wang at intel.com> wrote:
>
>> 1)
>> Width and height of Panel and Background depend on output's, therefore
>> they should be bound with output changes including mode, transform and
>> scale.
>>
>> 2)
>> Update the min_allocation before resize the panel and background
>> window. Add window_set_min_allocation function because it is invisible
>> outside window.c.
>
>An alternative to adding the set_min_allocation would be to cause the
>min_allocation to be something very small to begin with. The min_allocation is not
>important for the panel and background surfaces, because they follow special
>rules, and cannot be resized freely anyway.
>
>I'm fine with either way.
>
>>
>> Signed-off-by: Quanxian Wang <quanxian.wang at intel.com>
>> ---
>> clients/desktop-shell.c | 80
>> +++++++++++++++++++++++++++++++++++++++++++++++--
>> clients/window.c | 7 +++++ clients/window.h | 2 ++
>> 3 files changed, 87 insertions(+), 2 deletions(-)
>>
>> diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c index
>> a0c6b6d..4373448 100644
>> --- a/clients/desktop-shell.c
>> +++ b/clients/desktop-shell.c
>> @@ -103,6 +103,15 @@ struct output {
>>
>> struct panel *panel;
>> struct background *background;
>> + struct {
>> + int height;
>> + int width;
>> + uint32_t refresh;
>> + } mode;
>> +
>> + uint32_t interface_version;
>> + uint32_t transform;
>> + uint32_t scale;
>> };
>>
>> struct panel_launcher {
>> @@ -1145,6 +1154,45 @@ desktop_destroy_outputs(struct desktop
>> *desktop) }
>>
>> static void
>> +update_output(struct output *output)
>> +{
>> + struct panel *panel = output->panel;
>> + struct background *background = output->background;
>> + int width, height;
>> +
>> + if (!output)
>
>You already dereferenced 'output' above, checking for NULL here is useless.
>'output' can never be NULL anyway, right?
[Wang, Quanxian] right.
>
>> + return;
>> +
>> + width = output->mode.width;
>> + height = output->mode.height;
>> +
>> + switch (output->transform) {
>> + case WL_OUTPUT_TRANSFORM_90:
>> + case WL_OUTPUT_TRANSFORM_270:
>> + case WL_OUTPUT_TRANSFORM_FLIPPED_90:
>> + case WL_OUTPUT_TRANSFORM_FLIPPED_270:
>> + /* Swap width and height */
>> + width = output->mode.height;
>> + height = output->mode.width;
>> + break;
>> + default:
>> + break;
>> + }
>> +
>> + if (output->scale != 0) {
>
>If scale was initialized to 1, there would be no need for 'if'. Right?
[Wang, Quanxian] in testing, this cause the error because scale is set to 0, it happens when weston is started at the very beginning. Here we need to do that.
>
>> + width /= output->scale;
>> + height /= output->scale;
>> + }
>> +
>> + /* Set min_allocation of panel */
>
>Needless comment.
[Wang, Quanxian] will fix.
>
>> + window_set_min_allocation(panel->window, width, 32);
>> + window_set_min_allocation(background->window, width, height);
>> +
>> + window_schedule_resize(background->window, width, height);
>> + window_schedule_resize(panel->window, width, 32); }
>> +
>> +static void
>> output_handle_geometry(void *data,
>> struct wl_output *wl_output,
>> int x, int y,
>> @@ -1157,6 +1205,11 @@ output_handle_geometry(void *data, {
>> struct output *output = data;
>>
>> + output->transform = transform;
>> +
>> + if (output->interface_version < 2)
>> + update_output(output);
>> +
>> window_set_buffer_transform(output->panel->window,
>> transform); window_set_buffer_transform(output->background->window,
>> transform); }
>> @@ -1169,12 +1222,29 @@ output_handle_mode(void *data,
>> int height,
>> int refresh)
>> {
>> + struct output *output = data;
>> +
>> + if (flags & WL_OUTPUT_MODE_CURRENT) {
>> + if (!output)
>> + return;
>> +
>> + output->mode.width = width;
>> + output->mode.height = height;
>> + output->mode.refresh = refresh;
>> +
>> + if (output->interface_version < 2)
>> + update_output(output);
>> + }
>> }
>>
>> static void
>> output_handle_done(void *data,
>> struct wl_output *wl_output) {
>> + struct output *output = data;
>> +
>> + if (output->interface_version >= 2)
>> + update_output(output);
>
>The only case when this can be called is when the interface version is at least 2, so
>no need to check. The protocol specification guarantees that.
[Wang, Quanxian] yes. delete that
>
>> }
>>
>> static void
>> @@ -1184,6 +1254,11 @@ output_handle_scale(void *data, {
>> struct output *output = data;
>>
>> + output->scale = scale;
>> +
>> + if (output->interface_version < 2)
>> + update_output(output);
>
>The only case when this can be called is when the interface version is at least 2, so
>no need to call update_output() here.
[Wang, Quanxian] yes.
>
>> +
>> window_set_buffer_scale(output->panel->window, scale);
>> window_set_buffer_scale(output->background->window, scale); } @@
>> -1212,7 +1287,7 @@ output_init(struct output *output, struct desktop
>> *desktop) }
>>
>> static void
>> -create_output(struct desktop *desktop, uint32_t id)
>> +create_output(struct desktop *desktop, uint32_t id, uint32_t version)
>> {
>> struct output *output;
>>
>> @@ -1223,6 +1298,7 @@ create_output(struct desktop *desktop, uint32_t
>> id) output->output =
>> display_bind(desktop->display, id,
>> &wl_output_interface, 2); output->server_output_id = id;
>> + output->interface_version = version;
>
>This is not completely correct. In the future, a compositor may advertise
>wl_output interface version 3 or greater, if the protocol has been augmented.
>However, the valid version is the minimum of what the compositor advertises and
>what the client is written to support.
>Therefore the interface version should be min(version, 2).
>
>Note that create_output() already has a small bug in this: it always binds with
>version 2 regardless of what the compositor advertised.
>
>You can compare to the "desktop_shell" global handling which seems correct.
[Wang, Quanxian] I will check and update that. The change will be like that
1293 output->output =
1294 display_bind(desktop->display, id, &wl_output_interface, version);
1295 output->server_output_id = id;
1296 output->interface_version = (version < 2) ? version : 2;
>
>>
>> wl_output_add_listener(output->output, &output_listener, output); @@
>> -1247,7 +1323,7 @@ global_handler(struct display *display, uint32_t
>> id, desktop->interface_version);
>> desktop_shell_add_listener(desktop->shell,
>> &listener, desktop); } else if (!strcmp(interface, "wl_output")) {
>> - create_output(desktop, id);
>> + create_output(desktop, id, version);
>> }
>> }
>>
>> diff --git a/clients/window.c b/clients/window.c index
>> c8287e2..6c01222 100644
>> --- a/clients/window.c
>> +++ b/clients/window.c
>> @@ -3839,6 +3839,13 @@ undo_resize(struct window *window) }
>>
>> void
>> +window_set_min_allocation(struct window *window, int width, int
>> height) +{
>> + window->min_allocation.width = width;
>> + window->min_allocation.height = height; }
>> +
>> +void
>> window_schedule_resize(struct window *window, int width, int height)
>> {
>> /* We should probably get these numbers from the theme. */ diff
>> --git a/clients/window.h b/clients/window.h index 54b848b..b221b76
>> 100644
>> --- a/clients/window.h
>> +++ b/clients/window.h
>> @@ -336,6 +336,8 @@ void
>> window_schedule_redraw(struct window *window); void
>> window_schedule_resize(struct window *window, int width, int height);
>> +void
>> +window_set_min_allocation(struct window *window, int width, int
>> height);
>> void
>> window_damage(struct window *window, int32_t x, int32_t y,
>
>Thanks,
>pq
>_______________________________________________
>wayland-devel mailing list
>wayland-devel at lists.freedesktop.org
>http://lists.freedesktop.org/mailman/listinfo/wayland-devel
More information about the wayland-devel
mailing list