[Impeller] GLES: lazily allocate texture mip levels on first per-level write by bdero · Pull Request #186302 · flutter/flutter

bdero · 2026-05-10T02:52:45Z

glTexSubImage2D into a mip level not previously defined by glTexImage2D raises GL_INVALID_OPERATION. Both the lazy slice-init in BlitCopyBufferToTextureCommandGLES::Encode and TextureGLES::InitializeContentsIfNecessary only ever allocated level 0, so any non-zero mip upload aborted with a fatal GL error. The pre-existing cubemap path in InitializeContentsIfNecessary was also broken: it would re-bind a GL_TEXTURE_CUBE_MAP handle to GL_TEXTURE_2D and call glTexImage2D on the wrong target, a latent crash on the first cubemap sampler bind without a prior face upload.

#185890 depends on this PR.

Approach

Per-(slice, mip_level) initialization tracking with lazy allocation on first write:

slices_initialized_ becomes std::array<std::bitset<16>, 6> (six cubemap faces by up to 16 mip levels, covering a 32k base dimension).
InitializeContentsIfNecessary allocates only the base mip level. This is byte-identical to pre-fix behavior on the 2D path, so Impeller's snapshot pipeline (Picture.toImage(generate_mips: true) -> RenderTargetAllocator::CreateOffscreen[MSAA] -> render at level 0 -> glGenerateMipmap) keeps its existing GL footprint. For cubemaps the function now correctly binds GL_TEXTURE_CUBE_MAP and allocates the base level for each of the six face targets.
BlitCopyBufferToTextureCommandGLES::Encode allocates the requested (slice, mip_level) lazily before glTexSubImage2D. Per-level uploads pay only for the levels they touch, and writes to a level after glGenerateMipmap (or MarkContentsInitialized from the Android external-texture path) are skipped because those bits are already set.
OnSetContents (the legacy upload path) is updated to use the new bit-marking API. Side-effect fix: the old slices_initialized_ = AddOperation(...) assignment clobbered all six cubemap bits on every upload; the new API marks only the face that was actually written.

Behavior table

Code path	Pre-fix	This PR
2D mip_count=1 (overwhelming majority)	1 `TexImage2D`	identical
Snapshot path with `generate_mips=true`	1 `TexImage2D` upfront + `glGenerateMipmap` driven implicit allocations	identical
Flutter GPU per-level upload (#185890)	crash (`GL_INVALID_OPERATION` on `glTexSubImage2D`)	works; 1 `TexImage2D` per mip on first write
Cubemap face upload via blit	works	identical
Cubemap rebind via `Bind()` before any upload	crash (rebind to `GL_TEXTURE_2D` of cube handle)	works; bind to `GL_TEXTURE_CUBE_MAP` and allocate face base level

This PR does not have a dedicated issue: it is a prerequisite bug fix for the per-(mip, slice) upload path required by #185890, and the failure modes (GL_INVALID_OPERATION on glTexSubImage2D, cubemap rebind to wrong target) are documented above. Verified via new BlitCommandGLES unit tests covering base-level, non-zero-mip, repeated-upload, every-mip-on-first-write, and cubemap face cases (MockGLES wires glTexImage2D and glBindTexture to the existing MOCK_METHOD declarations); the full GLES test suite passes locally (749 tests); manually confirmed with flutter_tester_opengles --enable-flutter-gpu against gpu_test.dart from #185890 that Texture.overwrite writes to a non-zero mip level and the cubemap-face tests pass.

Pre-launch Checklist

gemini-code-assist

Code Review

This pull request modifies the GLES backend to allocate the complete mipmap chain using glTexImage2D during texture initialization and buffer-to-texture blit operations, ensuring storage is defined for all levels before sub-image updates. Review feedback highlights that the implementation in TextureGLES currently assumes a 2D texture target and needs to be updated to support cubemap faces. Furthermore, it is suggested to refactor the redundant mipmap dimension calculation and allocation logic into a shared helper method to reduce code duplication.

…l write `glTexSubImage2D` into a level not previously defined by `glTexImage2D` raises `GL_INVALID_OPERATION`. Both the lazy slice-init in `BlitCopyBufferToTextureCommandGLES::Encode` and `TextureGLES::InitializeContentsIfNecessary` only ever allocated level 0, so any non-zero mip upload aborted with a fatal GL error. The pre-existing `InitializeContentsIfNecessary` cubemap path was also broken: it would re-bind a `GL_TEXTURE_CUBE_MAP` handle to `GL_TEXTURE_2D` and call `glTexImage2D` on the wrong target (latent crash on first cubemap sampler bind without prior upload). Switch to per-`(slice, mip_level)` initialization tracking and allocate each level lazily on its first write: - `slices_initialized_` becomes `std::array<std::bitset<16>, 6>` (per-slice, per-mip). - `InitializeContentsIfNecessary` allocates only the base mip level (matching the pre-fix call count, so Impeller's snapshot pipeline keeps its `glGenerateMipmap`-driven allocation pattern unchanged). For cubemaps it now correctly binds `GL_TEXTURE_CUBE_MAP` and allocates the base level for each of the six face targets. - `BlitCopyBufferToTextureCommandGLES::Encode` allocates the requested `(slice, mip_level)` lazily before `glTexSubImage2D`, so non-zero mip uploads find storage already defined while existing single-level uploads pay no extra cost. Cubemap face targets and the per-(slice, mip) helpers are exercised by new `BlitCommandGLES` unit tests; the `glTexImage2D` and `glBindTexture` mocks in `MockGLES` are wired up to support them. Verified locally with flutter_tester_opengles --enable-flutter-gpu against the multi-mip gpu_test.dart from flutter#185890: the new "Texture.overwrite writes to a non-zero mip level" and cubemap-face tests pass on the GLES backend.

…tests The structured-binding loop variable copies a shared_ptr each iteration; take it by const reference instead.

gaaclarke

mostly looks good, there is some existing problems this is building off of that would be nice to avoid

gaaclarke · 2026-05-11T16:59:52Z

+  ///                        always 0).
+  /// @param[in]  mip_level  The mip level whose storage was allocated.
+  ///
+  void MarkSliceMipLevelInitialized(size_t slice, size_t mip_level) const;


I know you are following the existing pattern, but this is function for mutating, this shouldn't be a const function.

Yeah I agree, this is all sorts of wonky, and would be happy to clean it up. Would you mind if I deferred the full cleanup of the Texture mutation chain to a follow-up PR? Filed an issue: #186371

sure, if you pinky promise

Ah actually, thinking about this a bit more, the underlying mutable usage here seems like the canonical use case. GLES-specific lazy allocation tracking is purely an implementation detail (Metal and Vulkan allocate the full mip chain upfront and don't need any of this). Making the method non-const would force stuff like Bind() and SetAsFramebufferAttachment() to be non-const too, propagating a GLES-driver workaround into the type signatures across everything using the higher abstraction. I'd lean toward keeping MarkSliceMipLevelInitialized const for symmetry with MarkSliceInitialized, but happy to revert the MarkContentsInitialized() const change in this PR so we're not extending the pattern any further than necessary. Thoughts?

I don't wanna land this and then bait & switch you haha.

The whole thing seems fukakta, TextureGLES::InitializeContentsIfNecessary() is a const method too.

Yea, give it a shot if it becomes too much of a hassle let's stop the bleeding by not following that pattern here if we can avoid it.

propagating a GLES-driver workaround into the type signatures across everything using the higher abstraction.

Maybe if the mutable was percolated up it would make more sense than where it is now?

Give it a shot and if it get's hairy we can save it for another day.

(this is fine for now, feel free to explore outside of this pr)

Maybe if the mutable was percolated up it would make more sense than where it is now?

That's an interesting idea. Okay, I repurposed the issue and will land + see if I can make the weirdness down here better.

I tried it out and it was really easy to do: e36064c

Ah thanks, I'll just do it in this PR then

Alrighty, done in b794df4

gaaclarke · 2026-05-11T20:29:21Z

+  ///                        always 0).
+  /// @param[in]  mip_level  The mip level whose storage was allocated.
+  ///
+  void MarkSliceMipLevelInitialized(size_t slice, size_t mip_level) const;


sure, if you pinky promise

@gaaclarke

…hain The lazy `glTexImage2D` allocation introduced here mutates per-(slice, mip) bookkeeping that prior code expressed as `mutable` members behind `const` methods. Following @gaaclarke's review comment and his demonstration commit, drop `const` from the methods that actually mutate so the type signatures advertise the mutation honestly: * `TextureGLES::MarkContentsInitialized` * `TextureGLES::MarkSliceInitialized` * `TextureGLES::MarkSliceMipLevelInitialized` * `TextureGLES::InitializeContentsIfNecessary` * `TextureGLES::Bind` * `TextureGLES::SetAsFramebufferAttachment` Remove the `mutable` qualifier from `slice_mip_initialized_` and `fence_` accordingly. All non-test callers of the de-consted methods already hold non-const `TextureGLES` references and continue to compile. The one exception is `BufferBindingsGLES::BindTextures`, which reaches the texture via a `const` view into the bound-texture list; that call site uses `const_cast<TextureGLES&>(...).Bind()`, scoped with a comment explaining that the mutation is GLES-specific lazy bookkeeping and not visible to the higher abstraction. Resolves the const-correctness discussion at flutter#186371. Verified locally on `flutter_tester` (Metal) and `flutter_tester_opengles` (SwANGLE): gpu_test.dart still reports 36 passing + 1 skipped on both backends.

gaaclarke · 2026-05-12T00:33:08Z

+    // `fence_`). The mutation is implementation detail of the GLES backend
+    // and is invisible to the higher abstraction; the `const_cast` is
+    // confined to this one call site.
+    if (!const_cast<TextureGLES&>(texture_gles).Bind()) {


Doh, i didn't realize this got added. This is probably equally bad as the mutable instance variables. Sorry about that, thanks for the comment. Either way.

auto-submit · 2026-05-12T00:48:24Z

autosubmit label was removed for flutter/flutter/186302, because - The status or check suite Windows tool_integration_tests_4_10 has failed. Please fix the issues identified (or deflake) before re-applying this label.

Roll Flutter from 707dbc0420a3 to 23f6f5853f50 (149 revisions) flutter/flutter@707dbc0...23f6f58 2026-05-12 737941+loic-sharma@users.noreply.github.com Add 'cp: review' label to the manual cherrypick process (flutter/flutter#186158) 2026-05-12 engine-flutter-autoroll@skia.org Roll Packages from 19ec8b8 to 93cbed6 (3 revisions) (flutter/flutter#186401) 2026-05-12 30870216+gaaclarke@users.noreply.github.com Removes SDF option for macOS (always enabled) (flutter/flutter#186265) 2026-05-12 nico.reiab@gmail.com docs: fix typos in flutter_tools comments (flutter/flutter#186321) 2026-05-12 15619084+vashworth@users.noreply.github.com Pass XcodeBasedProject instead of String to functions in XcodeProjectInterpreter (flutter/flutter#186378) 2026-05-12 jason-simmons@users.noreply.github.com Update iOS scenario app test goldens to match changes from flutter/flutter#182662 (flutter/flutter#186390) 2026-05-12 engine-flutter-autoroll@skia.org Roll Skia from ad0aff15b9fa to 77a21bc723dc (2 revisions) (flutter/flutter#186396) 2026-05-12 32538273+ValentinVignal@users.noreply.github.com Migrate focus_node.unfocus.0.dart to use `RadioGroup` (flutter/flutter#183979) 2026-05-12 engine-flutter-autoroll@skia.org Roll Skia from 91d3c1e730af to ad0aff15b9fa (7 revisions) (flutter/flutter#186391) 2026-05-12 bdero@google.com [Flutter GPU] Allow customizing the vertex layout on a RenderPipeline (flutter/flutter#186310) 2026-05-12 97480502+b-luk@users.noreply.github.com Fix `EmbedderTest.CanRenderTextWithImpellerMetal` test breakage (flutter/flutter#186262) 2026-05-12 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from rFhU-YPqdCRCtCz7b... to z7ICmPtn4hspu02zk... (flutter/flutter#186384) 2026-05-12 bdero@google.com [Impeller] GLES: lazily allocate texture mip levels on first per-level write (flutter/flutter#186302) 2026-05-12 bdero@google.com [Android] Propagate --enable-flutter-gpu Intent extra to engine args (flutter/flutter#186298) 2026-05-11 47866232+chunhtai@users.noreply.github.com [ci] update no-response workflow to also look for old label name in e… (flutter/flutter#186373) 2026-05-11 bdero@google.com [ImpellerC] Write a depfile when --shader-bundle is in use (flutter/flutter#186341) 2026-05-11 nico.reiab@gmail.com docs: fix doubled-word typos in comments (flutter/flutter#186320) 2026-05-11 engine-flutter-autoroll@skia.org Roll Skia from 32281401997e to 91d3c1e730af (4 revisions) (flutter/flutter#186368) 2026-05-11 15619084+vashworth@users.noreply.github.com Show SwiftPM warnings right before iOS/macOS build (flutter/flutter#185984) 2026-05-11 15619084+vashworth@users.noreply.github.com Convert rebuilding-flutter-tool script to dart (flutter/flutter#185089) 2026-05-11 15619084+vashworth@users.noreply.github.com Use Xcode's LLDB (flutter/flutter#186273) 2026-05-11 mr-peipei@web.de Remove `currentMainUri` from `generateMainDartWithPluginRegistrant` (flutter/flutter#185907) 2026-05-11 engine-flutter-autoroll@skia.org Roll Skia from 2514f6b5f92b to 32281401997e (1 revision) (flutter/flutter#186349) 2026-05-11 engine-flutter-autoroll@skia.org Roll Packages from 92552b1 to 19ec8b8 (4 revisions) (flutter/flutter#186350) 2026-05-11 1063596+reidbaker@users.noreply.github.com Check for absolute paths in skills. (flutter/flutter#185632) 2026-05-11 engine-flutter-autoroll@skia.org Roll Skia from 9fb7d2814642 to 2514f6b5f92b (1 revision) (flutter/flutter#186347) 2026-05-11 engine-flutter-autoroll@skia.org Roll Skia from 8cafb209e836 to 9fb7d2814642 (4 revisions) (flutter/flutter#186335) 2026-05-10 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from sOBiPJb0xznDBZlf5... to rFhU-YPqdCRCtCz7b... (flutter/flutter#186328) 2026-05-10 engine-flutter-autoroll@skia.org Roll Skia from 05a03f99c74e to 8cafb209e836 (1 revision) (flutter/flutter#186315) 2026-05-10 bdero@google.com [Impeller] Vulkan: don't drop user-supplied viewport X, Y, and depth range (flutter/flutter#185886) 2026-05-09 mbrase@google.com Update Fuchsia tests to subpackage their child components (flutter/flutter#186259) 2026-05-09 victorsanniay@gmail.com Fix SelectableText crash with inline lambda contextMenuBuilder (flutter/flutter#184990) 2026-05-09 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from 5_TnhTsHSqtCx37o6... to sOBiPJb0xznDBZlf5... (flutter/flutter#186289) 2026-05-09 engine-flutter-autoroll@skia.org Roll Skia from dc78d4bd2efb to 05a03f99c74e (2 revisions) (flutter/flutter#186283) 2026-05-09 22373191+Hari-07@users.noreply.github.com Improve non rect platform view rendering (flutter/flutter#182662) 2026-05-08 engine-flutter-autoroll@skia.org Roll Skia from 31521f8508c7 to dc78d4bd2efb (1 revision) (flutter/flutter#186278) 2026-05-08 30870216+gaaclarke@users.noreply.github.com Moves wide_gamut_macos to arm64 (flutter/flutter#186214) 2026-05-08 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[iOS] Migrate VSyncClient to a pure Obj-C implementation (#186166)" (flutter/flutter#186266) 2026-05-08 engine-flutter-autoroll@skia.org Roll Skia from a00db8749edb to 31521f8508c7 (2 revisions) (flutter/flutter#186264) 2026-05-08 97480502+b-luk@users.noreply.github.com Optimize compatible `DrawDiffRoundRect` calls to use `DrawRoundRect` (flutter/flutter#186203) 2026-05-08 bdero@google.com [triage] Add Flutter GPU as a triage team (flutter/flutter#186263) 2026-05-08 dmgr@google.com doc: Unified Check-Run User manual (flutter/flutter#186210) 2026-05-08 engine-flutter-autoroll@skia.org Roll Skia from 5f7adf4403d6 to a00db8749edb (1 revision) (flutter/flutter#186257) 2026-05-08 engine-flutter-autoroll@skia.org Roll Packages from 0411f1d to 92552b1 (1 revision) (flutter/flutter#186256) 2026-05-08 34871572+gmackall@users.noreply.github.com Add logging to figure out jvm crashes for `hot_mode_tests` (flutter/flutter#186107) 2026-05-08 engine-flutter-autoroll@skia.org Roll Skia from 926c09741ce2 to 5f7adf4403d6 (3 revisions) (flutter/flutter#186242) ...

flutter-dashboard Bot added the CICD Run CI/CD label May 10, 2026

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

bdero mentioned this pull request May 10, 2026

[Flutter GPU] Allow allocating multi-mip textures and overwriting specific (mip, slice) levels #185890

Merged

gemini-code-assist Bot reviewed May 10, 2026

View reviewed changes

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

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

bdero changed the base branch from main to master May 10, 2026 03:53

This comment was marked as outdated.

Sign in to view

bdero force-pushed the bdero/impeller-gles-mip-allocation branch from 9abe189 to 2dc545f Compare May 10, 2026 04:00

github-actions Bot removed the CICD Run CI/CD label May 10, 2026

bdero force-pushed the bdero/impeller-gles-mip-allocation branch from 2dc545f to 1d9c1f4 Compare May 10, 2026 04:47

flutter-dashboard Bot added the CICD Run CI/CD label May 10, 2026

bdero force-pushed the bdero/impeller-gles-mip-allocation branch from 1d9c1f4 to 5b59c29 Compare May 10, 2026 07:25

github-actions Bot removed the CICD Run CI/CD label May 10, 2026

bdero added the CICD Run CI/CD label May 10, 2026

bdero changed the title ~~[Impeller] GLES: allocate full mip chain on first slice init~~ [Impeller] GLES: lazily allocate texture mip levels on first per-level write May 10, 2026

bdero force-pushed the bdero/impeller-gles-mip-allocation branch from 5b59c29 to ce4ef72 Compare May 10, 2026 07:45

github-actions Bot removed the CICD Run CI/CD label May 10, 2026

bdero added the CICD Run CI/CD label May 10, 2026

bdero requested a review from gaaclarke May 10, 2026 08:15

[Impeller] Fix clang-tidy performance-for-range-copy in blit command …

a371cf0

…tests The structured-binding loop variable copies a shared_ptr each iteration; take it by const reference instead.

github-actions Bot removed the CICD Run CI/CD label May 10, 2026

bdero added the CICD Run CI/CD label May 11, 2026

gaaclarke reviewed May 11, 2026

View reviewed changes

bdero mentioned this pull request May 11, 2026

[Impeller] Explore options for the const-on-mutation pattern in GLES TextureGLES #186371

Closed

bdero requested a review from gaaclarke May 11, 2026 20:27

gaaclarke previously approved these changes May 11, 2026

View reviewed changes

gaaclarke added the autosubmit Merge PR when tree becomes green via auto submit App label May 11, 2026

bdero removed the autosubmit Merge PR when tree becomes green via auto submit App label May 11, 2026

bdero enabled auto-merge May 11, 2026 21:18

bdero removed the autosubmit Merge PR when tree becomes green via auto submit App label May 11, 2026

bdero dismissed gaaclarke’s stale review via b794df4 May 11, 2026 23:14

github-actions Bot removed the CICD Run CI/CD label May 11, 2026

bdero added the CICD Run CI/CD label May 11, 2026

bdero requested a review from gaaclarke May 11, 2026 23:17

gaaclarke approved these changes May 12, 2026

View reviewed changes

bdero added the autosubmit Merge PR when tree becomes green via auto submit App label May 12, 2026

auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label May 12, 2026

bdero added the autosubmit Merge PR when tree becomes green via auto submit App label May 12, 2026

bdero added this pull request to the merge queue May 12, 2026

Merged via the queue into flutter:master with commit 2fe6ec5 May 12, 2026
208 checks passed

bdero deleted the bdero/impeller-gles-mip-allocation branch May 12, 2026 03:31

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

Piinks mentioned this pull request May 15, 2026

Flutter Notable Commits - 2026 #181433

Open

Conversation

bdero commented May 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Approach

Behavior table

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!

This comment was marked as outdated.

gaaclarke left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

auto-submit Bot commented May 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

bdero commented May 10, 2026 •

edited

Loading