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

[Impeller] Compute in Vulkan #42294

Merged
merged 17 commits into from
Jun 1, 2023
Merged

[Impeller] Compute in Vulkan #42294

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
efbb8b3
Feature detection for subgroup support in Vulkan
dnfield May 24, 2023
eab8f2c
fix english
dnfield May 24, 2023
37eaae7
start pass
dnfield May 26, 2023
f07bfd9
more
dnfield May 26, 2023
28d1457
Merge remote-tracking branch 'upstream/main' into vk_compute
dnfield May 26, 2023
2168461
KEEP GOING
dnfield May 27, 2023
9301a82
a little more
dnfield May 31, 2023
c88dd19
Fix command buffer usage, SSBOs
dnfield Jun 1, 2023
355eaed
Merge remote-tracking branch 'upstream/main' into vk_compute
dnfield Jun 1, 2023
14f4076
Fix completion callbacks for Vulkan
dnfield Jun 1, 2023
8615211
Chinmay review, license/malioc
dnfield Jun 1, 2023
b2b37ec
limits
dnfield Jun 1, 2023
becb828
typo
dnfield Jun 1, 2023
45aeb3b
...
dnfield Jun 1, 2023
53220ac
fix type clash on different arch
dnfield Jun 1, 2023
454ed44
fix size
dnfield Jun 1, 2023
d26abf2
more shaders
dnfield Jun 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions ci/licenses_golden/licenses_flutter
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1496,6 +1496,10 @@ ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/command_encoder_vk.cc
ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/command_encoder_vk.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/command_pool_vk.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/command_pool_vk.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/compute_pass_vk.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/compute_pass_vk.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/compute_pipeline_vk.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/compute_pipeline_vk.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/context_vk.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/context_vk.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/impeller/renderer/backend/vulkan/debug_report_vk.cc + ../../../flutter/LICENSE
Expand Down Expand Up @@ -4158,6 +4162,10 @@ FILE: ../../../flutter/impeller/renderer/backend/vulkan/command_encoder_vk.cc
FILE: ../../../flutter/impeller/renderer/backend/vulkan/command_encoder_vk.h
FILE: ../../../flutter/impeller/renderer/backend/vulkan/command_pool_vk.cc
FILE: ../../../flutter/impeller/renderer/backend/vulkan/command_pool_vk.h
FILE: ../../../flutter/impeller/renderer/backend/vulkan/compute_pass_vk.cc
FILE: ../../../flutter/impeller/renderer/backend/vulkan/compute_pass_vk.h
FILE: ../../../flutter/impeller/renderer/backend/vulkan/compute_pipeline_vk.cc
FILE: ../../../flutter/impeller/renderer/backend/vulkan/compute_pipeline_vk.h
FILE: ../../../flutter/impeller/renderer/backend/vulkan/context_vk.cc
FILE: ../../../flutter/impeller/renderer/backend/vulkan/context_vk.h
FILE: ../../../flutter/impeller/renderer/backend/vulkan/debug_report_vk.cc
Expand Down
3 changes: 3 additions & 0 deletions impeller/playground/backend/vulkan/playground_impl_vk.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "impeller/renderer/backend/vulkan/formats_vk.h"
#include "impeller/renderer/backend/vulkan/surface_vk.h"
#include "impeller/renderer/backend/vulkan/texture_vk.h"
#include "impeller/renderer/vk/compute_shaders_vk.h"
#include "impeller/scene/shaders/vk/scene_shaders_vk.h"

namespace impeller {
Expand All @@ -38,6 +39,8 @@ ShaderLibraryMappingsForPlayground() {
impeller_imgui_shaders_vk_length),
std::make_shared<fml::NonOwnedMapping>(impeller_scene_shaders_vk_data,
impeller_scene_shaders_vk_length),
std::make_shared<fml::NonOwnedMapping>(
impeller_compute_shaders_vk_data, impeller_compute_shaders_vk_length),
};
}

Expand Down
4 changes: 3 additions & 1 deletion impeller/playground/compute_playground_test.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,9 @@ class ComputePlaygroundTest

#define INSTANTIATE_COMPUTE_SUITE(playground) \
INSTANTIATE_TEST_SUITE_P( \
Compute, playground, ::testing::Values(PlaygroundBackend::kMetal), \
Compute, playground, \
::testing::Values(PlaygroundBackend::kMetal, \
PlaygroundBackend::kVulkan), \
[](const ::testing::TestParamInfo<ComputePlaygroundTest::ParamType>& \
info) { return PlaygroundBackendToString(info.param); });

Expand Down
2 changes: 1 addition & 1 deletion impeller/renderer/backend/metal/compute_pass_mtl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class ComputePassMTL final : public ComputePass {
bool IsValid() const override;

// |ComputePass|
void OnSetLabel(std::string label) override;
void OnSetLabel(const std::string& label) override;

// |ComputePass|
bool OnEncodeCommands(const Context& context,
Expand Down
6 changes: 3 additions & 3 deletions impeller/renderer/backend/metal/compute_pass_mtl.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,11 @@
return is_valid_;
}

void ComputePassMTL::OnSetLabel(std::string label) {
void ComputePassMTL::OnSetLabel(const std::string& label) {
if (label.empty()) {
return;
}
label_ = std::move(label);
label_ = label;
}

bool ComputePassMTL::OnEncodeCommands(const Context& context,
Expand All @@ -55,7 +55,7 @@
return false;
}

FML_DCHECK(!grid_size_.IsEmpty() && !thread_group_size_.IsEmpty());
FML_DCHECK(!grid_size.IsEmpty() && !thread_group_size.IsEmpty());

// TODO(dnfield): Support non-serial dispatch type on higher iOS versions.
auto compute_command_encoder = [buffer_ computeCommandEncoder];
Expand Down
4 changes: 4 additions & 0 deletions impeller/renderer/backend/vulkan/BUILD.gn
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ impeller_component("vulkan") {
"command_encoder_vk.h",
"command_pool_vk.cc",
"command_pool_vk.h",
"compute_pass_vk.cc",
"compute_pass_vk.h",
"compute_pipeline_vk.cc",
"compute_pipeline_vk.h",
"context_vk.cc",
"context_vk.h",
"debug_report_vk.cc",
Expand Down
1 change: 1 addition & 0 deletions impeller/renderer/backend/vulkan/allocator_vk.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,7 @@ std::shared_ptr<DeviceBuffer> AllocatorVK::OnCreateBuffer(
buffer_info.usage = vk::BufferUsageFlagBits::eVertexBuffer |
vk::BufferUsageFlagBits::eIndexBuffer |
vk::BufferUsageFlagBits::eUniformBuffer |
vk::BufferUsageFlagBits::eStorageBuffer |
vk::BufferUsageFlagBits::eTransferSrc |
vk::BufferUsageFlagBits::eTransferDst;
buffer_info.size = desc.size;
Expand Down
19 changes: 17 additions & 2 deletions impeller/renderer/backend/vulkan/capabilities_vk.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,19 @@ bool CapabilitiesVK::SetDevice(const vk::PhysicalDevice& device) {

device_properties_ = device.getProperties();

auto physical_properties_2 =
device.getProperties2<vk::PhysicalDeviceProperties2,
vk::PhysicalDeviceSubgroupProperties>();

// Currently shaders only want access to arithmetic subgroup features.
// If that changes this needs to get updated, and so does Metal (which right
// now assumes it from compile time flags based on the MSL target version).

supports_compute_subgroups_ =
!!(physical_properties_2.get<vk::PhysicalDeviceSubgroupProperties>()
.supportedOperations &
vk::SubgroupFeatureFlagBits::eArithmetic);

return true;
}

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

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

Choose a reason for hiding this comment

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

oh, this will work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

sg

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

return true;
}

// |Capabilities|
bool CapabilitiesVK::SupportsComputeSubgroups() const {
return false;
// Set by |SetDevice|.
return supports_compute_subgroups_;
}

// |Capabilities|
Expand Down
1 change: 1 addition & 0 deletions impeller/renderer/backend/vulkan/capabilities_vk.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ class CapabilitiesVK final : public Capabilities,
PixelFormat color_format_ = PixelFormat::kUnknown;
PixelFormat depth_stencil_format_ = PixelFormat::kUnknown;
vk::PhysicalDeviceProperties device_properties_;
bool supports_compute_subgroups_ = false;
bool is_valid_ = false;

bool HasExtension(const std::string& ext) const;
Expand Down
31 changes: 22 additions & 9 deletions impeller/renderer/backend/vulkan/command_buffer_vk.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "impeller/base/validation.h"
#include "impeller/renderer/backend/vulkan/blit_pass_vk.h"
#include "impeller/renderer/backend/vulkan/command_encoder_vk.h"
#include "impeller/renderer/backend/vulkan/compute_pass_vk.h"
#include "impeller/renderer/backend/vulkan/context_vk.h"
#include "impeller/renderer/backend/vulkan/formats_vk.h"
#include "impeller/renderer/backend/vulkan/render_pass_vk.h"
Expand Down Expand Up @@ -47,12 +48,13 @@ const std::shared_ptr<CommandEncoderVK>& CommandBufferVK::GetEncoder() const {
}

bool CommandBufferVK::OnSubmitCommands(CompletionCallback callback) {
const auto submit = encoder_->Submit();
if (callback) {
callback(submit ? CommandBuffer::Status::kCompleted
: CommandBuffer::Status::kError);
if (!callback) {
return encoder_->Submit();
}
return submit;
return encoder_->Submit([callback](bool submitted) {
callback(submitted ? CommandBuffer::Status::kCompleted
: CommandBuffer::Status::kError);
});
}

void CommandBufferVK::OnWaitUntilScheduled() {}
Expand All @@ -65,7 +67,7 @@ std::shared_ptr<RenderPass> CommandBufferVK::OnCreateRenderPass(
}
auto pass = std::shared_ptr<RenderPassVK>(new RenderPassVK(context, //
target, //
encoder_ //)
encoder_ //
));
if (!pass->IsValid()) {
return nullptr;
Expand All @@ -85,9 +87,20 @@ std::shared_ptr<BlitPass> CommandBufferVK::OnCreateBlitPass() const {
}

std::shared_ptr<ComputePass> CommandBufferVK::OnCreateComputePass() const {
// TODO(110622): Wire up compute passes in Vulkan.
IMPELLER_UNIMPLEMENTED;
return nullptr;
if (!IsValid()) {
return nullptr;
}
auto context = context_.lock();
if (!context) {
return nullptr;
}
auto pass = std::shared_ptr<ComputePassVK>(new ComputePassVK(context, //
encoder_ //
));
if (!pass->IsValid()) {
return nullptr;
}
return pass;
}

} // namespace impeller
26 changes: 21 additions & 5 deletions impeller/renderer/backend/vulkan/command_encoder_vk.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,13 +124,23 @@ bool CommandEncoderVK::IsValid() const {
return is_valid_;
}

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

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's being done below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fail_callback is used in the ScopedCleanupClosure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I think I see what you mean.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

if (!IsValid()) {
if (fail_callback) {
callback(false);
}
return false;
}

// Success or failure, you only get to submit once.
fml::ScopedCleanupClosure reset([&]() { Reset(); });
fml::ScopedCleanupClosure reset([&]() {
if (fail_callback) {
callback(false);
}
Reset();
});

InsertDebugMarker("QueueSubmit");

Expand All @@ -155,10 +165,16 @@ bool CommandEncoderVK::Submit() {
return false;
}

// Submit will proceed, call callback with true when it is done and do not
// call when `reset` is collected.
fail_callback = false;

return fence_waiter_->AddFence(
std::move(fence), [tracked_objects = std::move(tracked_objects_)] {
// Nothing to do, we just drop the tracked
// objects on the floor.
std::move(fence),
[callback, tracked_objects = std::move(tracked_objects_)] {
if (callback) {
callback(true);
}
});
}

Expand Down
5 changes: 4 additions & 1 deletion impeller/renderer/backend/vulkan/command_encoder_vk.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#pragma once

#include <functional>
#include <optional>
#include <set>

Expand Down Expand Up @@ -32,11 +33,13 @@ class FenceWaiterVK;

class CommandEncoderVK {
public:
using SubmitCallback = std::function<void(bool)>;

~CommandEncoderVK();

bool IsValid() const;

bool Submit();
bool Submit(SubmitCallback callback = {});

bool Track(std::shared_ptr<SharedObjectVK> object);

Expand Down
Loading