[PATCH weston 08/12 v2] weston: Port Wayland backend to new output handling API

Fri Aug 19 07:39:07 UTC 2016

On Thu, 18 Aug 2016 16:52:33 +0200
Armin Krezović <krezovic.armin at gmail.com> wrote:

> On 18.08.2016 12:31, Pekka Paalanen wrote:
> > On Wed, 17 Aug 2016 19:49:06 +0200
> > Armin Krezović <krezovic.armin at gmail.com> wrote:
> >   
> >> On 17.08.2016 17:07, Pekka Paalanen wrote:  
> >>> On Sun, 14 Aug 2016 17:28:17 +0200
> >>> Armin Krezović <krezovic.armin at gmail.com> wrote:  
> 
> SNIP
> 
> >>>> +static int
> >>>> +wayland_output_configure_hotplug(struct weston_output *base)
> >>>> +{
> >>>> +	struct wayland_output *output = to_wayland_output(base);
> >>>> +	struct wayland_parent_output *poutput = output->user_data;
> >>>>  
> >>>>  	if (poutput->current_mode) {
> >>>> -		mode = poutput->current_mode;
> >>>> +		output->poutput_mode = poutput->current_mode;
> >>>>  	} else if (poutput->preferred_mode) {
> >>>> -		mode = poutput->preferred_mode;
> >>>> +		output->poutput_mode = poutput->preferred_mode;
> >>>>  	} else if (!wl_list_empty(&poutput->mode_list)) {
> >>>> -		mode = container_of(poutput->mode_list.next,
> >>>> -				    struct weston_mode, link);
> >>>> -	} else {
> >>>> -		weston_log("No valid modes found.  Skipping output\n");
> >>>> -		return NULL;
> >>>> -	}
> >>>> -
> >>>> -	if (!wl_list_empty(&b->compositor->output_list)) {
> >>>> -		output = container_of(b->compositor->output_list.prev,
> >>>> -				      struct wayland_output, base.link);
> >>>> -		x = output->base.x + output->base.current_mode->width;
> >>>> +		output->poutput_mode = container_of(poutput->mode_list.next,
> >>>> +						    struct weston_mode, link);
> >>>>  	} else {
> >>>> -		x = 0;
> >>>> +		weston_log("No valid modes found. Cannot configure an output.\n");
> >>>> +		return -1;
> >>>>  	}
> >>>>  
> >>>> -	output = wayland_output_create(b, x, 0, mode->width, mode->height,
> >>>> -				       NULL, 0,
> >>>> -				       WL_OUTPUT_TRANSFORM_NORMAL, 1);
> >>>> -	if (!output)
> >>>> -		return NULL;
> >>>> +	if (wayland_output_configure(&output->base, output->poutput_mode->width,
> >>>> +				     output->poutput_mode->height) < 0)    
> >>>
> >>> If wayland_output_configure() multiplies by scale and applies
> >>> transformation, then passing mode->width and mode->height directly here
> >>> is wrong.
> >>>
> >>> The "video mode" is given in output pixels, so it is already multiplied
> >>> by scale and rotated by transform.
> >>>
> >>> I see the old code was equally wrong, particularly if the output was
> >>> rotated 90 degrees it would create a window with inverted aspect ratio.
> >>> Ok, so if this keeps the old broken behaviour, it's fine.
> >>>
> >>> Testing the upstream x11-backend, it looks like we expect to put the
> >>> logical output size (in global coordinates) in weston.ini, and then
> >>> expect the scale and transform to be applied on that to get the output
> >>> resolution in output pixels.
> >>>     
> >>
> >> This is for parent output (fs shell and sprawl). You can't change the size
> >> here. Scale and transform are also hardcoded to "scale = 1, transform = WL_OUTPUT_TRANSFORM_NORMAL"
> >> (well, they used to be and I let them remain that way).
> >>
> >> See:
> >>
> >> https://cgit.freedesktop.org/wayland/weston/tree/libweston/compositor-wayland.c#n1148
> >>
> >> and:
> >>
> >> https://github.com/krezovic/weston/blob/gsoc-v10/compositor/main.c#L1545
> >>
> >> Those values are set just for sake of it, it's not that anything uses them.  
> > 
> > Yes, but ideally we might use also the scale and transform from the
> > parent output.
> >   
> 
> But, if parent output was transformed, wouldn't that display the surface
> just as it should be? Applying transform might further scale/rotate it. If
> it were to be used for "show off" (in weston-info, for example), we could
> do that. But I doubt we want any additional transformations if parent output
> is already transformed.

It does not matter which transform we choose, the image will always
be shown the right way because we transform our drawing to match -
provided we get the dimensions right to begin with. Also scale matters
only for "sharpness". If we use a scale different from the parent, the
image just gets scaled by the parent compositor. (Obviously this does
not apply to the x11-backend, because there the "parent compositor"
does not scale nor rotate depending on how we draw.)

The reason why transform and scale are exposed to clients at all is
that it allows clients to draw their buffers in a way that could bypass
the compositing step, as the (parent) compositor does not need to scale
and rotate. So using the parent scale and transform would be an
optimization.

> So, for *_for_parent_output case, I've hardcoded transform and scale values
> in the backend, just before calls to wayland_output_set_size().

Yes, hardcoding is good for now.

> > Those values are no longer hardcoded in this site here, which makes
> > things harder to follow, and wayland_output_configure() does use them.
> > 
> > However, that doesn't really affect this patch. Except maybe needing a
> > comment about scale and transform.
> > 
> > Btw. wayland_output_configure() forgets to apply transform, but that's
> > how it was already.
> >   
> 
> Backend functions don't do that. It's done in libweston, (current) weston_output_init, rather.

Wayland_output_configure() does apply scale. Either you have to apply
both or neither, so something is off there. But that is for another
time to be fixed.

Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160819/5378d01f/attachment.sig>