Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[d3d8/9] Properly handle resource priority #4690

Merged
merged 5 commits into from
Feb 17, 2025

Conversation

WinterSnowfall
Copy link
Contributor

@WinterSnowfall WinterSnowfall commented Feb 14, 2025

The d3d8/9 behavior with regards to resource priorities is pretty straight forward:

  • D3D8/9Resource::SetPriority() only works for resources in D3DPOOL_MANAGED
  • everything else returns 0 above and on D3D8/9Resource::GetPriority() (esentially the value used above is ignored)

d3d9Ex, in its eternal wisdom decided to switch this up and:

  • D3D9Resource::SetPriority() only works for resources in D3DPOOL_DEFAULT
  • everything else returns 0 above and on D3D9Resource::GetPriority(), including anything in D3DPOOL_MANAGED
  • there's a list of constants you can use with SetPriority(), but they're not mandatory and apps can basically set pretty much anything they want

I'm aware our backend doesn't use the priority for anything really, but I guess some games might depend on it for their own internal stuff. Oh, and Wine has a d3d8/9 test for it that fails without this PR, which annoyed me.

So this is now pretty much native behavior, as annoying as it was to get everything in place within templateing nightmares and whatnot.

Still need to double check if the tests are happy, and I'll undraft after I've confirmed.

@WinterSnowfall WinterSnowfall force-pushed the priority branch 3 times, most recently from e3d2212 to a4fdff9 Compare February 15, 2025 11:50
@WinterSnowfall
Copy link
Contributor Author

Ended up having to fix some odd crashes with the x64 builds of the Wine device tests (which to be fair I haven't checked in like ages, since I didn't expect any foul play vs x32).

I've confirmed the priority behavior is now in line with expectations.

@WinterSnowfall WinterSnowfall marked this pull request as ready for review February 15, 2025 11:56
Copy link
Collaborator

@K0bin K0bin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few nits. Please tell me if you disagree with any of them or if it gets too nit-picky.

@WinterSnowfall WinterSnowfall force-pushed the priority branch 3 times, most recently from 599073c to c60305a Compare February 16, 2025 15:26
@doitsujin doitsujin merged commit 92523fc into doitsujin:master Feb 17, 2025
3 checks passed
@WinterSnowfall WinterSnowfall deleted the priority branch February 17, 2025 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants