[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.

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 Outdated

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

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

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

Open

11 tasks done

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

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kevmoo commented Sep 28, 2026 •

edited

Loading