[PATCH 3/5] wined3d: Fixup alpha component in surface_convert_format().

[Paul Gofman]

On 11/11/19 20:12, Henri Verbeet wrote:
> On Mon, 11 Nov 2019 at 20:03, Henri Verbeet <hverbeet at gmail.com> wrote:
>> On Mon, 11 Nov 2019 at 19:43, Paul Gofman <gofmanp at gmail.com> wrote:
>>> On 11/11/19 19:07, Henri Verbeet wrote:
>>>> On Fri, 8 Nov 2019 at 15:28, Paul Gofman <gofmanp at gmail.com> wrote:
>>>>> @@ -685,14 +762,22 @@ static struct wined3d_texture *surface_convert_format(struct wined3d_texture *sr
>>>>>      else
>>>>>      {
>>>>>          struct wined3d_box src_box = {0, 0, desc.width, desc.height, 0, 1};
>>>>> +        struct wined3d_bo_address conv_data;
>>>>>
>>>>>          TRACE("Using upload conversion.\n");
>>>>>
>>>>>          wined3d_texture_prepare_location(dst_texture, 0, context, WINED3D_LOCATION_TEXTURE_RGB);
>>>>> -        dst_texture->texture_ops->texture_upload_data(context, wined3d_const_bo_address(&src_data),
>>>>> +
>>>>> +        wined3d_fixup_alpha(context, src_format, dst_format, &src_data, src_row_pitch,
>>>>> +                desc.width, desc.height, &conv_data);
>>>>> +
>>>> Although X-channel data is undefined, it seems undesirable to modify
>>>> the source data. Wouldn't it be better to fixup the destination data
>>>> instead?
>>> But I thought I don't modify source data: wined3d_fixup_alpha()
>>> allocates temporary buffer and maps source data as read only. Or am I
>>> missing your point?
>>>
>> No, you're right, my bad.
> Although looking a bit closer at the patch, it still seems a
> relatively fragile way to fix the issue. It works because
> texture_upload_data() uses e.g. GL_BGRA/GL_UNSIGNED_INT_8_8_8_8_REV
> for uploading WINED3DFMT_B8G8R8X8_UNORM data, but the caller isn't
> supposed to care about that, and a different backend (e.g. Vulkan)
> could do uploads in a different way.

I think the only reason why this type of fixup is needed is because we have the underlying format for _B8G8R8X8_UNORM which has an explicit alpha. So I suppose if WINED3DFMT_B8G8R8X8_UNORM gets underlying format without alpha, this conversion should not break but rather become redundant, as probably upload conversion should put alpha of 1.0 in this case. Should I maybe move this alpha fixup to wined3d_texture_gl_upload_data()?