[Mesa-dev] [PATCH 4/4] i965/copy_image: Divide the x offsets by block width when using the blitter

[Jason Ekstrand]

On Wed, Sep 3, 2014 at 11:58 AM, Matt Turner <mattst88 at gmail.com> wrote:

> On Tue, Sep 2, 2014 at 3:49 PM, Jason Ekstrand <jason at jlekstrand.net>
> wrote:
> > Signed-off-by: Jason Ekstrand <jason.ekstrand at intel.com>
> > ---
> >  src/mesa/drivers/dri/i965/intel_copy_image.c | 31
> +++++++++++++++++++---------
> >  1 file changed, 21 insertions(+), 10 deletions(-)
> >
> > diff --git a/src/mesa/drivers/dri/i965/intel_copy_image.c
> b/src/mesa/drivers/dri/i965/intel_copy_image.c
> > index 8bda2dd..f7db56d 100644
> > --- a/src/mesa/drivers/dri/i965/intel_copy_image.c
> > +++ b/src/mesa/drivers/dri/i965/intel_copy_image.c
> > @@ -40,6 +40,7 @@ copy_image_with_blitter(struct brw_context *brw,
> >                          int src_width, int src_height)
> >  {
> >     GLuint bw, bh;
> > +   uint32_t src_image_x, src_image_y, dst_image_x, dst_image_y;
> >     int cpp;
> >
> >     /* The blitter doesn't understand multisampling at all. */
> > @@ -70,6 +71,9 @@ copy_image_with_blitter(struct brw_context *brw,
> >        return false;
> >     }
> >
> > +   intel_miptree_get_image_offset(src_mt, src_level, src_z,
> > +                                  &src_image_x, &src_image_y);
> > +
> >     if (_mesa_is_format_compressed(src_mt->format)) {
> >        _mesa_get_format_block_size(src_mt->format, &bw, &bh);
> >
> > @@ -83,10 +87,21 @@ copy_image_with_blitter(struct brw_context *brw,
> >        src_width /= (int)bw;
> >        src_height /= (int)bh;
> >
> > +      /* Inside of the miptree, the x offsets are storred in pixels
> while
>
> "stored" (the line above and below)
>
> > +       * the y offsets are storred in blocks.  We need to scale just
> the x
> > +       * offset.
> > +       */
> > +      src_image_x /= bw;
> > +
> >        cpp = _mesa_get_format_bytes(src_mt->format);
> >     } else {
> >        cpp = src_mt->cpp;
> >     }
> > +   src_x += src_image_x;
> > +   src_y += src_image_y;
> > +
> > +   intel_miptree_get_image_offset(dst_mt, dst_level, dst_z,
> > +                                  &dst_image_x, &dst_image_y);
> >
> >     if (_mesa_is_format_compressed(dst_mt->format)) {
> >        _mesa_get_format_block_size(dst_mt->format, &bw, &bh);
> > @@ -96,17 +111,13 @@ copy_image_with_blitter(struct brw_context *brw,
> >
> >        dst_x /= (int)bw;
> >        dst_y /= (int)bh;
> > -   }
> >
> > -   uint32_t src_image_x, src_image_y;
> > -   intel_miptree_get_image_offset(src_mt, src_level, src_z,
> > -                                  &src_image_x, &src_image_y);
> > -   src_x += src_image_x;
> > -   src_y += src_image_y;
> > -
> > -   uint32_t dst_image_x, dst_image_y;
> > -   intel_miptree_get_image_offset(dst_mt, dst_level, dst_z,
> > -                                  &dst_image_x, &dst_image_y);
> > +      /* Inside of the miptree, the x offsets are storred in pixels
> while
> > +       * the y offsets are storred in blocks.  We need to scale just
> the x
>
> "stored" (the line above and below)
>
> All of these patches look like legitimate bug fixes. There needs to be
> a Bugzilla: ... tag for bug 82804 (and presumably 83036 as well?) and
> some Cc: mesa-stable tags.
>

Yeah, doesn't fix 83036.  There are some subtests there that fail but
that's not because of problems in CopyImageSubData but because of precision
loss in GetTexImage.  I haven't gotten around to fixing GetTexImage yet;
one of these days...


>
> With those typos fixed and the appropriate tags added, the series is
>
> Reviewed-by: Matt Turner <mattst88 at gmail.com>
>

Pushed, thanks.
--Jason
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140903/baf323db/attachment-0001.html>