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

Generate layer unique id for raster cache key#8637

Merged
liyuqian merged 4 commits into
flutter-team-archive:masterfrom
liyuqian:fix_layer_id
Apr 18, 2019
Merged

Generate layer unique id for raster cache key#8637
liyuqian merged 4 commits into
flutter-team-archive:masterfrom
liyuqian:fix_layer_id

Conversation

@liyuqian

Copy link
Copy Markdown
Contributor

The raw pointer isn't a reliable id as the allocator can reuse an
address that's just been released for another layer.

This will fix Fuchsia bug FL-216.

This problem was not affecting non-Fuchsia Flutter probably because
non-Fuchsia Flutter purges the raster cache key much more frequently so
we won't see a key collision. In Fuchsia, as the key has to wait for the
Vulkan surface to render asynchronously, this suddenly becomes an issue.

The raw pointer isn't a reliable id as the allocator can reuse an
address that's just been released for another layer.

This will fix a Fuchsia retained rendering bug.

This problem was not affecting non-Fuchsia Flutter probably because
non-Fuchsia Flutter purges the raster cache key much more frequently so
we won't see a key collision. In Fuchsia, as the key has to wait for the
Vulkan surface to render asynchronously, this suddenly becomes an issue.
Comment thread flow/layers/layer.cc Outdated

Layer::~Layer() = default;

uint32_t Layer::NextUniqueID() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What is the rate that these ids are generated at, and how long are these objects expected to live?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Usually a few (mostly fewer than 10, I never see more than 100) per frame. So that's about 100-200 at full 60fps animations, and I've never seen more than 1000 per second.

A layer may live for a very long time if the scene doesn't change at all. But I'd never expect to have many layers alive at one time. I think it's safe to say that most of the time, no more than 100 are alive at the same time. In extreme cases, maybe never more than 1000 at a time (otherwise compositing all those layers will be very janky).

If we take 1000 layers/second as the generation speed, it'll take 49 days to cycle the unique id.

If we take that no more than 1000 layers are alive at a time, the chance of collision (given that we've cycled the unique id after 49 days, and those 1000 layers live forever) is less than 2^-20 per layer and less than 2^-10 (0.1%) per second.

If that doesn't sound safe enough, I'm happy to use uint64_t for the id.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If it's not hard to switch to uint64_t, then let's switch.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

@liyuqian liyuqian merged commit d79eb28 into flutter-team-archive:master Apr 18, 2019
@liyuqian liyuqian deleted the fix_layer_id branch May 23, 2019 18:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Development

Successfully merging this pull request may close these issues.

3 participants