Skip to content

Commit

Permalink
Gracefully handle duplicated entries in PictureLayerImpl::SetPaintWor…
Browse files Browse the repository at this point in the history
…kletInputs()

Clear the cached entry to nullopt so that the next duplicated
entry will get a nullopt instead of an invalid PaintRecord.

(cherry picked from commit 5c80767)

Bug: 1442690
Change-Id: Id5089f36820d9deffba464f2b18594fb86123528
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4570147
Reviewed-by: Justin Novosad <junov@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1150362}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4583309
Auto-Submit: Xianzhu Wang <wangxianzhu@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/5790@{#250}
Cr-Branched-From: 1d71a33-refs/heads/main@{#1148114}
  • Loading branch information
wangxianzhu authored and Chromium LUCI CQ committed Jun 2, 2023
1 parent 2d0fd17 commit 0decd62
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 1 deletion.
3 changes: 3 additions & 0 deletions cc/layers/picture_layer_impl.cc
Expand Up @@ -2114,6 +2114,9 @@ void PictureLayerImpl::SetPaintWorkletInputs(
// Attempt to re-use an existing PaintRecord if possible.
new_records[input] = std::make_pair(
paint_image_id, std::move(paint_worklet_records_[input].second));
// The move constructor of absl::optional does not clear the source to
// nullopt.
paint_worklet_records_[input].second = absl::nullopt;
}
paint_worklet_records_.swap(new_records);

Expand Down
31 changes: 31 additions & 0 deletions cc/layers/picture_layer_impl_unittest.cc
Expand Up @@ -6260,6 +6260,37 @@ TEST_F(LegacySWPictureLayerImplTest, PaintWorkletInputs) {
EXPECT_TRUE(pending_layer()->GetPaintWorkletRecordMap().contains(input3));
}

TEST_F(LegacySWPictureLayerImplTest, PaintWorkletInputsIdenticalEntries) {
gfx::Size layer_bounds(1000, 1000);

auto recording_source = FakeRecordingSource::CreateRecordingSource(
gfx::Rect(layer_bounds), layer_bounds);
scoped_refptr<TestPaintWorkletInput> input =
base::MakeRefCounted<TestPaintWorkletInput>(gfx::SizeF(100, 100));
PaintImage image = CreatePaintWorkletPaintImage(input);
recording_source->add_draw_image(image, gfx::Point(100, 100));
recording_source->add_draw_image(image, gfx::Point(100, 100));
recording_source->Rerecord();

// All inputs should be registered on the pending layer.
SetupPendingTree(recording_source->CreateRasterSource(), gfx::Size(),
Region(gfx::Rect(layer_bounds)));
EXPECT_EQ(pending_layer()->GetPaintWorkletRecordMap().size(), 1u);
EXPECT_TRUE(pending_layer()->GetPaintWorkletRecordMap().contains(input));

PaintRecord record;
pending_layer()->SetPaintWorkletRecord(input, record);
pending_layer()->picture_layer_tiling_set()->RemoveAllTiles();
recording_source->Rerecord();
pending_layer()->SetRasterSource(recording_source->CreateRasterSource(),
Region());
EXPECT_EQ(pending_layer()->GetPaintWorkletRecordMap().size(), 1u);
auto it = pending_layer()->GetPaintWorkletRecordMap().find(input);
ASSERT_NE(it, pending_layer()->GetPaintWorkletRecordMap().end());
// For now the paint record is cleared for multiple identical inputs.
EXPECT_FALSE(it->second.second.has_value());
}

TEST_F(LegacySWPictureLayerImplTest, NoTilingsUsesScaleOne) {
auto render_pass = viz::CompositorRenderPass::Create();

Expand Down
7 changes: 6 additions & 1 deletion cc/paint/paint_record.h
Expand Up @@ -25,6 +25,8 @@ class CC_PAINT_EXPORT PaintRecord {
public:
PaintRecord();
~PaintRecord();
// After move, the source is invalid (with nullptr buffer_) and can't be
// used anymore.
PaintRecord(PaintRecord&&);
PaintRecord& operator=(PaintRecord&&);
PaintRecord(const PaintRecord&);
Expand All @@ -34,7 +36,10 @@ class CC_PAINT_EXPORT PaintRecord {
return buffer_->EqualsForTesting(*other.buffer_);
}

const PaintOpBuffer& buffer() const { return *buffer_; }
const PaintOpBuffer& buffer() const {
CHECK(buffer_);
return *buffer_;
}

size_t size() const { return buffer_->size(); }
bool empty() const { return buffer_->empty(); }
Expand Down

0 comments on commit 0decd62

Please sign in to comment.