Re: [PATCH 3/5] wined3d: Prepare wined3d_format_convert_from_float for > 32bpp formats (v2).

[Henri Verbeet]

On Fri, 13 May 2022 at 14:55, Stefan Dösinger  wrote:
> @@ -6062,37 +6067,59 @@ DWORD wined3d_format_convert_from_float(const struct wined3d_format *format, con
>  for (i = 0; i < ARRAY_SIZE(float_conv); ++i)
>  {
> -
>    uint32_t *ret_i = ret;
> -
>    if (format_id != float_conv[i].format_id)
>        continue;
>
> -
>    ret = ((DWORD)((color->r * float_conv[i].mul.x) + 0.5f)) << float_conv[i].shift.x;
> -
>    ret |= ((DWORD)((color->g * float_conv[i].mul.y) + 0.5f)) << float_conv[i].shift.y;
> -
>    ret |= ((DWORD)((color->b * float_conv[i].mul.z) + 0.5f)) << float_conv[i].shift.z;
> -
>    ret |= ((DWORD)((color->a * float_conv[i].mul.w) + 0.5f)) << float_conv[i].shift.w;
> -
>    idx.x = float_conv[i].shift.x / 32;
> -
>    idx.y = float_conv[i].shift.y / 32;
> -
>    idx.z = float_conv[i].shift.z / 32;
> -
>    idx.w = float_conv[i].shift.w / 32;
> -
>    shift.x = float_conv[i].shift.x % 32;
> -
>    shift.y = float_conv[i].shift.y % 32;
> -
>    shift.z = float_conv[i].shift.z % 32;
> -
>    shift.w = float_conv[i].shift.w % 32;
> -
> -
>    ret_i[idx.x] = ((uint32_t)((color->r * float_conv[i].mul.x) + 0.5f)) << shift.x;
> -
>    ret_i[idx.y] |= ((uint32_t)((color->g * float_conv[i].mul.y) + 0.5f)) << shift.y;
> -
>    ret_i[idx.z] |= ((uint32_t)((color->b * float_conv[i].mul.z) + 0.5f)) << shift.z;
> -
>    ret_i[idx.w] |= ((uint32_t)((color->a * float_conv[i].mul.w) + 0.5f)) << shift.w;
>
> -
>    TRACE("Returning 0x%08x.\n", ret);
> -
>    TRACE("Returning 0x%08x 0x%08x 0x%08x 0x%08x.\n",
> -
>            ret_i[0], ret_i[1], ret_i[2], ret_i[3]);

That TRACE isn't safe, I'm afraid. I think technically this doesn't
quite do the right thing for formats with bpp < 4 like B5G6R5_UNORM,
although we should be fine in practice due to how this is used.
>  for (i = 0; i < ARRAY_SIZE(double_conv); ++i)
>  {
> -
>    uint32_t *ret_i;
> -
>    if (format_id != double_conv[i].format_id)
>        continue;
>
> -
>    ret = ((DWORD)((color->r * double_conv[i].mul.x) + 0.5)) << double_conv[i].shift.x;
> -
>    ret |= ((DWORD)((color->g * double_conv[i].mul.y) + 0.5)) << double_conv[i].shift.y;
> -
>    ret |= ((DWORD)((color->b * double_conv[i].mul.z) + 0.5)) << double_conv[i].shift.z;
> -
>    ret |= ((DWORD)((color->a * double_conv[i].mul.w) + 0.5)) << double_conv[i].shift.w;
> -
>    idx.x = float_conv[i].shift.x / 32;
> -
>    idx.y = float_conv[i].shift.y / 32;
> -
>    idx.z = float_conv[i].shift.z / 32;
> -
>    idx.w = float_conv[i].shift.w / 32;
> -
>    shift.x = float_conv[i].shift.x % 32;
> -
>    shift.y = float_conv[i].shift.y % 32;
> -
>    shift.z = float_conv[i].shift.z % 32;
> -
>    shift.w = float_conv[i].shift.w % 32;
>
> -
>    TRACE("Returning 0x%08x.\n", ret);
> -
>    ret_i = ret;
> -
>    ret_i[idx.x] = ((uint32_t)((color->r * double_conv[i].mul.x) + 0.5)) << double_conv[i].shift.x;
> -
>    ret_i[idx.y] |= ((uint32_t)((color->g * double_conv[i].mul.y) + 0.5)) << double_conv[i].shift.y;
> -
>    ret_i[idx.z] |= ((uint32_t)((color->b * double_conv[i].mul.z) + 0.5)) << double_conv[i].shift.z;
> -
>    ret_i[idx.w] |= ((uint32_t)((color->a * double_conv[i].mul.w) + 0.5)) << double_conv[i].shift.w;
>
> -
>    return ret;
> -
>    TRACE("Returning 0x%08x 0x%08x 0x%08x 0x%08x.\n", ret_i[0], ret_i[1], ret_i[2], ret_i[3]);

Likewise.