Skip to content

[ImpellerC] Write a depfile when --shader-bundle is in use#186341

Merged
bdero merged 2 commits into
flutter:masterfrom
bdero:bdero/impellerc-shaderbundle-depfile
May 12, 2026
Merged

[ImpellerC] Write a depfile when --shader-bundle is in use#186341
bdero merged 2 commits into
flutter:masterfrom
bdero:bdero/impellerc-shaderbundle-depfile

Conversation

@bdero

@bdero bdero commented May 11, 2026

Copy link
Copy Markdown
Member

Fixes #186340

When impellerc is invoked with both --depfile=<path> and --shader-bundle=<json>, the depfile is silently dropped. impellerc_main.cc takes an early return into GenerateShaderBundle(switches) when --shader-bundle is set, and the depfile-writing branch only runs on the non-bundle path. Build systems consuming impellerc for shader bundles (notably Dart's hooks framework, which flutter_gpu_shaders's buildShaderBundleJson goes through) have no way to discover which source files and transitive #include headers contributed to the produced bundle, and therefore cannot rerun the bundle build when any input changes. The user-visible effect is that editing a .frag referenced by a shader bundle manifest does not invalidate the cached bundle output until flutter clean is run, which makes shader iteration painful and confuses new users.

This change threads an optional std::set<std::string>* out_dependencies parameter through GenerateShaderBundleFlatbuffer, GenerateShaderFB, and GenerateShaderBackendFB. Each Compiler instance reports its GetIncludedFileNames() plus the primary shader source file into the set, which dedupes naturally across the five target-platform compiles per shader. After successful bundle generation, GenerateShaderBundle emits a Ninja-style depfile when --depfile is set, mirroring the format produced by Compiler::CreateDepfileContents on the single-shader compile path.

Manually verified end to end against a real bundle whose only shader transitively #includes a header: the resulting depfile lists both files. The new unit tests cover both the explicit-collector path (set contains the primary source files) and the null-collector path (callers that don't pass a set get exactly today's behaviour). Existing shader-bundle unit tests continue to pass unchanged.

Pre-launch Checklist

Threads a dependency-collecting set through GenerateShaderBundleFlatbuffer,
GenerateShaderFB, and GenerateShaderBackendFB. Each Compiler reports its
included file names plus the primary shader source file into the set,
which dedupes naturally across the five target-platform compiles per
shader. After successful bundle generation, GenerateShaderBundle emits a
Ninja-style depfile when --depfile is set, mirroring the format produced
by Compiler::CreateDepfileContents on the single-shader compile path.

Previously, --depfile was silently dropped in --shader-bundle mode
because the early return into GenerateShaderBundle bypassed OutputDepfile.
Build systems consuming impellerc for shader bundles (notably Dart's
hooks framework, which flutter_gpu_shaders goes through) had no way to
discover which source files and transitive #include headers contributed
to the bundle, and therefore couldn't rerun the bundle build when any
input changed.

Tests confirm the dependency set contains the primary source files. Both
the explicit-collector and null-collector paths are exercised. Existing
shader bundle tests continue to pass unchanged.

Fixes flutter#186340
@flutter-dashboard flutter-dashboard Bot added the CICD Run CI/CD label May 11, 2026
@github-actions github-actions Bot added engine flutter/engine related. See also e: labels. e: impeller Impeller rendering backend issues and features requests labels May 11, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request implements Ninja-style depfile generation for shader bundles by tracking source files and transitive includes during compilation. It updates the compilation pipeline to collect these dependencies, adds a utility to write them to disk, and includes unit tests to verify the new functionality. Review feedback recommends resolving paths relative to the specified working directory, simplifying string conversions in file operations, and using UTF-8 encoding for target paths to ensure cross-platform compatibility.

Comment thread engine/src/flutter/impeller/compiler/shader_bundle.cc Outdated
Comment thread engine/src/flutter/impeller/compiler/shader_bundle.cc Outdated
Comment thread engine/src/flutter/impeller/compiler/shader_bundle.cc Outdated
@github-actions github-actions Bot removed the CICD Run CI/CD label May 11, 2026
@github-project-automation github-project-automation Bot moved this to 🤔 Needs Triage in Flutter GPU May 11, 2026
@bdero bdero changed the title [Impeller] Write a depfile when --shader-bundle is in use [ImpellerC] Write a depfile when --shader-bundle is in use May 11, 2026
@bdero bdero moved this from 🤔 Needs Triage to ⚙️ In Progress in Flutter GPU May 11, 2026
@bdero bdero added the CICD Run CI/CD label May 11, 2026
@bdero
bdero requested a review from gaaclarke May 11, 2026 09:38

@gaaclarke gaaclarke left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks!

auto-submit Bot pushed a commit to flutter/packages that referenced this pull request May 14, 2026
Roll Flutter from 707dbc0420a3 to 23f6f5853f50 (149 revisions)

flutter/flutter@707dbc0...23f6f58

2026-05-12 [email protected] Add 'cp: review' label to the manual cherrypick process (flutter/flutter#186158)
2026-05-12 [email protected] Roll Packages from 19ec8b8 to 93cbed6 (3 revisions) (flutter/flutter#186401)
2026-05-12 [email protected] Removes SDF option for macOS (always enabled) (flutter/flutter#186265)
2026-05-12 [email protected] docs: fix typos in flutter_tools comments (flutter/flutter#186321)
2026-05-12 [email protected] Pass XcodeBasedProject instead of String to functions in XcodeProjectInterpreter (flutter/flutter#186378)
2026-05-12 [email protected] Update iOS scenario app test goldens to match changes from flutter/flutter#182662 (flutter/flutter#186390)
2026-05-12 [email protected] Roll Skia from ad0aff15b9fa to 77a21bc723dc (2 revisions) (flutter/flutter#186396)
2026-05-12 [email protected] Migrate focus_node.unfocus.0.dart to use `RadioGroup` (flutter/flutter#183979)
2026-05-12 [email protected] Roll Skia from 91d3c1e730af to ad0aff15b9fa (7 revisions) (flutter/flutter#186391)
2026-05-12 [email protected] [Flutter GPU] Allow customizing the vertex layout on a RenderPipeline (flutter/flutter#186310)
2026-05-12 [email protected] Fix `EmbedderTest.CanRenderTextWithImpellerMetal` test breakage (flutter/flutter#186262)
2026-05-12 [email protected] Roll Fuchsia Linux SDK from rFhU-YPqdCRCtCz7b... to z7ICmPtn4hspu02zk... (flutter/flutter#186384)
2026-05-12 [email protected] [Impeller] GLES: lazily allocate texture mip levels on first per-level write (flutter/flutter#186302)
2026-05-12 [email protected] [Android] Propagate --enable-flutter-gpu Intent extra to engine args (flutter/flutter#186298)
2026-05-11 [email protected] [ci] update no-response workflow to also look for old label name in e… (flutter/flutter#186373)
2026-05-11 [email protected] [ImpellerC] Write a depfile when --shader-bundle is in use (flutter/flutter#186341)
2026-05-11 [email protected] docs: fix doubled-word typos in comments (flutter/flutter#186320)
2026-05-11 [email protected] Roll Skia from 32281401997e to 91d3c1e730af (4 revisions) (flutter/flutter#186368)
2026-05-11 [email protected] Show SwiftPM warnings right before iOS/macOS build (flutter/flutter#185984)
2026-05-11 [email protected] Convert rebuilding-flutter-tool script to dart (flutter/flutter#185089)
2026-05-11 [email protected] Use Xcode's LLDB (flutter/flutter#186273)
2026-05-11 [email protected] Remove `currentMainUri` from `generateMainDartWithPluginRegistrant` (flutter/flutter#185907)
2026-05-11 [email protected] Roll Skia from 2514f6b5f92b to 32281401997e (1 revision) (flutter/flutter#186349)
2026-05-11 [email protected] Roll Packages from 92552b1 to 19ec8b8 (4 revisions) (flutter/flutter#186350)
2026-05-11 [email protected] Check for absolute paths in skills.  (flutter/flutter#185632)
2026-05-11 [email protected] Roll Skia from 9fb7d2814642 to 2514f6b5f92b (1 revision) (flutter/flutter#186347)
2026-05-11 [email protected] Roll Skia from 8cafb209e836 to 9fb7d2814642 (4 revisions) (flutter/flutter#186335)
2026-05-10 [email protected] Roll Fuchsia Linux SDK from sOBiPJb0xznDBZlf5... to rFhU-YPqdCRCtCz7b... (flutter/flutter#186328)
2026-05-10 [email protected] Roll Skia from 05a03f99c74e to 8cafb209e836 (1 revision) (flutter/flutter#186315)
2026-05-10 [email protected] [Impeller] Vulkan: don't drop user-supplied viewport X, Y, and depth range (flutter/flutter#185886)
2026-05-09 [email protected] Update Fuchsia tests to subpackage their child components (flutter/flutter#186259)
2026-05-09 [email protected] Fix SelectableText crash with inline lambda contextMenuBuilder (flutter/flutter#184990)
2026-05-09 [email protected] Roll Fuchsia Linux SDK from 5_TnhTsHSqtCx37o6... to sOBiPJb0xznDBZlf5... (flutter/flutter#186289)
2026-05-09 [email protected] Roll Skia from dc78d4bd2efb to 05a03f99c74e (2 revisions) (flutter/flutter#186283)
2026-05-09 [email protected] Improve non rect platform view rendering  (flutter/flutter#182662)
2026-05-08 [email protected] Roll Skia from 31521f8508c7 to dc78d4bd2efb (1 revision) (flutter/flutter#186278)
2026-05-08 [email protected] 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 [email protected] Roll Skia from a00db8749edb to 31521f8508c7 (2 revisions) (flutter/flutter#186264)
2026-05-08 [email protected] Optimize compatible `DrawDiffRoundRect` calls to use `DrawRoundRect` (flutter/flutter#186203)
2026-05-08 [email protected] [triage] Add Flutter GPU as a triage team (flutter/flutter#186263)
2026-05-08 [email protected] doc: Unified Check-Run User manual (flutter/flutter#186210)
2026-05-08 [email protected] Roll Skia from 5f7adf4403d6 to a00db8749edb (1 revision) (flutter/flutter#186257)
2026-05-08 [email protected] Roll Packages from 0411f1d to 92552b1 (1 revision) (flutter/flutter#186256)
2026-05-08 [email protected] Add logging to figure out jvm crashes for `hot_mode_tests` (flutter/flutter#186107)
2026-05-08 [email protected] Roll Skia from 926c09741ce2 to 5f7adf4403d6 (3 revisions) (flutter/flutter#186242)
...
stuartmorgan-g pushed a commit to flutter/core-packages that referenced this pull request Jun 9, 2026
Roll Flutter from 707dbc0420a3 to 23f6f5853f50 (149 revisions)

flutter/flutter@707dbc0...23f6f58

2026-05-12 [email protected] Add 'cp: review' label to the manual cherrypick process (flutter/flutter#186158)
2026-05-12 [email protected] Roll Packages from 19ec8b861676 to 93cbed65f336 (3 revisions) (flutter/flutter#186401)
2026-05-12 [email protected] Removes SDF option for macOS (always enabled) (flutter/flutter#186265)
2026-05-12 [email protected] docs: fix typos in flutter_tools comments (flutter/flutter#186321)
2026-05-12 [email protected] Pass XcodeBasedProject instead of String to functions in XcodeProjectInterpreter (flutter/flutter#186378)
2026-05-12 [email protected] Update iOS scenario app test goldens to match changes from flutter/flutter#182662 (flutter/flutter#186390)
2026-05-12 [email protected] Roll Skia from ad0aff15b9fa to 77a21bc723dc (2 revisions) (flutter/flutter#186396)
2026-05-12 [email protected] Migrate focus_node.unfocus.0.dart to use `RadioGroup` (flutter/flutter#183979)
2026-05-12 [email protected] Roll Skia from 91d3c1e730af to ad0aff15b9fa (7 revisions) (flutter/flutter#186391)
2026-05-12 [email protected] [Flutter GPU] Allow customizing the vertex layout on a RenderPipeline (flutter/flutter#186310)
2026-05-12 [email protected] Fix `EmbedderTest.CanRenderTextWithImpellerMetal` test breakage (flutter/flutter#186262)
2026-05-12 [email protected] Roll Fuchsia Linux SDK from rFhU-YPqdCRCtCz7b... to z7ICmPtn4hspu02zk... (flutter/flutter#186384)
2026-05-12 [email protected] [Impeller] GLES: lazily allocate texture mip levels on first per-level write (flutter/flutter#186302)
2026-05-12 [email protected] [Android] Propagate --enable-flutter-gpu Intent extra to engine args (flutter/flutter#186298)
2026-05-11 [email protected] [ci] update no-response workflow to also look for old label name in e… (flutter/flutter#186373)
2026-05-11 [email protected] [ImpellerC] Write a depfile when --shader-bundle is in use (flutter/flutter#186341)
2026-05-11 [email protected] docs: fix doubled-word typos in comments (flutter/flutter#186320)
2026-05-11 [email protected] Roll Skia from 32281401997e to 91d3c1e730af (4 revisions) (flutter/flutter#186368)
2026-05-11 [email protected] Show SwiftPM warnings right before iOS/macOS build (flutter/flutter#185984)
2026-05-11 [email protected] Convert rebuilding-flutter-tool script to dart (flutter/flutter#185089)
2026-05-11 [email protected] Use Xcode's LLDB (flutter/flutter#186273)
2026-05-11 [email protected] Remove `currentMainUri` from `generateMainDartWithPluginRegistrant` (flutter/flutter#185907)
2026-05-11 [email protected] Roll Skia from 2514f6b5f92b to 32281401997e (1 revision) (flutter/flutter#186349)
2026-05-11 [email protected] Roll Packages from 92552b16bcc1 to 19ec8b861676 (4 revisions) (flutter/flutter#186350)
2026-05-11 [email protected] Check for absolute paths in skills.  (flutter/flutter#185632)
2026-05-11 [email protected] Roll Skia from 9fb7d2814642 to 2514f6b5f92b (1 revision) (flutter/flutter#186347)
2026-05-11 [email protected] Roll Skia from 8cafb209e836 to 9fb7d2814642 (4 revisions) (flutter/flutter#186335)
2026-05-10 [email protected] Roll Fuchsia Linux SDK from sOBiPJb0xznDBZlf5... to rFhU-YPqdCRCtCz7b... (flutter/flutter#186328)
2026-05-10 [email protected] Roll Skia from 05a03f99c74e to 8cafb209e836 (1 revision) (flutter/flutter#186315)
2026-05-10 [email protected] [Impeller] Vulkan: don't drop user-supplied viewport X, Y, and depth range (flutter/flutter#185886)
2026-05-09 [email protected] Update Fuchsia tests to subpackage their child components (flutter/flutter#186259)
2026-05-09 [email protected] Fix SelectableText crash with inline lambda contextMenuBuilder (flutter/flutter#184990)
2026-05-09 [email protected] Roll Fuchsia Linux SDK from 5_TnhTsHSqtCx37o6... to sOBiPJb0xznDBZlf5... (flutter/flutter#186289)
2026-05-09 [email protected] Roll Skia from dc78d4bd2efb to 05a03f99c74e (2 revisions) (flutter/flutter#186283)
2026-05-09 [email protected] Improve non rect platform view rendering  (flutter/flutter#182662)
2026-05-08 [email protected] Roll Skia from 31521f8508c7 to dc78d4bd2efb (1 revision) (flutter/flutter#186278)
2026-05-08 [email protected] 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 [email protected] Roll Skia from a00db8749edb to 31521f8508c7 (2 revisions) (flutter/flutter#186264)
2026-05-08 [email protected] Optimize compatible `DrawDiffRoundRect` calls to use `DrawRoundRect` (flutter/flutter#186203)
2026-05-08 [email protected] [triage] Add Flutter GPU as a triage team (flutter/flutter#186263)
2026-05-08 [email protected] doc: Unified Check-Run User manual (flutter/flutter#186210)
2026-05-08 [email protected] Roll Skia from 5f7adf4403d6 to a00db8749edb (1 revision) (flutter/flutter#186257)
2026-05-08 [email protected] Roll Packages from cfdd1d3 to 92552b16bcc1 (1 revision) (flutter/flutter#186256)
2026-05-08 [email protected] Add logging to figure out jvm crashes for `hot_mode_tests` (flutter/flutter#186107)
2026-05-08 [email protected] Roll Skia from 926c09741ce2 to 5f7adf4403d6 (3 revisions) (flutter/flutter#186242)
...
creatorpiyush pushed a commit to creatorpiyush/packages that referenced this pull request Jun 10, 2026
…er#11700)

Roll Flutter from 707dbc0420a3 to 23f6f5853f50 (149 revisions)

flutter/flutter@707dbc0...23f6f58

2026-05-12 [email protected] Add 'cp: review' label to the manual cherrypick process (flutter/flutter#186158)
2026-05-12 [email protected] Roll Packages from 19ec8b8 to 93cbed6 (3 revisions) (flutter/flutter#186401)
2026-05-12 [email protected] Removes SDF option for macOS (always enabled) (flutter/flutter#186265)
2026-05-12 [email protected] docs: fix typos in flutter_tools comments (flutter/flutter#186321)
2026-05-12 [email protected] Pass XcodeBasedProject instead of String to functions in XcodeProjectInterpreter (flutter/flutter#186378)
2026-05-12 [email protected] Update iOS scenario app test goldens to match changes from flutter/flutter#182662 (flutter/flutter#186390)
2026-05-12 [email protected] Roll Skia from ad0aff15b9fa to 77a21bc723dc (2 revisions) (flutter/flutter#186396)
2026-05-12 [email protected] Migrate focus_node.unfocus.0.dart to use `RadioGroup` (flutter/flutter#183979)
2026-05-12 [email protected] Roll Skia from 91d3c1e730af to ad0aff15b9fa (7 revisions) (flutter/flutter#186391)
2026-05-12 [email protected] [Flutter GPU] Allow customizing the vertex layout on a RenderPipeline (flutter/flutter#186310)
2026-05-12 [email protected] Fix `EmbedderTest.CanRenderTextWithImpellerMetal` test breakage (flutter/flutter#186262)
2026-05-12 [email protected] Roll Fuchsia Linux SDK from rFhU-YPqdCRCtCz7b... to z7ICmPtn4hspu02zk... (flutter/flutter#186384)
2026-05-12 [email protected] [Impeller] GLES: lazily allocate texture mip levels on first per-level write (flutter/flutter#186302)
2026-05-12 [email protected] [Android] Propagate --enable-flutter-gpu Intent extra to engine args (flutter/flutter#186298)
2026-05-11 [email protected] [ci] update no-response workflow to also look for old label name in e… (flutter/flutter#186373)
2026-05-11 [email protected] [ImpellerC] Write a depfile when --shader-bundle is in use (flutter/flutter#186341)
2026-05-11 [email protected] docs: fix doubled-word typos in comments (flutter/flutter#186320)
2026-05-11 [email protected] Roll Skia from 32281401997e to 91d3c1e730af (4 revisions) (flutter/flutter#186368)
2026-05-11 [email protected] Show SwiftPM warnings right before iOS/macOS build (flutter/flutter#185984)
2026-05-11 [email protected] Convert rebuilding-flutter-tool script to dart (flutter/flutter#185089)
2026-05-11 [email protected] Use Xcode's LLDB (flutter/flutter#186273)
2026-05-11 [email protected] Remove `currentMainUri` from `generateMainDartWithPluginRegistrant` (flutter/flutter#185907)
2026-05-11 [email protected] Roll Skia from 2514f6b5f92b to 32281401997e (1 revision) (flutter/flutter#186349)
2026-05-11 [email protected] Roll Packages from 92552b1 to 19ec8b8 (4 revisions) (flutter/flutter#186350)
2026-05-11 [email protected] Check for absolute paths in skills.  (flutter/flutter#185632)
2026-05-11 [email protected] Roll Skia from 9fb7d2814642 to 2514f6b5f92b (1 revision) (flutter/flutter#186347)
2026-05-11 [email protected] Roll Skia from 8cafb209e836 to 9fb7d2814642 (4 revisions) (flutter/flutter#186335)
2026-05-10 [email protected] Roll Fuchsia Linux SDK from sOBiPJb0xznDBZlf5... to rFhU-YPqdCRCtCz7b... (flutter/flutter#186328)
2026-05-10 [email protected] Roll Skia from 05a03f99c74e to 8cafb209e836 (1 revision) (flutter/flutter#186315)
2026-05-10 [email protected] [Impeller] Vulkan: don't drop user-supplied viewport X, Y, and depth range (flutter/flutter#185886)
2026-05-09 [email protected] Update Fuchsia tests to subpackage their child components (flutter/flutter#186259)
2026-05-09 [email protected] Fix SelectableText crash with inline lambda contextMenuBuilder (flutter/flutter#184990)
2026-05-09 [email protected] Roll Fuchsia Linux SDK from 5_TnhTsHSqtCx37o6... to sOBiPJb0xznDBZlf5... (flutter/flutter#186289)
2026-05-09 [email protected] Roll Skia from dc78d4bd2efb to 05a03f99c74e (2 revisions) (flutter/flutter#186283)
2026-05-09 [email protected] Improve non rect platform view rendering  (flutter/flutter#182662)
2026-05-08 [email protected] Roll Skia from 31521f8508c7 to dc78d4bd2efb (1 revision) (flutter/flutter#186278)
2026-05-08 [email protected] 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 [email protected] Roll Skia from a00db8749edb to 31521f8508c7 (2 revisions) (flutter/flutter#186264)
2026-05-08 [email protected] Optimize compatible `DrawDiffRoundRect` calls to use `DrawRoundRect` (flutter/flutter#186203)
2026-05-08 [email protected] [triage] Add Flutter GPU as a triage team (flutter/flutter#186263)
2026-05-08 [email protected] doc: Unified Check-Run User manual (flutter/flutter#186210)
2026-05-08 [email protected] Roll Skia from 5f7adf4403d6 to a00db8749edb (1 revision) (flutter/flutter#186257)
2026-05-08 [email protected] Roll Packages from 0411f1d to 92552b1 (1 revision) (flutter/flutter#186256)
2026-05-08 [email protected] Add logging to figure out jvm crashes for `hot_mode_tests` (flutter/flutter#186107)
2026-05-08 [email protected] Roll Skia from 926c09741ce2 to 5f7adf4403d6 (3 revisions) (flutter/flutter#186242)
...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CICD Run CI/CD e: impeller Impeller rendering backend issues and features requests engine flutter/engine related. See also e: labels. flutter-gpu

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

[Impeller] impellerc --depfile is silently ignored in --shader-bundle mode

2 participants