[Impeller] Validate GLES texture units against the combined limit (#189332)
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
- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [AI contribution guidelines] and understand my
responsibilities, or I am not using AI tools.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [x] All existing and new tests are passing.
diff --git a/engine/src/flutter/impeller/renderer/backend/gles/buffer_bindings_gles.cc b/engine/src/flutter/impeller/renderer/backend/gles/buffer_bindings_gles.cc
index 2055e51..b5a6fad 100644
--- a/engine/src/flutter/impeller/renderer/backend/gles/buffer_bindings_gles.cc
+++ b/engine/src/flutter/impeller/renderer/backend/gles/buffer_bindings_gles.cc
@@ -493,6 +493,7 @@
ShaderStage stage,
size_t unit_start_index) {
size_t active_index = unit_start_index;
+ size_t stage_texture_count = 0;
for (auto i = 0u; i < texture_range.length; i++) {
const TextureAndSampler& data = bound_textures[texture_range.offset + i];
if (data.stage != stage) {
@@ -513,11 +514,21 @@
//--------------------------------------------------------------------------
/// Set the active texture unit.
///
- if (active_index >= gl.GetCapabilities()->GetMaxTextureUnits(stage)) {
+ /// Units are a combined resource; the per-stage limits cap only how many
+ /// samplers one stage references, not the unit indices they bind to.
+ ///
+ stage_texture_count++;
+ if (stage_texture_count > gl.GetCapabilities()->GetMaxTextureUnits(stage)) {
VALIDATION_LOG << "Texture units specified exceed the capabilities for "
"this shader stage.";
return std::nullopt;
}
+ if (active_index >=
+ gl.GetCapabilities()->max_combined_texture_image_units) {
+ VALIDATION_LOG << "Texture units specified exceed the combined texture "
+ "unit limit.";
+ return std::nullopt;
+ }
gl.ActiveTexture(GL_TEXTURE0 + active_index);
//--------------------------------------------------------------------------
diff --git a/engine/src/flutter/impeller/renderer/backend/gles/buffer_bindings_gles.h b/engine/src/flutter/impeller/renderer/backend/gles/buffer_bindings_gles.h
index 12cef03..c00b55e 100644
--- a/engine/src/flutter/impeller/renderer/backend/gles/buffer_bindings_gles.h
+++ b/engine/src/flutter/impeller/renderer/backend/gles/buffer_bindings_gles.h
@@ -21,6 +21,10 @@
FML_TEST_CLASS(BufferBindingsGLESTest, BindArrayData);
FML_TEST_CLASS(BufferBindingsGLESTest, BindUniformDataVerticesAndMatrices);
FML_TEST_CLASS(BufferBindingsGLESTest, BindUniformFailsWithoutFloatType);
+FML_TEST_CLASS(BufferBindingsGLESTest,
+ BindsTexturesAcrossThePerStageUnitBoundary);
+FML_TEST_CLASS(BufferBindingsGLESTest, RejectsTexturesBeyondThePerStageLimit);
+FML_TEST_CLASS(BufferBindingsGLESTest, RejectsTexturesBeyondTheCombinedLimit);
} // namespace testing
//------------------------------------------------------------------------------
@@ -66,6 +70,12 @@
BindUniformDataVerticesAndMatrices);
FML_FRIEND_TEST(testing::BufferBindingsGLESTest,
BindUniformFailsWithoutFloatType);
+ FML_FRIEND_TEST(testing::BufferBindingsGLESTest,
+ BindsTexturesAcrossThePerStageUnitBoundary);
+ FML_FRIEND_TEST(testing::BufferBindingsGLESTest,
+ RejectsTexturesBeyondThePerStageLimit);
+ FML_FRIEND_TEST(testing::BufferBindingsGLESTest,
+ RejectsTexturesBeyondTheCombinedLimit);
//----------------------------------------------------------------------------
/// @brief The arguments to glVertexAttribPointer.
///
diff --git a/engine/src/flutter/impeller/renderer/backend/gles/buffer_bindings_gles_unittests.cc b/engine/src/flutter/impeller/renderer/backend/gles/buffer_bindings_gles_unittests.cc
index 9bae5b1..494fc8d 100644
--- a/engine/src/flutter/impeller/renderer/backend/gles/buffer_bindings_gles_unittests.cc
+++ b/engine/src/flutter/impeller/renderer/backend/gles/buffer_bindings_gles_unittests.cc
@@ -5,11 +5,14 @@
#include "flutter/testing/testing.h" // IWYU pragma: keep
#include "gtest/gtest.h"
#include "impeller/core/shader_types.h"
+#include "impeller/core/texture_descriptor.h"
#include "impeller/renderer/backend/gles/buffer_bindings_gles.h"
#include "impeller/renderer/backend/gles/device_buffer_gles.h"
#include "impeller/renderer/backend/gles/formats_gles.h"
#include "impeller/renderer/backend/gles/reactor_gles.h"
+#include "impeller/renderer/backend/gles/sampler_library_gles.h"
#include "impeller/renderer/backend/gles/test/mock_gles.h"
+#include "impeller/renderer/backend/gles/texture_gles.h"
#include "impeller/renderer/command.h"
namespace impeller {
@@ -19,6 +22,7 @@
}
using ::testing::_;
+using ::testing::NiceMock;
TEST(BufferBindingsGLESTest, ToVertexAttribTypeSupportedFormats) {
EXPECT_EQ(ToVertexAttribType(VertexAttributeFormat::kFloat32x3),
@@ -361,5 +365,138 @@
/*expected_bound_size=*/32);
}
+namespace {
+
+// Owns the reactor, sampler, and metadata behind a set of texture bindings.
+struct BoundTexturesFixture {
+ std::shared_ptr<ReactorGLES> reactor;
+ std::shared_ptr<TestWorker> worker;
+ std::unique_ptr<SamplerLibrary> sampler_library;
+ raw_ptr<const Sampler> sampler;
+ std::vector<std::unique_ptr<ShaderMetadata>> metadata;
+ std::vector<TextureAndSampler> bound_textures;
+ absl::flat_hash_map<std::string, GLint> uniform_bindings;
+
+ explicit BoundTexturesFixture(std::unique_ptr<ProcTableGLES> proc_table) {
+ reactor = std::make_shared<ReactorGLES>(std::move(proc_table));
+ worker = std::make_shared<TestWorker>();
+ reactor->AddWorker(worker);
+ sampler_library = std::make_unique<SamplerLibraryGLES>(
+ /*supports_decal_sampler_address_mode=*/false);
+ sampler = sampler_library->GetSampler({});
+ }
+
+ void AddTextures(ShaderStage stage, size_t count) {
+ for (size_t i = 0; i < count; i++) {
+ TextureDescriptor desc;
+ desc.storage_mode = StorageMode::kDevicePrivate;
+ desc.type = TextureType::kTexture2D;
+ desc.format = PixelFormat::kR8G8B8A8UNormInt;
+ desc.size = {1, 1};
+ desc.mip_count = 1u;
+ desc.usage = TextureUsage::kShaderRead;
+ auto texture = std::make_shared<TextureGLES>(reactor, desc);
+ const std::string name = "tex" + std::to_string(metadata.size());
+ const std::string key = "TEX" + std::to_string(metadata.size());
+ uniform_bindings[key] = static_cast<GLint>(100 + metadata.size());
+ auto meta = std::make_unique<ShaderMetadata>();
+ meta->name = name;
+ TextureAndSampler data = {};
+ data.stage = stage;
+ data.texture = TextureResource(meta.get(), std::move(texture));
+ data.sampler = sampler;
+ metadata.push_back(std::move(meta));
+ bound_textures.push_back(std::move(data));
+ }
+ }
+};
+
+// Capabilities of a minimum-spec ES3 driver (16 per stage, 32 combined).
+std::unique_ptr<NiceMock<MockGLESImpl>> MakeSixteenUnitMockImpl() {
+ auto impl = std::make_unique<NiceMock<MockGLESImpl>>();
+ EXPECT_CALL(*impl, GetIntegerv(_, _))
+ .WillRepeatedly([](GLenum name, GLint* value) {
+ switch (name) {
+ case GL_MAX_TEXTURE_IMAGE_UNITS:
+ case GL_MAX_VERTEX_TEXTURE_IMAGE_UNITS:
+ *value = 16;
+ break;
+ case GL_MAX_COMBINED_TEXTURE_IMAGE_UNITS:
+ *value = 32;
+ break;
+ default:
+ break;
+ }
+ });
+ return impl;
+}
+
+} // namespace
+
+// One vertex texture pushes the last of 16 fragment samplers onto unit 16,
+// which must bind on a 16-per-stage driver since units are combined in GL.
+TEST(BufferBindingsGLESTest, BindsTexturesAcrossThePerStageUnitBoundary) {
+ std::shared_ptr<MockGLES> mock_gl = MockGLES::Init(MakeSixteenUnitMockImpl());
+ BoundTexturesFixture fixture(
+ std::make_unique<ProcTableGLES>(kMockResolverGLES));
+ fixture.AddTextures(ShaderStage::kVertex, 1);
+ fixture.AddTextures(ShaderStage::kFragment, 16);
+ ASSERT_TRUE(fixture.reactor->React());
+
+ BufferBindingsGLES bindings;
+ bindings.SetUniformBindings(std::move(fixture.uniform_bindings));
+ std::vector<BufferResource> bound_buffers;
+ EXPECT_TRUE(bindings.BindUniformData(
+ fixture.reactor->GetProcTable(), fixture.bound_textures, bound_buffers,
+ Range{0, fixture.bound_textures.size()}, Range{0, 0}));
+}
+
+// More samplers in one stage than its limit is still rejected.
+TEST(BufferBindingsGLESTest, RejectsTexturesBeyondThePerStageLimit) {
+ std::shared_ptr<MockGLES> mock_gl = MockGLES::Init(MakeSixteenUnitMockImpl());
+ BoundTexturesFixture fixture(
+ std::make_unique<ProcTableGLES>(kMockResolverGLES));
+ fixture.AddTextures(ShaderStage::kFragment, 17);
+ ASSERT_TRUE(fixture.reactor->React());
+
+ BufferBindingsGLES bindings;
+ bindings.SetUniformBindings(std::move(fixture.uniform_bindings));
+ std::vector<BufferResource> bound_buffers;
+ EXPECT_FALSE(bindings.BindUniformData(
+ fixture.reactor->GetProcTable(), fixture.bound_textures, bound_buffers,
+ Range{0, fixture.bound_textures.size()}, Range{0, 0}));
+}
+
+// Units past the combined limit are rejected even when each stage is within
+// its per-stage limit.
+TEST(BufferBindingsGLESTest, RejectsTexturesBeyondTheCombinedLimit) {
+ auto impl = std::make_unique<NiceMock<MockGLESImpl>>();
+ EXPECT_CALL(*impl, GetIntegerv(_, _))
+ .WillRepeatedly([](GLenum name, GLint* value) {
+ switch (name) {
+ case GL_MAX_TEXTURE_IMAGE_UNITS:
+ case GL_MAX_VERTEX_TEXTURE_IMAGE_UNITS:
+ case GL_MAX_COMBINED_TEXTURE_IMAGE_UNITS:
+ *value = 8;
+ break;
+ default:
+ break;
+ }
+ });
+ std::shared_ptr<MockGLES> mock_gl = MockGLES::Init(std::move(impl));
+ BoundTexturesFixture fixture(
+ std::make_unique<ProcTableGLES>(kMockResolverGLES));
+ fixture.AddTextures(ShaderStage::kVertex, 8);
+ fixture.AddTextures(ShaderStage::kFragment, 8);
+ ASSERT_TRUE(fixture.reactor->React());
+
+ BufferBindingsGLES bindings;
+ bindings.SetUniformBindings(std::move(fixture.uniform_bindings));
+ std::vector<BufferResource> bound_buffers;
+ EXPECT_FALSE(bindings.BindUniformData(
+ fixture.reactor->GetProcTable(), fixture.bound_textures, bound_buffers,
+ Range{0, fixture.bound_textures.size()}, Range{0, 0}));
+}
+
} // namespace testing
} // namespace impeller
diff --git a/engine/src/flutter/impeller/renderer/backend/gles/test/mock_gles.cc b/engine/src/flutter/impeller/renderer/backend/gles/test/mock_gles.cc
index f4d4815..dbd7024 100644
--- a/engine/src/flutter/impeller/renderer/backend/gles/test/mock_gles.cc
+++ b/engine/src/flutter/impeller/renderer/backend/gles/test/mock_gles.cc
@@ -98,7 +98,9 @@
*value = g_extensions.size();
} break;
case GL_MAX_COMBINED_TEXTURE_IMAGE_UNITS:
+ // Minimum default; a registered mock may overwrite it.
*value = 8;
+ CallMockMethod(&IMockGLESImpl::GetIntegerv, name, value);
break;
case GL_MAX_LABEL_LENGTH_KHR:
*value = 64;
