[Impeller] Compute in Vulkan #42294

dnfield · 2023-05-24T20:29:07Z

Updates capabilities checks for support
Fixes a bug where SSBOs were being treated as UBOs in render (and does the same work in compute).
Fixes CommandEncoderVK::Submit so that it takes a completion callback and CommandBufferVK uses it to avoid sending a kComplete status to callers when it should really be kPending.

jonahwilliams · 2023-05-24T20:40:57Z

impeller/renderer/backend/vulkan/capabilities_vk.cc

@@ -330,12 +343,14 @@ bool CapabilitiesVK::SupportsFramebufferFetch() const {

 // |Capabilities|
 bool CapabilitiesVK::SupportsCompute() const {
-  return false;
+  // Vulkan 1.1 requires support for compute.


oh, this will work?

It's not wired up yet. But capabilities wise it is supported.

More specifically,

engine/impeller/renderer/backend/vulkan/command_buffer_vk.cc

Line 89 in 64bed95

IMPELLER_UNIMPLEMENTED;

needs to get implemented for this to work in impeller, but the device will support it even though Impeller doesn't.

I think we'll start crashing on drawPoints on Vulkan if this returns true before its completely ready?

Ahh, yes. I'm not sure how big of a deal that is. I could make this patch still return false here but true on supports subgroups for now...?

(We'll hit a codepath that ends up doing an IMPELLER_UNIMPLEMENTED which will abort)

jonahwilliams

LGTM with question

dnfield · 2023-05-25T16:13:47Z

I'm going to hold off on landing this until I either get a sense of how long it'll take to fix the comptue implementation on Vulkan or I set the return value to false for now

jonahwilliams · 2023-06-01T18:22:49Z

impeller/renderer/prefix_sum_test.comp

@@ -31,7 +31,10 @@ void main() {
    value = input_data.data[ident];
  }

-  memory[ident] = value;
+  if (ident < BLOCK_SIZE) {


Technically we need to use a function constant sizing of the memory size so that it exactly corresponds to the threadgroup size.

auto-submit · 2023-06-01T19:58:22Z

auto label is removed for flutter/engine, pr: 42294, due to - The status or check suite Linux linux_license has failed. Please fix the issues identified (or deflake) before re-applying this label.

chinmaygarde · 2023-06-01T19:58:32Z

impeller/renderer/backend/vulkan/command_buffer_vk.cc

  }
-  return submit;
+  return encoder_->Submit([callback](bool submit) {
+    if (!submit) {


You missed the early return after the callback invocation on error. Thats why I prefer this stylistically too (and its drier).

callback(submitted ? kCompleted : kError);

That doesn't work - it fires the callback too early.

Ahh NVM I see.

chinmaygarde · 2023-06-01T20:00:26Z

impeller/renderer/backend/vulkan/command_encoder_vk.cc

-bool CommandEncoderVK::Submit() {
+bool CommandEncoderVK::Submit(SubmitCallback callback) {
+  // Make sure to call callback with `false` if anything returns early.
+  bool fail_callback = !!callback;


Prefer a ScopedCleanupClosure that returns false by default. On the one success case, release the closure and invoke it with true once.

That's being done below.

fail_callback is used in the ScopedCleanupClosure.

Oh, I think I see what you mean.

I thought there was some way to make this a little clearer with a second fml::ScopedCleanupClosure but I'm missing it. Maybe we can sync up offline.

Argh, nevermind again, I misread this and now I understand what you're saying :)

chinmaygarde · 2023-06-01T20:01:17Z

Sorry, just noticed an error on a second pass. Also some nits but those are not blockers.

auto-submit · 2023-06-01T20:33:45Z

auto label is removed for flutter/engine, pr: 42294, due to - The status or check suite Mac mac_host_engine has failed. Please fix the issues identified (or deflake) before re-applying this label.

The status or check suite Linux Android clang-tidy has failed. Please fix the issues identified (or deflake) before re-applying this label.

auto-submit · 2023-06-01T21:13:26Z

auto label is removed for flutter/engine, pr: 42294, due to - The status or check suite Linux Host clang-tidy has failed. Please fix the issues identified (or deflake) before re-applying this label.

…128119) flutter/engine@02d6fbb...c6e9383 2023-06-02 jason-simmons@users.noreply.github.com [Impeller] clang-tidy fixes (flutter/engine#42503) 2023-06-02 103135467+sealesj@users.noreply.github.com Allow for optional label to trigger vuln scan on patch PRs (flutter/engine#42494) 2023-06-02 skia-flutter-autoroll@skia.org Roll Skia from 881a8df6f9e9 to 47b0db43f6a4 (1 revision) (flutter/engine#42512) 2023-06-02 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from X4Pkixxtt3BkjRW9P... to PuYA-6NVHeHPlkCdk... (flutter/engine#42511) 2023-06-02 skia-flutter-autoroll@skia.org Roll Skia from 7777ee2bf7ef to 881a8df6f9e9 (1 revision) (flutter/engine#42510) 2023-06-02 skia-flutter-autoroll@skia.org Roll Dart SDK from 3d4d29d8f16b to 9d8df2a5210b (2 revisions) (flutter/engine#42509) 2023-06-02 skia-flutter-autoroll@skia.org Roll Skia from 02e706e9761b to 7777ee2bf7ef (2 revisions) (flutter/engine#42508) 2023-06-02 skia-flutter-autoroll@skia.org Roll Skia from 0c75f1877b37 to 02e706e9761b (2 revisions) (flutter/engine#42506) 2023-06-02 dkwingsmt@users.noreply.github.com Revert "[Rasterizer] Make resubmit information temporary" (flutter/engine#42455) 2023-06-02 skia-flutter-autoroll@skia.org Roll Skia from f4854a3d009d to 0c75f1877b37 (1 revision) (flutter/engine#42505) 2023-06-02 skia-flutter-autoroll@skia.org Roll Dart SDK from 0d3c310fd6d9 to 3d4d29d8f16b (3 revisions) (flutter/engine#42502) 2023-06-02 skia-flutter-autoroll@skia.org Roll Skia from 082a7d1f72f7 to f4854a3d009d (4 revisions) (flutter/engine#42500) 2023-06-01 godofredoc@google.com Do not retry lint or clang tidy tests. (flutter/engine#42498) 2023-06-01 dnfield@google.com [Impeller] Compute in Vulkan (flutter/engine#42294) 2023-06-01 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from yqJfBsLdfLP4_vbFu... to JQRQ1nH1ILNA--N_b... (flutter/engine#42499) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from X4Pkixxtt3Bk to PuYA-6NVHeHP fuchsia/sdk/core/mac-amd64 from yqJfBsLdfLP4 to JQRQ1nH1ILNA 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 jonahwilliams@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

Feature detection for subgroup support in Vulkan

efbb8b3

dnfield requested review from chinmaygarde and jonahwilliams May 24, 2023 20:29

dnfield self-assigned this May 24, 2023

dnfield added the e: impeller label May 24, 2023

fix english

eab8f2c

jonahwilliams reviewed May 24, 2023

View reviewed changes

jonahwilliams approved these changes May 24, 2023

View reviewed changes

chinmaygarde approved these changes May 25, 2023

View reviewed changes

chinmaygarde added the Work in progress (WIP) Not ready (yet) for review! label May 25, 2023

dnfield added 7 commits May 25, 2023 21:37

start pass

37eaae7

more

f07bfd9

Merge remote-tracking branch 'upstream/main' into vk_compute

28d1457

KEEP GOING

2168461

a little more

9301a82

Fix command buffer usage, SSBOs

c88dd19

Merge remote-tracking branch 'upstream/main' into vk_compute

355eaed

dnfield changed the title ~~[Impeller] Feature detection for subgroup support in Vulkan~~ [Impeller] Compute in Vulkan Jun 1, 2023

dnfield requested review from jonahwilliams and chinmaygarde June 1, 2023 18:19

jonahwilliams reviewed Jun 1, 2023

View reviewed changes

Fix completion callbacks for Vulkan

14f4076

dnfield added autosubmit Merge PR when tree becomes green via auto submit App and removed Work in progress (WIP) Not ready (yet) for review! labels Jun 1, 2023

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jun 1, 2023

chinmaygarde requested changes Jun 1, 2023

View reviewed changes

Chinmay review, license/malioc

8615211

chinmaygarde approved these changes Jun 1, 2023

View reviewed changes

dnfield added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 1, 2023

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jun 1, 2023

limits

b2b37ec

dnfield added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 1, 2023

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jun 1, 2023

dnfield added 5 commits June 1, 2023 14:16

typo

becb828

...

45aeb3b

fix type clash on different arch

53220ac

fix size

454ed44

more shaders

d26abf2

dnfield added the autosubmit Merge PR when tree becomes green via auto submit App label Jun 1, 2023

auto-submit bot merged commit b8ef52a into flutter:main Jun 1, 2023
29 checks passed

dnfield deleted the vk_compute branch June 2, 2023 05:59

engine-flutter-autoroll mentioned this pull request Jun 2, 2023

Roll Flutter Engine from 02d6fbb68b69 to c6e9383f2143 (15 revisions) flutter/flutter#128119

Merged

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

b8ef52a88 [Impeller] Compute in Vulkan (flutter/engine#42294)

3cb26a0

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Impeller] Compute in Vulkan #42294

[Impeller] Compute in Vulkan #42294

dnfield commented May 24, 2023 •

edited

jonahwilliams May 24, 2023

dnfield May 24, 2023

dnfield May 24, 2023

jonahwilliams May 24, 2023

dnfield May 24, 2023

jonahwilliams May 24, 2023

dnfield May 24, 2023

jonahwilliams left a comment

dnfield commented May 25, 2023

jonahwilliams Jun 1, 2023

auto-submit bot commented Jun 1, 2023

chinmaygarde Jun 1, 2023

dnfield Jun 1, 2023

dnfield Jun 1, 2023

chinmaygarde Jun 1, 2023

dnfield Jun 1, 2023

dnfield Jun 1, 2023

dnfield Jun 1, 2023

dnfield Jun 1, 2023

dnfield Jun 1, 2023

chinmaygarde commented Jun 1, 2023

auto-submit bot commented Jun 1, 2023

auto-submit bot commented Jun 1, 2023

[Impeller] Compute in Vulkan #42294

[Impeller] Compute in Vulkan #42294

Conversation

dnfield commented May 24, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonahwilliams left a comment

Choose a reason for hiding this comment

dnfield commented May 25, 2023

Choose a reason for hiding this comment

auto-submit bot commented Jun 1, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chinmaygarde commented Jun 1, 2023

auto-submit bot commented Jun 1, 2023

auto-submit bot commented Jun 1, 2023

dnfield commented May 24, 2023 •

edited