On 9 October 2011 15:21, Stefan Dösinger stefan@codeweavers.com wrote:
This is the first set of my P8 patches, they remove dead code that is left over from d3d8/9 style palettized texture support. See http://www.winehq.org/pipermail/wine-devel/2011-October/092744.html for the entire patchset.
I thought you asked me not to look at these patches yet because you still had some issues to fix. Regardless, I gave this a cursory look: - I can't say I find e.g. sprinkling d3dformat_is_palettized() over the code particularly elegant. - Do you have tests for the D3DERR_INVALIDCALL returns? - In patch 5 you remove wined3d_device_get_current_texture_palette() from the spec but not from the public header.
On Sunday 09 October 2011 16:40:11 Henri Verbeet wrote:
I thought you asked me not to look at these patches yet because you still had some issues to fix. Regardless, I gave this a cursory look:
Yeah, those were the ones that implemented blitting in a different way in wined3d and had issues with palette changes and SFLAG_CONVERTED. The patches I sent to wine-devel have this fixed, and the patches I sent to wine-patches aren't affected by this.
- I can't say I find e.g. sprinkling d3dformat_is_palettized() over
the code particularly elegant.
I suppose I could check for it in IDirect3DDevice8Impl_CreateSurface(and the d3d9 equivalent), if all calls that create surfaces run through it.
- Do you have tests for the D3DERR_INVALIDCALL returns?
I tested that and Windows returns D3D_OK. Windows also allows creating P8 offscreen plain surfaces, and it allows StretchRect operations between those surfaces. It doesn't allow StretchRect operations between P8 and non- Palettized surfaces. In the end I could not find any way to make use of those surfaces I could create.
I prefer to return INVALIDCALL over the half-baked implementation Windows has to find applications that try to use P8 surfaces in a way I may have missed.
- In patch 5 you remove wined3d_device_get_current_texture_palette()
from the spec but not from the public header.
Oops, thanks for catching that.
On 9 October 2011 17:31, Stefan Dösinger stefan@codeweavers.com wrote:
- I can't say I find e.g. sprinkling d3dformat_is_palettized() over the code particularly elegant.
I suppose I could check for it in IDirect3DDevice8Impl_CreateSurface(and the d3d9 equivalent), if all calls that create surfaces run through it.
- Do you have tests for the D3DERR_INVALIDCALL returns?
I tested that and Windows returns D3D_OK. Windows also allows creating P8 offscreen plain surfaces, and it allows StretchRect operations between those surfaces. It doesn't allow StretchRect operations between P8 and non- Palettized surfaces. In the end I could not find any way to make use of those surfaces I could create.
I prefer to return INVALIDCALL over the half-baked implementation Windows has to find applications that try to use P8 surfaces in a way I may have missed.
It's not really clear to me that refusing to create P8 surfaces in d3d8/9 really gains us much, or that it's really a prerequisite for the other patches. If you want to go that way though, you should probably just map D3DFORMAT_P8 to something that's not WINED3DFMT_P8_UINT, perhaps WINED3DFMT_UNSUPPORTED, and just let wined3d fail resource creation.
On Monday 10 October 2011 10:20:15 Henri Verbeet wrote:
It's not really clear to me that refusing to create P8 surfaces in d3d8/9 really gains us much, or that it's really a prerequisite for the other patches.
I guess you've got a point here, if we just allow the surfaces we'll end up with the same behavior as windows for probably the same reason - functionality for ddraw is exposed via d3d8/9, but because d3d8/9 don't have P8 primaries it is useless.
I'll still return D3DERR_INVALIDCALL from the palette setters/getters to remove the d3d8/9 palette API from wined3d, which simplifies the code there. We could implement them via the ddraw palette API, but without test cases showing how they're supposed to work it would be a shot in the dark and most likely dead code.
-
Henri Verbeet -
Stefan Dösinger