[Mesa-dev] [PATCH 1/2] vulkan/util: Introduce format utilities

Chad Versace chadversary at chromium.org
Tue Jun 27 23:29:23 UTC 2017


On Fri 23 Jun 2017, alexandros.frantzis at collabora.com wrote:
> From: Alexandros Frantzis <alexandros.frantzis at collabora.com>
> 
> Introduce utilities to describe, find and compare Vulkan formats based
> on their color component masks, taking into account system endianness.
> ---
>  src/vulkan/Makefile.sources      |   2 +
>  src/vulkan/util/vk_format_util.c | 173 +++++++++++++++++++++++++++++++++++++++
>  src/vulkan/util/vk_format_util.h | 100 ++++++++++++++++++++++
>  3 files changed, 275 insertions(+)
>  create mode 100644 src/vulkan/util/vk_format_util.c
>  create mode 100644 src/vulkan/util/vk_format_util.h
> 
> diff --git a/src/vulkan/Makefile.sources b/src/vulkan/Makefile.sources
> index 2cf7218e92..f4f1f14d5a 100644
> --- a/src/vulkan/Makefile.sources
> +++ b/src/vulkan/Makefile.sources
> @@ -17,6 +17,8 @@ VULKAN_WSI_X11_FILES := \
>  
>  VULKAN_UTIL_FILES := \
>  	util/vk_alloc.h \
> +	util/vk_format_util.c \
> +	util/vk_format_util.h \
>  	util/vk_util.c \
>  	util/vk_util.h
>  
> diff --git a/src/vulkan/util/vk_format_util.c b/src/vulkan/util/vk_format_util.c
> new file mode 100644
> index 0000000000..aa6d403e84
> --- /dev/null
> +++ b/src/vulkan/util/vk_format_util.c
> @@ -0,0 +1,173 @@
> +/*
> + * Copyright © 2017 Collabora Ltd.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + */
> +
> +#include "vk_format_util.h"
> +
> +#include "util/u_vector.h"
> +#include "util/u_math.h"
> +
> +#include <assert.h>
> +
> +static inline uint32_t
> +bswap24(uint32_t n)
> +{
> +   return (n & 0xff000000) |
> +          ((n >> 16) & 0x000000ff) |
> +          (n & 0x0000ff00) |
> +          ((n << 16) & 0x00ff0000);
> +}
> +
> +static inline uint32_t
> +bswap(uint32_t n, int bits)
> +{
> +   switch (bits)
> +   {

Mesa code-style puts the '{' on the same line as 'switch'.

> +   case 8:
> +      return n;
> +   case 16:
> +      return util_bswap16(n);
> +   case 24:
> +      return bswap24(n);
> +   case 32:
> +      return util_bswap32(n);
> +   default:
> +      assert(!"Invalid bswap bits");
> +      return n;

If someone passes an invalid param here, there's little point in
continuing, even in release builds, because continuing will produce only
undefined behavior. So let's optimize the release build by removing the
default case, like this:

    bswap(...)
    {
        switch (bits) {
        case 8:
            ...
        case 16:
            ...
        case 32:
            ...
        }

        unreachable("invalid bswap bits");
    }

> +   }
> +}
> +
> +struct format_spec {
> +   VkFormat format;
> +   struct vk_format_util_spec spec;
> +};
> +
> +#define VF(FMT, BPP, R, G, B, A, E, S) \
> +   {VK_FORMAT_##FMT, {BPP, R, G, B, A, VK_FORMAT_UTIL_ENDIANNESS_##E, VK_FORMAT_UTIL_##S}}
> +
> +// SRGB formats are presented to the user before the UNORM ones.

The comment should explain why the sRGB formats precede the others.

Also, Mesa code-style uses /* comments, not // comments.

> +static const struct format_spec format_specs[] = {
> +   VF(R8_SRGB, 8, 0x000000ff, 0x00000000, 0x00000000, 0x00000000, BIG, SWAPPABLE),
> +   VF(R8G8_SRGB, 16, 0x0000ff00, 0x000000ff, 0x00000000, 0x00000000, BIG, SWAPPABLE),
> +   VF(R8G8B8_SRGB, 24, 0x00ff0000, 0x0000ff00, 0x000000ff, 0x00000000, BIG, SWAPPABLE),
> +   VF(B8G8R8_SRGB, 24, 0x000000ff, 0x0000ff00, 0x00ff0000, 0x00000000, BIG, SWAPPABLE),
> +   VF(R8G8B8A8_SRGB, 32, 0xff000000, 0x00ff0000, 0x0000ff00, 0x000000ff, BIG, SWAPPABLE),
> +   VF(B8G8R8A8_SRGB, 32, 0x0000ff00, 0x00ff0000, 0xff000000, 0x000000ff, BIG, SWAPPABLE),
> +   VF(A8B8G8R8_SRGB_PACK32, 32, 0x000000ff, 0x0000ff00, 0x00ff0000, 0xff000000, NATIVE, SWAPPABLE),
> +
> +   VF(R8_UNORM, 8, 0x000000ff, 0x00000000, 0x00000000, 0x00000000, BIG, SWAPPABLE),
> +   VF(R8G8_UNORM, 16, 0x0000ff00, 0x000000ff, 0x00000000, 0x00000000, BIG, SWAPPABLE),
> +   VF(R8G8B8_UNORM, 24, 0x00ff0000, 0x0000ff00, 0x000000ff, 0x00000000, BIG, SWAPPABLE),
> +   VF(B8G8R8_UNORM, 24, 0x000000ff, 0x0000ff00, 0x00ff0000, 0x00000000, BIG, SWAPPABLE),
> +   VF(R8G8B8A8_UNORM, 32, 0xff000000, 0x00ff0000, 0x0000ff00, 0x000000ff, BIG, SWAPPABLE),
> +   VF(B8G8R8A8_UNORM, 32, 0x0000ff00, 0x00ff0000, 0xff000000, 0x000000ff, BIG, SWAPPABLE),
> +   VF(R4G4_UNORM_PACK8, 8, 0x000000f0, 0x0000000f, 0x00000000, 0x00000000, NATIVE, NON_SWAPPABLE),
> +   VF(R4G4B4A4_UNORM_PACK16, 16, 0x0000f000, 0x00000f00, 0x000000f0, 0x0000000f, NATIVE, NON_SWAPPABLE),
> +   VF(B4G4R4A4_UNORM_PACK16, 16, 0x000000f0, 0x00000f00, 0x0000f000, 0x0000000f, NATIVE, NON_SWAPPABLE),
> +   VF(R5G6B5_UNORM_PACK16, 16, 0x0000f800, 0x000007e0, 0x0000001f, 0x00000000, NATIVE, NON_SWAPPABLE),
> +   VF(B5G6R5_UNORM_PACK16, 16, 0x0000001f, 0x000007e0, 0x0000f800, 0x00000000, NATIVE, NON_SWAPPABLE),
> +   VF(R5G5B5A1_UNORM_PACK16, 16, 0x0000f800, 0x000007c0, 0x0000003e, 0x00000001, NATIVE, NON_SWAPPABLE),
> +   VF(B5G5R5A1_UNORM_PACK16, 16, 0x0000003e, 0x000007c0, 0x0000f800, 0x00000001, NATIVE, NON_SWAPPABLE),
> +   VF(A1R5G5B5_UNORM_PACK16, 16, 0x00007c00, 0x000003e0, 0x0000001f, 0x00008000, NATIVE, NON_SWAPPABLE),
> +   VF(A8B8G8R8_UNORM_PACK32, 32, 0x000000ff, 0x0000ff00, 0x00ff0000, 0xff000000, NATIVE, SWAPPABLE),
> +   VF(A2R10G10B10_UNORM_PACK32, 32, 0x3ff00000, 0x000ffc00, 0x000003ff, 0xc0000000, NATIVE, NON_SWAPPABLE),
> +   VF(A2B10G10R10_UNORM_PACK32, 32, 0x000003ff, 0x000ffc00, 0x3ff00000, 0xc0000000, NATIVE, NON_SWAPPABLE),
> +};

The concepts of "swappable" and "non-swappable", as applied to formats
here, is not a concept present in the Vulkan spec.

I'm not sure what swappable/non-swappable encodes in this table, and why
vk_compare_format_specs() is trying to do with swappability. I tried
reverse-engineering the patch's intent, and formed the following
observations and questions:

    1. The table partitions the formats into 3 classes:

        a. Big-endian and swappable. All non-packed formats belong to
           this class.

        b. Native-endian and non-swappable. All packed formats, except
           VK_FORMAT_A8B8G8R8_SRGB_PACK32 and
           VK_FORMAT_A8B8G8R8_UNORM_PACK32, belong to this class.

        c. Native-endian and swappable. Exactly
           VK_FORMAT_A8B8G8R8_SRGB_PACK32 and
           VK_FORMAT_A8B8G8R8_UNORM_PACK32 belong to this class.

    2. Why are no formats marked as little-endian?

    3. Why are the A8B8G8R8 formats special? Because each channel is
       byte-aligned?

I suspect that the table is actually trying to capture the
distinction between "packed" and "non-packed" formats, as defined
by the Vulkan spec, in section 32.2.1 Format Definition. (Some people
say "array" formats instead of "non-packed" formats). (The Vulkan spec
also defines the concept of "block" formats, but that's irrelevant to
this patch).

I don't believe the endianness scheme in this patch is not able to capture the
bit-layout of VK_FORMAT_R16G16B16A16_UNORM, but the Vulkan spec's notion
of "packed/non-packed" is. According to table "Byte mappings for
non-packed/compressed color formats" in the Vulkan 1.0.51 spec,
VK_FORMAT_R16G16B16A16_UNORM is a non-packed format where the red channel
is bytes 0-1, green is bytes 2-3, blue is bytes 4-5, etc, but the bytes
within each channel are ordered by host endianness.

I don't understand how swappable/non-swappable apply to
VK_FORMAT_R16G16B16A16_UNORM.

> +
> +#undef VF
> +
> +static inline vk_format_util_endianness
> +native_endianness()

In C, the empty parameter list is not what you want. The compiler
interprets the empty parameter parameter list as an old-style variadic
parameter list.  You want native_endianness(void) instead.

Anyway, this function should be replaced by the macros in
src/util/u_endian.h.

> +{
> +   const unsigned ui = 1;
> +   return *((const char *) &ui) == 1 ? VK_FORMAT_UTIL_ENDIANNESS_LITTLE :
> +                                       VK_FORMAT_UTIL_ENDIANNESS_BIG;
> +}
> +
> +void
> +vk_get_format_spec(VkFormat format,
> +                   struct vk_format_util_spec *spec)
> +{
> +   static const struct vk_format_util_spec empty_spec =
> +      {0, 0x0, 0x0, 0x0, 0x0, VK_FORMAT_UTIL_ENDIANNESS_NATIVE, VK_FORMAT_UTIL_NON_SWAPPABLE};

The verbose struct initializer should be replaced with `empty_spec = {0};`
The C99 spec guarantees that `= {0}` produces a zero-initialized struct
here.

> +
> +   for (size_t i = 0; i < ARRAY_SIZE(format_specs); i++) {
> +      if (format_specs[i].format == format) {
> +         *spec = format_specs[i].spec;
> +         return;
> +      }
> +   }
> +
> +   *spec = empty_spec;
> +}
> +
> +bool
> +vk_compare_format_specs(const struct vk_format_util_spec* spec1,
> +                        const struct vk_format_util_spec* spec2)
> +{
> +   const vk_format_util_endianness endianness1 =
> +      spec1->endianness == VK_FORMAT_UTIL_ENDIANNESS_NATIVE ?
> +      native_endianness() : spec1->endianness;
> +
> +   const vk_format_util_endianness endianness2 =
> +      spec2->endianness == VK_FORMAT_UTIL_ENDIANNESS_NATIVE ?
> +      native_endianness() : spec2->endianness;
> +
> +   if (endianness1 == endianness2) {
> +      return spec1->bits_per_pixel == spec2->bits_per_pixel &&
> +             spec1->red_mask == spec2->red_mask &&
> +             spec1->green_mask == spec2->green_mask &&
> +             spec1->blue_mask == spec2->blue_mask &&
> +             spec1->alpha_mask == spec2->alpha_mask;
> +   }
> +   else {

I don't understand why the 'else' branch exists. But that's likely due
to my confusion regarding swappable/non-swappable, explained above.

Also, Mesa code-style is `} else {`.

> +      if (spec1->swappable == VK_FORMAT_UTIL_SWAPPABLE) {
> +         return spec1->bits_per_pixel == spec2->bits_per_pixel &&
> +                bswap(spec1->red_mask, spec1->bits_per_pixel) == spec2->red_mask &&
> +                bswap(spec1->green_mask, spec1->bits_per_pixel) == spec2->green_mask &&
> +                bswap(spec1->blue_mask, spec1->bits_per_pixel) == spec2->blue_mask &&
> +                bswap(spec1->alpha_mask, spec1->bits_per_pixel) == spec2->alpha_mask;
> +      }
> +      else if (spec2->swappable == VK_FORMAT_UTIL_SWAPPABLE) {
> +         return spec1->bits_per_pixel == spec2->bits_per_pixel &&
> +                spec1->red_mask == bswap(spec2->red_mask, spec2->bits_per_pixel) &&
> +                spec1->green_mask == bswap(spec2->green_mask, spec2->bits_per_pixel) &&
> +                spec1->blue_mask == bswap(spec2->blue_mask, spec2->bits_per_pixel) &&
> +                spec1->alpha_mask == bswap(spec2->alpha_mask, spec2->bits_per_pixel);
> +      }
> +   }
> +
> +   return false;
> +}
> +
> +void
> +vk_find_matching_formats(const struct vk_format_util_spec *spec,
> +                         struct u_vector *formats)
> +{
> +   for (size_t i = 0; i < ARRAY_SIZE(format_specs); i++) {
> +      if (vk_compare_format_specs(spec, &format_specs[i].spec)) {
> +         VkFormat *f = u_vector_add(formats);
> +         if (f)
> +            *f = format_specs[i].format;
> +      }
> +   }
> +}
> diff --git a/src/vulkan/util/vk_format_util.h b/src/vulkan/util/vk_format_util.h
> new file mode 100644
> index 0000000000..a1f9d87abe
> --- /dev/null
> +++ b/src/vulkan/util/vk_format_util.h
> @@ -0,0 +1,100 @@
> +/*
> + * Copyright © 2017 Collabora Ltd.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + */
> +
> +#ifndef VK_FORMAT_UTIL_H
> +#define VK_FORMAT_UTIL_H
> +
> +#include <vulkan/vulkan.h>
> +#include <stdbool.h>
> +
> +struct u_vector;
> +
> +typedef enum vk_format_util_endianness {
> +   VK_FORMAT_UTIL_ENDIANNESS_NATIVE,
> +   VK_FORMAT_UTIL_ENDIANNESS_LITTLE,
> +   VK_FORMAT_UTIL_ENDIANNESS_BIG,
> +} vk_format_util_endianness;
> +
> +typedef enum vk_format_util_swappable {
> +   VK_FORMAT_UTIL_NON_SWAPPABLE,
> +   VK_FORMAT_UTIL_SWAPPABLE,
> +} vk_format_util_swappable;
> +
> +/*
> + * Structure describing a pixel format.
> + *
> + * The masks express the bit positions of the components in an native integer
> + * storing a pixel of this format.
> + *
> + * Examples:
> + *
> + *   * A pixel format which is always stored as a 0xR8G8B8A8 32-bit integer:
> + *
> + *     {32, 0xff000000, 0x00ff0000, 0x0000ff00, 0x000000ff, NATIVE, SWAPPABLE}
> + *
> + *   * A pixel format which is always stored as R8,G8,B8,A8 in memory order,
> + *     with R at the lowest address:
> + *
> + *     {32, 0xff000000, 0x00ff0000, 0x0000ff00, 0x000000ff, BIG, SWAPPABLE}
> + *     or
> + *     {32, 0x000000ff, 0x0000ff00, 0x00ff0000, 0xff000000, LITTLE, SWAPPABLE}
> + *
> + *   * A pixel format which is always stored as a 0xR5G6B5 16-bit integer,
> + *     and cannot change endianness by byte-swapping:
> + *
> + *     {16, 0x0000f800, 0x000007e0, 0x0000001f, 0x00000000, NATIVE, NON_SWAPPABLE},
> + *
> + */
> +struct vk_format_util_spec {
> +   int bits_per_pixel;
> +   uint32_t red_mask;
> +   uint32_t green_mask;
> +   uint32_t blue_mask;
> +   uint32_t alpha_mask;
> +   /* The endianness for which the masks are correct */
> +   vk_format_util_endianness endianness;
> +   /* Whether the masks can be byte-swapped to switch endianness */
> +   vk_format_util_swappable swappable;
> +};
> +
> +
> +/* Fill a vk_format_util_spec structure to describe a Vulkan format. */
> +void
> +vk_get_format_spec(VkFormat format,
> +                   struct vk_format_util_spec *spec);
> +
> +/* Compare two pixel formats described by vk_format_util_spec structures. */
> +bool
> +vk_compare_format_specs(const struct vk_format_util_spec *spec1,
> +                        const struct vk_format_util_spec *spec2);
> +
> +/*
> + * Find Vulkan pixel formats that match a vk_format_util_spec.
> + *
> + * Note that, at the moment, only UNORM and SRGB formats are considered.
> + */
> +void
> +vk_find_matching_formats(const struct vk_format_util_spec *spec,
> +                         struct u_vector *formats);
> +
> +#endif
> -- 
> 2.11.0
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list