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

Fix gles texture uniform array binding #75313

Merged

Conversation

ChibiDenDen
Copy link
Contributor

@ChibiDenDen ChibiDenDen commented Mar 25, 2023

Fix both texture uniform binding and texture object binding in gles (compatibility ) mode for godot 4

Before this change, the engine (editor / game) crashes whenever there is an array of sampler2D in compatibility mode

The following line in any shader crashes:
uniform sampler2D[10] textures;

Production edit: Fixes #76463

@YuriSizov
Copy link
Contributor

YuriSizov commented Mar 25, 2023

Thanks for opening a pull request! Please squash your commits into a single one. Make sure that the commit message stays short but descriptive (your original message and the title of this PR are a good option).

And to your knowledge, does this address any open issue?

@ChibiDenDen ChibiDenDen force-pushed the gles_sampler_uniform_array_fix branch from 48eaeef to e2bad9e Compare March 25, 2023 13:52
@ChibiDenDen
Copy link
Contributor Author

@YuriSizov thanks, I squashed the commits.
I tried to look at the open issues for keywords like "shader array" "shader crash" "sampler array" "sampler2d" and couldn't find anything similar, might have missed something though.

Should I open an issue describing the bugs fixed here?

@YuriSizov
Copy link
Contributor

YuriSizov commented Mar 25, 2023

Should I open an issue describing the bugs fixed here?

No, it's not necessary at this point. I was mostly curious if this would reduce our backlog of existing issues :)

@ChibiDenDen ChibiDenDen force-pushed the gles_sampler_uniform_array_fix branch from 47d4778 to 4eacfcf Compare March 25, 2023 20:14
@fire fire changed the title fix gles texture uniform array binding Fix gles texture uniform array binding Apr 1, 2023
Copy link
Member

@clayjohn clayjohn left a comment

Choose a reason for hiding this comment

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

Looks really good! I just have one little requested change because we try to avoid using auto unless absolutely necessary.

drivers/gles3/storage/material_storage.cpp Outdated Show resolved Hide resolved
@ChibiDenDen ChibiDenDen force-pushed the gles_sampler_uniform_array_fix branch from 4fcf9da to 719af76 Compare May 5, 2023 19:29
@clayjohn
Copy link
Member

This looks good to go, it just needs to be rebased to resolve conflicts

@ChibiDenDen ChibiDenDen force-pushed the gles_sampler_uniform_array_fix branch 2 times, most recently from 563c29f to 27da9ad Compare May 26, 2023 17:06
@YuriSizov YuriSizov requested a review from clayjohn May 26, 2023 17:08
drivers/gles3/shader_gles3.cpp Outdated Show resolved Hide resolved
drivers/gles3/shader_gles3.cpp Outdated Show resolved Hide resolved
@ChibiDenDen ChibiDenDen force-pushed the gles_sampler_uniform_array_fix branch from 27da9ad to 6d3634e Compare May 26, 2023 20:11
@YuriSizov YuriSizov merged commit 08fcf27 into godotengine:master May 27, 2023
@YuriSizov
Copy link
Contributor

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Godot crashes if creating uniform sampler2D array when using compatibility/GLES3 renderer
4 participants