[PATCH] Bug fix client apps because of output change

[Pekka Paalanen]

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.

> >
> >> +		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