Skip to content

Conversation

@flar
Copy link
Contributor

@flar flar commented Aug 20, 2025

While doing some code surveys to see what surface/context files are being used, I discovered that these source files defined classes that were not used anywhere, so deleting them as dead code.

@github-actions github-actions bot added the engine flutter/engine related. See also e: labels. label Aug 20, 2025
Copy link
Contributor

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

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 correctly removes the obsolete vulkan_window source files and their references in the build system and other headers. The change is clean and follows the stated purpose. I've found one minor issue in the updated documentation example where the hash value for the new example file seems to be outdated. Please see the specific comment for details. Otherwise, the changes look good.

Comment on lines 26 to 30
file_name="engine/src/flutter/display_list/display_list.h"; (printf "blob $(wc -c < "$file_name" | awk '{print $1}')\0"; cat "$file_name") | sha1sum
11a5a03d15ae21bde366e41291a7899eec44e5ae -

git ls-tree -r HEAD engine/src/flutter/vulkan/vulkan_window.h
100644 blob 11a5a03d15ae21bde366e41291a7899eec44e5ae engine/src/flutter/vulkan/vulkan_window.h
git ls-tree -r HEAD engine/src/flutter/display_list/display_list.h
100644 blob 11a5a03d15ae21bde366e41291a7899eec44e5ae engine/src/flutter/display_list/display_list.h
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The hash value 11a5a03d15ae21bde366e41291a7899eec44e5ae appears to be the hash of the deleted file vulkan_window.h. To keep the example in this documentation accurate, please update the hash to match the new file engine/src/flutter/display_list/display_list.h. You can regenerate it using the command on line 26.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch!

@flar
Copy link
Contributor Author

flar commented Aug 20, 2025

I'm still finding more potentially obsolete code in the vulkan directory. Some of it is referenced outside that directory, but by code that is, itself, also dead. Working my way through the spaghetti...

Copy link
Member

@chinmaygarde chinmaygarde left a comment

Choose a reason for hiding this comment

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

Another option is to just move the entirety of flutter/vulkan into the Fuchsia runner since that is the only user of this stuff subsystem.

@flar
Copy link
Contributor Author

flar commented Aug 25, 2025

While fuchsia is a major user of the other //vulkan files, they're also used in a bunch of test code. I started tracing it out and gave up. Is it worth getting rid of just these couple of files? I didn't find any others that wouldn't cause a huge yarn-pull through various parts of the engine.

@flar flar added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 25, 2025
@auto-submit auto-submit bot added this pull request to the merge queue Aug 26, 2025
Merged via the queue into flutter:master with commit 22a5223 Aug 26, 2025
186 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Aug 26, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 26, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 26, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 26, 2025
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Aug 26, 2025
flutter/flutter@a4cb00a...c65f01d

2025-08-26 [email protected] Roll Packages from fe66130 to 1ef712e (4 revisions) (flutter/flutter#174442)
2025-08-26 [email protected] Revert "Directly generate a Mach-O dynamic library using gen_snapshot (#171626) (flutter/flutter#174392)
2025-08-26 49699333+dependabot[bot]@users.noreply.github.com Bump codecov/codecov-action from 5.4.0 to 5.5.0 in the all-github-actions group (flutter/flutter#174436)
2025-08-26 [email protected] Roll Skia from 9daab16abbf9 to 21214d63fc40 (1 revision) (flutter/flutter#174431)
2025-08-26 [email protected] Roll Fuchsia Linux SDK from UiY8gj468PZUj6QTm... to L5zGzsIWIS8N36AFQ... (flutter/flutter#174430)
2025-08-26 [email protected] Roll Dart SDK from f1f90d413dd3 to 9054cd8af73c (2 revisions) (flutter/flutter#174428)
2025-08-26 [email protected] Roll Skia from afb5c22d9ba0 to 9daab16abbf9 (2 revisions) (flutter/flutter#174429)
2025-08-26 [email protected] Roll Skia from 2227187dbdcf to afb5c22d9ba0 (1 revision) (flutter/flutter#174425)
2025-08-26 [email protected] [iOS][Secure Paste] Custom edit menu actions (flutter/flutter#171825)
2025-08-26 [email protected] Make SystemUiOverlayStyle to be diagnosticable (flutter/flutter#174018)
2025-08-26 [email protected] Fix: Active step fully colored in vertical mode (flutter/flutter#173152)
2025-08-26 [email protected] Migrate to use `WidgetStateProperty` (flutter/flutter#174323)
2025-08-26 [email protected] Roll Skia from ed42a94ee066 to 2227187dbdcf (3 revisions) (flutter/flutter#174417)
2025-08-26 [email protected] Roll Dart SDK from a0e39d9b4a58 to f1f90d413dd3 (1 revision) (flutter/flutter#174409)
2025-08-26 [email protected] [Impeller] Flush the data written to the device buffer by RoundSuperellipseGeometry (flutter/flutter#174316)
2025-08-26 [email protected] Remove obsolete vulkan_window source files (flutter/flutter#174087)
2025-08-25 [email protected] [web] Migrate non-CanvasKit-specific tests to ui/ (flutter/flutter#174396)
2025-08-25 [email protected] Create Hot Restart over websocket test (flutter/flutter#173852)
2025-08-25 [email protected] Roll Dart SDK from e283a9e88242 to a0e39d9b4a58 (1 revision) (flutter/flutter#174383)
2025-08-25 [email protected] Update `master` CHANGELOG for 3.35.2 (flutter/flutter#174399)
2025-08-25 [email protected] Roll Skia from da724d312e65 to ed42a94ee066 (4 revisions) (flutter/flutter#174394)
2025-08-25 [email protected] Update dwds to 25.0.3 (flutter/flutter#174379)
2025-08-25 [email protected] Fix logic statements in year2023 documentation (flutter/flutter#174120)
2025-08-25 [email protected] Release thread-local resources when submitting a Flutter GPU command buffer (flutter/flutter#173663)
2025-08-25 [email protected] [web] Refactor LayerScene out of CanvasKit (flutter/flutter#174375)
2025-08-25 [email protected] Stream logs from `devicectl` and `lldb` (flutter/flutter#173724)
2025-08-25 [email protected] NavigatorPopScope examples no longer use deprecated onPop. (flutter/flutter#174291)
2025-08-25 [email protected] fix typo in test documentation function name (flutter/flutter#174297)

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 [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
mboetger pushed a commit to mboetger/flutter that referenced this pull request Sep 18, 2025
While doing some code surveys to see what surface/context files are
being used, I discovered that these source files defined classes that
were not used anywhere, so deleting them as dead code.
korca0220 pushed a commit to korca0220/flutter that referenced this pull request Sep 22, 2025
While doing some code surveys to see what surface/context files are
being used, I discovered that these source files defined classes that
were not used anywhere, so deleting them as dead code.
Jaineel-Mamtora pushed a commit to Jaineel-Mamtora/flutter_forked that referenced this pull request Sep 24, 2025
While doing some code surveys to see what surface/context files are
being used, I discovered that these source files defined classes that
were not used anywhere, so deleting them as dead code.
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 12, 2025
lucaantonelli pushed a commit to lucaantonelli/flutter that referenced this pull request Nov 21, 2025
While doing some code surveys to see what surface/context files are
being used, I discovered that these source files defined classes that
were not used anywhere, so deleting them as dead code.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

engine flutter/engine related. See also e: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants