Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[web] Fix image getting evicted after being consumed from CrossFrameCache #22072

Merged
merged 3 commits into from
Oct 23, 2020

Conversation

ferhatb
Copy link
Contributor

@ferhatb ferhatb commented Oct 23, 2020

Description

When SceneBuilder repaints pictures and PersistedPicture reuses a recycled BitmapCanvas, multiple BitmapCanvas's can point to same element cache. After a frame is painted, the cache evicts unused images although they get used by another Picture applyBitmapPaint. This PR makes sure elementCache is cleared when recycling a BitmapCanvas for future use.

Related Issues

flutter/flutter#60460

Tests

Added a test to make sure same image is reused across updates.

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the [contributor guide] and followed the process outlined there for submitting PRs.
  • I signed the [CLA].
  • I read and followed the [C++, Objective-C, Java style guides] for the engine.
  • I read the [tree hygiene] wiki page, which explains my responsibilities.
  • I updated/added relevant documentation.
  • All existing and new tests are passing.
  • I am willing to follow-up on review comments in a timely manner.

Reviewer Checklist

  • I have submitted any presubmit flakes in this PR using the [engine presubmit flakes form] before re-triggering the failure.

Breaking Change

Did any tests fail when you ran them? Please read [handling breaking changes].

@ferhatb ferhatb requested review from ditman and removed request for nturgut October 23, 2020 18:25
Copy link
Contributor

@yjbanov yjbanov left a comment

Choose a reason for hiding this comment

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

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants