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

[embedder] Allow for the backing stores to not be cached #22643

Closed
wants to merge 1 commit into from
Closed

[embedder] Allow for the backing stores to not be cached #22643

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 2 additions & 0 deletions shell/platform/embedder/embedder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -858,6 +858,8 @@ typedef struct {
/// Indicates if this backing store was updated since the last time it was
/// associated with a presented layer.
bool did_update;
/// Indicates whether this backing store can be cached and re-used.
bool is_cacheable;
Copy link
Member

Choose a reason for hiding this comment

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

Two issues:

  1. We ask our embedders to zero-initialize structs to their default values. This will make is so that layers that were previously cacheable (as they are now before this patch) will have their is_cacheable value set to false due to zero initialization. Consequently, if embedders update their engine, their layers will no longer be cached unless they update their code as well. This is a breaking change which we avoid. How about bool avoid_cache, bypass_cache, etc..?
  2. This entry is not at the bottom of the struct and will break the ABI. However, we are in a bind here as the union are not all pointers so we can't really put this at the bottom. However, we can add this flag to the FlutterLayer instead as that is a union of pointers.

Copy link
Contributor

Choose a reason for hiding this comment

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

Kaushik asked me to take over this PR.

I'm trying to figure a way around the problem with point 2, I'm not sure if there is an easy / clean way to add avoid_cache as a field to FlutterLayer since we only have the BackingStore / EmbedderRenderTarget at the point when we want to cache the render target.

Do you think it would be viable to add bool avoid_cache as a field in embedder_render_target.h and change FlutterBackingStoreCreateCallback to return two booleans (one for success, one for avoid_cache) such that we can set the avoid_cache in the render target after creating a backing store?

cc @iskakaushik

union {
/// The description of the OpenGL backing store.
FlutterOpenGLBackingStore open_gl;
Expand Down
9 changes: 7 additions & 2 deletions shell/platform/embedder/embedder_external_view_embedder.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

#include "flutter/shell/platform/embedder/embedder_layers.h"
#include "flutter/shell/platform/embedder/embedder_render_target.h"
#include "flutter/shell/platform/embedder/embedder_struct_macros.h"
#include "third_party/skia/include/gpu/GrDirectContext.h"

namespace flutter {
Expand Down Expand Up @@ -263,8 +264,12 @@ void EmbedderExternalViewEmbedder::SubmitFrame(
// Hold all rendered layers in the render target cache for one frame to
// see if they may be reused next frame.
for (auto& render_target : matched_render_targets) {
render_target_cache_.CacheRenderTarget(render_target.first,
std::move(render_target.second));
const FlutterBackingStore* backing_store =
render_target.second->GetBackingStore();
if (SAFE_ACCESS(backing_store, is_cacheable, true)) {
render_target_cache_.CacheRenderTarget(render_target.first,
std::move(render_target.second));
}
}

frame->Submit();
Expand Down
3 changes: 2 additions & 1 deletion shell/platform/embedder/tests/embedder_assertions.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,8 @@ inline bool operator==(const FlutterSoftwareBackingStore& a,
inline bool operator==(const FlutterBackingStore& a,
const FlutterBackingStore& b) {
if (!(a.struct_size == b.struct_size && a.user_data == b.user_data &&
a.type == b.type && a.did_update == b.did_update)) {
a.type == b.type && a.did_update == b.did_update &&
a.is_cacheable == b.is_cacheable)) {
return false;
}

Expand Down
3 changes: 3 additions & 0 deletions shell/platform/embedder/tests/embedder_test_backingstore_producer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ bool EmbedderTestBackingStoreProducer::CreateFramebuffer(

backing_store_out->type = kFlutterBackingStoreTypeOpenGL;
backing_store_out->user_data = surface.get();
backing_store_out->is_cacheable = true;
backing_store_out->open_gl.type = kFlutterOpenGLTargetTypeFramebuffer;
backing_store_out->open_gl.framebuffer.target = framebuffer_info.fFormat;
backing_store_out->open_gl.framebuffer.name = framebuffer_info.fFBOID;
Expand Down Expand Up @@ -124,6 +125,7 @@ bool EmbedderTestBackingStoreProducer::CreateTexture(
}

backing_store_out->type = kFlutterBackingStoreTypeOpenGL;
backing_store_out->is_cacheable = true;
backing_store_out->user_data = surface.get();
backing_store_out->open_gl.type = kFlutterOpenGLTargetTypeTexture;
backing_store_out->open_gl.texture.target = texture_info.fTarget;
Expand Down Expand Up @@ -160,6 +162,7 @@ bool EmbedderTestBackingStoreProducer::CreateSoftware(
}

backing_store_out->type = kFlutterBackingStoreTypeSoftware;
backing_store_out->is_cacheable = true;
backing_store_out->user_data = surface.get();
backing_store_out->software.allocation = pixmap.addr();
backing_store_out->software.row_bytes = pixmap.rowBytes();
Expand Down
5 changes: 5 additions & 0 deletions shell/platform/embedder/tests/embedder_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -552,6 +552,7 @@ TEST_F(EmbedderTest,
FlutterBackingStore backing_store = *layers[0]->backing_store;
backing_store.type = kFlutterBackingStoreTypeSoftware;
backing_store.did_update = true;
backing_store.is_cacheable = true;
Copy link
Member

Choose a reason for hiding this comment

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

Due to the earlier discussion on breaking changes, these test cases may not change.

backing_store.software.height = 600;

FlutterLayer layer = {};
Expand Down Expand Up @@ -585,6 +586,7 @@ TEST_F(EmbedderTest,
FlutterBackingStore backing_store = *layers[2]->backing_store;
backing_store.type = kFlutterBackingStoreTypeSoftware;
backing_store.did_update = true;
backing_store.is_cacheable = true;
backing_store.software.height = 600;

FlutterLayer layer = {};
Expand Down Expand Up @@ -618,6 +620,7 @@ TEST_F(EmbedderTest,
FlutterBackingStore backing_store = *layers[4]->backing_store;
backing_store.type = kFlutterBackingStoreTypeSoftware;
backing_store.did_update = true;
backing_store.is_cacheable = true;
backing_store.software.height = 600;

FlutterLayer layer = {};
Expand Down Expand Up @@ -862,6 +865,7 @@ TEST_F(EmbedderTest, VerifyB143464703WithSoftwareBackend) {
FlutterBackingStore backing_store = *layers[0]->backing_store;
backing_store.type = kFlutterBackingStoreTypeSoftware;
backing_store.did_update = true;
backing_store.is_cacheable = true;

FlutterLayer layer = {};
layer.struct_size = sizeof(layer);
Expand Down Expand Up @@ -894,6 +898,7 @@ TEST_F(EmbedderTest, VerifyB143464703WithSoftwareBackend) {
FlutterBackingStore backing_store = *layers[2]->backing_store;
backing_store.type = kFlutterBackingStoreTypeSoftware;
backing_store.did_update = true;
backing_store.is_cacheable = true;

FlutterLayer layer = {};
layer.struct_size = sizeof(layer);
Expand Down
30 changes: 30 additions & 0 deletions shell/platform/embedder/tests/embedder_unittests_gl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ TEST_F(EmbedderTest, CompositorMustBeAbleToRenderToOpenGLFramebuffer) {
backing_store.struct_size = sizeof(backing_store);
backing_store.type = kFlutterBackingStoreTypeOpenGL;
backing_store.did_update = true;
backing_store.is_cacheable = true;
backing_store.open_gl.type = kFlutterOpenGLTargetTypeFramebuffer;

FlutterLayer layer = {};
Expand Down Expand Up @@ -117,6 +118,7 @@ TEST_F(EmbedderTest, CompositorMustBeAbleToRenderToOpenGLFramebuffer) {
backing_store.struct_size = sizeof(backing_store);
backing_store.type = kFlutterBackingStoreTypeOpenGL;
backing_store.did_update = true;
backing_store.is_cacheable = true;
backing_store.open_gl.type = kFlutterOpenGLTargetTypeFramebuffer;

FlutterLayer layer = {};
Expand Down Expand Up @@ -178,6 +180,7 @@ TEST_F(EmbedderTest, RasterCacheDisabledWithPlatformViews) {
backing_store.struct_size = sizeof(backing_store);
backing_store.type = kFlutterBackingStoreTypeOpenGL;
backing_store.did_update = true;
backing_store.is_cacheable = true;
backing_store.open_gl.type = kFlutterOpenGLTargetTypeFramebuffer;

FlutterLayer layer = {};
Expand Down Expand Up @@ -210,6 +213,7 @@ TEST_F(EmbedderTest, RasterCacheDisabledWithPlatformViews) {
backing_store.struct_size = sizeof(backing_store);
backing_store.type = kFlutterBackingStoreTypeOpenGL;
backing_store.did_update = true;
backing_store.is_cacheable = true;
backing_store.open_gl.type = kFlutterOpenGLTargetTypeFramebuffer;

FlutterLayer layer = {};
Expand Down Expand Up @@ -282,6 +286,7 @@ TEST_F(EmbedderTest, RasterCacheEnabled) {
backing_store.struct_size = sizeof(backing_store);
backing_store.type = kFlutterBackingStoreTypeOpenGL;
backing_store.did_update = true;
backing_store.is_cacheable = true;
backing_store.open_gl.type = kFlutterOpenGLTargetTypeFramebuffer;

FlutterLayer layer = {};
Expand Down Expand Up @@ -351,6 +356,7 @@ TEST_F(EmbedderTest, CompositorMustBeAbleToRenderToOpenGLTexture) {
backing_store.struct_size = sizeof(backing_store);
backing_store.type = kFlutterBackingStoreTypeOpenGL;
backing_store.did_update = true;
backing_store.is_cacheable = true;
backing_store.open_gl.type = kFlutterOpenGLTargetTypeTexture;

FlutterLayer layer = {};
Expand Down Expand Up @@ -383,6 +389,7 @@ TEST_F(EmbedderTest, CompositorMustBeAbleToRenderToOpenGLTexture) {
backing_store.struct_size = sizeof(backing_store);
backing_store.type = kFlutterBackingStoreTypeOpenGL;
backing_store.did_update = true;
backing_store.is_cacheable = true;
backing_store.open_gl.type = kFlutterOpenGLTargetTypeTexture;

FlutterLayer layer = {};
Expand Down Expand Up @@ -443,6 +450,7 @@ TEST_F(EmbedderTest, CompositorMustBeAbleToRenderToSoftwareBuffer) {
backing_store.struct_size = sizeof(backing_store);
backing_store.type = kFlutterBackingStoreTypeSoftware;
backing_store.did_update = true;
backing_store.is_cacheable = true;
ASSERT_FLOAT_EQ(
backing_store.software.row_bytes * backing_store.software.height,
800 * 4 * 600.0);
Expand Down Expand Up @@ -477,6 +485,7 @@ TEST_F(EmbedderTest, CompositorMustBeAbleToRenderToSoftwareBuffer) {
backing_store.struct_size = sizeof(backing_store);
backing_store.type = kFlutterBackingStoreTypeSoftware;
backing_store.did_update = true;
backing_store.is_cacheable = true;
FlutterLayer layer = {};
layer.struct_size = sizeof(layer);
layer.type = kFlutterLayerContentTypeBackingStore;
Expand Down Expand Up @@ -537,6 +546,7 @@ TEST_F(EmbedderTest, CompositorMustBeAbleToRenderKnownScene) {
FlutterBackingStore backing_store = *layers[0]->backing_store;
backing_store.type = kFlutterBackingStoreTypeOpenGL;
backing_store.did_update = true;
backing_store.is_cacheable = true;
backing_store.open_gl.type = kFlutterOpenGLTargetTypeTexture;

FlutterLayer layer = {};
Expand Down Expand Up @@ -570,6 +580,7 @@ TEST_F(EmbedderTest, CompositorMustBeAbleToRenderKnownScene) {
FlutterBackingStore backing_store = *layers[2]->backing_store;
backing_store.type = kFlutterBackingStoreTypeOpenGL;
backing_store.did_update = true;
backing_store.is_cacheable = true;
backing_store.open_gl.type = kFlutterOpenGLTargetTypeTexture;

FlutterLayer layer = {};
Expand Down Expand Up @@ -603,6 +614,7 @@ TEST_F(EmbedderTest, CompositorMustBeAbleToRenderKnownScene) {
FlutterBackingStore backing_store = *layers[4]->backing_store;
backing_store.type = kFlutterBackingStoreTypeOpenGL;
backing_store.did_update = true;
backing_store.is_cacheable = true;
backing_store.open_gl.type = kFlutterOpenGLTargetTypeTexture;

FlutterLayer layer = {};
Expand Down Expand Up @@ -720,6 +732,7 @@ TEST_F(EmbedderTest, CustomCompositorMustWorkWithCustomTaskRunner) {
backing_store.struct_size = sizeof(backing_store);
backing_store.type = kFlutterBackingStoreTypeOpenGL;
backing_store.did_update = true;
backing_store.is_cacheable = true;
backing_store.open_gl.type = kFlutterOpenGLTargetTypeTexture;

FlutterLayer layer = {};
Expand Down Expand Up @@ -752,6 +765,7 @@ TEST_F(EmbedderTest, CustomCompositorMustWorkWithCustomTaskRunner) {
backing_store.struct_size = sizeof(backing_store);
backing_store.type = kFlutterBackingStoreTypeOpenGL;
backing_store.did_update = true;
backing_store.is_cacheable = true;
backing_store.open_gl.type = kFlutterOpenGLTargetTypeTexture;

FlutterLayer layer = {};
Expand Down Expand Up @@ -834,6 +848,7 @@ TEST_F(EmbedderTest, CompositorMustBeAbleToRenderWithRootLayerOnly) {
FlutterBackingStore backing_store = *layers[0]->backing_store;
backing_store.type = kFlutterBackingStoreTypeOpenGL;
backing_store.did_update = true;
backing_store.is_cacheable = true;
backing_store.open_gl.type = kFlutterOpenGLTargetTypeTexture;

FlutterLayer layer = {};
Expand Down Expand Up @@ -901,6 +916,7 @@ TEST_F(EmbedderTest, CompositorMustBeAbleToRenderWithPlatformLayerOnBottom) {
FlutterBackingStore backing_store = *layers[0]->backing_store;
backing_store.type = kFlutterBackingStoreTypeOpenGL;
backing_store.did_update = true;
backing_store.is_cacheable = true;
backing_store.open_gl.type = kFlutterOpenGLTargetTypeTexture;

FlutterLayer layer = {};
Expand Down Expand Up @@ -1021,6 +1037,7 @@ TEST_F(EmbedderTest,
FlutterBackingStore backing_store = *layers[0]->backing_store;
backing_store.type = kFlutterBackingStoreTypeOpenGL;
backing_store.did_update = true;
backing_store.is_cacheable = true;
backing_store.open_gl.type = kFlutterOpenGLTargetTypeTexture;

FlutterLayer layer = {};
Expand Down Expand Up @@ -1054,6 +1071,7 @@ TEST_F(EmbedderTest,
FlutterBackingStore backing_store = *layers[2]->backing_store;
backing_store.type = kFlutterBackingStoreTypeOpenGL;
backing_store.did_update = true;
backing_store.is_cacheable = true;
backing_store.open_gl.type = kFlutterOpenGLTargetTypeTexture;

FlutterLayer layer = {};
Expand Down Expand Up @@ -1087,6 +1105,7 @@ TEST_F(EmbedderTest,
FlutterBackingStore backing_store = *layers[4]->backing_store;
backing_store.type = kFlutterBackingStoreTypeOpenGL;
backing_store.did_update = true;
backing_store.is_cacheable = true;
backing_store.open_gl.type = kFlutterOpenGLTargetTypeTexture;

FlutterLayer layer = {};
Expand Down Expand Up @@ -1365,6 +1384,7 @@ TEST_F(EmbedderTest, CanRenderGradientWithCompositorOnNonRootLayer) {
FlutterBackingStore backing_store = *layers[0]->backing_store;
backing_store.type = kFlutterBackingStoreTypeOpenGL;
backing_store.did_update = true;
backing_store.is_cacheable = true;
backing_store.open_gl.type = kFlutterOpenGLTargetTypeFramebuffer;

FlutterLayer layer = {};
Expand Down Expand Up @@ -1398,6 +1418,7 @@ TEST_F(EmbedderTest, CanRenderGradientWithCompositorOnNonRootLayer) {
FlutterBackingStore backing_store = *layers[2]->backing_store;
backing_store.type = kFlutterBackingStoreTypeOpenGL;
backing_store.did_update = true;
backing_store.is_cacheable = true;
backing_store.open_gl.type = kFlutterOpenGLTargetTypeFramebuffer;

FlutterLayer layer = {};
Expand Down Expand Up @@ -1478,6 +1499,7 @@ TEST_F(EmbedderTest, CanRenderGradientWithCompositorOnNonRootLayerWithXform) {
FlutterBackingStore backing_store = *layers[0]->backing_store;
backing_store.type = kFlutterBackingStoreTypeOpenGL;
backing_store.did_update = true;
backing_store.is_cacheable = true;
backing_store.open_gl.type = kFlutterOpenGLTargetTypeFramebuffer;

FlutterLayer layer = {};
Expand Down Expand Up @@ -1511,6 +1533,7 @@ TEST_F(EmbedderTest, CanRenderGradientWithCompositorOnNonRootLayerWithXform) {
FlutterBackingStore backing_store = *layers[2]->backing_store;
backing_store.type = kFlutterBackingStoreTypeOpenGL;
backing_store.did_update = true;
backing_store.is_cacheable = true;
backing_store.open_gl.type = kFlutterOpenGLTargetTypeFramebuffer;

FlutterLayer layer = {};
Expand Down Expand Up @@ -1822,6 +1845,7 @@ TEST_F(EmbedderTest,
FlutterBackingStore backing_store = *layers[0]->backing_store;
backing_store.type = kFlutterBackingStoreTypeOpenGL;
backing_store.did_update = true;
backing_store.is_cacheable = true;
backing_store.open_gl.type = kFlutterOpenGLTargetTypeTexture;

FlutterLayer layer = {};
Expand Down Expand Up @@ -1855,6 +1879,7 @@ TEST_F(EmbedderTest,
FlutterBackingStore backing_store = *layers[2]->backing_store;
backing_store.type = kFlutterBackingStoreTypeOpenGL;
backing_store.did_update = true;
backing_store.is_cacheable = true;
backing_store.open_gl.type = kFlutterOpenGLTargetTypeTexture;

FlutterLayer layer = {};
Expand Down Expand Up @@ -1917,6 +1942,7 @@ TEST_F(
FlutterBackingStore backing_store = *layers[0]->backing_store;
backing_store.type = kFlutterBackingStoreTypeOpenGL;
backing_store.did_update = true;
backing_store.is_cacheable = true;
backing_store.open_gl.type = kFlutterOpenGLTargetTypeTexture;

FlutterLayer layer = {};
Expand Down Expand Up @@ -1950,6 +1976,7 @@ TEST_F(
FlutterBackingStore backing_store = *layers[2]->backing_store;
backing_store.type = kFlutterBackingStoreTypeOpenGL;
backing_store.did_update = true;
backing_store.is_cacheable = true;
backing_store.open_gl.type = kFlutterOpenGLTargetTypeTexture;

FlutterLayer layer = {};
Expand Down Expand Up @@ -2083,6 +2110,7 @@ TEST_F(EmbedderTest, PlatformViewMutatorsAreValid) {
FlutterBackingStore backing_store = *layers[0]->backing_store;
backing_store.type = kFlutterBackingStoreTypeOpenGL;
backing_store.did_update = true;
backing_store.is_cacheable = true;
backing_store.open_gl.type = kFlutterOpenGLTargetTypeTexture;

FlutterLayer layer = {};
Expand Down Expand Up @@ -2179,6 +2207,7 @@ TEST_F(EmbedderTest, PlatformViewMutatorsAreValidWithPixelRatio) {
FlutterBackingStore backing_store = *layers[0]->backing_store;
backing_store.type = kFlutterBackingStoreTypeOpenGL;
backing_store.did_update = true;
backing_store.is_cacheable = true;
backing_store.open_gl.type = kFlutterOpenGLTargetTypeTexture;

FlutterLayer layer = {};
Expand Down Expand Up @@ -2281,6 +2310,7 @@ TEST_F(EmbedderTest,
FlutterBackingStore backing_store = *layers[0]->backing_store;
backing_store.type = kFlutterBackingStoreTypeOpenGL;
backing_store.did_update = true;
backing_store.is_cacheable = true;
Copy link
Member

Choose a reason for hiding this comment

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

We need another test case where a frame is being rendered at the same size (there are already tests that do this) and where we should assert that the engine is indeed discarding layers with backing stores of identical size.

backing_store.open_gl.type = kFlutterOpenGLTargetTypeTexture;

FlutterLayer layer = {};
Expand Down