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

[impeller] combine sampler and texture maps. #44990

Merged
merged 3 commits into from
Aug 23, 2023
Merged

[impeller] combine sampler and texture maps. #44990

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3bd3b72
[impeller] combine sampler and texture maps.
Aug 23, 2023
73672db
update gles
Aug 23, 2023
7f553e1
bind fragment stage to fragment bindings
Aug 23, 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
10 changes: 0 additions & 10 deletions impeller/core/resource_binder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,6 @@ struct ResourceBinder {
const ShaderMetadata& metadata,
const BufferView& view) = 0;

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

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

virtual bool BindResource(ShaderStage stage,
const SampledImageSlot& slot,
const ShaderMetadata& metadata,
Expand Down
17 changes: 7 additions & 10 deletions impeller/renderer/backend/gles/buffer_bindings_gles.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,15 +305,15 @@ bool BufferBindingsGLES::BindTextures(const ProcTableGLES& gl,
const Bindings& bindings,
ShaderStage stage) const {
size_t active_index = 0;
for (const auto& texture : bindings.textures) {
const auto& texture_gles = TextureGLES::Cast(*texture.second.resource);
if (texture.second.GetMetadata() == nullptr) {
for (const auto& data : bindings.sampled_images) {
const auto& texture_gles = TextureGLES::Cast(*data.second.texture.resource);
if (data.second.texture.GetMetadata() == nullptr) {
VALIDATION_LOG << "No metadata found for texture binding.";
return false;
}

const auto uniform_key =
CreateUniformMemberKey(texture.second.GetMetadata()->name);
CreateUniformMemberKey(data.second.texture.GetMetadata()->name);
auto uniform = uniform_locations_.find(uniform_key);
if (uniform == uniform_locations_.end()) {
VALIDATION_LOG << "Could not find uniform for key: " << uniform_key;
Expand Down Expand Up @@ -341,12 +341,9 @@ bool BufferBindingsGLES::BindTextures(const ProcTableGLES& gl,
/// If there is a sampler for the texture at the same index, configure the
/// bound texture using that sampler.
///
auto sampler = bindings.samplers.find(texture.first);
if (sampler != bindings.samplers.end()) {
const auto& sampler_gles = SamplerGLES::Cast(*sampler->second.resource);
if (!sampler_gles.ConfigureBoundTexture(texture_gles, gl)) {
return false;
}
const auto& sampler_gles = SamplerGLES::Cast(*data.second.sampler.resource);
if (!sampler_gles.ConfigureBoundTexture(texture_gles, gl)) {
return false;
}

//--------------------------------------------------------------------------
Expand Down
24 changes: 6 additions & 18 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 @@ -187,22 +187,13 @@ static bool Bind(ComputePassBindingsCache& pass,

static bool Bind(ComputePassBindingsCache& pass,
size_t bind_index,
const Sampler& sampler,
const Texture& texture) {
if (!texture.IsValid()) {
if (!sampler.IsValid() || !texture.IsValid()) {
return false;
}

pass.SetTexture(bind_index, TextureMTL::Cast(texture).GetMTLTexture());
return true;
}

static bool Bind(ComputePassBindingsCache& pass,
size_t bind_index,
const Sampler& sampler) {
if (!sampler.IsValid()) {
return false;
}

pass.SetSampler(bind_index, SamplerMTL::Cast(sampler).GetMTLSamplerState());
return true;
}
Expand Down Expand Up @@ -239,16 +230,13 @@ static bool Bind(ComputePassBindingsCache& pass,
}
}

for (const auto& texture : command.bindings.textures) {
if (!Bind(pass_bindings, texture.first, *texture.second.resource)) {
return false;
}
}
for (const auto& sampler : command.bindings.samplers) {
if (!Bind(pass_bindings, sampler.first, *sampler.second.resource)) {
for (const auto& data : command.bindings.sampled_images) {
if (!Bind(pass_bindings, data.first, *data.second.sampler.resource,
*data.second.texture.resource)) {
return false;
}
}

// TODO(dnfield): use feature detection to support non-uniform threadgroup
// sizes.
// https://github.com/flutter/flutter/issues/110619
Expand Down
29 changes: 7 additions & 22 deletions impeller/renderer/backend/metal/render_pass_mtl.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -379,8 +379,9 @@ static bool Bind(PassBindingsCache& pass,
static bool Bind(PassBindingsCache& pass,
ShaderStage stage,
size_t bind_index,
const Sampler& sampler,
const Texture& texture) {
if (!texture.IsValid()) {
if (!sampler.IsValid() || !texture.IsValid()) {
return false;
}

Expand All @@ -395,18 +396,8 @@ static bool Bind(PassBindingsCache& pass,
}

return pass.SetTexture(stage, bind_index,
TextureMTL::Cast(texture).GetMTLTexture());
}

static bool Bind(PassBindingsCache& pass,
ShaderStage stage,
size_t bind_index,
const Sampler& sampler) {
if (!sampler.IsValid()) {
return false;
}

return pass.SetSampler(stage, bind_index,
TextureMTL::Cast(texture).GetMTLTexture()) &&
pass.SetSampler(stage, bind_index,
SamplerMTL::Cast(sampler).GetMTLSamplerState());
}

Expand All @@ -422,15 +413,9 @@ static bool Bind(PassBindingsCache& pass,
return false;
}
}
for (const auto& texture : bindings.textures) {
if (!Bind(pass_bindings, stage, texture.first,
*texture.second.resource)) {
return false;
}
}
for (const auto& sampler : bindings.samplers) {
if (!Bind(pass_bindings, stage, sampler.first,
*sampler.second.resource)) {
for (const auto& data : bindings.sampled_images) {
if (!Bind(pass_bindings, stage, data.first, *data.second.sampler.resource,
*data.second.texture.resource)) {
return false;
}
}
Expand Down
16 changes: 6 additions & 10 deletions impeller/renderer/backend/vulkan/compute_pass_vk.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ static bool UpdateBindingLayouts(const Bindings& bindings,

barrier.new_layout = vk::ImageLayout::eShaderReadOnlyOptimal;

for (const auto& [_, texture] : bindings.textures) {
if (!TextureVK::Cast(*texture.resource).SetLayout(barrier)) {
for (const auto& [_, data] : bindings.sampled_images) {
if (!TextureVK::Cast(*data.texture.resource).SetLayout(barrier)) {
return false;
}
}
Expand Down Expand Up @@ -88,21 +88,17 @@ static bool AllocateAndBindDescriptorSets(const ContextVK& context,
&writes, //
&vk_desc_set //
](const Bindings& bindings) -> bool {
for (const auto& [index, sampler_handle] : bindings.samplers) {
if (bindings.textures.find(index) == bindings.textures.end()) {
return false;
}

auto texture = bindings.textures.at(index).resource;
for (const auto& [index, data] : bindings.sampled_images) {
auto texture = data.texture.resource;
const auto& texture_vk = TextureVK::Cast(*texture);
const SamplerVK& sampler = SamplerVK::Cast(*sampler_handle.resource);
const SamplerVK& sampler = SamplerVK::Cast(*data.sampler.resource);

if (!encoder.Track(texture) ||
!encoder.Track(sampler.GetSharedSampler())) {
return false;
}

const SampledImageSlot& slot = bindings.sampled_images.at(index);
const SampledImageSlot& slot = data.slot;

vk::DescriptorImageInfo image_info;
image_info.imageLayout = vk::ImageLayout::eShaderReadOnlyOptimal;
Expand Down
16 changes: 6 additions & 10 deletions impeller/renderer/backend/vulkan/render_pass_vk.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,8 +283,8 @@ static bool UpdateBindingLayouts(const Bindings& bindings,

barrier.new_layout = vk::ImageLayout::eShaderReadOnlyOptimal;

for (const auto& [_, texture] : bindings.textures) {
if (!TextureVK::Cast(*texture.resource).SetLayout(barrier)) {
for (const auto& [_, data] : bindings.sampled_images) {
if (!TextureVK::Cast(*data.texture.resource).SetLayout(barrier)) {
return false;
}
}
Expand Down Expand Up @@ -331,21 +331,17 @@ static bool AllocateAndBindDescriptorSets(const ContextVK& context,
&writes, //
&vk_desc_set //
](const Bindings& bindings) -> bool {
for (const auto& [index, sampler_handle] : bindings.samplers) {
if (bindings.textures.find(index) == bindings.textures.end()) {
return false;
}

auto texture = bindings.textures.at(index).resource;
for (const auto& [index, data] : bindings.sampled_images) {
auto texture = data.texture.resource;
const auto& texture_vk = TextureVK::Cast(*texture);
const SamplerVK& sampler = SamplerVK::Cast(*sampler_handle.resource);
const SamplerVK& sampler = SamplerVK::Cast(*data.sampler.resource);

if (!encoder.Track(texture) ||
!encoder.Track(sampler.GetSharedSampler())) {
return false;
}

const SampledImageSlot& slot = bindings.sampled_images.at(index);
const SampledImageSlot& slot = data.slot;

vk::DescriptorImageInfo image_info;
image_info.imageLayout = vk::ImageLayout::eShaderReadOnlyOptimal;
Expand Down
60 changes: 15 additions & 45 deletions impeller/renderer/command.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,53 +83,32 @@ bool Command::DoBindResource(ShaderStage stage,
bool Command::BindResource(ShaderStage stage,
const SampledImageSlot& slot,
const ShaderMetadata& metadata,
const std::shared_ptr<const Texture>& texture) {
if (!texture || !texture->IsValid()) {
return false;
}

if (!slot.HasTexture()) {
return true;
}

switch (stage) {
case ShaderStage::kVertex:
vertex_bindings.textures[slot.texture_index] = {&metadata, texture};
return true;
case ShaderStage::kFragment:
fragment_bindings.textures[slot.texture_index] = {&metadata, texture};
return true;
case ShaderStage::kCompute:
VALIDATION_LOG << "Use ComputeCommands for compute shader stages.";
case ShaderStage::kTessellationControl:
case ShaderStage::kTessellationEvaluation:
case ShaderStage::kUnknown:
return false;
}

return false;
}

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) {
if (!sampler || !sampler->IsValid()) {
return false;
}

if (!slot.HasSampler()) {
if (!texture || !texture->IsValid()) {
return false;
}
if (!slot.HasSampler() || !slot.HasTexture()) {
return true;
}

switch (stage) {
case ShaderStage::kVertex:
vertex_bindings.samplers[slot.sampler_index] = {&metadata, sampler};
vertex_bindings.sampled_images[slot.sampler_index] = slot;
vertex_bindings.sampled_images[slot.sampler_index] = TextureAndSampler{
.slot = slot,
.texture = {&metadata, texture},
.sampler = {&metadata, sampler},
};
return true;
case ShaderStage::kFragment:
fragment_bindings.samplers[slot.sampler_index] = {&metadata, sampler};
fragment_bindings.sampled_images[slot.sampler_index] = slot;
fragment_bindings.sampled_images[slot.sampler_index] = TextureAndSampler{
.slot = slot,
.texture = {&metadata, texture},
.sampler = {&metadata, sampler},
};
return true;
case ShaderStage::kCompute:
VALIDATION_LOG << "Use ComputeCommands for compute shader stages.";
Expand All @@ -142,13 +121,4 @@ bool Command::BindResource(ShaderStage stage,
return false;
}

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) {
return BindResource(stage, slot, metadata, texture) &&
BindResource(stage, slot, metadata, sampler);
}

} // namespace impeller
27 changes: 11 additions & 16 deletions impeller/renderer/command.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,17 @@ using BufferResource = Resource<BufferView>;
using TextureResource = Resource<std::shared_ptr<const Texture>>;
using SamplerResource = Resource<std::shared_ptr<const Sampler>>;

/// @brief combines the texture, sampler and sampler slot information.
struct TextureAndSampler {
Copy link
Member

@gaaclarke gaaclarke Aug 23, 2023

Choose a reason for hiding this comment

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

This doesn't make the memory usage smaller. The Command size is smaller, but the map entries will be bigger. The way to reduce memory would be to use std::variant so that each entry is just the max of the different sizes plus the type signifier.

I tried this in the past (main...gaaclarke:engine:command-thinning) and found that the overhead of switching on the std::variant ate up the savings from smaller memory. Plus, std::variants are super cumbersome to work with.

Copy link
Contributor Author

@jonahwilliams jonahwilliams Aug 23, 2023

Choose a reason for hiding this comment

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

we don't need a std::variant, and its not just about the memory usage. The cost of computing hashes (std::find in the profile) is eating up CPU time.

Copy link
Contributor

@dnfield dnfield Aug 23, 2023

Choose a reason for hiding this comment

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

Why not just go to vectors in this patch?

Copy link
Contributor Author

@jonahwilliams jonahwilliams Aug 23, 2023

Choose a reason for hiding this comment

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

baby steps :)

SampledImageSlot slot;
TextureResource texture;
SamplerResource sampler;
};

struct Bindings {
std::map<size_t, ShaderUniformSlot> uniforms;
std::map<size_t, SampledImageSlot> sampled_images;
std::map<size_t, TextureAndSampler> sampled_images;
std::map<size_t, BufferResource> buffers;
std::map<size_t, TextureResource> textures;
std::map<size_t, SamplerResource> samplers;
};

//------------------------------------------------------------------------------
Expand Down Expand Up @@ -184,18 +189,6 @@ struct Command : public ResourceBinder {
const std::shared_ptr<const ShaderMetadata>& metadata,
const BufferView& view);

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

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

// |ResourceBinder|
bool BindResource(ShaderStage stage,
const SampledImageSlot& slot,
Expand All @@ -205,7 +198,9 @@ struct Command : public ResourceBinder {

BufferView GetVertexBuffer() const;

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

private:
template <class T>
Expand Down
Loading