[Mesa-dev] [PATCH 05/29] mesa: Consider internal base format in _mesa_format_convert
Iago Toral
itoral at igalia.com
Thu Nov 20 02:31:04 PST 2014
On Thu, 2014-11-20 at 08:42 +0100, Iago Toral wrote:
> On Wed, 2014-11-19 at 11:43 -0800, Jason Ekstrand wrote:
> > A couple of specific comments are below. More generally, why are you
> > only considering the base format on two cases? Do we never use it for
> > anything else?
>
> I thought about that too but when I looked at the original code it
> seemed that it only cared for the base format in these two scenarios, so
> I thought that maybe the conversions cases that could be affected are
> all handled in those two paths. I'll check again though, just in case I
> missed something.
I don't know how I came to that conclusion but it seems wrong after
looking at the original code in texstore.c, which considers the base
internal format in the integer, float and ubyte paths too, so we should
do the same in _mesa_format_convert. I'll fix that.
Iago
> > On Tue, Nov 18, 2014 at 1:23 AM, Iago Toral Quiroga
> > <itoral at igalia.com> wrote:
> > Add a dst_internal_format parameter to _mesa_format_convert,
> > that represents
> > the base internal format for texture/pixel uploads, so we can
> > do the right
> > thing when the driver has selected a different internal format
> > for the target
> > dst format.
> > ---
> > src/mesa/main/format_utils.c | 65
> > +++++++++++++++++++++++++++++++++++++++++++-
> > src/mesa/main/format_utils.h | 2 +-
> > 2 files changed, 65 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/mesa/main/format_utils.c
> > b/src/mesa/main/format_utils.c
> > index fc59e86..5964689 100644
> > --- a/src/mesa/main/format_utils.c
> > +++ b/src/mesa/main/format_utils.c
> > @@ -303,7 +303,7 @@ _mesa_compute_component_mapping(GLenum
> > inFormat, GLenum outFormat, GLubyte *map)
> > void
> > _mesa_format_convert(void *void_dst, uint32_t dst_format,
> > size_t dst_stride,
> > void *void_src, uint32_t src_format,
> > size_t src_stride,
> > - size_t width, size_t height)
> > + size_t width, size_t height, GLenum
> > dst_internal_format)
> > {
> > uint8_t *dst = (uint8_t *)void_dst;
> > uint8_t *src = (uint8_t *)void_src;
> > @@ -422,6 +422,36 @@ _mesa_format_convert(void *void_dst,
> > uint32_t dst_format, size_t dst_stride,
> > if (src_array_format.as_uint && dst_array_format.as_uint)
> > {
> > assert(src_array_format.normalized ==
> > dst_array_format.normalized);
> >
> > + /* If the base format of our dst is not the same as the
> > provided base
> > + * format it means that we are converting to a
> > different format
> > + * than the one originally requested by the client.
> > This can happen when
> > + * the internal base format requested is not supported
> > by the driver.
> > + * In this case we need to consider the requested
> > internal base format to
> > + * compute the correct swizzle operation from src to
> > dst. We will do this
> > + * by computing the swizzle transform
> > src->rgba->base->rgba->dst instead
> > + * of src->rgba->dst.
> > + */
> > + mesa_format dst_mesa_format;
> > + if (dst_format & MESA_ARRAY_FORMAT_BIT)
> > + dst_mesa_format =
> > _mesa_format_from_array_format(dst_format);
> > + else
> > + dst_mesa_format = dst_format;
> >
> >
> > Let's put an extra line here so it doesn't get confused with the below
> > if statement
> >
> >
> > + if (dst_internal_format !=
> > _mesa_get_format_base_format(dst_mesa_format)) {
> > + /* Compute src2rgba as src->rgba->base->rgba */
> > + uint8_t rgba2base[4], base2rgba[4], swizzle[4];
> > + _mesa_compute_component_mapping(GL_RGBA,
> > dst_internal_format, rgba2base);
> > + _mesa_compute_component_mapping(dst_internal_format,
> > GL_RGBA, base2rgba);
> > +
> > + for (i = 0; i < 4; i++) {
> > + if (base2rgba[i] > MESA_FORMAT_SWIZZLE_W)
> > + swizzle[i] = base2rgba[i];
> > + else
> > + swizzle[i] =
> > src2rgba[rgba2base[base2rgba[i]]];
> >
> >
> > This doesn't work for composing three swizzles. If you get a ZERO or
> > ONE in rgba2base, you'll read the wrong memory from src2rgba.
>
> Actually, the problem is worse, because the mapping written by
> _mesa_compute_component_mapping is a 6-component mapping and we are
> passing a 4-component array. I'll fix this.
>
> >
> >
> > + }
> > + memcpy(src2rgba, swizzle, sizeof(src2rgba));
> > + }
> > +
> > + /* Compute src2dst from src2rgba */
> > for (i = 0; i < 4; i++) {
> > if (rgba2dst[i] > MESA_FORMAT_SWIZZLE_W) {
> > src2dst[i] = rgba2dst[i];
> > @@ -539,9 +569,42 @@ _mesa_format_convert(void *void_dst,
> > uint32_t dst_format, size_t dst_stride,
> > src += src_stride;
> > }
> > } else {
> > + /* For some conversions, doing src->rgba->dst is not
> > enough and we
> > + * need to consider the base internal format. In
> > these cases a
> > + * swizzle operation is required to match the
> > semantics of the base
> > + * internal format requested:
> > src->rgba->swizzle->rgba->dst.
> > + *
> > + * We can detect these cases by checking if the
> > swizzle transform
> > + * for base->rgba->base is 0123. If it is not, then
> > we need
> > + * to do the swizzle operation (need_convert =
> > true).
> > + */
> > + GLubyte rgba2base[4], base2rgba[4], map[4];
> > + bool need_convert = false;
> > + mesa_format dst_mesa_format;
> > + if (dst_format & MESA_ARRAY_FORMAT_BIT)
> > + dst_mesa_format =
> > _mesa_format_from_array_format(dst_format);
> > + else
> > + dst_mesa_format = dst_format;
> >
> >
> > Blank line again
> >
> >
> >
> > + if (dst_internal_format !=
> > + _mesa_get_format_base_format(dst_mesa_format)) {
> > + _mesa_compute_component_mapping(GL_RGBA,
> > dst_internal_format,
> > + base2rgba);
> > +
> > _mesa_compute_component_mapping(dst_internal_format, GL_RGBA,
> > + rgba2base);
> > + for (i = 0; i < 4; ++i) {
> > + map[i] = base2rgba[rgba2base[i]];
> > + if (map[i] != i)
> > + need_convert = true;
> > + }
> > + }
> > +
> > for (row = 0; row < height; ++row) {
> > _mesa_unpack_rgba_row(src_format, width,
> > src, tmp_float + row *
> > width);
> > + if (need_convert)
> > + _mesa_swizzle_and_convert(tmp_float + row *
> > width, GL_FLOAT, 4,
> > + tmp_float + row *
> > width, GL_FLOAT, 4,
> > + map, false, width);
> > src += src_stride;
> > }
> > }
> > diff --git a/src/mesa/main/format_utils.h
> > b/src/mesa/main/format_utils.h
> > index 4237ad3..29ab4a0 100644
> > --- a/src/mesa/main/format_utils.h
> > +++ b/src/mesa/main/format_utils.h
> > @@ -158,6 +158,6 @@ _mesa_compute_component_mapping(GLenum
> > inFormat, GLenum outFormat, GLubyte *map)
> > void
> > _mesa_format_convert(void *void_dst, uint32_t dst_format,
> > size_t dst_stride,
> > void *void_src, uint32_t src_format,
> > size_t src_stride,
> > - size_t width, size_t height);
> > + size_t width, size_t height, GLenum
> > dst_internal_format);
> >
> > #endif
> > --
> > 1.9.1
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
> >
>
More information about the mesa-dev
mailing list