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
This seems like the Copy/Paste syndrom [:-)]
Anyway, I fixed it in one of my pending patch.
Yes! From a ZBuffer surface to another... Don't remember the game though... (because it crashed in another part of wine)
Wow... This is nice :-)
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.
Yeah, it's in my plans to have the ZBuffer a bit better handled to not crash when applications do strange things with it...
Proper Lock / Blt support will never be fast on any GL implementation I know though :-/
Yes this can be done as well. [:-)]
Fixed it in one of my patches too.
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.
Well, you can pass the W coordinates using the 1/W trick, no ? If you use the XYZRHW format and give it X, Y, Z, 1/W after your computation it should work, no ? Slow but working from what I can see...
Oh! Well I forgot this hack... [;-)]
I think to support this using GL, we should use the feedback buffer. Will surely be slow, but well, better than to write our own lighting :-)
Lionel
A bit late, as usual ;-)
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.
Well, you can pass the W coordinates using the 1/W trick, no ? If you use the XYZRHW format and give it X, Y, Z, 1/W after your computation it should work, no ? Slow but working from what I can see...
I was talking about the XYZ format. In your old code, you did only the world transformation manually, so the format is still XYZ. If the world transformation ever introduced a W value different from 1 (which should not happened because only the projection matrix must do this), we're stuck because we cannot pass this value to DrawPrimitive (because we use XYZ coordinates). What I've done is to transform and light all vertices and use the XYZRHW format.
Oh! Well I forgot this hack... [;-)]
I think to support this using GL, we should use the feedback buffer. Will surely be slow, but well, better than to write our own lighting :-)
I don't know much about feedback buffer... But maybe replacing (or improving?) our current hackish code of ProcessVertices with feedback buffer should do the trick. Once done, we could simply make execute buffers use this function.
Lionel
-
Christian Costa -
Lionel Ulmer