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

OpenGL: two stage texture binding bug #787

Merged
merged 1 commit into from
Feb 15, 2023

Conversation

allcreater
Copy link
Contributor

There was a specific problem with the OpenGL backend: despite the API providing support for at least 12(SG_MAX_SHADERSTAGE_IMAGES) slots for textures for the vertex shader and 12 for the fragment shader, the OpenGL implementation could bind only SG_MAX_SHADERSTAGE_IMAGES combined for both stages because of the length of the binding cache.

I propose fixing this issue by increasing the cache size to the value of SG_MAX_SHADERSTAGE_IMAGES * SG_NUM_SHADER_STAGES so that all images can be bound. The actual number of supported images is known at runtime, _sg.gl.max_combined_texture_image_units, and is already used to prevent unsupported bindings.

PS: the previous PR #766 could be used to work around these restrictions as well, but that is an independent improvement with its own goal to provide the possibility to increase limits itself (there are shaders where a total of 24 textures is not enough).

…STAGE_IMAGES both on vertex and fragment shader
@floooh
Copy link
Owner

floooh commented Feb 15, 2023

Ok, I'll switch back to "PR mode" for the a week or two, starting with this PR.

@floooh
Copy link
Owner

floooh commented Feb 15, 2023

Yep, the PR makes complete sense. I'll just change the enum to a define (since it's not a public constant and just in the implementation that's fine - such constants only need to be an enum if they are part of the language bindings).

@floooh floooh merged commit e6a6d0e into floooh:master Feb 15, 2023
@floooh
Copy link
Owner

floooh commented Feb 15, 2023

Ok merged. Thanks for the PR! I'll followup with a commit where I'll expose GL_MAX_COMBINED_TEXTURE_IMAGE_UNITS in the sg_limits struct.

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