[embedder] Allow for the backing stores to not be cached #22643
Conversation
iskakaushik
force-pushed
the
backing_stores_can_be_cacheable_or_not
branch
from
2e7bf6e to
62da1a6
Compare
| @@ -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; | |||
There was a problem hiding this comment.
Two issues:
- 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_cacheablevalue set tofalsedue 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 aboutbool avoid_cache,bypass_cache, etc..? - 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
FlutterLayerinstead as that is a union of pointers.
There was a problem hiding this comment.
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
| @@ -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; | |||
There was a problem hiding this comment.
Due to the earlier discussion on breaking changes, these test cases may not change.
| @@ -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; | |||
There was a problem hiding this comment.
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.
No description provided.