[PATCH] Bug fix client apps because of output change

[Wang, Quanxian]

>-----Original Message-----
>From: Pekka Paalanen [mailto:ppaalanen at gmail.com]
>Sent: Monday, March 10, 2014 5:58 PM
>To: Wang, Quanxian
>Cc: wayland-devel at lists.freedesktop.org
>Subject: Re: [PATCH] Bug fix client apps because of output change
>
>On Mon, 10 Mar 2014 08:23:45 +0000
>"Wang, Quanxian" <quanxian.wang at intel.com> 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