[PATCH 0/7] wined3d: blit set_shader texture_target activation cleanup

[Roderick Colenbrander]

On Wed, May 19, 2010 at 11:30 AM, Roderick Colenbrander
<thunderbird2k at gmail.com> wrote:
>>> At a later stage I might disallow calling set_shader when we don't need the shader.
>> How does the caller know when a shader is needed?
>>
>
> In the end set_shader will only be called by blit_shader->blit_surface.
>
> The main issue I'm facing is p8 uploads which are completely broken
> right now (a regression). At the beginning of the week I wanted to add
> BLIT_OP_COMPLEX_UPLOAD to fix this but Henri didn't like that. This
> 'fixed' the bug where d3dfmt_get_conv was not allowed by
> blit_supported to offer the 8-bit shader. This fix would only apply to
> the LoadLocation part which is where the 8-bit upload takes place.
>
> The suggested direction was to fix blit_supported to allow 8-bit blits
> under 'special circumstances'. Such a change would not only allow
> 8-bit uploads but also allow 8-bit -> 8-bit Blt/BltFast
> (blit_supported can't distinguish between the two uses since it has
> too vague information). Allowing the 8-bit -> 8-Blt/BltFast led to
> these changes (I'm not that happy about allowing those).
>
> At this point BltOverride (and some of the other helpers derived from
> it) blindly call set_shader when it is not needed which wouldn't work
> here. The set_shader call at this point lacks the knowledge (it has
> only one of the surfaces) to not apply the shader in this case. I
> don't think I can add the missing surface because not all places which
> call set_shader have it. For this reason I wanted to take the texture
> target activation out of 'set_shader' and lift this to 'blit_surface'
> which the current callers are supposed to become at some point. Some
> places (e.g. swapchain_blit) imply that that the shader is needed when
> a complex source fixup is detected and the same for blit_to_drawable,
> so in BltOverride/arbfp_blit_surface I can just not call set_shader.
>
> Right now I don't know anymore how it should be fixed at this stage
> without re-adding the ugly extension checks which existed before in
> d3dfmt_get_conv or other or other hacks like checking for
> 'is_complex_fixup' in d3dfmt_get_conv :(
>
> Roderick
>

So the main issue is that a p8 sysmem -> drawable upload on the same surface
is detected as a 'destination fixup' by blit_supported. The
blit_supported call is
used for p8 in d3dfmt_get_conv basically for format selection before it were GL
extension checks.

What I could try to do is take out the blit_supported call and rely on
the 'implicit
blit_supported' for now in the form of the formats table. When the
table is loaded
it already sets the P8 texturing format we want (it is not that nice).
Together with
this change I would also have to rewrite d3dfmt_get_conv to really get
rid of the format
fixups as they are a part of the issue.

Issues:
1) sysmem -> drawable uploads on the same surface don't work
2) when shaders nor paletted_texture is around, d3dfmt_get_conv
    receives an 'empty' description which it fills with the gl info
(bad and seems to cause some issues already)
( 3) format desc fixups are ugly )

I would propose to make three changes:
1) format desc changes
Add some format color keying flags to the format_desc (would be set
for all CK formats) and a conversion flag (this one would be set for
P8 and perhaps also for all CK formats)


2) add separate formats table for color keyed formats which
d3dfmt_get_conv would select from.
This could work like:

HRESULT d3dfmt_get_conv(..)
{
    if (color_keying_active)
    {
        /* search color key format list */
        /* At the moment this is performed in software for p8 */
        return desc;
    }

    /* In case of P8 we need a different format when we use drawpixels
     * compared to texturing when ext_paletted_texture or shaders are around */
    if (!use_textures && p8)
    {
        return p8_drawpixels_desc;
    }

    /* In case of P8 this when we arrive here we return the default
format which is in the formats table,
     * so if paletted textures are around then that format is
returned. This would be an implicit
     * blit_supported check. At a later stage the blit_shader should
perhaps be asked what format is needed.
     */
    return default_desc;
}


3) Get rid of CONVERT_TYPES and look at the flags in the format_desc
before calling d3dfmt_convert_surface (this call would also check
these flags to figure out the right conversion).


This change addresses some ugly things (format desc fixups), fixes
issue '2' and would also address the 8-bit 'complex upload' case. I
admit that the last one would be fixed in a 'sneaky' way. At a later
point the blit_shader could perhaps be asked for a format.

What do you think?

Roderick