Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[macOS] Clean up unused methods in FlutterRenderer #42196

Merged
merged 5 commits into from
May 21, 2023

Conversation

dkwingsmt
Copy link
Contributor

This PR removes the methods in FlutterRenderer that will never be used.

These methods allows the engine to get a drawable and present a drawable, but they will never be called on the macOS embedder. This is because they go through the FlutterMetalRendererConfig struct to GPUSurfaceMetalSkia, which is bypassed if the engine has a non-null external view embedder, which the macOS always provides.

Removing these methods not only cleans up the code (and the confusion), but also reduces the methods that might need to be migrated for the multi-view project.

After this change, FlutterRenderer is left with very few functionalities:

  • It hosts device and commandQueue.
  • It creates FlutterRendererConfig.
  • It implements FlutterTextureRegistry and FlutterTextureRegistrarDelegate, linking FlutterTexture and FlutterTextureRegistrar.

We might want to refactor this class, but that's for the future.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Copy link
Member

@cbracken cbracken left a comment

Choose a reason for hiding this comment

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

Nice. There's a bit of followup cleanup we should land. I'll send a patch.

zanderso pushed a commit to flutter/flutter that referenced this pull request May 22, 2023
…127325)

flutter/engine@f0f3fe7...e04c147

2023-05-22 [email protected] Roll Fuchsia Linux SDK from
NE_eTA29vOHN4goJL... to LrfeC0dLk8ToJVik5... (flutter/engine#42208)
2023-05-22 [email protected] Roll Fuchsia Mac SDK from
HPZKiLZlLCR6moOCE... to QAwORJOkyNl4J3x4Y... (flutter/engine#42207)
2023-05-22 [email protected] Roll Skia from 16c60e5bebfc to
ef226c5a7930 (1 revision) (flutter/engine#42206)
2023-05-22 [email protected] Roll Skia from 79088c6b7a33 to
16c60e5bebfc (1 revision) (flutter/engine#42205)
2023-05-22 [email protected] Roll Skia from 76303a5498e9 to
79088c6b7a33 (2 revisions) (flutter/engine#42204)
2023-05-22 [email protected] Roll Skia from 2d4ea9542e83 to
76303a5498e9 (1 revision) (flutter/engine#42203)
2023-05-22 [email protected] Roll Fuchsia Linux SDK from
88pzkUAkSKsJrNG38... to NE_eTA29vOHN4goJL... (flutter/engine#42202)
2023-05-21 [email protected] [macOS] Clean up unused
methods in FlutterRenderer (flutter/engine#42196)
2023-05-21 [email protected] Roll Fuchsia Mac SDK from
JU-dKW3CQIUzhbqWE... to HPZKiLZlLCR6moOCE... (flutter/engine#42201)
2023-05-21 [email protected] Roll Skia from 5b2005e47bf3 to
2d4ea9542e83 (1 revision) (flutter/engine#42200)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from 88pzkUAkSKsJ to LrfeC0dLk8To
  fuchsia/sdk/core/mac-amd64 from JU-dKW3CQIUz to QAwORJOkyNl4

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC [email protected],[email protected] on the revert to ensure that
a human
is aware of the problem.

To file a bug in Flutter:
https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
CaseyHillers pushed a commit to CaseyHillers/flutter that referenced this pull request May 24, 2023
…lutter#127325)

flutter/engine@f0f3fe7...e04c147

2023-05-22 [email protected] Roll Fuchsia Linux SDK from
NE_eTA29vOHN4goJL... to LrfeC0dLk8ToJVik5... (flutter/engine#42208)
2023-05-22 [email protected] Roll Fuchsia Mac SDK from
HPZKiLZlLCR6moOCE... to QAwORJOkyNl4J3x4Y... (flutter/engine#42207)
2023-05-22 [email protected] Roll Skia from 16c60e5bebfc to
ef226c5a7930 (1 revision) (flutter/engine#42206)
2023-05-22 [email protected] Roll Skia from 79088c6b7a33 to
16c60e5bebfc (1 revision) (flutter/engine#42205)
2023-05-22 [email protected] Roll Skia from 76303a5498e9 to
79088c6b7a33 (2 revisions) (flutter/engine#42204)
2023-05-22 [email protected] Roll Skia from 2d4ea9542e83 to
76303a5498e9 (1 revision) (flutter/engine#42203)
2023-05-22 [email protected] Roll Fuchsia Linux SDK from
88pzkUAkSKsJrNG38... to NE_eTA29vOHN4goJL... (flutter/engine#42202)
2023-05-21 [email protected] [macOS] Clean up unused
methods in FlutterRenderer (flutter/engine#42196)
2023-05-21 [email protected] Roll Fuchsia Mac SDK from
JU-dKW3CQIUzhbqWE... to HPZKiLZlLCR6moOCE... (flutter/engine#42201)
2023-05-21 [email protected] Roll Skia from 5b2005e47bf3 to
2d4ea9542e83 (1 revision) (flutter/engine#42200)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from 88pzkUAkSKsJ to LrfeC0dLk8To
  fuchsia/sdk/core/mac-amd64 from JU-dKW3CQIUz to QAwORJOkyNl4

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC [email protected],[email protected] on the revert to ensure that
a human
is aware of the problem.

To file a bug in Flutter:
https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants