[Impeller] Implement a `MockGLES`, that provides trampolines for `ProcGLESTable` #46433

matanlurey · 2023-09-30T02:40:32Z

Unblocks testing flutter/flutter#135715. See README.md for details.

I made one non-testing change to the proc_table itself, which is if a function call will fail, we will print out what is about to fail in the validation log. It was useful when debugging the test creation itself, and it's only enabled when GL call checks are enabled anyway.

@gaaclarke I originally implemented it with FML_THREAD_LOCAL, but figured doing a global lock essentially was the same thing, and would prevent parallel test runs from stepping on each-other in weird ways?

/cc @chinmaygarde for visibility.

…kGLES

gaaclarke

Code lgtm, docs lgtm, I just have some notes.

impeller/renderer/backend/gles/proc_table_gles.h

impeller/renderer/backend/gles/test/mock_gles.cc

…kGLES

matanlurey

Thanks for the review!

impeller/renderer/backend/gles/proc_table_gles.h

impeller/renderer/backend/gles/test/mock_gles.cc

matanlurey · 2023-10-03T21:30:18Z

Changed made. I'd like one more look from @gaaclarke and @jonahwilliams before submitting. Thanks!

…46392) Closes flutter/flutter#135715. I plan to add tests before submitting once #46433 lands, probably something around `glDebugMessageControl` being emitted once per context, since that's what we need to keep working for Pixel phones. ## Output <details> <summary>Screenshot of app running locally, showing lack of off-screen MSAA</summary> ![flutter_01](https://github.com/flutter/engine/assets/168174/23642582-ce4c-47dc-8edf-7e6cfd12a074) </details> ```shell $ fl run --enable-impeller --local-engine=$ENGINE/out/android_debug_unopt_arm64 --local-engine-host=$ENGINE/out/host_debug_unopt_arm64 Launching lib/main.dart on Pixel 5 in debug mode... Running Gradle task 'assembleDebug'... 33.7s ✓ Built build/app/outputs/flutter-apk/app-debug.apk. Installing build/app/outputs/flutter-apk/app-debug.apk... 17.8s E/flutter (17749): [ERROR:flutter/shell/platform/android/android_context_gl_impeller.cc(80)] Using the Impeller rendering backend (OpenGLES). Syncing files to device Pixel 5... 70ms Flutter run key commands. r Hot reload. 🔥🔥🔥 R Hot restart. h List all available interactive commands. d Detach (terminate "flutter run" but leave application running). c Clear the screen q Quit (terminate the application on the device). A Dart VM Service on Pixel 5 is available at: http://127.0.0.1:57181/CrsakIgCHFY=/ I/cle_opacity_foo(17749): Compiler allocated 4413KB to compile void android.view.ViewRootImpl.performTraversals() The Flutter DevTools debugger and profiler on Pixel 5 is available at: http://127.0.0.1:9101?uri=http://127.0.0.1:57181/CrsakIgCHFY=/ ```

jonahwilliams

LGTM

…s for `ProcGLESTable` (flutter/engine#46433)

…135938) flutter/engine@8ab7c88...eef2449 2023-10-03 skia-flutter-autoroll@skia.org Roll Skia from c66c89c56549 to 4cc56b6b3453 (1 revision) (flutter/engine#46525) 2023-10-03 matanlurey@users.noreply.github.com [Impeller] Implement a `MockGLES`, that provides trampolines for `ProcGLESTable` (flutter/engine#46433) 2023-10-03 matanlurey@users.noreply.github.com [Impeller] Invoke `glDebugMessageControl` before `glPushDebugGroup` (flutter/engine#46392) 2023-10-03 skia-flutter-autoroll@skia.org Roll Dart SDK from 318c46832196 to 817f77ceba40 (1 revision) (flutter/engine#46520) 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 chinmaygarde@google.com,rmistry@google.com,zra@google.com 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

…lutter#135938) flutter/engine@8ab7c88...eef2449 2023-10-03 skia-flutter-autoroll@skia.org Roll Skia from c66c89c56549 to 4cc56b6b3453 (1 revision) (flutter/engine#46525) 2023-10-03 matanlurey@users.noreply.github.com [Impeller] Implement a `MockGLES`, that provides trampolines for `ProcGLESTable` (flutter/engine#46433) 2023-10-03 matanlurey@users.noreply.github.com [Impeller] Invoke `glDebugMessageControl` before `glPushDebugGroup` (flutter/engine#46392) 2023-10-03 skia-flutter-autoroll@skia.org Roll Dart SDK from 318c46832196 to 817f77ceba40 (1 revision) (flutter/engine#46520) 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 chinmaygarde@google.com,rmistry@google.com,zra@google.com 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

…46392) Closes flutter/flutter#135715. I plan to add tests before submitting once #46433 lands, probably something around `glDebugMessageControl` being emitted once per context, since that's what we need to keep working for Pixel phones. ## Output <details> <summary>Screenshot of app running locally, showing lack of off-screen MSAA</summary> ![flutter_01](https://github.com/flutter/engine/assets/168174/23642582-ce4c-47dc-8edf-7e6cfd12a074) </details> ```shell $ fl run --enable-impeller --local-engine=$ENGINE/out/android_debug_unopt_arm64 --local-engine-host=$ENGINE/out/host_debug_unopt_arm64 Launching lib/main.dart on Pixel 5 in debug mode... Running Gradle task 'assembleDebug'... 33.7s ✓ Built build/app/outputs/flutter-apk/app-debug.apk. Installing build/app/outputs/flutter-apk/app-debug.apk... 17.8s E/flutter (17749): [ERROR:flutter/shell/platform/android/android_context_gl_impeller.cc(80)] Using the Impeller rendering backend (OpenGLES). Syncing files to device Pixel 5... 70ms Flutter run key commands. r Hot reload. 🔥🔥🔥 R Hot restart. h List all available interactive commands. d Detach (terminate "flutter run" but leave application running). c Clear the screen q Quit (terminate the application on the device). A Dart VM Service on Pixel 5 is available at: http://127.0.0.1:57181/CrsakIgCHFY=/ I/cle_opacity_foo(17749): Compiler allocated 4413KB to compile void android.view.ViewRootImpl.performTraversals() The Flutter DevTools debugger and profiler on Pixel 5 is available at: http://127.0.0.1:9101?uri=http://127.0.0.1:57181/CrsakIgCHFY=/ ```

@gaaclarke

…cGLESTable` (#46433) Unblocks testing flutter/flutter#135715. See `README.md` for details. I made one non-testing change to the proc_table itself, which is if a function call will fail, we will print out what is about to fail in the validation log. It was useful when debugging the test creation itself, and it's only enabled when GL call checks are enabled anyway. @gaaclarke I originally implemented it with `FML_THREAD_LOCAL`, but figured doing a global lock essentially was the same thing, and would prevent parallel test runs from stepping on each-other in weird ways? /cc @chinmaygarde for visibility.

matanlurey added 3 commits September 29, 2023 19:20

Implement a MockGLES handler, with a self-capturing proc table.

f1707a5

Tweak a bit.

dc7f799

Tweak, add README.

2a82e94

matanlurey requested review from jonahwilliams and gaaclarke September 30, 2023 02:40

github-actions bot added the e: impeller label Sep 30, 2023

matanlurey self-assigned this Sep 30, 2023

matanlurey changed the title ~~Implement a MockGLES, that provides trampolines for ProcGLESTable~~ [Impeller] Implement a MockGLES, that provides trampolines for ProcGLESTable Sep 30, 2023

matanlurey mentioned this pull request Sep 30, 2023

[Impeller] Invoke glDebugMessageControl before glPushDebugGroup #46392

Merged

matanlurey added 3 commits September 29, 2023 19:59

Merge remote-tracking branch 'upstream/main' into engine-impeller-Moc…

36489ab

…kGLES

Exclude gles/test from licensing.

cb5226d

Merge remote-tracking branch 'upstream/main' into engine-impeller-Moc…

8f05919

…kGLES

gaaclarke approved these changes Oct 3, 2023

View reviewed changes

matanlurey added 2 commits October 3, 2023 11:12

Merge remote-tracking branch 'upstream/main' into engine-impeller-Moc…

0bee06c

…kGLES

Review feedback.

3a860c5

matanlurey commented Oct 3, 2023

View reviewed changes

matanlurey requested a review from gaaclarke October 3, 2023 17:23

matanlurey added 2 commits October 3, 2023 13:15

Use try_lock.

271d6d1

Add TODO.

476957c

gaaclarke approved these changes Oct 3, 2023

View reviewed changes

jonahwilliams approved these changes Oct 3, 2023

View reviewed changes

matanlurey merged commit 0141ba2 into flutter:main Oct 3, 2023

engine-flutter-autoroll mentioned this pull request Oct 3, 2023

Roll Flutter Engine from 8ab7c8841ee5 to eef2449ff80b (4 revisions) flutter/flutter#135938

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 3, 2023

0141ba258 [Impeller] Implement a MockGLES, that provides trampoline…

1d70a67

…s for `ProcGLESTable` (flutter/engine#46433)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Impeller] Implement a `MockGLES`, that provides trampolines for `ProcGLESTable` #46433

[Impeller] Implement a `MockGLES`, that provides trampolines for `ProcGLESTable` #46433

Uh oh!

matanlurey commented Sep 30, 2023

Uh oh!

gaaclarke left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

matanlurey left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

matanlurey commented Oct 3, 2023

Uh oh!

jonahwilliams left a comment

Uh oh!

Uh oh!

[Impeller] Implement a MockGLES, that provides trampolines for ProcGLESTable #46433

[Impeller] Implement a MockGLES, that provides trampolines for ProcGLESTable #46433

Uh oh!

Conversation

matanlurey commented Sep 30, 2023

Uh oh!

gaaclarke left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

matanlurey left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

matanlurey commented Oct 3, 2023

Uh oh!

jonahwilliams left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

[Impeller] Implement a `MockGLES`, that provides trampolines for `ProcGLESTable` #46433

[Impeller] Implement a `MockGLES`, that provides trampolines for `ProcGLESTable` #46433