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

Validate depth formats on use with CopyRects #180

Merged
merged 1 commit into from
Oct 10, 2024

Conversation

WinterSnowfall
Copy link
Contributor

The D3D8 specs also say, with regards to CopyRects:

This method cannot be applied to surfaces whose formats are classified as depth stencil formats.

I have checked this claim against native drivers and they do indeed validate it.

|| Format == D3DFMT_D24X4S4
|| Format == D3DFMT_D24S8
|| Format == D3DFMT_D24X8;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure if these are used by any d3d8 game. But there are a few other depth stencil surfaces missing. Maybe it makes sense to just add them all now:

  • D3DFMT_S1D15 (72)
  • D3DFMT_S8D24 (74)
  • D3DFMT_D24S8 (75)
  • D3DFMT_X8D24 (76)
  • D3DFMT_X4S4D24 (78)
  • D3DFMT_D32F_LOCKABLE (82)
  • D3DFMT_D24FS8 (83)
  • D3DFMT_D32_LOCKABLE (84)

Copy link
Contributor Author

@WinterSnowfall WinterSnowfall Sep 22, 2024

Choose a reason for hiding this comment

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

None of these (except D3DFMT_D24S8, which I've already included above) are part of the D3D8 standard or D3DFORMAT enum, so the likelihood of any D3D8 game using them is doubtful. Some of them are included and used in D3D9, however, as you are aware, CopyRects is D3D8 specific.

Here's an excerpt on buffer formats taken from the DX8.1 SDK docs:
DSDX8

Copy link
Contributor

Choose a reason for hiding this comment

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

Keep in mind that some of the stencil formats were added to video cards after DX 8.1. They may still work with video cards. I don't think adding all known ones is bad.

However, maybe a better way, rather than using a function to check specific formats, is to just ask d3d9 if it is a stencil buffer by using the following code:

SourceDesc.Usage == D3DUSAGE_DEPTHSTENCIL

Copy link
Contributor Author

Choose a reason for hiding this comment

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

However, maybe a better way, rather than using a function to check specific formats, is to just ask d3d9 if it is a stencil buffer by using the following code:

SourceDesc.Usage == D3DUSAGE_DEPTHSTENCIL

That indeed sounds a lot better than adding formats which were not known by the d3d8 runtime. I will need to test this behavior against native d3d8, but will implement it if it checks out. Thank you for the suggestion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@elishacloud Sadly, that doesn't seem to be a correct approach. I checked, and you can create a texture and underlying surfaces that have a depth stencil format without specifying the D3DUSAGE_DEPTHSTENCIL flag. Those surfaces would then not fail the CopyRects validation, which is not the intended behavior. It seems to be format not usage specific, as the specs mention.

If you want, I can test to see if CopyRects validates any of the extra formats you've specified, however I doubt that is the case. The ones I've already included should be sufficient to prevent any unwanted CopyRects behavior in d3d8 games. If we notice exceptions to the rule, we can always refine it later.

Copy link
Contributor

Choose a reason for hiding this comment

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

I checked, and you can create a texture and underlying surfaces that have a depth stencil format without specifying the D3DUSAGE_DEPTHSTENCIL flag.

Ok, too bad. I guess the only way is to check the surface format. Thanks!

@WinterSnowfall WinterSnowfall marked this pull request as draft September 26, 2024 18:03
@WinterSnowfall WinterSnowfall marked this pull request as ready for review September 26, 2024 19:34
@crosire crosire merged commit b65caf1 into crosire:main Oct 10, 2024
@WinterSnowfall WinterSnowfall deleted the copyrects branch October 10, 2024 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants