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

Commit c408429

Browse files
authored
[Impeller] Fixed blit command missing tracking and added mock vulkan for tests (#41408)
Adds test for #41347 and adds an extra fix for flutter/flutter#125147. Testing this is possible because I've mocked out all of vulkan. We can expand on this in the future we want to be more robust. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See [testing the engine] for instructions on writing and running engine tests. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I signed the [CLA]. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style [testing the engine]: https://github.com/flutter/flutter/wiki/Testing-the-engine [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat
1 parent 34ece7a commit c408429

File tree

9 files changed

+431
-1
lines changed

9 files changed

+431
-1
lines changed

ci/licenses_golden/excluded_files

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,8 @@
144144
../../../flutter/impeller/golden_tests_harvester/test
145145
../../../flutter/impeller/image/README.md
146146
../../../flutter/impeller/playground
147+
../../../flutter/impeller/renderer/backend/vulkan/blit_command_vk_unittests.cc
148+
../../../flutter/impeller/renderer/backend/vulkan/test
147149
../../../flutter/impeller/renderer/capabilities_unittests.cc
148150
../../../flutter/impeller/renderer/compute_subgroup_unittests.cc
149151
../../../flutter/impeller/renderer/compute_unittests.cc

impeller/BUILD.gn

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,10 @@ impeller_component("impeller_unittests") {
9797
]
9898
}
9999

100+
if (impeller_enable_vulkan) {
101+
deps += [ "//flutter/impeller/renderer/backend/vulkan:vulkan_unittests" ]
102+
}
103+
100104
if (impeller_enable_compute) {
101105
deps += [ "renderer:compute_tessellation_unittests" ]
102106
}

impeller/renderer/backend/vulkan/BUILD.gn

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,19 @@
44

55
import("../../../tools/impeller.gni")
66

7+
impeller_component("vulkan_unittests") {
8+
testonly = true
9+
sources = [
10+
"blit_command_vk_unittests.cc",
11+
"test/mock_vulkan.cc",
12+
"test/mock_vulkan.h",
13+
]
14+
deps = [
15+
":vulkan",
16+
"//flutter/testing:testing_lib",
17+
]
18+
}
19+
720
impeller_component("vulkan") {
821
sources = [
922
"allocator_vk.cc",

impeller/renderer/backend/vulkan/blit_command_vk.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ bool BlitCopyTextureToBufferCommandVK::Encode(CommandEncoderVK& encoder) const {
100100
// cast source and destination to TextureVK
101101
const auto& src = TextureVK::Cast(*source);
102102

103-
if (!encoder.Track(source)) {
103+
if (!encoder.Track(source) || !encoder.Track(destination)) {
104104
return false;
105105
}
106106

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
// Copyright 2013 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
#include "flutter/testing/testing.h"
6+
#include "impeller/renderer/backend/vulkan/blit_command_vk.h"
7+
#include "impeller/renderer/backend/vulkan/command_encoder_vk.h"
8+
#include "impeller/renderer/backend/vulkan/test/mock_vulkan.h"
9+
10+
namespace impeller {
11+
namespace testing {
12+
13+
TEST(BlitCommandVkTest, BlitCopyTextureToTextureCommandVK) {
14+
auto context = CreateMockVulkanContext();
15+
auto pool = CommandPoolVK::GetThreadLocal(context.get());
16+
CommandEncoderVK encoder(context->GetDevice(), context->GetGraphicsQueue(),
17+
pool, context->GetFenceWaiter());
18+
BlitCopyTextureToTextureCommandVK cmd;
19+
cmd.source = context->GetResourceAllocator()->CreateTexture({
20+
.size = ISize(100, 100),
21+
});
22+
cmd.destination = context->GetResourceAllocator()->CreateTexture({
23+
.size = ISize(100, 100),
24+
});
25+
bool result = cmd.Encode(encoder);
26+
EXPECT_TRUE(result);
27+
EXPECT_TRUE(encoder.IsTracking(cmd.source));
28+
EXPECT_TRUE(encoder.IsTracking(cmd.destination));
29+
}
30+
31+
TEST(BlitCommandVkTest, BlitCopyTextureToBufferCommandVK) {
32+
auto context = CreateMockVulkanContext();
33+
auto pool = CommandPoolVK::GetThreadLocal(context.get());
34+
CommandEncoderVK encoder(context->GetDevice(), context->GetGraphicsQueue(),
35+
pool, context->GetFenceWaiter());
36+
BlitCopyTextureToBufferCommandVK cmd;
37+
cmd.source = context->GetResourceAllocator()->CreateTexture({
38+
.size = ISize(100, 100),
39+
});
40+
cmd.destination = context->GetResourceAllocator()->CreateBuffer({
41+
.size = 1,
42+
});
43+
bool result = cmd.Encode(encoder);
44+
EXPECT_TRUE(result);
45+
EXPECT_TRUE(encoder.IsTracking(cmd.source));
46+
EXPECT_TRUE(encoder.IsTracking(cmd.destination));
47+
}
48+
49+
TEST(BlitCommandVkTest, BlitGenerateMipmapCommandVK) {
50+
auto context = CreateMockVulkanContext();
51+
auto pool = CommandPoolVK::GetThreadLocal(context.get());
52+
CommandEncoderVK encoder(context->GetDevice(), context->GetGraphicsQueue(),
53+
pool, context->GetFenceWaiter());
54+
BlitGenerateMipmapCommandVK cmd;
55+
cmd.texture = context->GetResourceAllocator()->CreateTexture({
56+
.size = ISize(100, 100),
57+
.mip_count = 2,
58+
});
59+
bool result = cmd.Encode(encoder);
60+
EXPECT_TRUE(result);
61+
EXPECT_TRUE(encoder.IsTracking(cmd.texture));
62+
}
63+
64+
} // namespace testing
65+
} // namespace impeller

impeller/renderer/backend/vulkan/command_encoder_vk.cc

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,13 +58,27 @@ class TrackedObjectsVK {
5858
tracked_buffers_.insert(std::move(buffer));
5959
}
6060

61+
bool IsTracking(const std::shared_ptr<const DeviceBuffer>& buffer) const {
62+
if (!buffer) {
63+
return false;
64+
}
65+
return tracked_buffers_.find(buffer) != tracked_buffers_.end();
66+
}
67+
6168
void Track(std::shared_ptr<const TextureSourceVK> texture) {
6269
if (!texture) {
6370
return;
6471
}
6572
tracked_textures_.insert(std::move(texture));
6673
}
6774

75+
bool IsTracking(const std::shared_ptr<const TextureSourceVK>& texture) const {
76+
if (!texture) {
77+
return false;
78+
}
79+
return tracked_textures_.find(texture) != tracked_textures_.end();
80+
}
81+
6882
vk::CommandBuffer GetCommandBuffer() const { return *buffer_; }
6983

7084
DescriptorPoolVK& GetDescriptorPool() { return desc_pool_; }
@@ -172,6 +186,14 @@ bool CommandEncoderVK::Track(std::shared_ptr<const DeviceBuffer> buffer) {
172186
return true;
173187
}
174188

189+
bool CommandEncoderVK::IsTracking(
190+
const std::shared_ptr<const DeviceBuffer>& buffer) const {
191+
if (!IsValid()) {
192+
return false;
193+
}
194+
return tracked_objects_->IsTracking(buffer);
195+
}
196+
175197
bool CommandEncoderVK::Track(std::shared_ptr<const TextureSourceVK> texture) {
176198
if (!IsValid()) {
177199
return false;
@@ -190,6 +212,16 @@ bool CommandEncoderVK::Track(const std::shared_ptr<const Texture>& texture) {
190212
return Track(TextureVK::Cast(*texture).GetTextureSource());
191213
}
192214

215+
bool CommandEncoderVK::IsTracking(
216+
const std::shared_ptr<const Texture>& texture) const {
217+
if (!IsValid()) {
218+
return false;
219+
}
220+
std::shared_ptr<const TextureSourceVK> source =
221+
TextureVK::Cast(*texture).GetTextureSource();
222+
return tracked_objects_->IsTracking(source);
223+
}
224+
193225
std::optional<vk::DescriptorSet> CommandEncoderVK::AllocateDescriptorSet(
194226
const vk::DescriptorSetLayout& layout) {
195227
if (!IsValid()) {

impeller/renderer/backend/vulkan/command_encoder_vk.h

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,12 @@
1616

1717
namespace impeller {
1818

19+
namespace testing {
20+
class BlitCommandVkTest_BlitCopyTextureToTextureCommandVK_Test;
21+
class BlitCommandVkTest_BlitCopyTextureToBufferCommandVK_Test;
22+
class BlitCommandVkTest_BlitGenerateMipmapCommandVK_Test;
23+
} // namespace testing
24+
1925
class ContextVK;
2026
class DeviceBuffer;
2127
class Texture;
@@ -35,8 +41,12 @@ class CommandEncoderVK {
3541

3642
bool Track(std::shared_ptr<const DeviceBuffer> buffer);
3743

44+
bool IsTracking(const std::shared_ptr<const DeviceBuffer>& texture) const;
45+
3846
bool Track(const std::shared_ptr<const Texture>& texture);
3947

48+
bool IsTracking(const std::shared_ptr<const Texture>& texture) const;
49+
4050
bool Track(std::shared_ptr<const TextureSourceVK> texture);
4151

4252
vk::CommandBuffer GetCommandBuffer() const;
@@ -52,6 +62,12 @@ class CommandEncoderVK {
5262

5363
private:
5464
friend class ContextVK;
65+
friend class ::impeller::testing::
66+
BlitCommandVkTest_BlitCopyTextureToTextureCommandVK_Test;
67+
friend class ::impeller::testing::
68+
BlitCommandVkTest_BlitCopyTextureToBufferCommandVK_Test;
69+
friend class ::impeller::testing::
70+
BlitCommandVkTest_BlitGenerateMipmapCommandVK_Test;
5571

5672
vk::Device device_ = {};
5773
std::shared_ptr<QueueVK> queue_;

0 commit comments

Comments
 (0)