Repository navigation
[Impeller] Validate GLES texture units against the combined limit - #189332
Merged
auto-submit[bot] merged 4 commits intoAug 6, 2026
Merged
Conversation
bdero
marked this pull request as ready for review
July 12, 2026 03:43
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates the GLES backend's texture binding logic to track per-stage texture counts separately from active indices, validating both per-stage and combined texture unit limits. It also adds corresponding unit tests and updates the GLES mock implementation. Feedback suggests removing unnecessary friend declarations in buffer_bindings_gles.h since the new tests only access public APIs, and explicitly mocking GL_MAX_COMBINED_TEXTURE_IMAGE_UNITS in the combined limit test to improve robustness.
jason-simmons
previously approved these changes
Jul 13, 2026
walley892
previously approved these changes
Jul 14, 2026
bdero
force-pushed
the
bdero/gles-combined-texture-units
branch
2 times, most recently
from
July 30, 2026 03:53
9f4d1c9 to
750b7b1
Compare
BufferBindingsGLES allocates fragment stage texture units after the vertex stage's, but validated the running unit index against the per-stage maximum. On a driver reporting the GLES minimum of 16 fragment units (ANGLE on D3D11), a draw whose fragment shader uses all 16 samplers then failed validation as soon as the vertex stage sampled a texture, since the last fragment sampler lands on unit index 16. Texture units are a combined resource in GL, glActiveTexture and sampler uniform values are valid up to the combined limit, and the per-stage limits only bound how many samplers a single stage references. Validate the unit index against the combined limit and the per-stage sampler count against the per-stage limit.
bdero
force-pushed
the
bdero/gles-combined-texture-units
branch
from
August 6, 2026 00:19
750b7b1 to
adadc02
Compare
Member
Author
|
Getting around to cleaning up/rebasing my PRs. This one should be ready to go! |
jason-simmons
approved these changes
Aug 6, 2026
This was referenced Aug 6, 2026
This was referenced Aug 7, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
Fixes #189331.
The GLES backend allocates fragment stage texture units after the vertex stage's, but validated the running unit index against the per-stage maximum. On a driver reporting the minimum 16 fragment units (ANGLE on D3D11), a draw using all 16 fragment samplers plus a vertex stage texture failed validation and the render pass aborted, crashing skinned-mesh rendering in Flutter GPU apps on Windows.
Texture units are a combined resource in GL; the per-stage limits bound only how many samplers one stage references. This validates the unit index against the combined limit and the per-stage sampler count against the per-stage limit. Adds unit tests for the previously rejected case and both overflow cases.
Pre-launch Checklist
///).