[v2 PATCH 2/3] wined3d: Handle scale with multisampled destination in fbo_blitter_blit().

[Paul Gofman]

On 3/10/20 11:11, Henri Verbeet wrote:
> On Mon, 9 Mar 2020 at 18:45, Paul Gofman <gofmanp at gmail.com> wrote:
>> @@ -938,12 +938,17 @@ static DWORD fbo_blitter_blit(struct wined3d_blitter *blitter, enum wined3d_blit
>>      {
>>          TRACE("Colour blit.\n");
>>
>> -        /* Resolve the source surface first if needed. */
>> -        if (wined3d_texture_gl_is_multisample_location(wined3d_texture_gl(src_texture), src_location)
>> -                && (src_texture->resource.format->id != dst_texture->resource.format->id
>> +        /* Resolve the source and destination surfaces if needed. */
>> +        if (src_texture->resource.format->id != dst_texture->resource.format->id
>>                  || abs(src_rect->bottom - src_rect->top) != abs(dst_rect->bottom - dst_rect->top)
>> -                || abs(src_rect->right - src_rect->left) != abs(dst_rect->right - dst_rect->left)))
>> -            src_location = WINED3D_LOCATION_RB_RESOLVED;
>> +                || abs(src_rect->right - src_rect->left) != abs(dst_rect->right - dst_rect->left))
>> +        {
>> +            if (wined3d_texture_gl_is_multisample_location(wined3d_texture_gl(dst_texture), dst_location))
>> +                dst_location = WINED3D_LOCATION_RB_RESOLVED;
>> +
> What initially bothered me about this is that it breaks the blitter
> interface a little; i.e., a caller asking for a blit to
> WINED3D_LOCATION_RB_MULTISAMPLE can potentially end up not getting a
> an up-to-date WINED3D_LOCATION_RB_MULTISAMPLE. I think that still
> works out in practice for all current callers, but it seems
> undesirable in the long term. It would be easy enough to avoid that by
> adding a wined3d_texture_load_location() call with the requested
> destination location, but then that would introduce a potentially
> redundant extra blit.

An extra wined3d_texture_load_location() for requested destintation
location was introduced in the first version when resolve was in
texture2d_blt_fbo(), but I thought one of the reasons it is actually
better to do in fbo_blitter_blit() is that we can avoid that. Doesn't
dst_location returned from blitter have the purpose to give us some
flexibility to leave the different dst_location?

>
> After giving it some more thought though, I'm wondering whether
> perhaps the best option would be for texture2d_blt() to not request
> scaled multi-sample blits in the first place, unless it knows that at
> least one blitter can handle those. (I.e., introducing something like
> "d3d_info->scaled_resolve".)

I am not sure I fully understand. Currently fbo blitter thinks it can
always handle that, just does it wrong when the destination texture is
also multisampled. GLSL blitter does not support support source
multisampled textures, but should be fine with multisampled destination.
Do you mean to just do not support scaled resolves in FBO blitter, or do
not support if EXT_framebuffer_multisample_blit_scaled is not available
(if it is available, we still need to resolve destination texture)?

As a separate question, if no blitter is decided to be able to handle
scaled resolve, should texture2d_blt() do the resolve first and then
scaled blit?