[Impeller] Deduplicate GLES render pass state by kevmoo · Pull Request #193427 · flutter/flutter

kevmoo · 2026-09-28T02:58:19Z

Deduplicates redundant OpenGL ES state transitions across draw commands within RenderPassGLES (EncodeCommandsInReactor):

Per-Pass Orthogonal GLES State Tracking (RenderPassStateCache):
- Encapsulates pass-scoped GL state tracking inside RenderPassStateCache (BlendStateCache, StencilFaceStateCache, StencilStateCache, and DepthStateCache in an anonymous namespace in render_pass_gles.cc), tracking enabled/disabled state (GL_BLEND, GL_STENCIL_TEST, GL_DEPTH_TEST, GL_SCISSOR_TEST), active program handle (std::optional<HandleGLES> via HandleGLES::Equal{} so pipeline variants sharing a compiled UniqueHandleGLES skip redundant program binds and _impeller_y_flip uniform updates), color mask, blend equations/factors, depth compare function and write mask, front/back stencil functions/operations/write masks, viewport, scissor rect, cull mode, and winding order across draw calls within EncodeCommandsInReactor.
- ResetGLState initializes the tracked state once at pass entry before glClear, eliminating the redundant pass-exit ResetGLState call while ensuring no state leaks across passes, ReactorGLES handle operations, or BlitPassGLES commands.

WebGL 2.0 Call Count & CPU Command-Encoding Reduction (Web Impeller / WIMP)

Workload (`1280x720`, `wimp_st`)	Metric	Before	After	Delta
`bouncy` (`128` nodes, `441` draws)	Total WebGL2 calls / frame	`13,234`	`7,983`	`-39.7%` (`-5,251` calls)
	`glBlendEquationSeparate` + `glBlendFuncSeparate` / frame	`802`	`4`	`-99.5%`
	`glStencilFuncSeparate` + `Op` + `Mask` / frame	`1,327`	`9`	`-99.3%`
	`glColorMask` / frame	`443`	`3`	`-99.3%`
	`glEnable` + `glDisable` / frame	`1,506`	`225`	`-85.1%`
	`glDepthFunc` + `glDepthMask` / frame	`884`	`161`	`-81.8%`
	`glUseProgram` / frame	`441`	`105`	`-76.2%`
	`glUniform1fv` (`_impeller_y_flip`) / frame	`441`	`105`	`-76.2%`
	`glScissor` / frame	`175`	`117`	`-33.1%`
	CPU command-encoding raster time (`--disable-gl-drawing-for-tests`)	`13.41 ms`	`12.47 ms`	`-7.0%` (`-0.94 ms`, `1.08x` faster)
`grid` (`1000` nodes, `5,908` draws)	Total WebGL2 calls / frame	`154,521`	`94,517`	`-38.8%` (`-60,004` calls)
	`glBlendEquationSeparate` + `glBlendFuncSeparate` / frame	`9,474`	`338`	`-96.4%`
	`glStencilFuncSeparate` + `Op` + `Mask` / frame	`17,726`	`844`	`-95.2%`
	`glColorMask` / frame	`5,909`	`337`	`-94.3%`
	`glEnable` + `glDisable` / frame	`18,071`	`2,691`	`-85.1%`
	`glUseProgram` / frame	`5,908`	`2,197`	`-62.8%`
	`glUniform1fv` (`_impeller_y_flip`) / frame	`5,908`	`2,197`	`-62.8%`
	`glDepthFunc` + `glDepthMask` / frame	`11,817`	`6,051`	`-48.8%`
	CPU command-encoding raster time (`--disable-gl-drawing-for-tests`)	`60.86 ms`	`56.13 ms`	`-7.8%` (`-4.73 ms`, `1.08x` faster)

Fixes #192147

Pre-launch Checklist

…ameters Track orthogonal GL state variables (blend enable/factors/ops, color write mask, stencil enable/front/back state, depth enable/compare/mask, scissor rect, and bound program/y-flip uniform) across commands in RenderPassGLES::EncodeCommandsInReactor, and cache the configured SamplerDescriptor key on non-wrapped TextureGLES instances so SamplerGLES::ConfigureBoundTexture skips redundant glTexParameteri calls. Fixes flutter#192147

…edupe tests - Track the bound program in `RenderPassGLES::EncodeCommandsInReactor` via `std::optional<HandleGLES>` (`HandleGLES::Equal{}`) instead of `const PipelineGLES*` so pipeline variants sharing a compiled `UniqueHandleGLES` skip redundant `glUseProgram` and `_impeller_y_flip` uniform updates. - Add `RenderPassGLESCommandTest.DeduplicatesEnabledBlendStencilDepthAndSharedProgramAcrossVariants` to verify enabled blend, depth, and stencil deduplication (including `stencil_reference` changes) and shared program handle deduplication across `PipelineGLES` instances. - Extend `BufferBindingsGLESTest.SkipsRedundantSamplerConfigurationOnSameTexture` to verify that `TextureGLES::WrapTexture(...)` bypasses the sampler parameter cache and re-emits `glTexParameteri` on every bind.

gemini-code-assist

Code Review

This pull request introduces state caching in the Impeller GLES backend to deduplicate redundant OpenGL state changes, including sampler parameters, blending, stencil, depth, scissor, and program bindings. Feedback on these changes highlights two critical issues: first, the cached sampler key on TextureGLES is not invalidated when texture storage or mip levels are mutated, which can lead to incorrect sampler configurations; second, GL_SCISSOR_TEST is enabled for clipped draws but is never disabled for subsequent unclipped draws within the same render pass, potentially causing incorrect clipping.

gaaclarke

Nice, this is good practice when working with opengl and something we should do.

gaaclarke

Have you done some local testing to see the performance delta? It would be nice to know what we are getting in return of maintaining caches which can be bug vectors.

…er caching

kevmoo · 2026-09-29T23:37:06Z

Thanks for the review @gaaclarke! In a4bf0ec I refactored all the per-pass state caches into structs with methods inside an anonymous namespace (RenderPassStateCache, BlendStateCache, StencilFaceStateCache, StencilStateCache, DepthStateCache), documented the HandleGLES monotonic UniqueID invariant on BindProgram, and split the SamplerGLES caching out of this PR.

On local Web Impeller (WIMP / WebGL 2.0) benchmarks, RenderPassGLES state deduplication alone (a4bf0ecda5e) produces the following deltas:

bouncy (128 nodes, 441 draws/frame):
- WebGL2 API calls / frame: 13,234 -> 7,983 (-39.7% / -5,251 calls, including glBlend* 802 -> 4, glStencil* 1,327 -> 9, glColorMask 443 -> 3, glEnable/Disable 1,506 -> 225, glDepth* 884 -> 161, glUseProgram 441 -> 105)
- CPU command-encoding raster time (--disable-gl-drawing-for-tests): 13.41 ms -> 12.47 ms (-7.0% / -0.94 ms, 1.08x faster)
grid (1000 nodes, 5,908 draws/frame):
- WebGL2 API calls / frame: 154,521 -> 94,517 (-38.8% / -60,004 calls, including glBlend* 9,474 -> 338, glStencil* 17,726 -> 844, glColorMask 5,909 -> 337, glEnable/Disable 18,071 -> 2,691, glUseProgram 5,908 -> 2,197)
- CPU command-encoding raster time (--disable-gl-drawing-for-tests): 60.86 ms -> 56.13 ms (-7.8% / -4.73 ms, 1.08x faster)

Updated the PR description table with these numbers as well. PTAL!

@gaaclarke

Addressed all 10 review comments in a4bf0ec (refactored state caches into structs with methods in anonymous namespace, split out SamplerGLES caching, and added local performance benchmarks); re-requesting review from @gaaclarke.

RenderPass::SetScissor only populates pending_.scissor for the next Draw() call (which moves pending_ into commands_ and resets pending_). Subsequent Draw() calls within the same clip scope have command.scissor == std::nullopt, meaning the active scissor rect remains unchanged (matching Metal and Vulkan; see flutter-team-archive/engine#56494). Do not disable GL_SCISSOR_TEST or clear the cached scissor when command_scissor is std::nullopt in RenderPassStateCache::ConfigureScissor.

gaaclarke · 2026-10-01T22:24:11Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces state caching mechanisms, including BlendStateCache, StencilStateCache, DepthStateCache, and RenderPassStateCache, to deduplicate OpenGL state changes across commands in the GLES backend. Unit tests are added to verify this deduplication behavior. Feedback points out a potential null pointer dereference in BlendStateCache::Configure when handling depth-only or stencil-only passes where the color attachment descriptor is null.

gaaclarke

lgtm, thanks

flutter/flutter@e89fd0a...d03768e 2026-10-02 [email protected] Roll Skia from 7b7326917e77 to 9e88bf828078 (8 revisions) (flutter/flutter#193694) 2026-10-02 [email protected] [Widget Preview] Provide descriptive error when widget preview is unconstrained (flutter/flutter#193005) 2026-10-02 [email protected] Roll Dart SDK from 0e7642b85457 to ac1a97aae47d (26 revisions) (flutter/flutter#193692) 2026-10-02 [email protected] Remove unused Fuchsia sysmem header files (flutter/flutter#193244) 2026-10-02 [email protected] [Windows] Preserve composing extent in setEditingState (flutter/flutter#189968) 2026-10-02 [email protected] [docs] Make Border.symmetric docs more explicit about their arguments (flutter/flutter#193222) 2026-10-02 [email protected] [Embedder] Support render texture for vulkan (flutter/flutter#188855) 2026-10-02 [email protected] Updated Remaining Engine Defaults to SDK 37 (flutter/flutter#190429) 2026-10-02 [email protected] Document that enableSuggestions: false can disable keyboard languages on Android (flutter/flutter#192714) 2026-10-02 [email protected] [flutter_tools] Explicitly track host CPU architecture in command result analytics (flutter/flutter#191836) 2026-10-02 [email protected] [web] Preserve DOM focus on role update and honor isAccessibilityFocusBlocked (flutter/flutter#192963) 2026-10-02 [email protected] [flutter_tools] Include base href in web hot reload script paths (flutter/flutter#193678) 2026-10-02 [email protected] [Impeller] Deduplicate GLES render pass state (flutter/flutter#193427) 2026-10-02 [email protected] Roll Skia from f2d68e0b8863 to 7b7326917e77 (16 revisions) (flutter/flutter#193676) 2026-10-01 [email protected] Fix analysis failures due to missing `const` (flutter/flutter#193685) 2026-10-01 [email protected] Removes a11y_assessment app (flutter/flutter#193671) 2026-10-01 [email protected] test: configure Xvfb and openbox for windowing_test (flutter/flutter#193529) 2026-10-01 [email protected] Sync CHANGELOG.md from stable (flutter/flutter#193666) 2026-10-01 [email protected] Avoid using relative path in Process.start (flutter/flutter#193664) 2026-10-01 [email protected] [AGP 9.1.0 Migration #6] Deliver Flutter assets as a generated assets source directory on the app path (flutter/flutter#192488) 2026-10-01 [email protected] ci(bringup): android_java17_build_android_host_app_with_module_aar is green (flutter/flutter#193580) 2026-10-01 [email protected] [flutter_tools] Fix Use dependency graph to determine plugin initialization order (flutter/flutter#191591) 2026-10-01 [email protected] [flutter_tools] Migrate DaemonCommand and Daemon domains to constructor DI (flutter/flutter#193542) If this roll has caused a breakage, revert this CL and set the roller to dry run mode using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…StateCache - Store `VertexAttribStateCache` and `configured_program_y_flip` inside `RenderPassStateCache` in `render_pass_gles.cc` (matching `flutter#193427`), eliminating mutable `configured_y_flip_` from `PipelineGLES`. - Track `bound_sampler_keys` (`std::optional<uint64_t>`) and `bound_textures` (`std::optional<GLuint>`) per texture unit in `VertexAttribStateCache` so the branch is self-contained off `master` (`86798946adc`). - Add `BufferBindingsGLESTest` unit tests for vertex attribute deduplication, active texture/sampler binding deduplication, and dirty-tracked UBO uploads.

@gaaclarke

…lutter#193922) ### Rationale Split out from flutter#193427 per @gaaclarke's review feedback. OpenGL ES does not expose standalone sampler objects on the paths used by `SamplerGLES`; instead, `SamplerGLES::ConfigureBoundTexture` emits up to 5 `glTexParameteri` calls (`GL_TEXTURE_MIN_FILTER`, `GL_TEXTURE_MAG_FILTER`, `GL_TEXTURE_MAX_LEVEL`, `GL_TEXTURE_WRAP_S`, `GL_TEXTURE_WRAP_T`) plus optional `GL_TEXTURE_MAX_ANISOTROPY_EXT` on the bound GL texture object every time a texture is bound for a draw call. On glyph-atlas-heavy and tiled scenes (`bouncy@128`, `grid@1000`), hundreds or thousands of consecutive draw calls sample the same texture with an identical `SamplerDescriptor`, issuing up to `12,258` redundant `glTexParameteri`/`glTexParameterf` calls per frame after flutter#193427. Within a render pass, non-wrapped `TextureGLES` instances own an immutable `UniqueHandleGLES` (`HandleGLES`) and immutable `TextureDescriptor` / `mip_count`. Caching the last configured `SamplerDescriptor::ToKey` per `HandleGLES` in `SamplerStateCache` inside `RenderPassStateCache` (alongside `BlendStateCache`, `StencilStateCache`, and `DepthStateCache`) skips redundant `glTexParameter*` calls across draws and pipelines within the pass without storing mutable cached state on `TextureGLES`. ### Summary of Changes - [`engine/src/flutter/impeller/renderer/backend/gles/texture_gles.h`](https://github.com/kevmoo/flutter/blob/f9f4ba51255/engine/src/flutter/impeller/renderer/backend/gles/texture_gles.h): Add a read-only `const HandleGLES& GetHandle() const` accessor matching `PipelineGLES::GetProgramHandle()`. - [`engine/src/flutter/impeller/renderer/backend/gles/buffer_bindings_gles.h`](https://github.com/kevmoo/flutter/blob/f9f4ba51255/engine/src/flutter/impeller/renderer/backend/gles/buffer_bindings_gles.h) & [`engine/src/flutter/impeller/renderer/backend/gles/buffer_bindings_gles.cc`](https://github.com/kevmoo/flutter/blob/f9f4ba51255/engine/src/flutter/impeller/renderer/backend/gles/buffer_bindings_gles.cc): Add `struct SamplerStateCache` (`absl::flat_hash_map<HandleGLES, uint64_t, HandleGLES::Hash, HandleGLES::Equal> configured_samplers;` + `ConfigureBoundTexture(...)`) and pass an optional `SamplerStateCache*` into `BufferBindingsGLES::BindUniformData` and `BufferBindingsGLES::BindTextures`. Wrapped external textures (`IsWrapped()`) bypass the cache so external callers mutating raw GL texture parameters are unaffected. - [`engine/src/flutter/impeller/renderer/backend/gles/render_pass_gles.cc`](https://github.com/kevmoo/flutter/blob/f9f4ba51255/engine/src/flutter/impeller/renderer/backend/gles/render_pass_gles.cc): Add `SamplerStateCache sampler;` to `RenderPassStateCache` and pass `&state_cache.sampler` into `vertex_desc_gles->BindUniformData(...)`. - [`engine/src/flutter/impeller/renderer/backend/gles/buffer_bindings_gles_unittests.cc`](https://github.com/kevmoo/flutter/blob/f9f4ba51255/engine/src/flutter/impeller/renderer/backend/gles/buffer_bindings_gles_unittests.cc): Add `BufferBindingsGLESTest.SkipsRedundantSamplerConfigurationOnSameTexture` covering repeated binds with an identical sampler descriptor, re-configuration when the sampler descriptor changes, cross-pipeline deduplication/invalidation via a shared `SamplerStateCache`, and cache bypass for wrapped external textures (`TextureGLES::WrapTexture`). ### Flow / Surface Delta #### 1. Deterministic WebGL2 Call-Count Reductions (`0%` Variance) Measured on top of merged flutter#193427 (`origin/master` @ `a554b8ac180` vs. `daad7727f54` across steady-state frames): | Workload (`1280x720`, `wimp_st`) | Metric | Pre-Change (post-`flutter#193427`) | Post-Change | Absolute Delta | Delta (%) | Reduction | | :--- | :--- | :---: | :---: | :---: | :---: | :--- | | **`bouncy` (`128` nodes, `441` draws, `247` textured)** | `glTexParameteri` + `glTexParameterf` / frame | `1,482` (`1,235` + `247`) | `~6` (`1` per pass) | `-1,476` | **`-99.6%`** | **Deduplicated per pass** | | | Total GL calls / frame | `7,827` | `6,336` | `-1,491` | **`-19.0%`** | **`1.24x` fewer calls** (`-52.2%` / `2.09x` cumulative w/ `flutter#193427`) | | **`grid` (`100` nodes, `660` draws, `245` textured)** | `glTexParameteri` + `glTexParameterf` / frame | `1,470` (`1,225` + `245`) | `~6` (`1` per pass) | `-1,464` | **`-99.6%`** | **Deduplicated per pass** | | | Total GL calls / frame | `10,674` | `9,213` | `-1,461` | **`-13.7%`** | **`1.16x` fewer calls** | | **`grid` (`1000` nodes, `5,908` draws, `2,043` textured)** | `glTexParameteri` + `glTexParameterf` / frame | `12,258` (`10,215` + `2,043`) | `~6` (`1` per pass) | `-12,252` | **`-99.95%`** | **Deduplicated per pass** | | | Total GL calls / frame | `93,669` | `81,285` | `-12,384` | **`-13.2%`** | **`1.15x` fewer calls** (`-47.4%` / `1.90x` cumulative w/ `flutter#193427`) | In `100us` V8/Wasm CPU profiling of the `DedicatedWorker` raster thread on `grid@1000` (`--profile --no-strip-wasm`), redundant `glTexParameteri` (`9.75 ms` Wasm self-time + `19.76 ms` JS binding) and `glTexParameterf` (`12.80 ms` JS binding) accounted for **`42.31 ms` (`2.09%` of total active worker CPU time)** across a `2.02s` steady-state window. #### 2. Hardware WebGL2 `rasterTimeMs` A/B (`AMD Radeon 780M`, `n=5` Interleaved ABBA Pairs) Measured on headed Chrome 154 (`ANGLE (AMD, AMD Radeon 780M Graphics (radeonsi phoenix ACO), OpenGL ES 3.2)`, `--disable-gpu-vsync --disable-frame-rate-limit`, `10` samples/run × `5` interleaved ABBA pairs per arm): | Workload (`1280x720`, `wimp_st`) | Pre-Change (`a554b8ac180`) | Post-Change (`daad7727f54`) | Absolute Delta | Delta (%) | Speedup | | :--- | :---: | :---: | :---: | :---: | :---: | | **`bouncy` (`64` nodes)** | `1.412 ms` | `1.382 ms` | `-0.030 ms` | `-2.15%` | `1.02x` | | **`bouncy` (`128` nodes)** | `1.842 ms` | `1.756 ms` | `-0.086 ms` | **`-4.66%`** | **`1.05x`** | | **`grid` (`100` nodes)** | `1.783 ms` | `1.727 ms` | `-0.057 ms` | **`-3.17%`** | **`1.03x`** | | **`grid` (`1000` nodes)** | `16.385 ms` | `15.996 ms` | `-0.389 ms` | **`-2.37%`** | **`1.02x`** | **How much to trust these numbers:** - **Call counts (Table 1) are exact (`0%` run-to-run variance)**: Every frame deterministically eliminates `1,464`–`12,252` redundant `glTexParameter*` calls across the boundary. - **Wall-clock `rasterTimeMs` (Table 2) has a `~1.2%–2.0%` run-to-run noise floor**: - **`bouncy@128` (`-4.66%`)** and **`grid@100` (`-3.17%`)** clear the noise floor consistently (`5/5` interleaved Post runs beat Pre). - **`grid@1000` (`-2.37%`, `-0.39 ms`)** sits just above the `~1.5%–2.0%` noise floor (`4/5` pairs faster) and aligns with the `2.09%` worker CPU profile share of `glTexParameter*` when `5,908` draw calls dominate frame time. - **`bouncy@64` (`-2.15%`, `-0.03 ms`)** is inside the noise floor at `<1.5 ms` frame times and should be treated as flat/within noise. ### Verification - Built and ran `host_debug_unopt:impeller_unittests --gtest_filter="*BufferBindingsGLES*:*RenderPassGLES*:*SamplerGLES*:*TextureGLES*"` locally (`37/37` passed, including `BufferBindingsGLESTest.SkipsRedundantSamplerConfigurationOnSameTexture`). - Built `wasm_release:skwasm/wimp.js` and verified both deterministic WebGL2 call counts and `n=5` interleaved hardware WebGL2 `rasterTimeMs` medians (`AMD Radeon 780M`) in Chrome 154 via `flutter-wasm-compare`. Related to flutter#192147 Towards flutter#174980 ## 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 in-code documentation (doc comments with `///`). - [x] If this PR introduces a new feature or capability, I created and linked a website documentation issue or PR in [flutter/website] (or verified none is needed). - [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.  [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [flutter/website]: https://github.com/flutter/website [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md

- Merge origin/master (including flutter#193922 SamplerStateCache) into wimp-track-b-gles3-state-dedup. - Convert struct VertexAttribStateCache to class VertexAttribStateCache with private data members and encapsulated state-tracking methods matching the review style from flutter#193922 and flutter#193427. - DelegateSamplerStateCache::IsTextureConfigured in BindAndConfigureTexture.

Deduplicates redundant OpenGL ES state transitions across draw commands within `RenderPassGLES` (`EncodeCommandsInReactor`): 1. **Per-Pass Orthogonal GLES State Tracking (`RenderPassStateCache`)**: - Encapsulates pass-scoped GL state tracking inside `RenderPassStateCache` (`BlendStateCache`, `StencilFaceStateCache`, `StencilStateCache`, and `DepthStateCache` in an anonymous namespace in `render_pass_gles.cc`), tracking enabled/disabled state (`GL_BLEND`, `GL_STENCIL_TEST`, `GL_DEPTH_TEST`, `GL_SCISSOR_TEST`), active program handle (`std::optional<HandleGLES>` via `HandleGLES::Equal{}` so pipeline variants sharing a compiled `UniqueHandleGLES` skip redundant program binds and `_impeller_y_flip` uniform updates), color mask, blend equations/factors, depth compare function and write mask, front/back stencil functions/operations/write masks, viewport, scissor rect, cull mode, and winding order across draw calls within `EncodeCommandsInReactor`. - `ResetGLState` initializes the tracked state once at pass entry before `glClear`, eliminating the redundant pass-exit `ResetGLState` call while ensuring no state leaks across passes, `ReactorGLES` handle operations, or `BlitPassGLES` commands. ### WebGL 2.0 Call Count & CPU Command-Encoding Reduction (Web Impeller / WIMP) | Workload (`1280x720`, `wimp_st`) | Metric | Before | After | Delta | | :--- | :--- | :---: | :---: | :---: | | **`bouncy` (`128` nodes, `441` draws)** | Total WebGL2 calls / frame | `13,234` | `7,983` | **`-39.7%` (`-5,251` calls)** | | | `glBlendEquationSeparate` + `glBlendFuncSeparate` / frame | `802` | `4` | **`-99.5%`** | | | `glStencilFuncSeparate` + `Op` + `Mask` / frame | `1,327` | `9` | **`-99.3%`** | | | `glColorMask` / frame | `443` | `3` | **`-99.3%`** | | | `glEnable` + `glDisable` / frame | `1,506` | `225` | **`-85.1%`** | | | `glDepthFunc` + `glDepthMask` / frame | `884` | `161` | **`-81.8%`** | | | `glUseProgram` / frame | `441` | `105` | **`-76.2%`** | | | `glUniform1fv` (`_impeller_y_flip`) / frame | `441` | `105` | **`-76.2%`** | | | `glScissor` / frame | `175` | `117` | **`-33.1%`** | | | CPU command-encoding raster time (`--disable-gl-drawing-for-tests`) | `13.41 ms` | `12.47 ms` | **`-7.0%` (`-0.94 ms`, `1.08x` faster)** | | **`grid` (`1000` nodes, `5,908` draws)** | Total WebGL2 calls / frame | `154,521` | `94,517` | **`-38.8%` (`-60,004` calls)** | | | `glBlendEquationSeparate` + `glBlendFuncSeparate` / frame | `9,474` | `338` | **`-96.4%`** | | | `glStencilFuncSeparate` + `Op` + `Mask` / frame | `17,726` | `844` | **`-95.2%`** | | | `glColorMask` / frame | `5,909` | `337` | **`-94.3%`** | | | `glEnable` + `glDisable` / frame | `18,071` | `2,691` | **`-85.1%`** | | | `glUseProgram` / frame | `5,908` | `2,197` | **`-62.8%`** | | | `glUniform1fv` (`_impeller_y_flip`) / frame | `5,908` | `2,197` | **`-62.8%`** | | | `glDepthFunc` + `glDepthMask` / frame | `11,817` | `6,051` | **`-48.8%`** | | | CPU command-encoding raster time (`--disable-gl-drawing-for-tests`) | `60.86 ms` | `56.13 ms` | **`-7.8%` (`-4.73 ms`, `1.08x` faster)** | Fixes flutter#192147 ## 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 in-code documentation (doc comments with `///`). - [x] If this PR introduces a new feature or capability, I created and linked a website documentation issue or PR in [flutter/website] (or verified none is needed). - [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.  [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [flutter/website]: https://github.com/flutter/website [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md

@gaaclarke

…lutter#193922) ### Rationale Split out from flutter#193427 per @gaaclarke's review feedback. OpenGL ES does not expose standalone sampler objects on the paths used by `SamplerGLES`; instead, `SamplerGLES::ConfigureBoundTexture` emits up to 5 `glTexParameteri` calls (`GL_TEXTURE_MIN_FILTER`, `GL_TEXTURE_MAG_FILTER`, `GL_TEXTURE_MAX_LEVEL`, `GL_TEXTURE_WRAP_S`, `GL_TEXTURE_WRAP_T`) plus optional `GL_TEXTURE_MAX_ANISOTROPY_EXT` on the bound GL texture object every time a texture is bound for a draw call. On glyph-atlas-heavy and tiled scenes (`bouncy@128`, `grid@1000`), hundreds or thousands of consecutive draw calls sample the same texture with an identical `SamplerDescriptor`, issuing up to `12,258` redundant `glTexParameteri`/`glTexParameterf` calls per frame after flutter#193427. Within a render pass, non-wrapped `TextureGLES` instances own an immutable `UniqueHandleGLES` (`HandleGLES`) and immutable `TextureDescriptor` / `mip_count`. Caching the last configured `SamplerDescriptor::ToKey` per `HandleGLES` in `SamplerStateCache` inside `RenderPassStateCache` (alongside `BlendStateCache`, `StencilStateCache`, and `DepthStateCache`) skips redundant `glTexParameter*` calls across draws and pipelines within the pass without storing mutable cached state on `TextureGLES`. ### Summary of Changes - [`engine/src/flutter/impeller/renderer/backend/gles/texture_gles.h`](https://github.com/kevmoo/flutter/blob/f9f4ba51255/engine/src/flutter/impeller/renderer/backend/gles/texture_gles.h): Add a read-only `const HandleGLES& GetHandle() const` accessor matching `PipelineGLES::GetProgramHandle()`. - [`engine/src/flutter/impeller/renderer/backend/gles/buffer_bindings_gles.h`](https://github.com/kevmoo/flutter/blob/f9f4ba51255/engine/src/flutter/impeller/renderer/backend/gles/buffer_bindings_gles.h) & [`engine/src/flutter/impeller/renderer/backend/gles/buffer_bindings_gles.cc`](https://github.com/kevmoo/flutter/blob/f9f4ba51255/engine/src/flutter/impeller/renderer/backend/gles/buffer_bindings_gles.cc): Add `struct SamplerStateCache` (`absl::flat_hash_map<HandleGLES, uint64_t, HandleGLES::Hash, HandleGLES::Equal> configured_samplers;` + `ConfigureBoundTexture(...)`) and pass an optional `SamplerStateCache*` into `BufferBindingsGLES::BindUniformData` and `BufferBindingsGLES::BindTextures`. Wrapped external textures (`IsWrapped()`) bypass the cache so external callers mutating raw GL texture parameters are unaffected. - [`engine/src/flutter/impeller/renderer/backend/gles/render_pass_gles.cc`](https://github.com/kevmoo/flutter/blob/f9f4ba51255/engine/src/flutter/impeller/renderer/backend/gles/render_pass_gles.cc): Add `SamplerStateCache sampler;` to `RenderPassStateCache` and pass `&state_cache.sampler` into `vertex_desc_gles->BindUniformData(...)`. - [`engine/src/flutter/impeller/renderer/backend/gles/buffer_bindings_gles_unittests.cc`](https://github.com/kevmoo/flutter/blob/f9f4ba51255/engine/src/flutter/impeller/renderer/backend/gles/buffer_bindings_gles_unittests.cc): Add `BufferBindingsGLESTest.SkipsRedundantSamplerConfigurationOnSameTexture` covering repeated binds with an identical sampler descriptor, re-configuration when the sampler descriptor changes, cross-pipeline deduplication/invalidation via a shared `SamplerStateCache`, and cache bypass for wrapped external textures (`TextureGLES::WrapTexture`). ### Flow / Surface Delta #### 1. Deterministic WebGL2 Call-Count Reductions (`0%` Variance) Measured on top of merged flutter#193427 (`origin/master` @ `a554b8ac180` vs. `daad7727f54` across steady-state frames): | Workload (`1280x720`, `wimp_st`) | Metric | Pre-Change (post-`flutter#193427`) | Post-Change | Absolute Delta | Delta (%) | Reduction | | :--- | :--- | :---: | :---: | :---: | :---: | :--- | | **`bouncy` (`128` nodes, `441` draws, `247` textured)** | `glTexParameteri` + `glTexParameterf` / frame | `1,482` (`1,235` + `247`) | `~6` (`1` per pass) | `-1,476` | **`-99.6%`** | **Deduplicated per pass** | | | Total GL calls / frame | `7,827` | `6,336` | `-1,491` | **`-19.0%`** | **`1.24x` fewer calls** (`-52.2%` / `2.09x` cumulative w/ `flutter#193427`) | | **`grid` (`100` nodes, `660` draws, `245` textured)** | `glTexParameteri` + `glTexParameterf` / frame | `1,470` (`1,225` + `245`) | `~6` (`1` per pass) | `-1,464` | **`-99.6%`** | **Deduplicated per pass** | | | Total GL calls / frame | `10,674` | `9,213` | `-1,461` | **`-13.7%`** | **`1.16x` fewer calls** | | **`grid` (`1000` nodes, `5,908` draws, `2,043` textured)** | `glTexParameteri` + `glTexParameterf` / frame | `12,258` (`10,215` + `2,043`) | `~6` (`1` per pass) | `-12,252` | **`-99.95%`** | **Deduplicated per pass** | | | Total GL calls / frame | `93,669` | `81,285` | `-12,384` | **`-13.2%`** | **`1.15x` fewer calls** (`-47.4%` / `1.90x` cumulative w/ `flutter#193427`) | In `100us` V8/Wasm CPU profiling of the `DedicatedWorker` raster thread on `grid@1000` (`--profile --no-strip-wasm`), redundant `glTexParameteri` (`9.75 ms` Wasm self-time + `19.76 ms` JS binding) and `glTexParameterf` (`12.80 ms` JS binding) accounted for **`42.31 ms` (`2.09%` of total active worker CPU time)** across a `2.02s` steady-state window. #### 2. Hardware WebGL2 `rasterTimeMs` A/B (`AMD Radeon 780M`, `n=5` Interleaved ABBA Pairs) Measured on headed Chrome 154 (`ANGLE (AMD, AMD Radeon 780M Graphics (radeonsi phoenix ACO), OpenGL ES 3.2)`, `--disable-gpu-vsync --disable-frame-rate-limit`, `10` samples/run × `5` interleaved ABBA pairs per arm): | Workload (`1280x720`, `wimp_st`) | Pre-Change (`a554b8ac180`) | Post-Change (`daad7727f54`) | Absolute Delta | Delta (%) | Speedup | | :--- | :---: | :---: | :---: | :---: | :---: | | **`bouncy` (`64` nodes)** | `1.412 ms` | `1.382 ms` | `-0.030 ms` | `-2.15%` | `1.02x` | | **`bouncy` (`128` nodes)** | `1.842 ms` | `1.756 ms` | `-0.086 ms` | **`-4.66%`** | **`1.05x`** | | **`grid` (`100` nodes)** | `1.783 ms` | `1.727 ms` | `-0.057 ms` | **`-3.17%`** | **`1.03x`** | | **`grid` (`1000` nodes)** | `16.385 ms` | `15.996 ms` | `-0.389 ms` | **`-2.37%`** | **`1.02x`** | **How much to trust these numbers:** - **Call counts (Table 1) are exact (`0%` run-to-run variance)**: Every frame deterministically eliminates `1,464`–`12,252` redundant `glTexParameter*` calls across the boundary. - **Wall-clock `rasterTimeMs` (Table 2) has a `~1.2%–2.0%` run-to-run noise floor**: - **`bouncy@128` (`-4.66%`)** and **`grid@100` (`-3.17%`)** clear the noise floor consistently (`5/5` interleaved Post runs beat Pre). - **`grid@1000` (`-2.37%`, `-0.39 ms`)** sits just above the `~1.5%–2.0%` noise floor (`4/5` pairs faster) and aligns with the `2.09%` worker CPU profile share of `glTexParameter*` when `5,908` draw calls dominate frame time. - **`bouncy@64` (`-2.15%`, `-0.03 ms`)** is inside the noise floor at `<1.5 ms` frame times and should be treated as flat/within noise. ### Verification - Built and ran `host_debug_unopt:impeller_unittests --gtest_filter="*BufferBindingsGLES*:*RenderPassGLES*:*SamplerGLES*:*TextureGLES*"` locally (`37/37` passed, including `BufferBindingsGLESTest.SkipsRedundantSamplerConfigurationOnSameTexture`). - Built `wasm_release:skwasm/wimp.js` and verified both deterministic WebGL2 call counts and `n=5` interleaved hardware WebGL2 `rasterTimeMs` medians (`AMD Radeon 780M`) in Chrome 154 via `flutter-wasm-compare`. Related to flutter#192147 Towards flutter#174980 ## 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 in-code documentation (doc comments with `///`). - [x] If this PR introduces a new feature or capability, I created and linked a website documentation issue or PR in [flutter/website] (or verified none is needed). - [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.  [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [flutter/website]: https://github.com/flutter/website [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md

github-actions Bot added engine flutter/engine related. See also e: labels. e: impeller Impeller rendering backend issues and features requests labels Sep 28, 2026

kevmoo requested a review from gaaclarke September 29, 2026 03:11

kevmoo marked this pull request as ready for review September 29, 2026 03:11

gemini-code-assist Bot reviewed Sep 29, 2026

View reviewed changes

Comment thread engine/src/flutter/impeller/renderer/backend/gles/texture_gles.h Resolved

Comment thread engine/src/flutter/impeller/renderer/backend/gles/render_pass_gles.cc Resolved

gaaclarke previously requested changes Sep 29, 2026

View reviewed changes

gaaclarke reviewed Sep 29, 2026

View reviewed changes

kevmoo added 2 commits September 29, 2026 22:32

Disable GL_SCISSOR_TEST when transitioning to unscissored commands

00ed46d

Refactor RenderPassGLES state caches into structs and split out sampl…

a4bf0ec

…er caching

kevmoo changed the title ~~[Impeller] Deduplicate GLES render pass state and texture sampler parameters~~ [Impeller] Deduplicate GLES render pass state Sep 29, 2026

kevmoo requested a review from gaaclarke September 29, 2026 23:37

kevmoo added the CICD Run CI/CD label Oct 1, 2026

gemini-code-assist Bot reviewed Oct 1, 2026

View reviewed changes

Comment thread engine/src/flutter/impeller/renderer/backend/gles/render_pass_gles.cc Resolved

gaaclarke approved these changes Oct 1, 2026

View reviewed changes

kevmoo added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 1, 2026

auto-submit Bot added this pull request to the merge queue Oct 2, 2026

Merged via the queue into flutter:master with commit 3964e0e Oct 2, 2026
23 checks passed

flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 2, 2026

kevmoo deleted the impeller-gles-state-dedupe branch October 2, 2026 05:00

bdero mentioned this pull request Oct 5, 2026

[Impeller] Expand the vertex attribute formats and gate them by backend capability #190642

Open

10 tasks done

kevmoo mentioned this pull request Oct 6, 2026

[Impeller] Cache GLES sampler configuration in RenderPassStateCache #193922

Merged

11 tasks done

kevmoo mentioned this pull request Oct 14, 2026

[Impeller] Reduce per-frame CPU and GLES state overhead on WebGL2 ☂️ #194408

Open

4 of 11 tasks

kevinsoul mentioned this pull request Oct 20, 2026

[Linux] Impeller (default since 3.47) causes jank on pointer-driven repaints on AMD radeonsi/Mesa (Wayland); Skia opt-out is smooth — affects debug and release #194433

Closed

This was referenced Nov 1, 2026

[Impeller][OpenGLES] ~2x raster time vs Skia on Mali-G31 (no Vulkan), unchanged on beta and master #195767

Open

[Impeller] Impeller is significantly slower than Skia on Adreno 702 (QCM2290) — 3.5x worse raster, Flutter 3.47.2 #192364

Open

Conversation

kevmoo commented Sep 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

WebGL 2.0 Call Count & CPU Command-Encoding Reduction (Web Impeller / WIMP)

Pre-launch Checklist

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

gaaclarke left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gaaclarke left a comment

Choose a reason for hiding this comment

Uh oh!

kevmoo commented Sep 29, 2026

Uh oh!

gaaclarke commented Oct 1, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

gaaclarke left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kevmoo commented Sep 28, 2026 •

edited

Loading