[PATCH v14 21/41] [XXX] compositor-drm: Only disallow scaling for overlay planes
daniel at fooishbar.org
Wed Feb 7 16:30:36 UTC 2018
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.
> 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:
> /* 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.
More information about the wayland-devel