On Fri, 23 Feb 2018 17:25:49 +0000 Daniel Stone <dan...@fooishbar.org> wrote:
> Hi Pekka, > > On 23 January 2018 at 14:20, Pekka Paalanen <ppaala...@gmail.com> wrote: > > On Wed, 20 Dec 2017 12:26:36 +0000 Daniel Stone <dani...@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
pgpPhYSouVe3t.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
