[PATCH v14 19/41] compositor-drm: Extract buffer->plane co-ord translation

[Pekka Paalanen]

On Fri, 23 Feb 2018 17:25:49 +0000
Daniel Stone <daniel at fooishbar.org> wrote:

> Hi Pekka,
> 
> On 23 January 2018 at 14:20, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> > On Wed, 20 Dec 2017 12:26:36 +0000 Daniel Stone <daniels at collabora.com> wrote:  
> >> +     if (sx1 < 0)
> >> +             sx1 = 0;
> >> +     if (sy1 < 0)
> >> +             sy1 = 0;
> >> +     if (sx2 > wl_fixed_from_int(ev->surface->width))
> >> +             sx2 = wl_fixed_from_int(ev->surface->width);
> >> +     if (sy2 > wl_fixed_from_int(ev->surface->height))
> >> +             sy2 = wl_fixed_from_int(ev->surface->height);  
> >
> > If the clamps changed something, we'd need equivalent adjustment in the
> > dest, but that seems to be ignored for now.  
> 
> Hm ... I am feeling particularly dense today, but I can't get my head
> around this. Why/how would we need to translate the dest region?

The function currently works like this:

1. Starting from the view parameters, compute the destination rectangle
   in the framebuffer.

2. Starting from the view parameters, compute the source rectangle in
   the buffer.

3. Source rectangle is clamped to the accessible buffer area.

If step 3 changes the source rectangle position in the buffer area, and
we do not take that into account in the destination rectangle, it will
look like as if the image has been shifted slightly. E.g. if computed
src_x was -1, and it gets clamped to 0, then the whole resulting image
will get shifted by one source pixel.

One possible solution would be to compute the source rectangle first,
clamp it, and then starting from the source rectangle compute the
destination rectangle. Whether the destination rectangle ends up any
different depends on the actual values in the transformation.

I think one could also take this further to take advantage of the
fractional source coordinates of KMS, by first computing the
destination coordinates from the view parameters, then computing the
source coordinates from the destination coordinates (which may produce
fractional values), clamp, and then re-compute the destination
coordinates from the final source coordinates.

However, as I gave my R-b already, I don't think this is important
enough to work on right now. Testing it would require carefully
constructing special test cases. We also seem to still lack an
automatable way to test this, so it would be a manual test. And it
seems like it would be easy to break this by accident.

It would be a lot of work for what seems to be a "rounding effect"
leading to the clamps even changing anything.

Also, this patch is just extracting code into a new function, not
intending to fix it, so it's fine.


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