Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Export GPU symbols for embedder #54662

Merged
merged 1 commit into from
Aug 22, 2024
Merged

Conversation

jwinarske
Copy link
Contributor

This PR exports GPU symbols for the embedder library.

List which issues are fixed by this PR. You must list at least one issue.
flutter/flutter#153196

If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.

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 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.

@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

@bdero
Copy link
Member

bdero commented Aug 20, 2024

Nice find.

@jwinarske jwinarske force-pushed the jw/gpu_symbols branch 2 times, most recently from f5f052b to 3593453 Compare August 20, 2024 23:00
@bdero
Copy link
Member

bdero commented Aug 20, 2024

Test-wise, I think adding something that forces one of the symbols to get called in the GLFW embedder smoketest would suffice.

Perhaps just:

diff --git a/examples/glfw/main.dart b/examples/glfw/main.dart
index 557d020a1d..97c81efc6f 100644
--- a/examples/glfw/main.dart
+++ b/examples/glfw/main.dart
@@ -2,11 +2,15 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.

+import '../../lib/gpu/lib/gpu.dart' as gpu;
 import 'package:flutter/material.dart';
 import 'package:flutter/foundation.dart'
     show debugDefaultTargetPlatformOverride;

 void main() {
+  // Ensure Flutter GPU symbols are available by forcing the GPU context to instantiate.
+  gpu.gpuContext;
+
   // This is a hack to make Flutter think you are running on Google Fuchsia,
   // otherwise you will get an error about running from an unsupported platform.
   debugDefaultTargetPlatformOverride = TargetPlatform.fuchsia;

That's enough to trigger a symbol call.

@bdero
Copy link
Member

bdero commented Aug 20, 2024

Ah actually, Impeller isn't enabled in that example, but we can still verify that the symbol is being called properly. Something like this:

diff --git a/examples/glfw/main.dart b/examples/glfw/main.dart
index 557d020a1d..ceb5eda137 100644
--- a/examples/glfw/main.dart
+++ b/examples/glfw/main.dart
@@ -2,11 +2,22 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.

+import '../../lib/gpu/lib/gpu.dart' as gpu;
 import 'package:flutter/material.dart';
 import 'package:flutter/foundation.dart'
     show debugDefaultTargetPlatformOverride;

 void main() {
+  // Ensure Flutter GPU symbols are available by forcing the GPU context to instantiate.
+  try {
+    // ignore: unnecessary_statements
+    gpu.gpuContext; // Force the context to instantiate.
+  } catch (e) {
+    // If impeller is not enabled, make sure the exception isn't about symbols missing.
+    assert(e.toString().contains(
+        'Flutter GPU requires the Impeller rendering backend to be enabled.'));
+  }
+
   // This is a hack to make Flutter think you are running on Google Fuchsia,
   // otherwise you will get an error about running from an unsupported platform.
   debugDefaultTargetPlatformOverride = TargetPlatform.fuchsia;

@jwinarske
Copy link
Contributor Author

@bdero makes sense

@jwinarske jwinarske force-pushed the jw/gpu_symbols branch 3 times, most recently from 8320ba3 to 9de0b70 Compare August 21, 2024 13:43
@jwinarske
Copy link
Contributor Author

The relative path is not working. It should be relative to the myapp/lib folder. I tried all variations, no dice.

Moved to import 'package:flutter_gpu/gpu.dart' as gpu; and added flutter pub add flutter_gpu --sdk=flutter to run.sh.

That part is fine, only the app hits an unrelated problem:

In order to run your application, type:

  $ cd myapp
  $ flutter run

Your application code is in myapp/lib/main.dart.

Resolving dependencies... 
Downloading packages... 
+ flutter_gpu 0.0.0 from sdk flutter
  material_color_utilities 0.11.1 (0.12.0 available)
Changed 1 dependency!
1 package has newer versions incompatible with dependency constraints.
Try `flutter pub outdated` for more information.
Resolving dependencies... 
Downloading packages... 
  material_color_utilities 0.11.1 (0.12.0 available)
Got dependencies!
1 package has newer versions incompatible with dependency constraints.
Try `flutter pub outdated` for more information.

lib/main.dart:1:1: Error: The specified language version is too high. The highest supported language version is 3.5.
// Copyright 2013 The Flutter Authors. All rights reserved.
^
../../../../../../../../flutter/packages/flutter/lib/src/semantics/semantics.dart:2925:7: Error: No named parameter with the name 'linkUrl'.
      linkUrl: data.linkUrl?.toString() ?? '',
      ^^^^^^^
Target kernel_snapshot_program failed: Exception

@jwinarske
Copy link
Contributor Author

@bdero
I just wrapped my due diligence; problem was on my end. We are good to go. Assert hits if symbols are not available. This is the happy path output:

Creating project myapp...
Resolving dependencies in `myapp`... 
Downloading packages... 
Got dependencies in `myapp`.
Wrote 18 files.

All done!
You can find general documentation for Flutter at: https://docs.flutter.dev/
Detailed API documentation is available at: https://api.flutter.dev/
If you prefer video documentation, consider: https://www.youtube.com/c/flutterdev

In order to run your application, type:

  $ cd myapp
  $ flutter run

Your application code is in myapp/lib/main.dart.

Resolving dependencies... 
Downloading packages... 
+ flutter_gpu 0.0.0 from sdk flutter
  material_color_utilities 0.11.1 (0.12.0 available)
Changed 1 dependency!
1 package has newer versions incompatible with dependency constraints.
Try `flutter pub outdated` for more information.
Resolving dependencies... 
Downloading packages... 
  material_color_utilities 0.11.1 (0.12.0 available)
Got dependencies!
1 package has newer versions incompatible with dependency constraints.
Try `flutter pub outdated` for more information.

/home/tcna/workspace-automation/app/engine/src/flutter/examples/glfw/debug
[ERROR:flutter/shell/platform/embedder/embedder_surface_gl_skia.cc(123)] Could not create a resource context for async texture uploads. Expect degraded performance. Set a valid make_resource_current callback on FlutterOpenGLRendererConfig.
[ERROR:flutter/shell/platform/embedder/embedder_surface_gl_skia.cc(123)] Could not create a resource context for async texture uploads. Expect degraded performance. Set a valid make_resource_current callback on FlutterOpenGLRendererConfig.
flutter: The Dart VM service is listening on http://127.0.0.1:34919/E_SicMZ-fEE=/

Copy link
Member

@bdero bdero left a comment

Choose a reason for hiding this comment

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

LGTM

@bdero bdero added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 22, 2024
@auto-submit auto-submit bot merged commit 85d4be0 into flutter:main Aug 22, 2024
30 checks passed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 22, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Aug 22, 2024
…153902)

flutter/engine@b94e009...85d4be0

2024-08-22 [email protected] Export GPU symbols for embedder (flutter/engine#54662)
2024-08-22 [email protected] [Impeller] Remove a log message in the Vulkan back end that is visible during engine startup (flutter/engine#54699)
2024-08-22 [email protected] Roll Skia from 3cd00377cefc to 34aa8ce13af6 (3 revisions) (flutter/engine#54698)
2024-08-21 [email protected] macOS: Make framework creation consistent with iOS (flutter/engine#54685)
2024-08-21 [email protected] Roll Skia from 69f4bd859025 to 3cd00377cefc (8 revisions) (flutter/engine#54693)
2024-08-21 [email protected] Roll Dart SDK from 060e40916514 to 025bf8d376d3 (1 revision) (flutter/engine#54692)
2024-08-21 [email protected] Split tests out of Linux Android artifact creation builds (flutter/engine#54683)
2024-08-21 [email protected] Roll Skia from 249d3f07c4d5 to 69f4bd859025 (5 revisions) (flutter/engine#54691)
2024-08-21 [email protected] [iOS] Tweak note about OpenGL support on mac in a user facing log. (flutter/engine#54690)
2024-08-21 [email protected] [Impeller] use blit pass to resize decoded images. (flutter/engine#54606)
2024-08-21 [email protected] Remove spammy warning message on `FlutterView` (flutter/engine#54686)
2024-08-21 [email protected] [Impeller] Perform integrity checks for Vulkan pipeline caches. (flutter/engine#54654)
2024-08-21 [email protected] docs: use test: all rather than editing .ci.yaml (flutter/engine#54667)
2024-08-21 [email protected] Reland "[DisplayList] Allow random access to ops through indexing" (flutter/engine#54676)
2024-08-21 [email protected] Roll Skia from 51ac9d93850c to 249d3f07c4d5 (2 revisions) (flutter/engine#54684)
2024-08-21 [email protected] iOS,macOS: Don't archive extra framework metadata (flutter/engine#54674)
2024-08-21 [email protected] Roll Dart SDK from 48f9b96d71e7 to 060e40916514 (1 revision) (flutter/engine#54682)
2024-08-21 [email protected] [web] annotate obscured text fields as passwords (flutter/engine#54664)
2024-08-21 [email protected] Roll Skia from c31e2ca59bd9 to 51ac9d93850c (2 revisions) (flutter/engine#54681)
2024-08-21 [email protected] [engine] reland weaken affinity of raster/ui to non-e core instead of only fast core (flutter/engine#54616)
2024-08-21 [email protected] Roll Skia from c00866df101a to c31e2ca59bd9 (2 revisions) (flutter/engine#54680)
2024-08-21 [email protected] Roll Skia from 39e5118034f4 to c00866df101a (1 revision) (flutter/engine#54678)
2024-08-21 [email protected] Roll Skia from 221ada80b174 to 39e5118034f4 (1 revision) (flutter/engine#54677)
2024-08-21 [email protected] Roll Skia from d576296091e0 to 221ada80b174 (2 revisions) (flutter/engine#54675)
2024-08-21 [email protected] Roll Dart SDK from 49f655b526c7 to 48f9b96d71e7 (1 revision) (flutter/engine#54672)
2024-08-21 [email protected] Roll Fuchsia Linux SDK from 3a16kOsyFmJh3lo7e... to XGzE3idakwfQZ68pb... (flutter/engine#54671)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from 3a16kOsyFmJh to XGzE3idakwfQ

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],[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://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
@jwinarske jwinarske deleted the jw/gpu_symbols branch August 22, 2024 15:01
Buchimi pushed a commit to Buchimi/flutter that referenced this pull request Sep 2, 2024
…lutter#153902)

flutter/engine@b94e009...85d4be0

2024-08-22 [email protected] Export GPU symbols for embedder (flutter/engine#54662)
2024-08-22 [email protected] [Impeller] Remove a log message in the Vulkan back end that is visible during engine startup (flutter/engine#54699)
2024-08-22 [email protected] Roll Skia from 3cd00377cefc to 34aa8ce13af6 (3 revisions) (flutter/engine#54698)
2024-08-21 [email protected] macOS: Make framework creation consistent with iOS (flutter/engine#54685)
2024-08-21 [email protected] Roll Skia from 69f4bd859025 to 3cd00377cefc (8 revisions) (flutter/engine#54693)
2024-08-21 [email protected] Roll Dart SDK from 060e40916514 to 025bf8d376d3 (1 revision) (flutter/engine#54692)
2024-08-21 [email protected] Split tests out of Linux Android artifact creation builds (flutter/engine#54683)
2024-08-21 [email protected] Roll Skia from 249d3f07c4d5 to 69f4bd859025 (5 revisions) (flutter/engine#54691)
2024-08-21 [email protected] [iOS] Tweak note about OpenGL support on mac in a user facing log. (flutter/engine#54690)
2024-08-21 [email protected] [Impeller] use blit pass to resize decoded images. (flutter/engine#54606)
2024-08-21 [email protected] Remove spammy warning message on `FlutterView` (flutter/engine#54686)
2024-08-21 [email protected] [Impeller] Perform integrity checks for Vulkan pipeline caches. (flutter/engine#54654)
2024-08-21 [email protected] docs: use test: all rather than editing .ci.yaml (flutter/engine#54667)
2024-08-21 [email protected] Reland "[DisplayList] Allow random access to ops through indexing" (flutter/engine#54676)
2024-08-21 [email protected] Roll Skia from 51ac9d93850c to 249d3f07c4d5 (2 revisions) (flutter/engine#54684)
2024-08-21 [email protected] iOS,macOS: Don't archive extra framework metadata (flutter/engine#54674)
2024-08-21 [email protected] Roll Dart SDK from 48f9b96d71e7 to 060e40916514 (1 revision) (flutter/engine#54682)
2024-08-21 [email protected] [web] annotate obscured text fields as passwords (flutter/engine#54664)
2024-08-21 [email protected] Roll Skia from c31e2ca59bd9 to 51ac9d93850c (2 revisions) (flutter/engine#54681)
2024-08-21 [email protected] [engine] reland weaken affinity of raster/ui to non-e core instead of only fast core (flutter/engine#54616)
2024-08-21 [email protected] Roll Skia from c00866df101a to c31e2ca59bd9 (2 revisions) (flutter/engine#54680)
2024-08-21 [email protected] Roll Skia from 39e5118034f4 to c00866df101a (1 revision) (flutter/engine#54678)
2024-08-21 [email protected] Roll Skia from 221ada80b174 to 39e5118034f4 (1 revision) (flutter/engine#54677)
2024-08-21 [email protected] Roll Skia from d576296091e0 to 221ada80b174 (2 revisions) (flutter/engine#54675)
2024-08-21 [email protected] Roll Dart SDK from 49f655b526c7 to 48f9b96d71e7 (1 revision) (flutter/engine#54672)
2024-08-21 [email protected] Roll Fuchsia Linux SDK from 3a16kOsyFmJh3lo7e... to XGzE3idakwfQZ68pb... (flutter/engine#54671)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from 3a16kOsyFmJh to XGzE3idakwfQ

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],[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://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
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
autosubmit Merge PR when tree becomes green via auto submit App flutter-gpu
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

2 participants