dxvk icon indicating copy to clipboard operation
dxvk copied to clipboard

[d3d8] Wine device tests fixes (Part 2) - LockRect/LockBox

Open WinterSnowfall opened this issue 1 year ago • 6 comments

Built on top of #4320, I'm now expanding and hoping to fix (most) of the Wine tests dealing with surface locks (LockRect/LockBox validations).

Work so far:

  • d3d8/9 seem to validate the boundaries of the locking rectangle for D3DPOOL_DEFAULT surfaces with formats that need to be block aligned, surfaces created via CreateImageSurface and cube textures outside of D3DPOOL_DEFAULT (the last one being specific to D3D8). ~If only it were so simple, though. Essentially all validation are done on surfaces or subsurfaces of textures so we need some way to determine the origin of a surface. To that end I've added a D3DRESOURCETYPE Type variable part of D3D9_COMMON_TEXTURE_DESC.~
  • a ~very highly cursed~ alignment lock check for block aligned formats, ~performed only on direct calls to IDirect3DSurface9->LockRect()~. Turns out this is only performed on the first mip level, which makes a whole lot more sense and isn't hyper-cursed.
  • a similar validation on volume textures with block aligned formats
  • added general format volume texture validations on creation and locking (there seems to be some pool and flag specific behavior here)
  • handled D3D8-specific LockRect/LockBox pLockedRect/pLockedBox clearing behavior, together with sanitizing return values on LockImage failures

WinterSnowfall avatar Oct 07 '24 19:10 WinterSnowfall

As an indication of progress:

Current master: -- D3D8: 0020:device: 51333 tests executed (34 marked as todo, 0 as flaky, 1086 failures), 1 skipped. -- D3D9: 0020:device: 155334 tests executed (78 marked as todo, 0 as flaky, 2017 failures), 6 skipped.

This PR: -- D3D8: 0020:device: 51131 tests executed (34 marked as todo, 0 as flaky, 822 failures), 1 skipped. -- D3D9: 0020:device: 155132 tests executed (78 marked as todo, 0 as flaky, 1802 failures), 6 skipped.

WinterSnowfall avatar Oct 07 '24 22:10 WinterSnowfall

I might fix some Rect/Box clearing issues on Unlock, but the PR is largely done at this point, and a potential powder keg, so I'll leave it up for testing for quite a bit.

WinterSnowfall avatar Oct 08 '24 14:10 WinterSnowfall

I have tested this with all of the d3d8 and d3d9 games in my prefix (that's somewhere in the range of 200+ titles) and haven't seen any downsides. That being said, I'd appreciate if @K0bin could have another look over the PR as well, since it largely also affects d3d9.

I might fix other stuff too, but I'm calling Part 2 complete and ready for merge.

WinterSnowfall avatar Oct 10 '24 14:10 WinterSnowfall

I'd much prefer having the locking changes in the device in LockImage unless you have a very good reason for not putting it there.

One of the validations is for negative coordinate positions in the rect, which happens before the conversion to a box (whose positions are stored as UINT). I will move what I can, but at least some of it will have to stay in place, I'm afraid.

WinterSnowfall avatar Oct 11 '24 20:10 WinterSnowfall

@K0bin Hopefully it looks a bit better now. I've moved everything that didn't need to stay at LockRect/LockBox level into LockImage. The LockRect dimension validations need to stay in place for the reasons stated above.

WinterSnowfall avatar Oct 11 '24 21:10 WinterSnowfall

Turns out the changes done to address the feedback above weren't that scary, so we should be good. Verified the same number of d3d8/d3d9 device tests pass with the new status quo.

WinterSnowfall avatar Oct 18 '24 14:10 WinterSnowfall