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