[PATCH v14 21/41] [XXX] compositor-drm: Only disallow scaling for overlay planes

[Daniel Stone]

Hi Pekka,

On 24 January 2018 at 10:28, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> On Wed, 20 Dec 2017 12:26:38 +0000
> Daniel Stone <daniels at collabora.com> wrote:
>> Now we have a more comprehensive overview of the transform we're going
>> to apply, use this to explicitly disallow scaling and rotation.
>>
>> XXX: This does not actually disallow rotation for square surfaces.
>>      We would have to build up a full buffer->view->output transform
>>      matrix, and then analyse that.
>
> Hi,
>
> I think the most important bit is missing in the commit message: why?
> What's wrong with the old checks?
>
> Is it only the wp_viewport state or something more?
>
> Maybe we could use this patch and add a helper that explicitly checks
> all the little bits of transformations to ensure there is no rotation?
> It would not have to allow all cases, e.g. if the transformation matrix
> inverts the buffer/output transform rotation we could just ignore and
> reject that case. The helper would essentially be:
>
> bool
> supported()
> {
>         /* XXX: This is safe, but still rejects some valid configurations */
>         if (viewport->buffer.transform != output->base.transform)
>                 return false;
>         if (!drm_view_transform_supported(ev))
>                 return false;
>         return true;
> }
>
> wp_viewport cannot produce a rotation, so there is no need to check it.

Right. If I remember correctly, it was noting how the various plane
implementations all failed to account for total wp_viewport/view
transformations correctly, or in some cases at all.

> That might allow to keep this and the following patches that unify the
> transformation handling between primary/overlay/cursor planes.
>
> It might even pay off to integrate this check into
> drm_plane_state_coords_for_view(), adding a success/reject return
> value. That function does assume that there is no end-to-end rotation
> at all. It won't work for arbitrarily rotated views, and for 90-degree
> step rotations it may produce negative width and/or height to imply a
> flip. I think it should explicitly fail rather than produce garbage
> when the preconditions are violated.

I think that's a better option. I'll look at making
plane_state_coords_for_view fail for anything but crop/scale, and then
leave it up to the users to check for crop/scale if they need to.

Thanks!

Cheers,
Daniel