Skip to content

Commit a71c0c6

Browse files
[Impeller] Support access the descriptor of PipelineFuture directly (flutter#37415)
* [Impeller] Support access the descriptor of PipelineFuture directly * Tweak code * Clean code
1 parent a42b2d9 commit a71c0c6

15 files changed

+92
-81
lines changed

impeller/entity/contents/content_context.cc

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -219,11 +219,9 @@ ContentContext::ContentContext(std::shared_ptr<Context> context)
219219
yuv_to_rgb_filter_pipelines_[{}] =
220220
CreateDefaultPipeline<YUVToRGBFilterPipeline>(*context_);
221221

222-
// Pipelines that are variants of the base pipelines with custom descriptors.
223-
// TODO(98684): Rework this API to allow fetching the descriptor without
224-
// waiting for the pipeline to build.
225-
if (auto solid_fill_pipeline = solid_fill_pipelines_[{}]->WaitAndGet()) {
226-
auto clip_pipeline_descriptor = solid_fill_pipeline->GetDescriptor();
222+
if (solid_fill_pipelines_[{}]->GetDescriptor().has_value()) {
223+
auto clip_pipeline_descriptor =
224+
solid_fill_pipelines_[{}]->GetDescriptor().value();
227225
clip_pipeline_descriptor.SetLabel("Clip Pipeline");
228226
// Disable write to all color attachments.
229227
auto color_attachments =

impeller/entity/contents/runtime_effect_contents.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ bool RuntimeEffectContents::Render(const ContentContext& renderer,
126126
options.primitive_type = geometry_result.type;
127127
options.ApplyToPipelineDescriptor(desc);
128128

129-
auto pipeline = context->GetPipelineLibrary()->GetPipeline(desc).get();
129+
auto pipeline = context->GetPipelineLibrary()->GetPipeline(desc).Get();
130130
if (!pipeline) {
131131
VALIDATION_LOG << "Failed to get or create runtime effect pipeline.";
132132
return false;

impeller/playground/imgui/imgui_impl_impeller.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ bool ImGui_ImplImpeller_Init(
102102
}
103103

104104
bd->pipeline =
105-
context->GetPipelineLibrary()->GetPipeline(std::move(desc)).get();
105+
context->GetPipelineLibrary()->GetPipeline(std::move(desc)).Get();
106106
IM_ASSERT(bd->pipeline != nullptr && "Could not create ImGui pipeline.");
107107

108108
bd->sampler = context->GetSamplerLibrary()->GetSampler({});

impeller/renderer/backend/gles/pipeline_library_gles.cc

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -180,8 +180,9 @@ PipelineFuture<PipelineDescriptor> PipelineLibraryGLES::GetPipeline(
180180
}
181181

182182
if (!reactor_) {
183-
return RealizedFuture<std::shared_ptr<Pipeline<PipelineDescriptor>>>(
184-
nullptr);
183+
return {
184+
descriptor,
185+
RealizedFuture<std::shared_ptr<Pipeline<PipelineDescriptor>>>(nullptr)};
185186
}
186187

187188
auto vert_function = descriptor.GetEntrypointForStage(ShaderStage::kVertex);
@@ -190,14 +191,16 @@ PipelineFuture<PipelineDescriptor> PipelineLibraryGLES::GetPipeline(
190191
if (!vert_function || !frag_function) {
191192
VALIDATION_LOG
192193
<< "Could not find stage entrypoint functions in pipeline descriptor.";
193-
return RealizedFuture<std::shared_ptr<Pipeline<PipelineDescriptor>>>(
194-
nullptr);
194+
return {
195+
descriptor,
196+
RealizedFuture<std::shared_ptr<Pipeline<PipelineDescriptor>>>(nullptr)};
195197
}
196198

197199
auto promise = std::make_shared<
198200
std::promise<std::shared_ptr<Pipeline<PipelineDescriptor>>>>();
199-
auto future = PipelineFuture<PipelineDescriptor>{promise->get_future()};
200-
pipelines_[descriptor] = future;
201+
auto pipeline_future =
202+
PipelineFuture<PipelineDescriptor>{descriptor, promise->get_future()};
203+
pipelines_[descriptor] = pipeline_future;
201204
auto weak_this = weak_from_this();
202205

203206
auto result = reactor_->AddOperation(
@@ -243,19 +246,17 @@ PipelineFuture<PipelineDescriptor> PipelineLibraryGLES::GetPipeline(
243246
});
244247
FML_CHECK(result);
245248

246-
return future;
249+
return pipeline_future;
247250
}
248251

249252
// |PipelineLibrary|
250253
PipelineFuture<ComputePipelineDescriptor> PipelineLibraryGLES::GetPipeline(
251254
ComputePipelineDescriptor descriptor) {
252255
auto promise = std::make_shared<
253256
std::promise<std::shared_ptr<Pipeline<ComputePipelineDescriptor>>>>();
254-
auto future =
255-
PipelineFuture<ComputePipelineDescriptor>{promise->get_future()};
256257
// TODO(dnfield): implement compute for GLES.
257258
promise->set_value(nullptr);
258-
return future;
259+
return {descriptor, promise->get_future()};
259260
}
260261

261262
// |PipelineLibrary|

impeller/renderer/backend/metal/pipeline_library_mtl.mm

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -93,14 +93,16 @@
9393
}
9494

9595
if (!IsValid()) {
96-
return RealizedFuture<std::shared_ptr<Pipeline<PipelineDescriptor>>>(
97-
nullptr);
96+
return {
97+
descriptor,
98+
RealizedFuture<std::shared_ptr<Pipeline<PipelineDescriptor>>>(nullptr)};
9899
}
99100

100101
auto promise = std::make_shared<
101102
std::promise<std::shared_ptr<Pipeline<PipelineDescriptor>>>>();
102-
auto future = PipelineFuture<PipelineDescriptor>{promise->get_future()};
103-
pipelines_[descriptor] = future;
103+
auto pipeline_future =
104+
PipelineFuture<PipelineDescriptor>{descriptor, promise->get_future()};
105+
pipelines_[descriptor] = pipeline_future;
104106
auto weak_this = weak_from_this();
105107

106108
auto completion_handler =
@@ -132,7 +134,7 @@
132134
[device_ newRenderPipelineStateWithDescriptor:GetMTLRenderPipelineDescriptor(
133135
descriptor)
134136
completionHandler:completion_handler];
135-
return future;
137+
return pipeline_future;
136138
}
137139

138140
PipelineFuture<ComputePipelineDescriptor> PipelineLibraryMTL::GetPipeline(
@@ -143,15 +145,17 @@
143145
}
144146

145147
if (!IsValid()) {
146-
return RealizedFuture<std::shared_ptr<Pipeline<ComputePipelineDescriptor>>>(
147-
nullptr);
148+
return {
149+
descriptor,
150+
RealizedFuture<std::shared_ptr<Pipeline<ComputePipelineDescriptor>>>(
151+
nullptr)};
148152
}
149153

150154
auto promise = std::make_shared<
151155
std::promise<std::shared_ptr<Pipeline<ComputePipelineDescriptor>>>>();
152-
auto future =
153-
PipelineFuture<ComputePipelineDescriptor>{promise->get_future()};
154-
compute_pipelines_[descriptor] = future;
156+
auto pipeline_future = PipelineFuture<ComputePipelineDescriptor>{
157+
descriptor, promise->get_future()};
158+
compute_pipelines_[descriptor] = pipeline_future;
155159
auto weak_this = weak_from_this();
156160

157161
auto completion_handler =
@@ -185,7 +189,7 @@ new ComputePipelineMTL(weak_this,
185189
descriptor)
186190
options:MTLPipelineOptionNone
187191
completionHandler:completion_handler];
188-
return future;
192+
return pipeline_future;
189193
}
190194

191195
// |PipelineLibrary|

impeller/renderer/backend/vulkan/pipeline_library_vk.cc

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -61,14 +61,16 @@ PipelineFuture<PipelineDescriptor> PipelineLibraryVK::GetPipeline(
6161
}
6262

6363
if (!IsValid()) {
64-
return RealizedFuture<std::shared_ptr<Pipeline<PipelineDescriptor>>>(
65-
nullptr);
64+
return {
65+
descriptor,
66+
RealizedFuture<std::shared_ptr<Pipeline<PipelineDescriptor>>>(nullptr)};
6667
}
6768

6869
auto promise = std::make_shared<
6970
std::promise<std::shared_ptr<Pipeline<PipelineDescriptor>>>>();
70-
auto future = PipelineFuture<PipelineDescriptor>{promise->get_future()};
71-
pipelines_[descriptor] = future;
71+
auto pipeline_future =
72+
PipelineFuture<PipelineDescriptor>{descriptor, promise->get_future()};
73+
pipelines_[descriptor] = pipeline_future;
7274

7375
auto weak_this = weak_from_this();
7476

@@ -86,19 +88,17 @@ PipelineFuture<PipelineDescriptor> PipelineLibraryVK::GetPipeline(
8688
weak_this, descriptor, std::move(pipeline_create_info)));
8789
});
8890

89-
return future;
91+
return pipeline_future;
9092
}
9193

9294
// |PipelineLibrary|
9395
PipelineFuture<ComputePipelineDescriptor> PipelineLibraryVK::GetPipeline(
9496
ComputePipelineDescriptor descriptor) {
9597
auto promise = std::make_shared<
9698
std::promise<std::shared_ptr<Pipeline<ComputePipelineDescriptor>>>>();
97-
auto future =
98-
PipelineFuture<ComputePipelineDescriptor>{promise->get_future()};
9999
// TODO(dnfield): implement compute for GLES.
100100
promise->set_value(nullptr);
101-
return future;
101+
return {descriptor, promise->get_future()};
102102
}
103103

104104
static vk::AttachmentDescription CreatePlaceholderAttachmentDescription(

impeller/renderer/compute_pipeline_descriptor.h

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,4 @@ class ComputePipelineDescriptor final
5353
std::shared_ptr<const ShaderFunction> entrypoint_;
5454
};
5555

56-
using ComputePipelineMap = std::unordered_map<
57-
ComputePipelineDescriptor,
58-
std::shared_future<std::shared_ptr<Pipeline<ComputePipelineDescriptor>>>,
59-
ComparableHash<ComputePipelineDescriptor>,
60-
ComparableEqual<ComputePipelineDescriptor>>;
61-
6256
} // namespace impeller

impeller/renderer/compute_unittests.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ TEST_P(ComputeTest, CanCreateComputePass) {
2929
SamplePipelineBuilder::MakeDefaultPipelineDescriptor(*context);
3030
ASSERT_TRUE(pipeline_desc.has_value());
3131
auto compute_pipeline =
32-
context->GetPipelineLibrary()->GetPipeline(pipeline_desc).get();
32+
context->GetPipelineLibrary()->GetPipeline(pipeline_desc).Get();
3333
ASSERT_TRUE(compute_pipeline);
3434

3535
auto cmd_buffer = context->CreateCommandBuffer();

impeller/renderer/pipeline.cc

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
// found in the LICENSE file.
44

55
#include "impeller/renderer/pipeline.h"
6+
#include <optional>
67

78
#include "compute_pipeline_descriptor.h"
89
#include "impeller/base/promise.h"
@@ -24,8 +25,8 @@ PipelineFuture<PipelineDescriptor> CreatePipelineFuture(
2425
const Context& context,
2526
std::optional<PipelineDescriptor> desc) {
2627
if (!context.IsValid()) {
27-
return RealizedFuture<std::shared_ptr<Pipeline<PipelineDescriptor>>>(
28-
nullptr);
28+
return {desc, RealizedFuture<std::shared_ptr<Pipeline<PipelineDescriptor>>>(
29+
nullptr)};
2930
}
3031

3132
return context.GetPipelineLibrary()->GetPipeline(std::move(desc));
@@ -35,8 +36,10 @@ PipelineFuture<ComputePipelineDescriptor> CreatePipelineFuture(
3536
const Context& context,
3637
std::optional<ComputePipelineDescriptor> desc) {
3738
if (!context.IsValid()) {
38-
return RealizedFuture<std::shared_ptr<Pipeline<ComputePipelineDescriptor>>>(
39-
nullptr);
39+
return {
40+
desc,
41+
RealizedFuture<std::shared_ptr<Pipeline<ComputePipelineDescriptor>>>(
42+
nullptr)};
4043
}
4144

4245
return context.GetPipelineLibrary()->GetPipeline(std::move(desc));
@@ -51,7 +54,8 @@ template <typename T>
5154
PipelineFuture<T> Pipeline<T>::CreateVariant(
5255
std::function<void(T& desc)> descriptor_callback) const {
5356
if (!descriptor_callback) {
54-
return RealizedFuture<std::shared_ptr<Pipeline<T>>>(nullptr);
57+
return {std::nullopt,
58+
RealizedFuture<std::shared_ptr<Pipeline<T>>>(nullptr)};
5559
}
5660

5761
auto copied_desc = desc_;
@@ -62,7 +66,7 @@ PipelineFuture<T> Pipeline<T>::CreateVariant(
6266
if (!library) {
6367
VALIDATION_LOG << "The library from which this pipeline was created was "
6468
"already collected.";
65-
return RealizedFuture<std::shared_ptr<Pipeline<T>>>(nullptr);
69+
return {desc_, RealizedFuture<std::shared_ptr<Pipeline<T>>>(nullptr)};
6670
}
6771

6872
return library->GetPipeline(std::move(copied_desc));

impeller/renderer/pipeline.h

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,15 @@ class PipelineLibrary;
2020
template <typename PipelineDescriptor_>
2121
class Pipeline;
2222

23-
// TODO(csg): Using a simple future is sub-optimal since callers that want to
24-
// eagerly create and cache pipeline variants will have to await on the future
25-
// to get its pipeline descriptor (unless they have explicitly cached it). This
26-
// would be a concurrency pessimization.
27-
//
28-
// Use a struct that stores the future and the descriptor separately.
29-
template <class T>
30-
using PipelineFuture = std::shared_future<std::shared_ptr<Pipeline<T>>>;
23+
template <typename T>
24+
struct PipelineFuture {
25+
std::optional<T> descriptor;
26+
std::shared_future<std::shared_ptr<Pipeline<T>>> future;
27+
28+
const std::shared_ptr<Pipeline<T>> Get() const { return future.get(); }
29+
30+
bool IsValid() const { return future.valid(); }
31+
};
3132

3233
//------------------------------------------------------------------------------
3334
/// @brief Describes the fixed function and programmable aspects of
@@ -107,12 +108,16 @@ class RenderPipelineT {
107108
return pipeline_;
108109
}
109110
did_wait_ = true;
110-
if (pipeline_future_.valid()) {
111-
pipeline_ = pipeline_future_.get();
111+
if (pipeline_future_.IsValid()) {
112+
pipeline_ = pipeline_future_.Get();
112113
}
113114
return pipeline_;
114115
}
115116

117+
std::optional<PipelineDescriptor> GetDescriptor() const {
118+
return pipeline_future_.descriptor;
119+
}
120+
116121
private:
117122
PipelineFuture<PipelineDescriptor> pipeline_future_;
118123
std::shared_ptr<Pipeline<PipelineDescriptor>> pipeline_;
@@ -145,8 +150,8 @@ class ComputePipelineT {
145150
return pipeline_;
146151
}
147152
did_wait_ = true;
148-
if (pipeline_future_.valid()) {
149-
pipeline_ = pipeline_future_.get();
153+
if (pipeline_future_.IsValid()) {
154+
pipeline_ = pipeline_future_.Get();
150155
}
151156
return pipeline_;
152157
}

impeller/renderer/pipeline_descriptor.h

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -138,10 +138,4 @@ class PipelineDescriptor final : public Comparable<PipelineDescriptor> {
138138
PrimitiveType primitive_type_ = PrimitiveType::kTriangle;
139139
};
140140

141-
using PipelineMap = std::unordered_map<
142-
PipelineDescriptor,
143-
std::shared_future<std::shared_ptr<Pipeline<PipelineDescriptor>>>,
144-
ComparableHash<PipelineDescriptor>,
145-
ComparableEqual<PipelineDescriptor>>;
146-
147141
} // namespace impeller

impeller/renderer/pipeline_library.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ PipelineFuture<PipelineDescriptor> PipelineLibrary::GetPipeline(
1818
auto promise = std::make_shared<
1919
std::promise<std::shared_ptr<Pipeline<PipelineDescriptor>>>>();
2020
promise->set_value(nullptr);
21-
return promise->get_future();
21+
return {descriptor, promise->get_future()};
2222
}
2323

2424
PipelineFuture<ComputePipelineDescriptor> PipelineLibrary::GetPipeline(
@@ -29,7 +29,7 @@ PipelineFuture<ComputePipelineDescriptor> PipelineLibrary::GetPipeline(
2929
auto promise = std::make_shared<
3030
std::promise<std::shared_ptr<Pipeline<ComputePipelineDescriptor>>>>();
3131
promise->set_value(nullptr);
32-
return promise->get_future();
32+
return {descriptor, promise->get_future()};
3333
}
3434

3535
} // namespace impeller

impeller/renderer/pipeline_library.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,17 @@ namespace impeller {
1515

1616
class Context;
1717

18+
using PipelineMap = std::unordered_map<PipelineDescriptor,
19+
PipelineFuture<PipelineDescriptor>,
20+
ComparableHash<PipelineDescriptor>,
21+
ComparableEqual<PipelineDescriptor>>;
22+
23+
using ComputePipelineMap =
24+
std::unordered_map<ComputePipelineDescriptor,
25+
PipelineFuture<ComputePipelineDescriptor>,
26+
ComparableHash<ComputePipelineDescriptor>,
27+
ComparableEqual<ComputePipelineDescriptor>>;
28+
1829
class PipelineLibrary : public std::enable_shared_from_this<PipelineLibrary> {
1930
public:
2031
virtual ~PipelineLibrary();

0 commit comments

Comments
 (0)