[Impeller] Populate float_type when loading Flutter GPU shader bundles#185874
Closed
bdero wants to merge 1 commit into
Closed
[Impeller] Populate float_type when loading Flutter GPU shader bundles#185874bdero wants to merge 1 commit into
bdero wants to merge 1 commit into
Conversation
The GLES backend requires ShaderStructMemberMetadata::float_type to determine which glUniform* call to use when binding uniform data. However, when Flutter GPU loads shaders from a shader bundle (.shaderbundle), float_type was never populated and defaulted to std::nullopt, causing a fatal crash on the GLES backend: "Float uniform should have a float type." This adds DeriveShaderFloatType() to shader_types.h which infers the float_type from the base ShaderType and element size in bytes, and uses it when loading shader bundle metadata. This matches what runtime_effect_contents.cc already does for runtime effect shaders. Tests added: - DeriveShaderFloatTypeFromElementSize: verifies the type derivation for all float subtypes, non-float types, and unknown sizes. - BindUniformFailsWithoutFloatType: verifies the GLES backend correctly rejects uniforms with missing float_type metadata.
Member
Author
|
Closing this in favor of #185879, which is the proper solution that achieves feature parity with RuntimeEffect. |
bdero
added a commit
to bdero/flutter
that referenced
this pull request
May 1, 2026
…ata (flutter#185879) Flutter GPU shader bundles serialize uniform struct field metadata via a flatbuffer schema (`shader_bundle.fbs`) that carries per-field type and size information, but does not carry the underlying vector/column dimensions. This is enough to bind uniforms on Metal and Vulkan, but it isn't enough to distinguish shapes that share the same `element_size_in_bytes`, most notably `vec4` vs `mat2`. The GLES backend uses `ShaderStructMemberMetadata::float_type` to pick the right `glUniform*` call, and the bundle loader had no way to populate it. PR flutter#182229 ("Turns on most of fragment_shader_test.dart for opengles") introduced `ShaderFloatType`, the GLES validation that requires it, and the fix for runtime effect shaders (`runtime_effect_contents.cc`), but the equivalent fix for the Flutter GPU shader bundle loader (`lib/gpu/shader_library.cc`) was not made. As a result, every flutter_gpu app with a uniform block crashes the GLES backend on all platforms (Windows ANGLE, Linux GLES, Android GLES) with: ``` [ERROR:flutter/impeller/renderer/backend/gles/buffer_bindings_gles.cc(409)] Break on 'ImpellerValidationBreak' to inspect point of failure: Float uniform should have a float type. ``` ## Approach The size-only signal already carried by the bundle is not sufficient to derive `float_type` correctly for `mat2` and `mat3`, which would otherwise collide with `vec2` arrays and `vec3` arrays. (impellerc serializes a `mat2` uniform with `element_size_in_bytes = 8` and `array_elements = 2`, which is the same shape as a length-2 `vec2` array. Same story for `mat3`.) To fix this without regressing matrix-typed uniforms, this PR extends the schema to carry `vec_size` and `columns` (matching what `ShaderInput` already does for vertex inputs), and uses them to derive `float_type` in the bundle loader via the same shape-based dispatch that runtime effects already use. The shader bundle format version is bumped from 1 to 2 because old bundles do not carry the new fields. Old bundles loaded against a new engine fail with a clear error pointing at the impellerc/SDK version mismatch, rather than silently rendering with the wrong `glUniform*` call. ## Tests - `shader_bundle_unittests.cc`: verify the bundle carries `vec_size` and `columns` for the existing `mvp` (mat4) and `color` (vec4) test fixtures. Exhaustively cover `DeriveShaderFloatType` for float scalars, all vector sizes, all square matrices, non-square shapes, non-float types, and zero (legacy bundle) values. - `buffer_bindings_gles_unittests.cc`: regression guard asserting that the GLES backend rejects a float uniform that arrives without `float_type` populated. If a future change forgets to populate `float_type` in any code path (shader bundle loader, runtime effects, or anything else), this test catches it at unit-test time instead of at runtime. - `gpu_test.dart`: the eight re-enabled tests cover triangle rendering, polygon mode, MSAA, indexed draw, stencil, cull mode, scissor, and viewport on the opengles backend, end-to-end through the bundle loader. ## Issues Fixes flutter#184953 Fixes flutter#183530 May also resolve the following issues, all of which surface as GLES uniform binding failures and share the same root-cause family. To be verified against the linked reproducers after merge: - flutter#176875 - flutter#164438 - flutter#164745 This supersedes the smaller fix in [`bdero/flutter-gpu-float-types`](https://github.com/bdero/flutter/tree/bdero/flutter-gpu-float-types) (draft flutter#185874), which derived `float_type` from `element_size_in_bytes` only and would have misclassified `mat2`/`mat3` as `vec2`/`vec3`.
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
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.
The GLES backend requires
ShaderStructMemberMetadata::float_typeto determine whichglUniform*call to use when binding uniform data. However, when Flutter GPU loads shaders from a.shaderbundle,float_typewas never populated and defaulted tostd::nullopt, causing a fatal crash on the GLES backend on all platforms (Windows ANGLE, Linux GLES, Android GLES):This adds
DeriveShaderFloatType()toshader_types.hwhich infersfloat_typefrom the baseShaderTypeand element size in bytes (4 →kFloat, 8 →kVec2, 12 →kVec3, 16 →kVec4, 64 →kMat4), and uses it when loading shader bundle metadata. This matches whatruntime_effect_contents.ccalready does for runtime effect shaders.Tests added:
DeriveShaderFloatTypeFromElementSize— verifies the type derivation for all float subtypes, non-float types, and unknown sizes.BindUniformFailsWithoutFloatType— regression guard verifying the GLES backend rejects uniforms with missingfloat_typemetadata.Issues
Fixes #184953
Fixes #183530
May also resolve the following issues, all of which surface as GLES uniform binding failures and share the same root-cause family. To be verified against the linked reproducers after merge: