Skip to content

Commit

Permalink
Revert "Collect textures from toImageSync safely (flutter#35073)" (fl…
Browse files Browse the repository at this point in the history
…utter#35281)

This reverts commit 4ac52f2.
  • Loading branch information
dnfield authored Aug 9, 2022
1 parent 29aea24 commit 3f7ccb2
Show file tree
Hide file tree
Showing 27 changed files with 257 additions and 602 deletions.
30 changes: 0 additions & 30 deletions common/graphics/texture.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,6 @@

namespace flutter {

ContextListener::ContextListener() = default;

ContextListener::~ContextListener() = default;

Texture::Texture(int64_t id) : id_(id) {}

Texture::~Texture() = default;
Expand All @@ -23,12 +19,6 @@ void TextureRegistry::RegisterTexture(std::shared_ptr<Texture> texture) {
mapping_[texture->Id()] = texture;
}

void TextureRegistry::RegisterContextListener(
uintptr_t id,
std::weak_ptr<ContextListener> image) {
images_[id] = std::move(image);
}

void TextureRegistry::UnregisterTexture(int64_t id) {
auto found = mapping_.find(id);
if (found == mapping_.end()) {
Expand All @@ -38,36 +28,16 @@ void TextureRegistry::UnregisterTexture(int64_t id) {
mapping_.erase(found);
}

void TextureRegistry::UnregisterContextListener(uintptr_t id) {
images_.erase(id);
}

void TextureRegistry::OnGrContextCreated() {
for (auto& it : mapping_) {
it.second->OnGrContextCreated();
}

for (const auto& [id, weak_image] : images_) {
if (auto image = weak_image.lock()) {
image->OnGrContextCreated();
} else {
images_.erase(id);
}
}
}

void TextureRegistry::OnGrContextDestroyed() {
for (auto& it : mapping_) {
it.second->OnGrContextDestroyed();
}

for (const auto& [id, weak_image] : images_) {
if (auto image = weak_image.lock()) {
image->OnGrContextDestroyed();
} else {
images_.erase(id);
}
}
}

std::shared_ptr<Texture> TextureRegistry::GetTexture(int64_t id) {
Expand Down
32 changes: 8 additions & 24 deletions common/graphics/texture.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,22 +16,7 @@ class GrDirectContext;

namespace flutter {

class ContextListener {
public:
ContextListener();
~ContextListener();

// Called from raster thread.
virtual void OnGrContextCreated() = 0;

// Called from raster thread.
virtual void OnGrContextDestroyed() = 0;

private:
FML_DISALLOW_COPY_AND_ASSIGN(ContextListener);
};

class Texture : public ContextListener {
class Texture {
public:
explicit Texture(int64_t id); // Called from UI or raster thread.
virtual ~Texture(); // Called from raster thread.
Expand All @@ -44,6 +29,12 @@ class Texture : public ContextListener {
const SkSamplingOptions& sampling,
const SkPaint* paint = nullptr) = 0;

// Called from raster thread.
virtual void OnGrContextCreated() = 0;

// Called from raster thread.
virtual void OnGrContextDestroyed() = 0;

// Called on raster thread.
virtual void MarkNewFrameAvailable() = 0;

Expand All @@ -54,6 +45,7 @@ class Texture : public ContextListener {

private:
int64_t id_;

FML_DISALLOW_COPY_AND_ASSIGN(Texture);
};

Expand All @@ -64,16 +56,9 @@ class TextureRegistry {
// Called from raster thread.
void RegisterTexture(std::shared_ptr<Texture> texture);

// Called from raster thread.
void RegisterContextListener(uintptr_t id,
std::weak_ptr<ContextListener> image);

// Called from raster thread.
void UnregisterTexture(int64_t id);

// Called from the raster thread.
void UnregisterContextListener(uintptr_t id);

// Called from raster thread.
std::shared_ptr<Texture> GetTexture(int64_t id);

Expand All @@ -85,7 +70,6 @@ class TextureRegistry {

private:
std::map<int64_t, std::shared_ptr<Texture>> mapping_;
std::map<uintptr_t, std::weak_ptr<ContextListener>> images_;

FML_DISALLOW_COPY_AND_ASSIGN(TextureRegistry);
};
Expand Down
11 changes: 4 additions & 7 deletions flow/compositor_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,11 @@ std::optional<SkRect> FrameDamage::ComputeClipRect(
}

CompositorContext::CompositorContext()
: texture_registry_(std::make_shared<TextureRegistry>()),
raster_time_(fixed_refresh_rate_updater_),
: raster_time_(fixed_refresh_rate_updater_),
ui_time_(fixed_refresh_rate_updater_) {}

CompositorContext::CompositorContext(Stopwatch::RefreshRateUpdater& updater)
: texture_registry_(std::make_shared<TextureRegistry>()),
raster_time_(updater),
ui_time_(updater) {}
: raster_time_(updater), ui_time_(updater) {}

CompositorContext::~CompositorContext() = default;

Expand Down Expand Up @@ -163,12 +160,12 @@ RasterStatus CompositorContext::ScopedFrame::Raster(
}

void CompositorContext::OnGrContextCreated() {
texture_registry_->OnGrContextCreated();
texture_registry_.OnGrContextCreated();
raster_cache_.Clear();
}

void CompositorContext::OnGrContextDestroyed() {
texture_registry_->OnGrContextDestroyed();
texture_registry_.OnGrContextDestroyed();
raster_cache_.Clear();
}

Expand Down
6 changes: 2 additions & 4 deletions flow/compositor_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -175,9 +175,7 @@ class CompositorContext {

RasterCache& raster_cache() { return raster_cache_; }

std::shared_ptr<TextureRegistry> texture_registry() {
return texture_registry_;
}
TextureRegistry& texture_registry() { return texture_registry_; }

const Stopwatch& raster_time() const { return raster_time_; }

Expand All @@ -187,7 +185,7 @@ class CompositorContext {

private:
RasterCache raster_cache_;
std::shared_ptr<TextureRegistry> texture_registry_;
TextureRegistry texture_registry_;
Stopwatch raster_time_;
Stopwatch ui_time_;
LayerSnapshotStore layer_snapshot_store_;
Expand Down
4 changes: 2 additions & 2 deletions flow/layers/layer.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ struct PrerollContext {
// These allow us to paint in the end of subtree Preroll.
const Stopwatch& raster_time;
const Stopwatch& ui_time;
std::shared_ptr<TextureRegistry> texture_registry;
TextureRegistry& texture_registry;
const bool checkerboard_offscreen_layers;
const float frame_device_pixel_ratio = 1.0f;

Expand Down Expand Up @@ -132,7 +132,7 @@ struct PaintContext {
ExternalViewEmbedder* view_embedder;
const Stopwatch& raster_time;
const Stopwatch& ui_time;
std::shared_ptr<TextureRegistry> texture_registry;
TextureRegistry& texture_registry;
const RasterCache* raster_cache;
const bool checkerboard_offscreen_layers;
const float frame_device_pixel_ratio = 1.0f;
Expand Down
5 changes: 3 additions & 2 deletions flow/layers/layer_tree.cc
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ sk_sp<DisplayList> LayerTree::Flatten(const SkRect& bounds) {

MutatorsStack unused_stack;
const FixedRefreshRateStopwatch unused_stopwatch;
TextureRegistry unused_texture_registry;
SkMatrix root_surface_transformation;
// No root surface transformation. So assume identity.
root_surface_transformation.reset();
Expand All @@ -182,7 +183,7 @@ sk_sp<DisplayList> LayerTree::Flatten(const SkRect& bounds) {
.surface_needs_readback = false,
.raster_time = unused_stopwatch,
.ui_time = unused_stopwatch,
.texture_registry = nullptr,
.texture_registry = unused_texture_registry,
.checkerboard_offscreen_layers = false,
.frame_device_pixel_ratio = device_pixel_ratio_
// clang-format on
Expand All @@ -201,7 +202,7 @@ sk_sp<DisplayList> LayerTree::Flatten(const SkRect& bounds) {
.view_embedder = nullptr,
.raster_time = unused_stopwatch,
.ui_time = unused_stopwatch,
.texture_registry = nullptr,
.texture_registry = unused_texture_registry,
.raster_cache = nullptr,
.checkerboard_offscreen_layers = false,
.frame_device_pixel_ratio = device_pixel_ratio_,
Expand Down
10 changes: 5 additions & 5 deletions flow/layers/layer_tree_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ TEST_F(LayerTreeTest, PrerollContextInitialization) {
MutatorsStack mock_mutators;
FixedRefreshRateStopwatch mock_raster_time;
FixedRefreshRateStopwatch mock_ui_time;
std::shared_ptr<TextureRegistry> mock_registry;
TextureRegistry mock_registry;

auto expect_defaults = [&mock_mutators, &mock_raster_time, &mock_ui_time,
&mock_registry](const PrerollContext& context) {
Expand All @@ -218,7 +218,7 @@ TEST_F(LayerTreeTest, PrerollContextInitialization) {

EXPECT_EQ(&context.raster_time, &mock_raster_time);
EXPECT_EQ(&context.ui_time, &mock_ui_time);
EXPECT_EQ(context.texture_registry.get(), mock_registry.get());
EXPECT_EQ(&context.texture_registry, &mock_registry);
EXPECT_EQ(context.checkerboard_offscreen_layers, false);
EXPECT_EQ(context.frame_device_pixel_ratio, 1.0f);

Expand All @@ -244,11 +244,11 @@ TEST_F(LayerTreeTest, PaintContextInitialization) {
// PaintContext that this test must be revisited and updated.
// If any fields get removed or replaced, then the expect_defaults closure
// will fail to compile, again bringing attention to updating this test.
EXPECT_EQ(sizeof(PaintContext), size_t(112));
EXPECT_EQ(sizeof(PaintContext), size_t(104));

FixedRefreshRateStopwatch mock_raster_time;
FixedRefreshRateStopwatch mock_ui_time;
std::shared_ptr<TextureRegistry> mock_registry;
TextureRegistry mock_registry;

auto expect_defaults = [&mock_raster_time, &mock_ui_time,
&mock_registry](const PaintContext& context) {
Expand All @@ -258,7 +258,7 @@ TEST_F(LayerTreeTest, PaintContextInitialization) {
EXPECT_EQ(context.view_embedder, nullptr);
EXPECT_EQ(&context.raster_time, &mock_raster_time);
EXPECT_EQ(&context.ui_time, &mock_ui_time);
EXPECT_EQ(context.texture_registry.get(), mock_registry.get());
EXPECT_EQ(&context.texture_registry, &mock_registry);
EXPECT_EQ(context.raster_cache, nullptr);
EXPECT_EQ(context.checkerboard_offscreen_layers, false);
EXPECT_EQ(context.frame_device_pixel_ratio, 1.0f);
Expand Down
3 changes: 2 additions & 1 deletion flow/layers/performance_overlay_layer_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ static void TestPerformanceOverlayLayerGold(int refresh_rate) {

ASSERT_TRUE(surface != nullptr);

flutter::TextureRegistry unused_texture_registry;
flutter::PaintContext paintContext = {
// clang-format off
.internal_nodes_canvas = nullptr,
Expand All @@ -66,7 +67,7 @@ static void TestPerformanceOverlayLayerGold(int refresh_rate) {
.view_embedder = nullptr,
.raster_time = mock_stopwatch,
.ui_time = mock_stopwatch,
.texture_registry = nullptr,
.texture_registry = unused_texture_registry,
.raster_cache = nullptr,
.checkerboard_offscreen_layers = false,
.frame_device_pixel_ratio = 1.0f,
Expand Down
4 changes: 1 addition & 3 deletions flow/layers/texture_layer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,7 @@ void TextureLayer::Paint(PaintContext& context) const {
FML_DCHECK(needs_painting(context));

std::shared_ptr<Texture> texture =
context.texture_registry
? context.texture_registry->GetTexture(texture_id_)
: nullptr;
context.texture_registry.GetTexture(texture_id_);
if (!texture) {
TRACE_EVENT_INSTANT0("flutter", "null texture");
return;
Expand Down
10 changes: 5 additions & 5 deletions flow/layers/texture_layer_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ TEST_F(TextureLayerTest, PaintingEmptyLayerDies) {
false, DlImageSampling::kNearestNeighbor);

// Ensure the texture is located by the Layer.
preroll_context()->texture_registry->RegisterTexture(mock_texture);
preroll_context()->texture_registry.RegisterTexture(mock_texture);

layer->Preroll(preroll_context(), SkMatrix());
EXPECT_EQ(layer->paint_bounds(), kEmptyRect);
Expand All @@ -62,7 +62,7 @@ TEST_F(TextureLayerTest, PaintBeforePrerollDies) {
layer_offset, layer_size, texture_id, false, DlImageSampling::kLinear);

// Ensure the texture is located by the Layer.
preroll_context()->texture_registry->RegisterTexture(mock_texture);
preroll_context()->texture_registry.RegisterTexture(mock_texture);

EXPECT_DEATH_IF_SUPPORTED(layer->Paint(paint_context()),
"needs_painting\\(context\\)");
Expand All @@ -78,7 +78,7 @@ TEST_F(TextureLayerTest, PaintingWithLinearSampling) {
layer_offset, layer_size, texture_id, false, DlImageSampling::kLinear);

// Ensure the texture is located by the Layer.
preroll_context()->texture_registry->RegisterTexture(mock_texture);
preroll_context()->texture_registry.RegisterTexture(mock_texture);

layer->Preroll(preroll_context(), SkMatrix());
EXPECT_EQ(layer->paint_bounds(),
Expand Down Expand Up @@ -124,12 +124,12 @@ TEST_F(TextureLayerTest, OpacityInheritance) {
layer_offset, layer_size, texture_id, false, DlImageSampling::kLinear);

// Ensure the texture is located by the Layer.
preroll_context()->texture_registry->RegisterTexture(mock_texture);
preroll_context()->texture_registry.RegisterTexture(mock_texture);

// The texture layer always reports opacity compatibility.
PrerollContext* context = preroll_context();
context->subtree_can_inherit_opacity = false;
context->texture_registry->RegisterTexture(mock_texture);
context->texture_registry.RegisterTexture(mock_texture);
layer->Preroll(context, SkMatrix::I());
EXPECT_TRUE(context->subtree_can_inherit_opacity);

Expand Down
Loading

0 comments on commit 3f7ccb2

Please sign in to comment.