[Flutter GPU] Allow attaching specific texture mip levels and slices for rendering#187685
Conversation
…for rendering Surfaces the Impeller HAL support for rendering into a specific texture mip level and slice to Flutter GPU. - Adds mipLevel and slice (default 0) to ColorAttachment and DepthStencilAttachment, selecting the subresource to render into. - Validates the ranges in Dart against the texture's mipLevelCount and sliceCount. - Adds GpuContext.doesSupportFramebufferRenderMipmap, lifting the GLES capability onto the base Capabilities interface. Metal and Vulkan always support it. Fixes flutter#150455
There was a problem hiding this comment.
Code Review
This pull request adds support for rendering into specific texture mipmap levels and slices within Flutter GPU. It introduces the SupportsFramebufferRenderMipmap capability check across GLES, Vulkan, and standard backends, exposes it to Dart via GpuContext.doesSupportFramebufferRenderMipmap, and updates ColorAttachment and DepthStencilAttachment to accept and validate mipLevel and slice parameters. The review feedback identifies compilation errors in the new tests due to the use of the undefined Colors.lime property, suggesting Colors.green as a replacement.
Move the mip/slice range checks into RenderTarget._validateAttachments and add a cross-attachment size consistency check. These guard against undefined behavior in release builds, where the engine-side checks are compiled out, so they run unconditionally rather than under an assert. Also validates the MSAA resolve texture's mip level and slice.
There was a problem hiding this comment.
Code Review
This pull request adds support for rendering into specific texture slices and non-zero mip levels in Flutter GPU. It updates backend capabilities, exposes the new properties through the C++ and Dart APIs, and implements attachment validation in RenderTarget. Feedback suggests refactoring the validation logic in Dart to avoid allocating records/tuples, thereby reducing garbage collection pressure during render pass creation.
Replace the (int, int) record in _validateAttachments with plain int width/height locals and drop the _attachmentMipSize helper, avoiding per-call allocations in this unconditionally-run path.
gaaclarke
left a comment
There was a problem hiding this comment.
Some minor grievances, otherwise LGTM!
| // Rendering into a non-zero mip level needs ES 3.0 or | ||
| // GL_OES_fbo_render_mipmap on the GLES backend. | ||
| if (!gpu.gpuContext.doesSupportFramebufferRenderMipmap) { | ||
| return; |
There was a problem hiding this comment.
you want to skip, not return in this case
| } | ||
| if (slice < 0 || slice >= texture.sliceCount) { | ||
| throw Exception( | ||
| "$label slice ($slice) must be in the range [0, ${texture.sliceCount}) for textures of type ${texture.textureType}", |
There was a problem hiding this comment.
'[0, ${texture.sliceCount-1}]' is my clear imo
There was a problem hiding this comment.
These match the half-open [0, N) wording already used by Texture.overwrite's mip/slice validation, so I kept the two consistent.
There was a problem hiding this comment.
On second thought, I agree and we should just use the simpler notation everywhere.
| } | ||
| }, skip: !(impellerEnabled && flutterGpuEnabled)); | ||
|
|
||
| test('ColorAttachment throws for an out-of-range slice', () async { |
There was a problem hiding this comment.
Thanks for adding these. I think it important we try to be a positive experience to work with despite (or especially because) it's a low level concept.
| // |Capabilities| | ||
| /// Core ES 2.0 only allows mip level 0; ES 3.0+ and the | ||
| /// GL_OES_fbo_render_mipmap extension lift that restriction. | ||
| bool SupportsFramebufferRenderMipmap() const override; |
There was a problem hiding this comment.
nit: Instead of having a virtual function, shouldn't we just have a protected bool we can set? I don't think we need to have custom logic that gets executed each time. We are just going to calculate it once. Virtual functions have higher overhead and worse locality.
If we are calling it only ever from Dart it's not a big deal. I didn't see us using it elsewhere.
There was a problem hiding this comment.
This follows the existing Capabilities interface, where every capability is a virtual getter. GLES already caches it in supports_fbo_render_mipmap_ (computed once) and Metal/Vulkan just return true, and it's only queried from Dart at render-pass setup, so the virtual isn't on a hot path.
There was a problem hiding this comment.
I do think this whole capabilities mechanism aught to just be simplified. We really don't need the dynamic dispatch at all. Added a cleanup issue to track here: #187704
When the backend cannot render into non-zero mip levels, report the test as skipped via markTestSkipped rather than silently passing.
Switch the attachment and Texture.overwrite validation messages from half-open [0, N) to inclusive [0, N-1], which states the maximum valid value directly. Keeps the two validation paths consistent.
Rendering into a non-zero mip level aborts on the GLES backend with an incomplete framebuffer: the texture storage path uses mutable, lazily allocated glTexImage2D levels, which are not framebuffer-attachment complete for a non-base level. This was never exercised before (the renderer test skips GLES), and surfaced when the Flutter GPU tests ran it on the ES 3.0 (SwiftShader) backend. Make CapabilitiesGLES::SupportsFramebufferRenderMipmap return false so the path is rejected gracefully and the capability is honest. Cube map face (slice) rendering is unaffected. Metal and Vulkan are unchanged.
|
CI surfaced that rendering into a non-zero mip level aborts on the GLES backend with an incomplete framebuffer, since Impeller's GLES texture storage isn't framebuffer-complete for a non-base mip level and it appears unsupported on some drivers... I've disabled |
flutter/flutter@66aaa9a...c0a1129 2026-06-10 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#187740) 2026-06-09 burak.karahan@mail.ru Remove Material import from view chrome style test (flutter/flutter#186994) 2026-06-09 jason-simmons@users.noreply.github.com [Impeller] Remove unused DeviceHolderVK reference from CommandBufferVK (flutter/flutter#187705) 2026-06-09 engine-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from KNe93cf5wU4xG2d-m... to 8azSyvz57mKcPqTwk... (flutter/flutter#187745) 2026-06-09 1063596+reidbaker@users.noreply.github.com Add android-agent agent.json and update reidbaker-agent skills (flutter/flutter#187746) 2026-06-09 engine-flutter-autoroll@skia.org Roll Skia from aeed11c35004 to 9f02102df298 (9 revisions) (flutter/flutter#187744) 2026-06-09 bdero@google.com [Impeller] Remove the texture coordinate system Y-flip workaround (flutter/flutter#187686) 2026-06-09 41687333+rlueders@users.noreply.github.com [Impeller] Retry uncompressed when fixed-rate compression is exhausted (flutter/flutter#187586) 2026-06-09 burak.karahan@mail.ru Remove Material import from implicit animation tests (flutter/flutter#186673) 2026-06-09 engine-flutter-autoroll@skia.org Roll Packages from 13b49f4 to bd297cf (4 revisions) (flutter/flutter#187739) 2026-06-09 30870216+gaaclarke@users.noreply.github.com Updates dia_dll.py to support vs2026 (flutter/flutter#187714) 2026-06-09 bdero@google.com [Flutter GPU] Allow attaching specific texture mip levels and slices for rendering (flutter/flutter#187685) 2026-06-09 engine-flutter-autoroll@skia.org Roll Dart SDK from 39f1c44e294f to f3441f2067ae (1 revision) (flutter/flutter#187711) 2026-06-09 bdero@google.com [flutter_tools] Hot reload Flutter GPU shader bundles (flutter/flutter#187654) 2026-06-09 katelovett@google.com Update triage links (flutter/flutter#187709) 2026-06-09 engine-flutter-autoroll@skia.org Roll Skia from 43f135735152 to aeed11c35004 (11 revisions) (flutter/flutter#187721) 2026-06-09 jason-simmons@users.noreply.github.com Use workspace resolution for the meta package in dev/integration_tests/record_use_test_package (flutter/flutter#187733) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC louisehsu@google.com,stuartmorgan@google.com 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
The package only runs on the Flutter master channel, but pub.dev scores packages by analyzing them on stable, so the flutter constraint has to be satisfiable by stable to keep the pana score. Raise the lower bound from the stale 3.29.0-1.0.pre.242 to the current latest stable 3.44.0, and document the real requirement (a master build from 2026-06-09, which added render-to-mip-level Flutter GPU support in flutter/flutter#187685) in the README, the pubspec comment, and the CHANGELOG.
…for rendering (flutter#187685) Fixes flutter#150455 Surfaces the Impeller HAL support for rendering into a specific texture mip level and slice (landed in flutter#187470) to Flutter GPU. - Adds `mipLevel` and `slice` (both default 0) to `ColorAttachment` and `DepthStencilAttachment`. They select the subresource of the attachment texture to render into: a non-zero mip level, a cube map face, or both. - Validates, in Dart and unconditionally, that each attachment's `mipLevel` and `slice` are in range (including the MSAA resolve texture) and that all attachments resolve to the same size. Out-of-range subresources and size mismatches are undefined behavior in release, where the engine-side checks are compiled out. - Adds `GpuContext.doesSupportFramebufferRenderMipmap`, lifted onto the base `Capabilities` interface so it can be queried. Rendering into a cube face is always available. Rendering into a non-zero mip level returns true on Metal and Vulkan and false on GLES: the GLES texture storage path yields an incomplete framebuffer for a non-base mip attachment, so the capability is conservative until that is reworked (tracked separately). Cube face rendering on GLES is unaffected. Tests cover rendering into a cube slice, rendering into a non-zero mip level (gated on the capability), and the range and size validation. ## 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. - [ ] All existing and new tests are passing. <!-- Links --> [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/ [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
Fixes #150455
Surfaces the Impeller HAL support for rendering into a specific texture mip level and slice (landed in #187470) to Flutter GPU.
mipLevelandslice(both default 0) toColorAttachmentandDepthStencilAttachment. They select the subresource of the attachment texture to render into: a non-zero mip level, a cube map face, or both.mipLevelandsliceare in range (including the MSAA resolve texture) and that all attachments resolve to the same size. Out-of-range subresources and size mismatches are undefined behavior in release, where the engine-side checks are compiled out.GpuContext.doesSupportFramebufferRenderMipmap, lifted onto the baseCapabilitiesinterface so it can be queried. Rendering into a cube face is always available. Rendering into a non-zero mip level returns true on Metal and Vulkan and false on GLES: the GLES texture storage path yields an incomplete framebuffer for a non-base mip attachment, so the capability is conservative until that is reworked (tracked separately). Cube face rendering on GLES is unaffected.Tests cover rendering into a cube slice, rendering into a non-zero mip level (gated on the capability), and the range and size validation.
Pre-launch Checklist
///).