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

Correct stencil upload bugs #11192

Merged
merged 3 commits into from
Jun 18, 2018
Merged

Correct stencil upload bugs #11192

merged 3 commits into from
Jun 18, 2018

Conversation

unknownbrackets
Copy link
Collaborator

@unknownbrackets unknownbrackets commented Jun 18, 2018

GLES was missing the semantic definitions. I guess it was just using the last ones on desktop or something. Now it works fine on mobile again.

Vulkan definitely looks like a driver bug, and it's a bug that could easily be affecting regular rendering too. I didn't test extensively but at least this case is affected:

  • Stencil testing is enabled, thus stencil is being written.
  • Alpha or color testing is enabled, and discard; is used in the shader.

I suppose we could statically write to depth (Adreno only?) whenever we also generate a discard. (EDIT by hrydgard: a commit for this has now been added)

If we bother detecting that case, we might put layout(early_fragment_tests) in; for GL4.2+/Vulkan/GLES3.1+? It might clarify some case with hardware tessellation or direct depal, etc.

Fixes #10634.

-[Unknown]

In case a buffer has been resized recently, we want to upload just the
detected drawable area, probably.  Before this was inconsistent depending
on the render resolution.
We write a static depth value, which will be ignored, to force the driver
to support discard.
@unknownbrackets unknownbrackets added this to the v1.7.0 milestone Jun 18, 2018
@unknownbrackets
Copy link
Collaborator Author

Seeing that it could be interesting to use ARM_shader_framebuffer_fetch_depth_stencil as an alternative to dual source blending (using in-shader blending replacing destColor.a), although that's probably impacted by early depth tests too.

-[Unknown]

@hrydgard
Copy link
Owner

I see you added a workaround by writing to depth - did layout(early_fragment_tests) do anything?

@hrydgard hrydgard merged commit fb70dad into hrydgard:master Jun 18, 2018
@hrydgard
Copy link
Owner

Good catch with the semantics by the way ... oops.

@unknownbrackets unknownbrackets deleted the stencil branch June 18, 2018 13:44
@unknownbrackets
Copy link
Collaborator Author

unknownbrackets commented Jun 18, 2018

layout(early_fragment_tests) is the exact opposite of what we want in this case (we need the tests to be late so it will write our stencil only when not discarded.)

I meant we could add that to the regular drawing fragment shaders to explicitly allow early tests in cases where we don't discard. And regular drawing is probably impacted by this driver bug.

-[Unknown]

@unknownbrackets
Copy link
Collaborator Author

Posted the issue on their forum at least:
https://developer.qualcomm.com/forum/qdn-forums/software/adreno-gpu-sdk/35916

-[Unknown]

@hrydgard
Copy link
Owner

Oh right, forgot that it's indeed a hint the other way :)

They're usually not nearly as responsive as say ARM in their forum, but good to have it reported.

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