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

[Impeller] Move BufferView/Texture/Sampler when binding. #48628

Merged
auto-submit merged 2 commits into from
Dec 4, 2023
Merged

[Impeller] Move BufferView/Texture/Sampler when binding. #48628

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7834161
[Impeller] move buffer_view/texture/sampler when binding.
Dec 4, 2023
11a60e7
++
Dec 4, 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
6 changes: 3 additions & 3 deletions impeller/core/resource_binder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,13 @@ struct ResourceBinder {
virtual bool BindResource(ShaderStage stage,
const ShaderUniformSlot& slot,
const ShaderMetadata& metadata,
const BufferView& view) = 0;
BufferView view) = 0;

virtual bool BindResource(ShaderStage stage,
const SampledImageSlot& slot,
const ShaderMetadata& metadata,
const std::shared_ptr<const Texture>& texture,
const std::shared_ptr<const Sampler>& sampler) = 0;
std::shared_ptr<const Texture> texture,
std::shared_ptr<const Sampler> sampler) = 0;
};

} // namespace impeller
26 changes: 13 additions & 13 deletions impeller/renderer/command.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,23 +33,23 @@ BufferView Command::GetVertexBuffer() const {
bool Command::BindResource(ShaderStage stage,
const ShaderUniformSlot& slot,
const ShaderMetadata& metadata,
const BufferView& view) {
return DoBindResource(stage, slot, &metadata, view);
BufferView view) {
return DoBindResource(stage, slot, &metadata, std::move(view));
}

bool Command::BindResource(
ShaderStage stage,
const ShaderUniformSlot& slot,
const std::shared_ptr<const ShaderMetadata>& metadata,
const BufferView& view) {
return DoBindResource(stage, slot, metadata, view);
BufferView view) {
return DoBindResource(stage, slot, metadata, std::move(view));
}

template <class T>
bool Command::DoBindResource(ShaderStage stage,
const ShaderUniformSlot& slot,
const T metadata,
const BufferView& view) {
BufferView view) {
FML_DCHECK(slot.ext_res_0 != VertexDescriptor::kReservedVertexBufferIndex);
if (!view) {
return false;
Expand All @@ -58,11 +58,11 @@ bool Command::DoBindResource(ShaderStage stage,
switch (stage) {
case ShaderStage::kVertex:
vertex_bindings.buffers[slot.ext_res_0] = {
.slot = slot, .view = BufferResource(metadata, view)};
.slot = slot, .view = BufferResource(metadata, std::move(view))};
Copy link
Member

@gaaclarke gaaclarke Dec 4, 2023

Choose a reason for hiding this comment

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

If I'm reading the code correctly, this used to be a copy here, but now is a move which should be faster.

return true;
case ShaderStage::kFragment:
fragment_bindings.buffers[slot.ext_res_0] = {
.slot = slot, .view = BufferResource(metadata, view)};
.slot = slot, .view = BufferResource(metadata, std::move(view))};
Copy link
Member

@gaaclarke gaaclarke Dec 4, 2023

Choose a reason for hiding this comment

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

If you accidentally omit the std::move here, we get a linter failure right?

Copy link
Contributor Author

@jonahwilliams jonahwilliams Dec 4, 2023

Choose a reason for hiding this comment

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

We do not :(

return true;
case ShaderStage::kCompute:
VALIDATION_LOG << "Use ComputeCommands for compute shader stages.";
Expand All @@ -78,8 +78,8 @@ bool Command::DoBindResource(ShaderStage stage,
bool Command::BindResource(ShaderStage stage,
const SampledImageSlot& slot,
const ShaderMetadata& metadata,
const std::shared_ptr<const Texture>& texture,
const std::shared_ptr<const Sampler>& sampler) {
std::shared_ptr<const Texture> texture,
std::shared_ptr<const Sampler> sampler) {
if (!sampler || !sampler->IsValid()) {
return false;
}
Expand All @@ -94,15 +94,15 @@ bool Command::BindResource(ShaderStage stage,
case ShaderStage::kVertex:
vertex_bindings.sampled_images[slot.sampler_index] = TextureAndSampler{
.slot = slot,
.texture = {&metadata, texture},
.sampler = {&metadata, sampler},
.texture = {&metadata, std::move(texture)},
.sampler = {&metadata, std::move(sampler)},
};
return true;
case ShaderStage::kFragment:
fragment_bindings.sampled_images[slot.sampler_index] = TextureAndSampler{
.slot = slot,
.texture = {&metadata, texture},
.sampler = {&metadata, sampler},
.texture = {&metadata, std::move(texture)},
.sampler = {&metadata, std::move(sampler)},
};
return true;
case ShaderStage::kCompute:
Expand Down
10 changes: 5 additions & 5 deletions impeller/renderer/command.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,19 +189,19 @@ struct Command : public ResourceBinder {
bool BindResource(ShaderStage stage,
const ShaderUniformSlot& slot,
const ShaderMetadata& metadata,
const BufferView& view) override;
BufferView view) override;

bool BindResource(ShaderStage stage,
const ShaderUniformSlot& slot,
const std::shared_ptr<const ShaderMetadata>& metadata,
const BufferView& view);
BufferView view);

// |ResourceBinder|
bool BindResource(ShaderStage stage,
const SampledImageSlot& slot,
const ShaderMetadata& metadata,
const std::shared_ptr<const Texture>& texture,
const std::shared_ptr<const Sampler>& sampler) override;
std::shared_ptr<const Texture> texture,
std::shared_ptr<const Sampler> sampler) override;

BufferView GetVertexBuffer() const;

Expand All @@ -212,7 +212,7 @@ struct Command : public ResourceBinder {
bool DoBindResource(ShaderStage stage,
const ShaderUniformSlot& slot,
T metadata,
const BufferView& view);
BufferView view);
};

} // namespace impeller
20 changes: 10 additions & 10 deletions impeller/renderer/compute_command.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ namespace impeller {
bool ComputeCommand::BindResource(ShaderStage stage,
const ShaderUniformSlot& slot,
const ShaderMetadata& metadata,
const BufferView& view) {
BufferView view) {
if (stage != ShaderStage::kCompute) {
VALIDATION_LOG << "Use Command for non-compute shader stages.";
return false;
Expand All @@ -23,16 +23,16 @@ bool ComputeCommand::BindResource(ShaderStage stage,
return false;
}

bindings.buffers[slot.ext_res_0] = {.slot = slot, .view = {&metadata, view}};
bindings.buffers[slot.ext_res_0] = {.slot = slot,
.view = {&metadata, std::move(view)}};
return true;
}

bool ComputeCommand::BindResource(
ShaderStage stage,
const SampledImageSlot& slot,
const ShaderMetadata& metadata,
const std::shared_ptr<const Texture>& texture,
const std::shared_ptr<const Sampler>& sampler) {
bool ComputeCommand::BindResource(ShaderStage stage,
const SampledImageSlot& slot,
const ShaderMetadata& metadata,
std::shared_ptr<const Texture> texture,
std::shared_ptr<const Sampler> sampler) {
if (stage != ShaderStage::kCompute) {
VALIDATION_LOG << "Use Command for non-compute shader stages.";
return false;
Expand All @@ -49,8 +49,8 @@ bool ComputeCommand::BindResource(

bindings.sampled_images[slot.sampler_index] = TextureAndSampler{
.slot = slot,
.texture = {&metadata, texture},
.sampler = {&metadata, sampler},
.texture = {&metadata, std::move(texture)},
.sampler = {&metadata, std::move(sampler)},
};

return false;
Expand Down
6 changes: 3 additions & 3 deletions impeller/renderer/compute_command.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,14 @@ struct ComputeCommand : public ResourceBinder {
bool BindResource(ShaderStage stage,
const ShaderUniformSlot& slot,
const ShaderMetadata& metadata,
const BufferView& view) override;
BufferView view) override;

// |ResourceBinder|
bool BindResource(ShaderStage stage,
const SampledImageSlot& slot,
const ShaderMetadata& metadata,
const std::shared_ptr<const Texture>& texture,
const std::shared_ptr<const Sampler>& sampler) override;
std::shared_ptr<const Texture> texture,
std::shared_ptr<const Sampler> sampler) override;

constexpr explicit operator bool() const {
return pipeline && pipeline->IsValid();
Expand Down