RE: [PATCH] Bug fix client apps because of output change
>-Original Message- >From: wayland-devel-boun...@lists.freedesktop.org >[mailto:wayland-devel-boun...@lists.freedesktop.org] On Behalf Of Pekka >Paalanen >Sent: Tuesday, March 11, 2014 4:26 PM >To: Wang, Quanxian >Cc: wayland-devel@lists.freedesktop.org >Subject: Re: [PATCH] Bug fix client apps because of output change > >On Tue, 11 Mar 2014 01:48:49 + >"Wang, Quanxian" wrote: > >> Thanks. >> >> Quanxian >> >> >> >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; >> > >> >Do not use the server advertized version in bind without checking it >> >first. If the server version grows, the client still needs to bind >> >with the old version, until the client code is ported to support the new >> >version. >> [Wang, Quanxian] Sorry for misunderstanding the interface version issue. >Currently my understanding is that client apps(desktop-shell) is based on >wl_output which version is under 2(including 2), so whatever wl_output upgrade >or not, we can use only the function provided by wl_output interface under the >version 2. Right? >> Another question is, since version is from server, why not keep it as >> original >value? when you do some operation, you just check and choose what needed >based on the value(just like update_output). My suggestion is when you upgrade >client app to use wl_output, we don't need to continue maintain such code.(For >example from 2 to 3). The only update is the new code which want to use new >function provided by newly wl_output interface. What is your idea about that? >> > > >Let me put it more clearly. The client has been coded to support version 2. The >server advertises version N. The agreed interface version that both can deal >with >is then min(2, N). > >You have to tell the server, what the agreed version is: you have to call >display_bind() with version min(2, N). This way the server knows, that it must >not >send events added in versions 3 or greater, because those would crash the >client. > >Also the client knows, that it cannot use requests added in versions >N+1 or greater. > >It does not matter whether you set output->interface_version = N or min(2, N), >because the code in the client is anyway comparing that to what it actually is >coded >for. It's just more logical to me to store the agreed version, not the server >version. [Wang, Quanxian] Thanks. Got that. > > >Thanks, >pq >___ >wayland-devel mailing list >wayland-devel@lists.freedesktop.org >http://lists.freedesktop.org/mailman/listinfo/wayland-devel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] Bug fix client apps because of output change
On Tue, 11 Mar 2014 01:40:18 +
"Wang, Quanxian" wrote:
>
>
> >-Original Message-
> >From: Pekka Paalanen [mailto:ppaala...@gmail.com]
> >Sent: Monday, March 10, 2014 7:05 PM
> >To: Wang, Quanxian
> >Cc: wayland-devel@lists.freedesktop.org
> >Subject: Re: [PATCH] Bug fix client apps because of output change
> >
> >On Mon, 10 Mar 2014 10:58:00 +
> >"Wang, Quanxian" wrote:
> >
> >>
> >>
> >> >-Original Message-
> >> >From: Pekka Paalanen [mailto:ppaala...@gmail.com]
> >> >Sent: Monday, March 10, 2014 5:58 PM
> >> >To: Wang, Quanxian
> >> >Cc: wayland-devel@lists.freedesktop.org
> >> >Subject: Re: [PATCH] Bug fix client apps because of output change
> >> >
> >> >On Mon, 10 Mar 2014 08:23:45 +
> >> >"Wang, Quanxian" wrote:
> >> >
> >> >> Thanks Pq. Comments below.
> >> >...
> >> >> >> @@ -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.
> >> >
> >> >Why does that happen? Is Weston sending an event with scale=0? If so,
> >> >then that is a Weston bug and should be fixed. Until you get any
> >> >event the scale is 1, because that is the implicit scale in interface
> >> >version 1.
> >> [Wang, Quanxian] yes. Got geometry event with scale=0.
> >
> >What do you mean? wl_output.geometry does not carry scale at all, hence until
> >you receive the first wl_output.scale you should assume scale=1.
> >The easiest way to do that is to initialize output::scale to 1 in
> >create_output(). You
> >should never have zero.
> [Wang, Quanxian] sorry, is scale event instead of geometry. Yes, I can
> initialize scale to be 1, but at the same time, I need to get the scale event
> from server. Basically I still need to do some checking for that.
>
You cannot require receiving the scale event, because the
protocol specification says:
This event contains scaling geometry information
that is not in the geometry event. It may be sent after
binding the output object or if the output scale changes
later. If it is not sent, the client should assume a
scale of 1.
However, if wl_output agreed interface version is at least 2, then you
are guaranteed to receive the done event when you have received all
output property events.
> After that, I will check why scale(0) is sent by wl_output and push the patch
> to fix that.
I suspect it doesn't really happen, but if it does, it is a bug in the
compositor, yes.
Thanks,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] Bug fix client apps because of output change
On Tue, 11 Mar 2014 01:48:49 + "Wang, Quanxian" wrote: > Thanks. > > Quanxian > > >> >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; > > > >Do not use the server advertized version in bind without checking it first. > >If the > >server version grows, the client still needs to bind with the old version, > >until the > >client code is ported to support the new version. > [Wang, Quanxian] Sorry for misunderstanding the interface version issue. > Currently my understanding is that client apps(desktop-shell) is based on > wl_output which version is under 2(including 2), so whatever wl_output > upgrade or not, we can use only the function provided by wl_output interface > under the version 2. Right? > Another question is, since version is from server, why not keep it as > original value? when you do some operation, you just check and choose what > needed based on the value(just like update_output). My suggestion is when you > upgrade client app to use wl_output, we don't need to continue maintain such > code.(For example from 2 to 3). The only update is the new code which want to > use new function provided by newly wl_output interface. What is your idea > about that? > > Let me put it more clearly. The client has been coded to support version 2. The server advertises version N. The agreed interface version that both can deal with is then min(2, N). You have to tell the server, what the agreed version is: you have to call display_bind() with version min(2, N). This way the server knows, that it must not send events added in versions 3 or greater, because those would crash the client. Also the client knows, that it cannot use requests added in versions N+1 or greater. It does not matter whether you set output->interface_version = N or min(2, N), because the code in the client is anyway comparing that to what it actually is coded for. It's just more logical to me to store the agreed version, not the server version. Thanks, pq ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
RE: [PATCH] Bug fix client apps because of output change
Thanks. Quanxian >> >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; > >Do not use the server advertized version in bind without checking it first. If >the >server version grows, the client still needs to bind with the old version, >until the >client code is ported to support the new version. [Wang, Quanxian] Sorry for misunderstanding the interface version issue. Currently my understanding is that client apps(desktop-shell) is based on wl_output which version is under 2(including 2), so whatever wl_output upgrade or not, we can use only the function provided by wl_output interface under the version 2. Right? Another question is, since version is from server, why not keep it as original value? when you do some operation, you just check and choose what needed based on the value(just like update_output). My suggestion is when you upgrade client app to use wl_output, we don't need to continue maintain such code.(For example from 2 to 3). The only update is the new code which want to use new function provided by newly wl_output interface. What is your idea about that? > > >Thanks, >pq ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
RE: [PATCH] Bug fix client apps because of output change
>-Original Message-
>From: Pekka Paalanen [mailto:ppaala...@gmail.com]
>Sent: Monday, March 10, 2014 7:05 PM
>To: Wang, Quanxian
>Cc: wayland-devel@lists.freedesktop.org
>Subject: Re: [PATCH] Bug fix client apps because of output change
>
>On Mon, 10 Mar 2014 10:58:00 +
>"Wang, Quanxian" wrote:
>
>>
>>
>> >-Original Message-
>> >From: Pekka Paalanen [mailto:ppaala...@gmail.com]
>> >Sent: Monday, March 10, 2014 5:58 PM
>> >To: Wang, Quanxian
>> >Cc: wayland-devel@lists.freedesktop.org
>> >Subject: Re: [PATCH] Bug fix client apps because of output change
>> >
>> >On Mon, 10 Mar 2014 08:23:45 +
>> >"Wang, Quanxian" wrote:
>> >
>> >> Thanks Pq. Comments below.
>> >...
>> >> >> @@ -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.
>> >
>> >Why does that happen? Is Weston sending an event with scale=0? If so,
>> >then that is a Weston bug and should be fixed. Until you get any
>> >event the scale is 1, because that is the implicit scale in interface
>> >version 1.
>> [Wang, Quanxian] yes. Got geometry event with scale=0.
>
>What do you mean? wl_output.geometry does not carry scale at all, hence until
>you receive the first wl_output.scale you should assume scale=1.
>The easiest way to do that is to initialize output::scale to 1 in
>create_output(). You
>should never have zero.
[Wang, Quanxian] sorry, is scale event instead of geometry. Yes, I can
initialize scale to be 1, but at the same time, I need to get the scale event
from server. Basically I still need to do some checking for that.
After that, I will check why scale(0) is sent by wl_output and push the patch
to fix that.
>
>
>Thanks,
>pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] Bug fix client apps because of output change
On Mon, 10 Mar 2014 10:58:00 +
"Wang, Quanxian" wrote:
>
>
> >-Original Message-
> >From: Pekka Paalanen [mailto:ppaala...@gmail.com]
> >Sent: Monday, March 10, 2014 5:58 PM
> >To: Wang, Quanxian
> >Cc: wayland-devel@lists.freedesktop.org
> >Subject: Re: [PATCH] Bug fix client apps because of output change
> >
> >On Mon, 10 Mar 2014 08:23:45 +
> >"Wang, Quanxian" wrote:
> >
> >> Thanks Pq. Comments below.
> >...
> >> >> @@ -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.
> >
> >Why does that happen? Is Weston sending an event with scale=0? If so, then
> >that
> >is a Weston bug and should be fixed. Until you get any event the scale is 1,
> >because that is the implicit scale in interface version 1.
> [Wang, Quanxian] yes. Got geometry event with scale=0.
What do you mean? wl_output.geometry does not carry scale at all, hence
until you receive the first wl_output.scale you should assume scale=1.
The easiest way to do that is to initialize output::scale to 1 in
create_output(). You should never have zero.
Thanks,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
RE: [PATCH] Bug fix client apps because of output change
>-Original Message-
>From: Pekka Paalanen [mailto:ppaala...@gmail.com]
>Sent: Monday, March 10, 2014 5:58 PM
>To: Wang, Quanxian
>Cc: wayland-devel@lists.freedesktop.org
>Subject: Re: [PATCH] Bug fix client apps because of output change
>
>On Mon, 10 Mar 2014 08:23:45 +
>"Wang, Quanxian" wrote:
>
>> Thanks Pq. Comments below.
>...
>> >> @@ -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.
>
>Why does that happen? Is Weston sending an event with scale=0? If so, then that
>is a Weston bug and should be fixed. Until you get any event the scale is 1,
>because that is the implicit scale in interface version 1.
[Wang, Quanxian] yes. Got geometry event with scale=0.
>
>> >
>> >> + width /= output->scale;
>> >> + height /= output->scale;
>> >> + }
>...
>> >> 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;
>
>Do not use the server advertized version in bind without checking it first. If
>the
>server version grows, the client still needs to bind with the old version,
>until the
>client code is ported to support the new version.
>
>
>Thanks,
>pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] Bug fix client apps because of output change
On Mon, 10 Mar 2014 08:23:45 +
"Wang, Quanxian" wrote:
> Thanks Pq. Comments below.
...
> >> @@ -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.
Why does that happen? Is Weston sending an event with scale=0? If so,
then that is a Weston bug and should be fixed. Until you get any event
the scale is 1, because that is the implicit scale in interface version
1.
> >
> >> + width /= output->scale;
> >> + height /= output->scale;
> >> + }
...
> >>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;
Do not use the server advertized version in bind without checking it
first. If the server version grows, the client still needs to bind with
the old version, until the client code is ported to support the new
version.
Thanks,
pq
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
RE: [PATCH] Bug fix client apps because of output change
Thanks Pq. Comments below.
>-Original Message-
>From: wayland-devel-boun...@lists.freedesktop.org
>[mailto:wayland-devel-boun...@lists.freedesktop.org] On Behalf Of Pekka
>Paalanen
>Sent: Monday, March 10, 2014 3:47 PM
>To: Wang, Quanxian
>Cc: wayland-devel@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 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
>> ---
>> 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;
>>
RE: [PATCH] Bug fix client apps because of output change
>-Original Message-
>From: wayland-devel-boun...@lists.freedesktop.org
>[mailto:wayland-devel-boun...@lists.freedesktop.org] On Behalf Of Pekka
>Paalanen
>Sent: Monday, March 10, 2014 3:47 PM
>To: Wang, Quanxian
>Cc: wayland-devel@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. ;-)
[Wang, Quanxian] that is fine.
>
>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.
[Wang, Quanxian] I will resend it again at last with title adding
"desktop-shell:". :)
>
>On Thu, 6 Mar 2014 18:31:14 +0800
>Quanxian Wang 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
>> ---
>> 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?
>
>> +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?
>
>> +width /= output->scale;
>> +height /= output->scale;
>> +}
>> +
>> +/* Set min_allocation of panel */
>
>Needless comment.
>
>> +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);
>> +
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 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
> ---
> 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?
> + 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?
> + width /= output->scale;
> + height /= output->scale;
> + }
> +
> + /* Set min_allocation of panel */
Needless comment.
> + 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.
> }
>
> 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 cas
RE: [PATCH] Bug fix client apps because of output change
I have double checked the patch and retesting it.
The bug is caused by not setting background window size after output change.
This fix has been included in previous patch. Pq's comment is right. We should
set background min allocation before resize.
Why did I get this? Because I just use another patch which only comment
min_allocation of background for testing. This caused the bug I mentioned.
Just use previous patch is fine.
Thanks
Regards
Quanxian Wang
>-Original Message-
>From: Wang, Quanxian
>Sent: Friday, March 07, 2014 10:44 AM
>To: wayland-devel@lists.freedesktop.org
>Cc: ppaala...@gmail.com
>Subject: RE: [PATCH] Bug fix client apps because of output change
>
>Ignore this, I have found a bug. After bug fixing, I will resend the patch.
>Sorry
>about that.
>
>I have two monitors.
>VGA1(left) + HDMI3(right)
>
>The bug is shown when you set the mode of HDMI3 to 800x600, and then move
>layout as HDMI3(left) + VGA1(right), and then change to mode 1920x1200.
>It will have some issue.
>
>Sorry about that.
>
>Regards
>
>Quanxian Wang
>
>>-Original Message-
>>From: Wang, Quanxian
>>Sent: Thursday, March 06, 2014 6:31 PM
>>To: wayland-devel@lists.freedesktop.org
>>Cc: ppaala...@gmail.com; Wang, Quanxian
>>Subject: [PATCH] Bug fix client apps because of output change
>>
>>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.
>>
>>Signed-off-by: Quanxian Wang
>>---
>> 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)
>>+ 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) {
>>+ width /= output->scale;
>>+ height /= output->scale;
>>+ }
>>+
>>+ /* Set min_allocation of panel */
>>+ 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
RE: [PATCH] Bug fix client apps because of output change
Ignore this, I have found a bug. After bug fixing, I will resend the patch.
Sorry about that.
I have two monitors.
VGA1(left) + HDMI3(right)
The bug is shown when you set the mode of HDMI3 to 800x600, and then move
layout as HDMI3(left) + VGA1(right), and then change to mode 1920x1200.
It will have some issue.
Sorry about that.
Regards
Quanxian Wang
>-Original Message-
>From: Wang, Quanxian
>Sent: Thursday, March 06, 2014 6:31 PM
>To: wayland-devel@lists.freedesktop.org
>Cc: ppaala...@gmail.com; Wang, Quanxian
>Subject: [PATCH] Bug fix client apps because of output change
>
>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.
>
>Signed-off-by: Quanxian Wang
>---
> 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)
>+ 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) {
>+ width /= output->scale;
>+ height /= output->scale;
>+ }
>+
>+ /* Set min_allocation of panel */
>+ 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);
> }
>
> 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);
>+
> 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
RE: [PATCH] Bug fix client apps because of output change
Hi, Pq >> >>Is there a reason why the background window does not need the >>min_allocation changed? >[Wang, Quanxian] With testing, this doesn't make error. Let me double check >that. [Wang, Quanxian] According to the logic, we need set the min_allocation because of background should overwrite the whole output. I have done with testing with set and not set. Nothing wrong happens in this condition. I add that in next updated patch. ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
RE: [PATCH] Bug fix client apps because of output change
>-Original Message-
>From: Pekka Paalanen [mailto:ppaala...@gmail.com]
>Sent: Thursday, March 06, 2014 4:42 PM
>To: Wang, Quanxian
>Cc: wayland-devel@lists.freedesktop.org
>Subject: Re: [PATCH] Bug fix client apps because of output change
>
>On Thu, 6 Mar 2014 16:25:42 +0800
>Quanxian Wang 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.
>>
>> Signed-off-by: Quanxian Wang
>> ---
>> clients/desktop-shell.c | 70
>> +
>> clients/window.c| 7 + clients/window.h| 2 ++
>> 3 files changed, 79 insertions(+)
>>
>> diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c index
>> a0c6b6d..9ccba34 100644
>> --- a/clients/desktop-shell.c
>> +++ b/clients/desktop-shell.c
>> @@ -96,6 +96,12 @@ struct background {
>> uint32_t color;
>> };
>>
>> +struct mode {
>> +int height;
>> +int width;
>> +uint32_t refresh;
>> +};
>> +
>> struct output {
>> struct wl_output *output;
>> uint32_t server_output_id;
>> @@ -103,6 +109,9 @@ struct output {
>>
>> struct panel *panel;
>> struct background *background;
>> +struct mode *mode;
>
>Hi,
>
>I think you could simplify this code quite a bit by just having the struct mode
>embed in struct output, instead of dynamically allocating it. You will always
>have
>exactly one struct mode for each struct output, right?
[Wang, Quanxian] fine. Got it.
>
>> +uint32_t transform;
>> +uint32_t scale;
>> };
>>
>> struct panel_launcher {
>> @@ -1128,6 +1137,10 @@ output_destroy(struct output *output) {
>> background_destroy(output->background);
>> panel_destroy(output->panel);
>> +
>> +if (output->mode)
>> +free(output->mode);
>> +
>> wl_output_destroy(output->output);
>> wl_list_remove(&output->link);
>>
>> @@ -1145,6 +1158,44 @@ 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 || !output->mode)
>> +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) {
>> +width /= output->scale;
>> +height /= output->scale;
>> +}
>> +
>> +/* Set min_allocation of panel */
>> +window_set_min_allocation(panel->window, width, 32);
>
>Is there a reason why the background window does not need the min_allocation
>changed?
[Wang, Quanxian] With testing, this doesn't make error. Let me double check
that.
>
>> +
>> +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 +1208,8 @@ output_handle_geometry(void *data, {
>> struct output *output = data;
>>
>> +output->transform = transform;
>> +update_output(output);
>> window_set_buffer_transform(output->panel->window,
>> transform); window_set_buffer_transform(output->background->window,
>> transform); }
>> @@ -1169,6 +1222,17 @@ output_handle_mode(void *data,
>> int height,
>> int refresh)
>> {
>> +struct output *output = data;
>> +
>
Re: [PATCH] Bug fix client apps because of output change
On Thu, 6 Mar 2014 16:25:42 +0800
Quanxian Wang 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.
>
> Signed-off-by: Quanxian Wang
> ---
> clients/desktop-shell.c | 70
> +
> clients/window.c| 7 + clients/window.h| 2 ++
> 3 files changed, 79 insertions(+)
>
> diff --git a/clients/desktop-shell.c b/clients/desktop-shell.c
> index a0c6b6d..9ccba34 100644
> --- a/clients/desktop-shell.c
> +++ b/clients/desktop-shell.c
> @@ -96,6 +96,12 @@ struct background {
> uint32_t color;
> };
>
> +struct mode {
> + int height;
> + int width;
> + uint32_t refresh;
> +};
> +
> struct output {
> struct wl_output *output;
> uint32_t server_output_id;
> @@ -103,6 +109,9 @@ struct output {
>
> struct panel *panel;
> struct background *background;
> + struct mode *mode;
Hi,
I think you could simplify this code quite a bit by just having the
struct mode embed in struct output, instead of dynamically allocating
it. You will always have exactly one struct mode for each struct
output, right?
> + uint32_t transform;
> + uint32_t scale;
> };
>
> struct panel_launcher {
> @@ -1128,6 +1137,10 @@ output_destroy(struct output *output)
> {
> background_destroy(output->background);
> panel_destroy(output->panel);
> +
> + if (output->mode)
> + free(output->mode);
> +
> wl_output_destroy(output->output);
> wl_list_remove(&output->link);
>
> @@ -1145,6 +1158,44 @@ 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 || !output->mode)
> + 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) {
> + width /= output->scale;
> + height /= output->scale;
> + }
> +
> + /* Set min_allocation of panel */
> + window_set_min_allocation(panel->window, width, 32);
Is there a reason why the background window does not need the
min_allocation changed?
> +
> + 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 +1208,8 @@ output_handle_geometry(void *data,
> {
> struct output *output = data;
>
> + output->transform = transform;
> + update_output(output);
> window_set_buffer_transform(output->panel->window,
> transform); window_set_buffer_transform(output->background->window,
> transform); }
> @@ -1169,6 +1222,17 @@ output_handle_mode(void *data,
> int height,
> int refresh)
> {
> + struct output *output = data;
> +
> + if (flags & WL_OUTPUT_MODE_CURRENT) {
> + if (!output && !output->mode)
> + return;
> +
> + output->mode->width = width;
> + output->mode->height = height;
> + output->mode->refresh = refresh;
> + update_output(output);
> + }
> }
>
> static void
> @@ -1184,6 +1248,8 @@ output_handle_scale(void *data,
> {
> struct output *output = data;
>
> + output->scale = scale;
> + update_output(output);
I think calling update_output() would be better done from the
wl_output.done event handler, because there you are guaranteed to have
all the relevant events received. Otherwise you race the scheduled
resize against receiving all the related events.
However, the catch is that wl_output.done is in interface version >= 2,
but not in version 1. For version 1 you'd need just call
update_output() directly like you already do.
> window_set_buffer_scale(output->panel->window, scale);
> window_set_buffer_scale(output->background->window, scale);
> }
> @@ -1206,6 +1272,10 @@ output_init(struct output *output, struct
> desktop *desktop) output->output, surface);
>
> output->background = background_create(desktop);
> + output->mode = (
