Skip to content

Commit

Permalink
[Impeller] dont cache failed render target allocations. (#45895)
Browse files Browse the repository at this point in the history
Otherwise we'll null de-ref when trying to create a texture with the same descriptor later on.
  • Loading branch information
jonahwilliams authored and harryterkelsen committed Oct 23, 2023
1 parent 1216347 commit 5c80851
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 0 deletions.
4 changes: 4 additions & 0 deletions impeller/entity/render_target_cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,16 @@ std::shared_ptr<Texture> RenderTargetCache::CreateTexture(

for (auto& td : texture_data_) {
const auto other_desc = td.texture->GetTextureDescriptor();
FML_DCHECK(td.texture != nullptr);
if (!td.used_this_frame && desc == other_desc) {
td.used_this_frame = true;
return td.texture;
}
}
auto result = RenderTargetAllocator::CreateTexture(desc);
if (result == nullptr) {
return result;
}
texture_data_.push_back(
TextureData{.used_this_frame = true, .texture = result});
return result;
Expand Down
23 changes: 23 additions & 0 deletions impeller/entity/render_target_cache_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,21 @@ class TestAllocator : public Allocator {

std::shared_ptr<DeviceBuffer> OnCreateBuffer(
const DeviceBufferDescriptor& desc) override {
if (should_fail) {
return nullptr;
}
return std::make_shared<MockDeviceBuffer>(desc);
};

virtual std::shared_ptr<Texture> OnCreateTexture(
const TextureDescriptor& desc) override {
if (should_fail) {
return nullptr;
}
return std::make_shared<MockTexture>(desc);
};

bool should_fail = false;
};

TEST(RenderTargetCacheTest, CachesUsedTexturesAcrossFrames) {
Expand Down Expand Up @@ -62,5 +70,20 @@ TEST(RenderTargetCacheTest, CachesUsedTexturesAcrossFrames) {
ASSERT_EQ(render_target_cache.CachedTextureCount(), 1u);
}

TEST(RenderTargetCacheTest, DoesNotPersistFailedAllocations) {
auto allocator = std::make_shared<TestAllocator>();
auto render_target_cache = RenderTargetCache(allocator);
auto desc = TextureDescriptor{
.format = PixelFormat::kR8G8B8A8UNormInt,
.size = ISize(100, 100),
.usage = static_cast<TextureUsageMask>(TextureUsage::kRenderTarget)};

render_target_cache.Start();
allocator->should_fail = true;

ASSERT_EQ(render_target_cache.CreateTexture(desc), nullptr);
ASSERT_EQ(render_target_cache.CachedTextureCount(), 0u);
}

} // namespace testing
} // namespace impeller

0 comments on commit 5c80851

Please sign in to comment.