Re: [PATCH 5/7] wined3d: Copy channel info for typed formats from typeless formats.

[Henri Verbeet]

On 10 February 2016 at 02:14, Józef Kucia <jkucia(a)codeweavers.com> wrote:
> @@ -1436,8 +1408,7 @@ static inline int getFmtIdx(enum wined3d_format_id format_id)
>  {
>      /* First check if the format is at the position of its value.
>       * This will catch the argb formats before the loop is entered. */
> -    if (format_id < (sizeof(formats) / sizeof(*formats))
> -            && formats[format_id].id == format_id)
> +    if (format_id < ARRAY_SIZE(formats) && formats[format_id].id == format_id)
>      {
>          return format_id;
>      }
> @@ -1445,10 +1416,10 @@ static inline int getFmtIdx(enum wined3d_format_id format_id)
>      {
>          unsigned int i;
>
> -        for (i = 0; i < (sizeof(formats) / sizeof(*formats)); ++i)
> -        {
> +        for (i = 0; i < ARRAY_SIZE(formats); ++i)
>              if (formats[i].id == format_id) return i;
> -        }
> +        for (i = 0; i < ARRAY_SIZE(typed_formats); ++i)
> +            if (typed_formats[i].id == format_id) return ARRAY_SIZE(formats) + i;
>      }
>      return -1;
>  }
> I'm not sure this is really the way to go. I'd prefer to just reorder
> things in such a way that the format id is the index in all cases
> except for FourCC formats. Patch 7/7 goes a long way in that
> direction, but to me it looks more complicated than just having a
> small table to remap the FourCC formats to an index. One additional
> advantage would be that we could then also remap e.g. DXT2 and DXT3 to
> BC2. E.g.:

struct
{
    enum wined3d_format_id id;
    unsigned int idx;
}
remap[] =
{
    {UYVY, FOURCC_BASE}
    {YUY2, FOURCC_BASE + 1}
    {YV12, FOURCC_BASE + 2}
    {DXT1, BC1},
    {DXT2, BC2},
    {DXT3, BC2},
    {DXT4, BC3},
    {DXT5, BC3},
};

[PATCH 5/7] wined3d: Copy channel info for typed formats from typeless formats.

[Józef Kucia]

On Wed, Feb 10, 2016 at 4:20 PM, Henri Verbeet <hverbeet(a)gmail.com> wrote:
> On 10 February 2016 at 02:14, Józef Kucia <jkucia(a)codeweavers.com> wrote:
> > @@ -1436,8 +1408,7 @@ static inline int getFmtIdx(enum wined3d_format_id format_id)
> >  {
> >      /* First check if the format is at the position of its value.
> >       * This will catch the argb formats before the loop is entered. */
> > -    if (format_id < (sizeof(formats) / sizeof(*formats))
> > -            && formats[format_id].id == format_id)
> > +    if (format_id < ARRAY_SIZE(formats) && formats[format_id].id == format_id)
> >      {
> >          return format_id;
> >      }
> > @@ -1445,10 +1416,10 @@ static inline int getFmtIdx(enum wined3d_format_id format_id)
> >      {
> >          unsigned int i;
> >
> > -        for (i = 0; i < (sizeof(formats) / sizeof(*formats)); ++i)
> > -        {
> > +        for (i = 0; i < ARRAY_SIZE(formats); ++i)
> >              if (formats[i].id == format_id) return i;
> > -        }
> > +        for (i = 0; i < ARRAY_SIZE(typed_formats); ++i)
> > +            if (typed_formats[i].id == format_id) return ARRAY_SIZE(formats) + i;
> >      }
> >      return -1;
> >  }
> > I'm not sure this is really the way to go. I'd prefer to just reorder
> > things in such a way that the format id is the index in all cases
> > except for FourCC formats. Patch 7/7 goes a long way in that
> > direction, but to me it looks more complicated than just having a
> > small table to remap the FourCC formats to an index. One additional
> > advantage would be that we could then also remap e.g. DXT2 and DXT3 to
> > BC2. E.g.:

It's mostly temporary before something equivalent to patch 7/7. We
could reorder format ids in enum wined3d_format_id so the only thing
needed would be a table to remap the FourCC formats. However, I think
this is very fragile and easy to break. Also, it makes some things
harder to change. For example, changing a default internal format
requires reordering entries in the typed_formats table.

FWIW, in the current git, there are only two formats which are stored
at the index that is equal to the format id (including
WINED3DFMT_UNKNOWN).

[PATCH 5/7] wined3d: Copy channel info for typed formats from typeless formats.

[Henri Verbeet]

On 10 February 2016 at 17:22, Józef Kucia <joseph.kucia(a)gmail.com> wrote:
> > I'm not sure this is really the way to go. I'd prefer to just reorder
> > things in such a way that the format id is the index in all cases
> > except for FourCC formats. Patch 7/7 goes a long way in that
> > direction, but to me it looks more complicated than just having a
> > small table to remap the FourCC formats to an index. One additional
> > advantage would be that we could then also remap e.g. DXT2 and DXT3 to
> > BC2. E.g.:
>
> It's mostly temporary before something equivalent to patch 7/7.
> Yeah, but it could probably be avoided if that patch came first.
> We
> could reorder format ids in enum wined3d_format_id so the only thing
> needed would be a table to remap the FourCC formats. However, I think
> this is very fragile and easy to break. Also, it makes some things
> harder to change. For example, changing a default internal format
> requires reordering entries in the typed_formats table.
> Oh, no, I don't mean for the various tables in utils.c, just for
> gl_info.formats. So conceptually what patch 7/7 does, but instead of
> looking for holes to put the FourCC formats in just have the remap
> table for those.