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

d3d11: samplerstate hashing fix #2500

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

Velro
Copy link
Contributor

@Velro Velro commented Apr 30, 2021

there's a case where you can request two different sampler states and they end up resolving down to the same sampler state in d3d11 internally, causing DX_CHECK_REFCOUNT(sampler, 1); to trigger.

Namely if you do
BGFX_SAMPLER_MIN_ANISOTROPIC | BGFX_SAMPLER_MAG_ANISOTROPIC
and
BGFX_SAMPLER_MIN_ANISOTROPIC | BGFX_SAMPLER_MAG_ANISOTROPIC | BGFX_SAMPLER_MIP_POINT
it'll trigger that assert.

you can repro the assert with this patch to bump.cpp example

diff --git a/examples/06-bump/bump.cpp b/examples/06-bump/bump.cpp
index 31afa7200..d99022568 100644
--- a/examples/06-bump/bump.cpp
+++ b/examples/06-bump/bump.cpp
@@ -282,8 +282,11 @@ public:
                                                bgfx::setIndexBuffer(m_ibh);

                                                // Bind textures.
-                                               bgfx::setTexture(0, s_texColor,  m_textureColor);
-                                               bgfx::setTexture(1, s_texNormal, m_textureNormal);
+                                               const uint32_t samplerFlagsA = BGFX_SAMPLER_MIN_ANISOTROPIC | BGFX_SAMPLER_MAG_ANISOTROPIC;
+                                               const uint32_t samplerFlagsB = BGFX_SAMPLER_MIN_ANISOTROPIC | BGFX_SAMPLER_MAG_ANISOTROPIC | BGFX_SAMPLER_MIP_POINT;
+
+                                               bgfx::setTexture(0, s_texColor,  m_textureColor, samplerFlagsA);
+                                               bgfx::setTexture(1, s_texNormal, m_textureNormal, samplerFlagsB);

this PR fixes things so these two requests do end up hashing to the same value. I'm hashing more memory than before, there's probably a way to compact the D3D11_TEXTURE_ADDRESS_MODE's better I didn't explore it much.

@Velro Velro requested a review from bkaradzic as a code owner April 30, 2021 05:35
@Velro
Copy link
Contributor Author

Velro commented Apr 30, 2021

sorry the git history is all weird I am super weak git-foo. If you have a quick tip on how to fix it I'll do it otherwise feel free to just yoink this and paste it

@bkaradzic
Copy link
Owner

Ok, I looked into your PR, but doesn't make sense why passing just addressing modes would fix this. I'll need to investigate.

@bkaradzic
Copy link
Owner

sorry the git history is all weird I am super weak git-foo.

Btw, if you syncing branches with latest, use rebase instead of merge.

… that only differ in cmpfunc would have collided
@bkaradzic
Copy link
Owner

I'm trying to repro this but it's not triggering the error you have.

@Velro
Copy link
Contributor Author

Velro commented May 4, 2021

I just repro'd the assert on another machine with that patch against latest. Here's all the debug output up to hitting the assert if that gives any hints

bump_example_debug_output.txt

@Velro
Copy link
Contributor Author

Velro commented May 4, 2021

genie command
..\bx\tools\bin\windows\genie --with-windows="10.0" --with-examples --with-tools vs2019
no command-line args to bump.exe

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.

2 participants