[Impeller] Compute shader support #35750

dnfield · 2022-08-26T22:01:24Z

This doesn't really do anything yet.

still left:

Update impellerc to reflect right types when working with compute shaders
Write some tests
Figure out how to wire this up to the Context.

dnfield · 2022-08-26T22:07:23Z

I guess it's .. kind of wired to context already, but will have to test to make sure it's working. Need impellerc in line first.

chinmaygarde

So exciting! Everything looks great. I don't think we should attempt compute for ES though.

chinmaygarde · 2022-08-26T22:13:32Z

impeller/renderer/backend/gles/command_buffer_gles.cc


+// |CommandBuffer|
+std::shared_ptr<ComputePass> CommandBufferGLES::OnCreateComputePass() const {
+  if (!IsValid()) {


Let's put the validity checks in the base class.

chinmaygarde · 2022-08-26T22:14:02Z

impeller/renderer/backend/gles/command_buffer_gles.cc

+  if (!IsValid()) {
+    return nullptr;
+  }
+  // TODO(dnfield): implement compute for GLES.


This isn't possible in GLES (anything < 3.2). So I think its best to just return nullptr with a comment here saying that compute passes aren't supported.

Is the idea that >= 3.2 will just have Vulkan anyway?

I was assuming we could use extension methods to check if it's available and if so, use them.

Yeah. Have a clear separation between a modern and legacy renderer with little effort made to update the legacy one.

chinmaygarde · 2022-08-26T22:16:56Z

impeller/renderer/command.cc

      fragment_bindings.buffers[slot.binding] = {&metadata, view};
      return true;
+    case ShaderStage::kCompute:
+      FML_DCHECK(false);


FML_UNREACHABLE for WIP stuff so we don't miss it?

dnfield · 2022-08-27T22:10:21Z

Vulkan is broken, I'll fix that up after I fix impellerc.

chinmaygarde

Sweet! Can wait to tinker on this.

chinmaygarde · 2022-08-30T19:54:46Z

impeller/renderer/backend/metal/compute_pass_mtl.mm

+  }
+
+  // TODO(dnfield): Support non-serial dispatch type on higher iOS versions.
+  auto compute_command_encoder = [buffer_ computeCommandEncoder];


We should be able to support this use cases using just creating multiple compute passes. This, instead of relying on the Metal API feature. I think we can get rid of this TODO.

My assumption is that Vulkan would have similar API for collecting compute information, using whatever necessary barriers, whether for use in a subsequent vertex shader or back on the host.

chinmaygarde · 2022-08-30T19:56:17Z

impeller/renderer/backend/metal/compute_pass_mtl.mm

+      auto_pop_debug_marker.Release();
+    }
+
+    // TODO(dnfield): update the compute pipeline descriptor so that it can set


File issues (in a later patch is fine) for these and cross reference them here?

Here and elsewhere.

chinmaygarde · 2022-08-30T19:57:14Z

impeller/renderer/backend/metal/compute_pass_mtl.mm

+  [encoder dispatchThreadgroups:MTLSizeMake(32, 32, 1)
+          threadsPerThreadgroup:MTLSizeMake(32, 32, 1)];
+
+  // TODO(dnfield): add some kind of completion handling logic for reading


IMO, this would need the addition of fences/events as a separate API that allows for resource synchronization between not only compute passes but compute and render passes as well.

chinmaygarde · 2022-08-30T19:58:24Z

impeller/renderer/command.cc

      fragment_bindings.buffers[slot.binding] = {&metadata, view};
      return true;
+    case ShaderStage::kCompute:
+      FML_UNREACHABLE();


Perhaps just a validation log that returns false?

chinmaygarde · 2022-08-30T19:59:13Z

impeller/renderer/compute_command.cc

+                                  const ShaderUniformSlot& slot,
+                                  const ShaderMetadata& metadata,
+                                  BufferView view) {
+  FML_DCHECK(stage == ShaderStage::kCompute);


Instead of a DCHECK that is all but useless for non-engine developers, just return false?

chinmaygarde · 2022-08-30T20:03:06Z

@dnfield Per your comment in the chat thread, marking this as a non-draft.

auto-submit · 2022-08-31T16:57:04Z

The status or check suite Mac Unopt has failed. Please fix the issues identified (or deflake) before re-applying this label.

…110727) * 5a1a855b2 Roll Clang from 41d4067d0b7c to 039b969b32b6 (flutter/engine#35840) * 3a1d6a9b0 Roll Skia from e0b87738eca7 to 45fe3b5a6710 (9 revisions) (flutter/engine#35841) * fb067eab7 [Impeller] Compute shader support (flutter/engine#35750) * a53384841 Roll Dart SDK from 5a2737b71877 to fec63626f078 (2 revisions) (flutter/engine#35844) * 3dc4e4971 roll CanvasKit to 0.36.1 (attempt 2) (flutter/engine#35839) * 9b14b25eb Reland: Adds a reusable FragmentShader (flutter/engine#35846) * ac34d3e63 [fuchsia] Workflow scripts. (flutter/engine#35822) * f0f8d10b8 Revert "Roll Dart SDK from 5a2737b71877 to fec63626f078 (2 revisions) (#35844)" (flutter/engine#35849)

Compute

08b51b4

dnfield added the e: impeller label Aug 26, 2022

dnfield requested review from bdero and chinmaygarde August 26, 2022 22:06

chinmaygarde reviewed Aug 26, 2022

View reviewed changes

chinmaygarde assigned dnfield Aug 26, 2022

chinmaygarde added the Work in progress (WIP) Not ready (yet) for review! label Aug 27, 2022

chinmaygarde changed the title ~~[Impeller] Compute shader support (WIP)~~ [Impeller] Compute shader support Aug 27, 2022

dnfield added 2 commits August 29, 2022 21:04

test, update impellerc

4bd9e0c

vulkan stuff

fc5b449

chinmaygarde approved these changes Aug 30, 2022

View reviewed changes

chinmaygarde removed the Work in progress (WIP) Not ready (yet) for review! label Aug 30, 2022

chinmaygarde marked this pull request as ready for review August 30, 2022 20:03

dnfield added 8 commits August 30, 2022 14:33

chinmay review and blobcat stuff

7f9530e

missed one

bc31a52

fix buildg

2ca37ff

one more

586743e

license file, vk switch

c38be7b

one more

09f4004

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

05f5b73

fix license file

be48526

dnfield added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 31, 2022

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Aug 31, 2022

give type a distinct name

4f92119

dnfield added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 31, 2022

auto-submit bot merged commit fb067ea into flutter:main Aug 31, 2022

dnfield deleted the compute branch August 31, 2022 19:07

engine-flutter-autoroll mentioned this pull request Aug 31, 2022

Roll Flutter Engine from d993e0d55fc6 to fb067eab70c5 (3 revisions) flutter/flutter#110698

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 31, 2022

fb067eab7 [Impeller] Compute shader support (flutter/engine#35750)

4b321ab

engine-flutter-autoroll mentioned this pull request Aug 31, 2022

Roll Flutter Engine from d993e0d55fc6 to a53384841ac0 (4 revisions) flutter/flutter#110701

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 31, 2022

fb067eab7 [Impeller] Compute shader support (flutter/engine#35750)

6a84a2d

engine-flutter-autoroll mentioned this pull request Aug 31, 2022

Roll Flutter Engine from d993e0d55fc6 to 3dc4e497169d (5 revisions) flutter/flutter#110703

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 31, 2022

fb067eab7 [Impeller] Compute shader support (flutter/engine#35750)

9da50ff

engine-flutter-autoroll mentioned this pull request Aug 31, 2022

Roll Flutter Engine from d993e0d55fc6 to 9b14b25ebdd7 (6 revisions) flutter/flutter#110719

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 31, 2022

fb067eab7 [Impeller] Compute shader support (flutter/engine#35750)

5da6dca

engine-flutter-autoroll mentioned this pull request Aug 31, 2022

Roll Flutter Engine from d993e0d55fc6 to ac34d3e63998 (7 revisions) flutter/flutter#110720

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 31, 2022

fb067eab7 [Impeller] Compute shader support (flutter/engine#35750)

4972d14

engine-flutter-autoroll mentioned this pull request Aug 31, 2022

Roll Flutter Engine from d993e0d55fc6 to f0f8d10b8809 (8 revisions) flutter/flutter#110727

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 31, 2022

fb067eab7 [Impeller] Compute shader support (flutter/engine#35750)

db62e7d

[Impeller] Compute shader support #35750

[Impeller] Compute shader support #35750

Uh oh!

Conversation

dnfield commented Aug 26, 2022 • edited by chinmaygarde Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dnfield commented Aug 26, 2022

Uh oh!

chinmaygarde left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dnfield commented Aug 27, 2022

Uh oh!

chinmaygarde left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chinmaygarde commented Aug 30, 2022

Uh oh!

auto-submit bot commented Aug 31, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dnfield commented Aug 26, 2022 •

edited by chinmaygarde

Loading