[D3D] Some execute buffers rewrites + misc fixes

[Christian Costa]

Hi,

Sorry, for a unknown reason my wine-devel delivery had been disabled.
I enabled it but I've just seen your posts in the archives...

> Some comments on 'small' parts... I will have to commit the patch on 
> my tree
> to try to understand the resulting code to understand it better [:-)]


>> +    if ( IsEqualGUID( &IID_IDirectDrawSurface, riid ) ||
>> +         IsEqualGUID( &IID_IDirectDrawSurface2, riid ) ||
>> +         IsEqualGUID( &IID_IDirectDrawSurface3, riid ) ) {
>> +        IDirectDrawSurface7_AddRef(ICOM_INTERFACE(This->surface, 
>> IDirectDrawSurface7));
>> +        *obp = ICOM_INTERFACE(This->surface, IDirectDrawSurface3);
>> +    TRACE("  Return IDirectDrawSurface3 interface %p\n", *obp);
>> +    return S_OK;
>> +    }
>> +    if ( IsEqualGUID( &IID_IDirectDrawSurface3, riid ) ||
>> +         IsEqualGUID( &IID_IDirectDrawSurface7, riid ) ) { +        
>> IDirectDrawSurface7_AddRef(ICOM_INTERFACE(This->surface, 
>> IDirectDrawSurface7));
>> +        *obp = ICOM_INTERFACE(This->surface, IDirectDrawSurface7);
>> +    TRACE("  Return IDirectDrawSurface7 interface %p\n", *obp);
>> +    return S_OK;
>> +    }
>

> Well, it's a bit strange to have 'IID_IDirectDrawSurface3' twice, no 
> [:-)] ?


This seems like the Copy/Paste syndrom [:-)]

>>      /* We only support the BLT with DEPTH_FILL for now */
>> -    if (This->ddraw_owner->d3d != NULL) {
>> +    if ((dwFlags & DDBLT_DEPTHFILL) && This->ddraw_owner->d3d != 
>> NULL) {
>>          if (This->ddraw_owner->d3d->current_device != NULL) {
>>          
>> This->ddraw_owner->d3d->current_device->clear(This->ddraw_owner->d3d->current_device, 
>>
>>                                0, NULL, /* Clear the whole screen */
>

> Did you add this check here because some game were actually doing a 
> 'normal'
> blit on the ZBuffer ?


Yes! From a ZBuffer surface to another... Don't remember the game 
though... (because it crashed in another part of wine)

> What happens in this case ?


When doing the standard blit, the app set the DDBLT struct pointer to 
NULL so
without the check this simply crashes when accessing the depthfill 
member of the structure.

> Moreover, you could also add here (which I forgot to do in my patch),
> support for the 'dst locking rectangle' in case an application tries to
> clear only part of the buffer (see example in the Blt overide function in
> d3ddevice/mesa.c for the 'Colorfill' case).


Yes this can be done as well. [:-)]

> Well, I have a comment... But it's in a part of the code that will be so
> rarely used that it could be completely thrown away anyway [:-)]


Sure, It's barely used. [:-)]

> Basically, you are doing the full projection (without doing the clipping
> though) in software... And this does not go well at all with lighting.


Yes, there is no lighting yet.

> This is why I choose in the previous code to only do the world 
> transformation to
> NOT impact at all lighting (and it worked for Twist AFAIK). So I am
> wondering why you choose to go all the way and did not do exactly as I 
> did
> (basically, only do the WORLD transform and set only the PROJ / VIEW one
> using the D3D7 API and set the WORLD to Identity as we already did the
> transform).


Because the world transformation introduced the W coordinate which 
cannot be passed
to drawing functions. But well, this coordinate should be always be 1 
after the world transformation so we can ommit it.

 >Even better would have been to have reused fully our VertexBuffer code 
and

> our (hacky [:-)] ) TransformVertices code.


Oh! Well I forgot this hack... [;-)]

Bye

Christian