Added test to assert the vulkan embedder threadsafe vkqueue usage #45732

gaaclarke · 2023-09-12T20:32:43Z

fixes flutter/flutter#133933

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.

bdero

LGTM

bdero · 2023-09-12T20:40:11Z

shell/platform/embedder/embedder.h

@@ -757,6 +757,9 @@ typedef struct {
  /// The queue family index of the VkQueue supplied in the next field.
  uint32_t queue_family_index;
  /// VkQueue handle.
+  /// The queue should not be used without protection from a mutex to make sure
+  /// it is not used simultaneously with other threads. That mutex should match
+  /// the one injected via the |get_instance_proc_address_callback|.


Should we link to flutter/flutter#134573 (bug about fixing this behavior)?

cbracken

lgtm with a small suggestion

shell/platform/embedder/tests/embedder_test_context_vulkan.h

shell/platform/embedder/tests/embedder_vk_unittests.cc

… usage (flutter/engine#45732)

…134597) flutter/engine@c90fadf...7c78ea2 2023-09-12 zanderso@users.noreply.github.com Revert "Lazily allocate RasterCacheItems only when caching is enabled" (flutter/engine#45734) 2023-09-12 1961493+harryterkelsen@users.noreply.github.com Revert "Use a single OffscreenCanvas for rendering in CanvasKit" (flutter/engine#45744) 2023-09-12 chinmaygarde@google.com [Impeller] Patch the compiler to account for subpass inputs and PSO metadata. (flutter/engine#45739) 2023-09-12 chinmaygarde@google.com [Impeller] Fix swapchain recreation for non-polling cases. (flutter/engine#45740) 2023-09-12 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from QgAHx3BtJfN3TmodS... to UCa49z8fu0hD9cypj... (flutter/engine#45738) 2023-09-12 ychris@google.com [ios] upload extension safe artifacts (flutter/engine#45664) 2023-09-12 30870216+gaaclarke@users.noreply.github.com Added test to assert the vulkan embedder threadsafe vkqueue usage (flutter/engine#45732) 2023-09-12 chinmaygarde@google.com [Impeller] If validations are enabled but not found, still create the VK context. (flutter/engine#45674) 2023-09-12 skia-flutter-autoroll@skia.org Roll Skia from 211d63b1e1f5 to 2d295711337c (7 revisions) (flutter/engine#45729) 2023-09-12 1961493+harryterkelsen@users.noreply.github.com Use a single OffscreenCanvas for rendering in CanvasKit (flutter/engine#42672) Also rolling transitive DEPS: fuchsia/sdk/core/mac-amd64 from QgAHx3BtJfN3 to UCa49z8fu0hD 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 bdero@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#134597) flutter/engine@c90fadf...7c78ea2 2023-09-12 zanderso@users.noreply.github.com Revert "Lazily allocate RasterCacheItems only when caching is enabled" (flutter/engine#45734) 2023-09-12 1961493+harryterkelsen@users.noreply.github.com Revert "Use a single OffscreenCanvas for rendering in CanvasKit" (flutter/engine#45744) 2023-09-12 chinmaygarde@google.com [Impeller] Patch the compiler to account for subpass inputs and PSO metadata. (flutter/engine#45739) 2023-09-12 chinmaygarde@google.com [Impeller] Fix swapchain recreation for non-polling cases. (flutter/engine#45740) 2023-09-12 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from QgAHx3BtJfN3TmodS... to UCa49z8fu0hD9cypj... (flutter/engine#45738) 2023-09-12 ychris@google.com [ios] upload extension safe artifacts (flutter/engine#45664) 2023-09-12 30870216+gaaclarke@users.noreply.github.com Added test to assert the vulkan embedder threadsafe vkqueue usage (flutter/engine#45732) 2023-09-12 chinmaygarde@google.com [Impeller] If validations are enabled but not found, still create the VK context. (flutter/engine#45674) 2023-09-12 skia-flutter-autoroll@skia.org Roll Skia from 211d63b1e1f5 to 2d295711337c (7 revisions) (flutter/engine#45729) 2023-09-12 1961493+harryterkelsen@users.noreply.github.com Use a single OffscreenCanvas for rendering in CanvasKit (flutter/engine#42672) Also rolling transitive DEPS: fuchsia/sdk/core/mac-amd64 from QgAHx3BtJfN3 to UCa49z8fu0hD 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 bdero@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

Added test to assert the vulkan embedder threadsafe vkqueue usage

9549d05

gaaclarke requested review from cbracken and chinmaygarde as code owners September 12, 2023 20:32

gaaclarke mentioned this pull request Sep 12, 2023

Add warning about Vulkan queue usage in embedder.h #45731

Closed

gaaclarke requested a review from bdero September 12, 2023 20:33

bdero approved these changes Sep 12, 2023

View reviewed changes

cbracken approved these changes Sep 12, 2023

View reviewed changes

shell/platform/embedder/tests/embedder_test_context_vulkan.h Show resolved Hide resolved

shell/platform/embedder/tests/embedder_vk_unittests.cc Outdated Show resolved Hide resolved

gaaclarke added 2 commits September 12, 2023 14:03

added link to an issue

aad7830

switched to strncmp

be68d0e

gaaclarke added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 12, 2023

auto-submit bot merged commit f358bde into flutter:main Sep 12, 2023
28 checks passed

engine-flutter-autoroll mentioned this pull request Sep 13, 2023

Roll Flutter Engine from c90fadf45d44 to 7c78ea210c19 (10 revisions) flutter/flutter#134597

Merged

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

f358bde6c Added test to assert the vulkan embedder threadsafe vkqueue…

0ad1c8f

… usage (flutter/engine#45732)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added test to assert the vulkan embedder threadsafe vkqueue usage #45732

Added test to assert the vulkan embedder threadsafe vkqueue usage #45732

gaaclarke commented Sep 12, 2023

bdero left a comment

bdero Sep 12, 2023

gaaclarke Sep 12, 2023

cbracken left a comment

Added test to assert the vulkan embedder threadsafe vkqueue usage #45732

Added test to assert the vulkan embedder threadsafe vkqueue usage #45732

Conversation

gaaclarke commented Sep 12, 2023

Pre-launch Checklist

bdero left a comment

Choose a reason for hiding this comment

bdero Sep 12, 2023

Choose a reason for hiding this comment

gaaclarke Sep 12, 2023

Choose a reason for hiding this comment

cbracken left a comment

Choose a reason for hiding this comment