| Message ID | 86a929ypsa.fsf@hiro.keithp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Keith Packard <keithp@keithp.com> writes: > Kertesz Laszlo <laszlo.kertesz@gmail.com> writes: > >> Ok, rebuilt the xserver package with debugging symbols (seems that >> checkinstall strips stuff by default). I got a bigger gdb.txt. See if it >> helps. > > I found a bug -- glamor_xv_put_image was mis-computing the number of > lines of changed video when the client drew only a subset of the > image. I think the client is drawing at src_y=1, src_h=239 for some > weird reason (I suspect a bug in the client). > > Try this patch: > > From eaa4225413b31314070f9a52d9290649e79a3b0f Mon Sep 17 00:00:00 2001 > From: Keith Packard <keithp@keithp.com> > Date: Sat, 27 Dec 2014 09:11:33 -0800 > Subject: [PATCH] glamor: Fix nlines in glamor_xv_put_image when src_y is odd > > The number of lines of video to update in the texture needs to be > computed from the height of the updated source, not the full height of > the source. > > Signed-off-by: Keith Packard <keithp@keithp.com> > --- > glamor/glamor_xv.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/glamor/glamor_xv.c b/glamor/glamor_xv.c > index 1c877da..83e24ad 100644 > --- a/glamor/glamor_xv.c > +++ b/glamor/glamor_xv.c > @@ -435,7 +435,7 @@ glamor_xv_put_image(glamor_port_private *port_priv, > } > > top = (src_y) & ~1; > - nlines = (src_y + height) - top; > + nlines = (src_y + src_h) - top; > > switch (id) { > case FOURCC_YV12: If the point is to upload only from the src_[xywh] recctangle, shouldn't the glamor_upload_sub_pixmap_to_texture() calls be using src_w instead of width, too?
On Sat, 2014-12-27 at 09:18 -0800, Keith Packard wrote: > Kertesz Laszlo <laszlo.kertesz@gmail.com> writes: > > > Ok, rebuilt the xserver package with debugging symbols (seems that > > checkinstall strips stuff by default). I got a bigger gdb.txt. See if it > > helps. > > I found a bug -- glamor_xv_put_image was mis-computing the number of > lines of changed video when the client drew only a subset of the > image. I think the client is drawing at src_y=1, src_h=239 for some > weird reason (I suspect a bug in the client). > > Try this patch: > Tried it and it works. I had a ~10 min Skype call and had no issues.
Eric Anholt <eric@anholt.net> writes: >> --- a/glamor/glamor_xv.c >> +++ b/glamor/glamor_xv.c >> @@ -435,7 +435,7 @@ glamor_xv_put_image(glamor_port_private *port_priv, >> } >> >> top = (src_y) & ~1; >> - nlines = (src_y + height) - top; >> + nlines = (src_y + src_h) - top; >> >> switch (id) { >> case FOURCC_YV12: > > If the point is to upload only from the src_[xywh] recctangle, shouldn't > the glamor_upload_sub_pixmap_to_texture() calls be using src_w instead > of width, too? It doesn't need to, but it could as an optimization. Skipping lines at the top and bottom is also just an optimization as the source rectangle defines a subset of the provided buffer, after all. I just fixed that optimization.
Keith Packard <keithp@keithp.com> writes: > Eric Anholt <eric@anholt.net> writes: > >>> --- a/glamor/glamor_xv.c >>> +++ b/glamor/glamor_xv.c >>> @@ -435,7 +435,7 @@ glamor_xv_put_image(glamor_port_private *port_priv, >>> } >>> >>> top = (src_y) & ~1; >>> - nlines = (src_y + height) - top; >>> + nlines = (src_y + src_h) - top; >>> >>> switch (id) { >>> case FOURCC_YV12: >> >> If the point is to upload only from the src_[xywh] recctangle, shouldn't >> the glamor_upload_sub_pixmap_to_texture() calls be using src_w instead >> of width, too? > > It doesn't need to, but it could as an optimization. Skipping lines at > the top and bottom is also just an optimization as the source rectangle > defines a subset of the provided buffer, after all. I just fixed that > optimization. glamor_xv_render is trying to scale from dst coords to src coords using multiplication by src_w / dst_w, though, so if the src pixmap was width wide instead of src_w wide, I think you'd be rendering wrong.
Keith Packard <keithp@keithp.com> writes: > Eric Anholt <eric@anholt.net> writes: > >>> --- a/glamor/glamor_xv.c >>> +++ b/glamor/glamor_xv.c >>> @@ -435,7 +435,7 @@ glamor_xv_put_image(glamor_port_private *port_priv, >>> } >>> >>> top = (src_y) & ~1; >>> - nlines = (src_y + height) - top; >>> + nlines = (src_y + src_h) - top; >>> >>> switch (id) { >>> case FOURCC_YV12: >> >> If the point is to upload only from the src_[xywh] recctangle, shouldn't >> the glamor_upload_sub_pixmap_to_texture() calls be using src_w instead >> of width, too? > > It doesn't need to, but it could as an optimization. Skipping lines at > the top and bottom is also just an optimization as the source rectangle > defines a subset of the provided buffer, after all. I just fixed that > optimization. FWIW, even as is: Reviewed-by: Eric Anholt <eric@anholt.net>
Eric Anholt <eric@anholt.net> writes:
> Reviewed-by: Eric Anholt <eric@anholt.net>
Merged.
09230a2..d723928 master -> master
From eaa4225413b31314070f9a52d9290649e79a3b0f Mon Sep 17 00:00:00 2001 From: Keith Packard <keithp@keithp.com> Date: Sat, 27 Dec 2014 09:11:33 -0800 Subject: [PATCH] glamor: Fix nlines in glamor_xv_put_image when src_y is odd The number of lines of video to update in the texture needs to be computed from the height of the updated source, not the full height of the source. Signed-off-by: Keith Packard <keithp@keithp.com> --- glamor/glamor_xv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/glamor/glamor_xv.c b/glamor/glamor_xv.c index 1c877da..83e24ad 100644 --- a/glamor/glamor_xv.c +++ b/glamor/glamor_xv.c @@ -435,7 +435,7 @@ glamor_xv_put_image(glamor_port_private *port_priv, } top = (src_y) & ~1; - nlines = (src_y + height) - top; + nlines = (src_y + src_h) - top; switch (id) { case FOURCC_YV12: -- 2.1.4