Re: [PATCH 3/5] wined3d: Always store the palette index in the alpha component.

[Henri Verbeet]

On 12 May 2014 15:12, Stefan Dösinger <stefan(a)codeweavers.com> wrote:
> @@ -1458,8 +1457,7 @@ static void surface_download_data(struct wined3d_surface *surface, const struct
>          int src_pitch = 0;
>          int dst_pitch = 0;
>
> -        /* In case of P8 the index is stored in the alpha component if the primary render target uses P8. */
> -        if (format->id == WINED3DFMT_P8_UINT && swapchain_is_p8(surface->resource.device->swapchains[0]))
> +        if (format->id == WINED3DFMT_P8_UINT)
>          {
>              gl_format = GL_ALPHA;
>              gl_type = GL_UNSIGNED_BYTE;
> This branch is pretty pointless, the format / type for
> WINED3DFMT_P8_UINT is always GL_ALPHA / GL_UNSIGNED_BYTE. (If
> ARB_fragment_program is supported, and the format is supported at all
> anyway.)
> @@ -3256,53 +3252,19 @@ static void read_from_framebuffer(struct wined3d_surface *surface, DWORD dst_loc
>      switch (surface->resource.format->id)
>      {
>          case WINED3DFMT_P8_UINT:
> -        {
> -            if (swapchain_is_p8(context->swapchain))
> -            {
> -                /* In case of P8 render targets the index is stored in the alpha component */
> -                fmt = GL_ALPHA;
> -                type = GL_UNSIGNED_BYTE;
> -                mem = data.addr;
> -                bpp = surface->resource.format->byte_count;
> -            }
> -            else
> -            {
> -                /* GL can't return palettized data, so read ARGB pixels into a
> -                 * separate block of memory and convert them into palettized format
> -                 * in software. Slow, but if the app means to use palettized render
> -                 * targets and locks it...
> -                 *
> -                 * Use GL_RGB, GL_UNSIGNED_BYTE to read the surface for performance reasons
> -                 * Don't use GL_BGR as in the WINED3DFMT_R8G8B8 case, instead watch out
> -                 * for the color channels when palettizing the colors.
> -                 */
> -                fmt = GL_RGB;
> -                type = GL_UNSIGNED_BYTE;
> -                pitch *= 3;
> -                mem = HeapAlloc(GetProcessHeap(), 0, surface->resource.size * 3);
> -                if (!mem)
> -                {
> -                    ERR("Out of memory\n");
> -                    return;
> -                }
> -                bpp = surface->resource.format->byte_count * 3;
> -            }
> -        }
> -        break;
> +            fmt = GL_ALPHA;
> +            type = GL_UNSIGNED_BYTE;
> +            break;
>
> default:
> -            mem = data.addr;
>              fmt = surface->resource.format->glFormat;
>              type = surface->resource.format->glType;
> -            bpp = surface->resource.format->byte_count;
>      }
> Like above.
> if (!srcIsUpsideDown)
>      {
> -        UINT len;
>          /* glReadPixels returns the image upside down, and there is no way to prevent this.
> -            Flip the lines in software */
> -        len = surface->resource.width * bpp;
> +         * Flip the lines in software.
> +         * TODO: Consider flipping the image in an FBO. */

Possibly, but that would be a decision for surface_load_sysmem(), not
read_from_framebuffer(). Note that reading the front buffer isn't
particularly fast on Windows either though.
> +        row = HeapAlloc(GetProcessHeap(), 0, pitch);
> +            goto error;
> This looks unintentional.
> @@ -3088,38 +3088,7 @@ DWORD wined3d_format_convert_from_float(const struct wined3d_surface *surface, c
>      }
>
> if (format->id == WINED3DFMT_P8_UINT)
> -    {
> -        const RGBQUAD *e;
> -        BYTE r, g, b, a;
> -
> -        if (!surface->palette)
> -        {
> -            WARN("Surface doesn't have a palette, returning 0.\n");
> -            return 0;
> -        }
> -
> -        r = (BYTE)((color->r * 255.0f) + 0.5f);
> -        g = (BYTE)((color->g * 255.0f) + 0.5f);
> -        b = (BYTE)((color->b * 255.0f) + 0.5f);
> -        a = (BYTE)((color->a * 255.0f) + 0.5f);
> -
> -        e = &surface->palette->colors[a];
> -        if (e->rgbRed == r && e->rgbGreen == g && e->rgbBlue == b)
> -            return a;
> -
> -        WARN("Alpha didn't match index, searching full palette.\n");
> -
> -        for (i = 0; i < 256; ++i)
> -        {
> -            e = &surface->palette->colors[i];
> -            if (e->rgbRed == r && e->rgbGreen == g && e->rgbBlue == b)
> -                return i;
> -        }
> -
> -        FIXME("Unable to convert color to palette index.\n");
> -
> -        return 0;
> -    }
> +        return (BYTE)((color->a * 255.0f) + 0.5f);
> So this could just use the table now, right?