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

D3D9 state cache cleanup #15723

Merged
merged 3 commits into from
Jul 30, 2022
Merged

D3D9 state cache cleanup #15723

merged 3 commits into from
Jul 30, 2022

Conversation

hrydgard
Copy link
Owner

@hrydgard hrydgard commented Jul 24, 2022

Trying to move some stuff on top of thin3d in a separate branch (stencil upload, other future effects), and running into various annoyances. So let's fix them separately.

I think this is safe enough, there really isn't anything complex in here, and the D3D9 backend seems to work just fine.

(Yeah yeah, I should just get around to doing the release at this point...)

@hrydgard hrydgard added D3D9 Direct3D 9 Code Cleanup Cleanup to make future work easier. Needs to be done sometimes. labels Jul 24, 2022
@@ -304,6 +304,7 @@ class TextureShaderApplierDX9 {
}

void Shade() {
// Intentionally bypassing the dxstate cache here (and using .Restore to recover afterwards). Not sure if this is a good idea.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This was done in other backends too, and never changed in D3D9. I think the issue was about order of state processing - by the time it got here, it'd already applied state, so if you changed state here it would break the actual draw. The order of operations may be different now.

-[Unknown]

@unknownbrackets
Copy link
Collaborator

I wonder if GPU_DX9::ReapplyGfxState() is needed anymore, then? Really, ideally everything should be based on the state desired when the draw is being constructed. I know when we changed the order of things for Vulkan or GL or something not that long ago, it broke some things and we had to make it careful.

My feeling is that we should merge this right after the next release.

-[Unknown]

@hrydgard hrydgard added this to the v1.14.0 milestone Jul 24, 2022
@hrydgard
Copy link
Owner Author

hrydgard commented Jul 29, 2022

I will remove GPU_DX9::ReapplyGfxState() , and the ugly stuff in Shade(), in an additional PR after this, for easier user-bisection.

I'll start merging post-1.13 stuff tomorrow unless something big comes up.

@hrydgard hrydgard merged commit 512382c into master Jul 30, 2022
@hrydgard hrydgard deleted the d3d9-state-cache-cleanup branch July 30, 2022 06:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code Cleanup Cleanup to make future work easier. Needs to be done sometimes. D3D9 Direct3D 9
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants