Compat: fix GL computation of storage limits #5029
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kainino0x
commented
Dec 19, 2024
Contributor
|
Previews, as seen when this build job started (42219e5): |
Contributor
Author
|
Revised! |
Contributor
Author
|
@greggman review bump now that I'm back |
greggman
reviewed
Jan 4, 2025
Contributor
greggman
left a comment
There was a problem hiding this comment.
Sigh.... I can't remember what we look at before. Re-checking.
min(MAX_SHADER_STORAGE_BUFFER_BINDINGS, MAX_COMPUTE_SHADER_STORAGE_BLOCKS)
min(MAX_SHADER_STORAGE_BUFFER_BINDINGS, MAX_FRAGMENT_SHADER_STORAGE_BLOCKS)
min(MAX_SHADER_STORAGE_BUFFER_BINDINGS, MAX_VERTEX_SHADER_STORAGE_BLOCKS)
is okay but it's not complete.
There's the issue of MAX_COMBINED_SHADER_STORAGE_BLOCKS and there are devices for which would affect any render pipeline and there are several devices that support less than vertex+fragment limits.
GL_MAX_COMBINED_SHADER_STORAGE_BLOCKS vs
GL_MAX_FRAGMENT_SHADER_STORAGE_BLOCKS
GL_MAX_VERTEX_SHADER_STORAGE_BLOCKS
total valid entries: 3365
num entries where GL_MAX_FRAGMENT_SHADER_STORAGE_BLOCKS+
GL_MAX_VERTEX_SHADER_STORAGE_BLOCKS > COMBINED: 403
Combos where COMBINED < GL_MAX_FRAGMENT_SHADER_STORAGE_BLOCKS +
GL_MAX_VERTEX_SHADER_STORAGE_BLOCKS
MAX MAX total MAX count
FRAGMENT VERTEX of COMBINED of
SHADER SHADER limits SHADER opengles.gpuinfo.org
STORAGE STORAGE <--- STORAGE entries
BLOCKS BLOCKS BLOCKS
------- ------ ----- ------- -------------------
8 8 16 8 359
64 64 128 64 21
35 35 70 35 15
60 60 120 60 4
32 32 64 32 2
16 16 32 16 1
8192 2048 10240 8192 1
Those include
Adreno (TM) 510
Adreno (TM) 540
Adreno (TM) 640
PowerVR Rogue G6200
PowerVR Rogue GE8320
...etc...
Do you want those details here? To include these devices, it seems like the best a implementation can do this this.
min(MAX_SHADER_STORAGE_BUFFER_BINDINGS, MAX_COMPUTE_SHADER_STORAGE_BLOCKS, MAX_COMBINED_SHADER_STORAGE_BLOCKS)
min(MAX_SHADER_STORAGE_BUFFER_BINDINGS, MAX_FRAGMENT_SHADER_STORAGE_BLOCKS, MAX_COMBINED_SHADER_STORAGE_BLOCKS / 2)
min(MAX_SHADER_STORAGE_BUFFER_BINDINGS, MAX_VERTEX_SHADER_STORAGE_BLOCKS, MAX_COMBINED_SHADER_STORAGE_BLOCKS / 2)
On the majority of devices, MAX_COMBINED_SHADER_STORAGE_BLOCKS will be > the sum of both
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I noticed the formula for
maxStorageBuffersPerShaderStagewas outdated. I also wanted to add the formula for the new limits, but I'm not sure what they all are.