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

[Impeller] Wait for the previous AHB texture to be fully recyclable. #52588

Merged
merged 2 commits into from
May 7, 2024

Conversation

chinmaygarde
Copy link
Member

@chinmaygarde chinmaygarde commented May 6, 2024

The Android surface transaction completion handler documentation states:

Buffers which are replaced or removed from the scene in the transaction
invoking this callback may be reused after this point.

However, this is NOT sufficient. One also needs to be obtain the previous release fence and perform a wait for the buffer to be safely reusable.

This patch adds a GPU side wait for this fence using available extensions.

Fixes flutter/flutter#147758

@chinmaygarde chinmaygarde added the Work in progress (WIP) Not ready (yet) for review! label May 6, 2024
@chinmaygarde chinmaygarde requested a review from jonahwilliams May 6, 2024 22:45
@chinmaygarde
Copy link
Member Author

Still WIP as I am not able to explain a validation error on early semaphore destruction. But it may be that I am doing something wrong where the tracked objects are being collected too early.

@jonahwilliams
Copy link
Member

Oh, missed your comment here. i will check this out and play around with it for a bit, I have a few ideas.

@chinmaygarde
Copy link
Member Author

Ok, the validation errors due to bad tracking are now fixed. Need to verify the waits actually fix our original issue (they should) and also patchup the tracing in the present ready fence case as I believe I made the same mistake earlier.

Copy link
Member

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

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

LGTM

The Android surface transaction completion handler documentation
states:

```
Buffers which are replaced or removed from the scene in the transaction
invoking this callback may be reused after this point.
```

However, this is NOT sufficient. One also needs to be obtain the
previous release fence and perform a wait for the buffer to be safely
reusable.

This patch adds a GPU side wait for this fence using available
extensions.
@chinmaygarde chinmaygarde removed the Work in progress (WIP) Not ready (yet) for review! label May 7, 2024
@chinmaygarde
Copy link
Member Author

Experimenting with the macrobenchmarks app, I couldn't see the checkerboarding we saw last week. I think this is good to go.

@chinmaygarde chinmaygarde added the autosubmit Merge PR when tree becomes green via auto submit App label May 7, 2024
@auto-submit auto-submit bot merged commit 42898e0 into flutter:main May 7, 2024
31 checks passed
@chinmaygarde chinmaygarde deleted the render_ready_wait branch May 7, 2024 18:34
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 7, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request May 7, 2024
…147939)

flutter/engine@d88b7db...42898e0

2024-05-07 chinmaygarde@google.com [Impeller] Wait for the previous AHB texture to be fully recyclable. (flutter/engine#52588)
2024-05-07 jason-simmons@users.noreply.github.com Manual Skia roll to 2319f1ae8fe42525f8b6a1969a1cee67bdbee290 (flutter/engine#52615)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC rmistry@google.com,zra@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
autosubmit Merge PR when tree becomes green via auto submit App e: impeller
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Impeller] Ensure a wait on the previous buffers fence/semaphore is performed when recycling AHB textures.
3 participants