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.

Sign up for GitHub

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

Jump to bottom

[Impeller] Give a fixed timeout for acquireNextImageKHR. #44241

Merged
merged 1 commit into from
Aug 2, 2023
Merged

[Impeller] Give a fixed timeout for acquireNextImageKHR. #44241

merged 1 commit into from
Aug 2, 2023

Conversation

jonahwilliams
Copy link
Member

Fixes flutter/flutter#131610

Since we're not blocking on presentation, we can have multiple images pending. This is generally good, but for the backpressure from acquireNextImageKHR to be valid it needs to have a finite timeout.

Pick a somewhat arbitrary but fairly long ms duration. See also: flutter/flutter#131698

@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

chinmaygarde
chinmaygarde approved these changes Aug 2, 2023
View reviewed changes
impeller/renderer/backend/vulkan/swapchain_impl_vk.cc
*sync->render_ready, // signal semaphore
nullptr // fence
*swapchain_, // swapchain
1'000'000'000, // timeout (ns) 1000ms
Copy link
Member

Choose a reason for hiding this comment

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

I was about to say, "if this is a magic number anyway, let's make it the same as the Metal backend.". The docs for [CAMetalLayer nextDrawable] say "If all drawables are in use, the layer waits up to one second for one to become available, after which it returns nil.". If this was intentional, amazing attention to detail!

Copy link
Member Author

Choose a reason for hiding this comment

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

😉

@chinmaygarde chinmaygarde changed the title [Impeller] give a fixed timeout for acquireNextImageKHR [Impeller] Give a fixed timeout for acquireNextImageKHR. Aug 2, 2023
@jonahwilliams jonahwilliams added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 2, 2023
@auto-submit auto-submit bot merged commit a86fe8a into flutter:main Aug 2, 2023
26 checks passed
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 2, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 2, 2023
@engine-flutter-autoroll
a86fe8a65 [Impeller] Give a fixed timeout for acquireNextImageKHR. (f…
45249ed 
…lutter/engine#44241)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 2, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 2, 2023
@engine-flutter-autoroll
a86fe8a65 [Impeller] Give a fixed timeout for acquireNextImageKHR. (f…
a81f751 
…lutter/engine#44241)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 2, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 2, 2023
@engine-flutter-autoroll
a86fe8a65 [Impeller] Give a fixed timeout for acquireNextImageKHR. (f…
cb94cdb 
…lutter/engine#44241)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 2, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 2, 2023
@engine-flutter-autoroll
a86fe8a65 [Impeller] Give a fixed timeout for acquireNextImageKHR. (f…
c4402f5 
…lutter/engine#44241)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 2, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 2, 2023
@engine-flutter-autoroll
a86fe8a65 [Impeller] Give a fixed timeout for acquireNextImageKHR. (f…
cabcdfe 
…lutter/engine#44241)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 2, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 2, 2023
@engine-flutter-autoroll
a86fe8a65 [Impeller] Give a fixed timeout for acquireNextImageKHR. (f…
25df567 
…lutter/engine#44241)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 2, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 2, 2023
@engine-flutter-autoroll
a86fe8a65 [Impeller] Give a fixed timeout for acquireNextImageKHR. (f…
81247db 
…lutter/engine#44241)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 2, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 2, 2023
@engine-flutter-autoroll
a86fe8a65 [Impeller] Give a fixed timeout for acquireNextImageKHR. (f…
84a7c8e 
…lutter/engine#44241)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 2, 2023
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 2, 2023
@engine-flutter-autoroll
a86fe8a65 [Impeller] Give a fixed timeout for acquireNextImageKHR. (f…
abb7348 
…lutter/engine#44241)
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Aug 2, 2023
@engine-flutter-autoroll
Manual roll Flutter Engine from 9dae7b708bda to d6b962d0e36d (25 revi…
f9e4567 
…sions) (#131785)

Manual roll requested by jacksongardner@google.com

flutter/engine@9dae7b7...d6b962d

2023-08-02 zanderso@users.noreply.github.com Revert "[Impeller] Support for rendering Android Platform Views into a HardwareBuffer backed texture." (flutter/engine#44262)
2023-08-02 skia-flutter-autoroll@skia.org Roll Skia from c6577d328585 to 6009cc6d7d80 (2 revisions) (flutter/engine#44264)
2023-08-02 skia-flutter-autoroll@skia.org Roll ANGLE from 11cef17b53ac to d61a50c155bd (1 revision) (flutter/engine#44261)
2023-08-02 skia-flutter-autoroll@skia.org Roll Skia from 1c0bba7c1053 to c6577d328585 (2 revisions) (flutter/engine#44260)
2023-08-02 skia-flutter-autoroll@skia.org Roll Dart SDK from 8ff03ebf7eaa to afbaf4216fc8 (1 revision) (flutter/engine#44259)
2023-08-02 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from 3JSF6hXLRdAK1wUN1... to Hx7ap5qcoqRIknnnG... (flutter/engine#44258)
2023-08-02 skia-flutter-autoroll@skia.org Roll ANGLE from a21631c02e45 to 11cef17b53ac (1 revision) (flutter/engine#44256)
2023-08-02 skia-flutter-autoroll@skia.org Roll Skia from 6807d8b8a9d3 to 1c0bba7c1053 (1 revision) (flutter/engine#44255)
2023-08-02 skia-flutter-autoroll@skia.org Roll ANGLE from 5d4b3645d0dc to a21631c02e45 (1 revision) (flutter/engine#44252)
2023-08-02 skia-flutter-autoroll@skia.org Roll Skia from 6087a5224c6f to 6807d8b8a9d3 (2 revisions) (flutter/engine#44250)
2023-08-02 skia-flutter-autoroll@skia.org Roll Skia from 6cb888889ad8 to 6087a5224c6f (1 revision) (flutter/engine#44249)
2023-08-02 jonahwilliams@google.com [Impeller] Give a fixed timeout for acquireNextImageKHR. (flutter/engine#44241)
2023-08-02 skia-flutter-autoroll@skia.org Roll Dart SDK from 7c03426705da to 8ff03ebf7eaa (2 revisions) (flutter/engine#44243)
2023-08-02 skia-flutter-autoroll@skia.org Roll Skia from 4deae93198f9 to 6cb888889ad8 (1 revision) (flutter/engine#44240)
2023-08-01 hterkelsen@users.noreply.github.com [canvaskit] Enable CanvasKit to compute tight SkPicture bounds (flutter/engine#43361)
2023-08-01 skia-flutter-autoroll@skia.org Roll Dart SDK from 197fc0d7fea2 to 7c03426705da (2 revisions) (flutter/engine#44237)
2023-08-01 john@johnmccutchan.com [Impeller] Support for rendering Android Platform Views into a HardwareBuffer backed texture. (flutter/engine#44087)
2023-08-01 yjbanov@google.com [web:a11y] add platform view role (flutter/engine#44188)
2023-08-01 skia-flutter-autoroll@skia.org Roll Skia from 18cf818e044f to 4deae93198f9 (1 revision) (flutter/engine#44236)
2023-08-01 skia-flutter-autoroll@skia.org Roll Skia from 58c031441cbb to 18cf818e044f (6 revisions) (flutter/engine#44234)
2023-08-01 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from g0dgOL2IlZQJCK4El... to 3JSF6hXLRdAK1wUN1... (flutter/engine#44233)
2023-08-01 skia-flutter-autoroll@skia.org Roll ANGLE from b53d99d87e6a to 5d4b3645d0dc (1 revision) (flutter/engine#44231)
2023-08-01 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from lwCo6le6r0X-Srvx3... to KPSWBhOvG6piddBQG... (flutter/engine#44230)
2023-08-01 skia-flutter-autoroll@skia.org Roll Skia from d53f7b880651 to 58c031441cbb (2 revisions) (flutter/engine#44229)
2023-08-01 58529443+srujzs@users.noreply.github.com Remove extends JSTypedArray from JSUint8Array1 (flutter/engine#44175)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from lwCo6le6r0X- to KPSWBhOvG6pi
  fuchsia/sdk/core/mac-amd64 from g0dgOL2IlZQJ to Hx7ap5qcoqRI

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 jacksongardner@google.com,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://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Aug 2, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 2, 2023
@engine-flutter-autoroll
a86fe8a65 [Impeller] Give a fixed timeout for acquireNextImageKHR. (f…
cc33b8c 
…lutter/engine#44241)
vashworth pushed a commit to vashworth/flutter that referenced this pull request Aug 2, 2023
@engine-flutter-autoroll @vashworth
Manual roll Flutter Engine from 9dae7b708bda to d6b962d0e36d (25 revi…
908ed76 
…sions) (flutter#131785)

Manual roll requested by jacksongardner@google.com

flutter/engine@9dae7b7...d6b962d

2023-08-02 zanderso@users.noreply.github.com Revert "[Impeller] Support for rendering Android Platform Views into a HardwareBuffer backed texture." (flutter/engine#44262)
2023-08-02 skia-flutter-autoroll@skia.org Roll Skia from c6577d328585 to 6009cc6d7d80 (2 revisions) (flutter/engine#44264)
2023-08-02 skia-flutter-autoroll@skia.org Roll ANGLE from 11cef17b53ac to d61a50c155bd (1 revision) (flutter/engine#44261)
2023-08-02 skia-flutter-autoroll@skia.org Roll Skia from 1c0bba7c1053 to c6577d328585 (2 revisions) (flutter/engine#44260)
2023-08-02 skia-flutter-autoroll@skia.org Roll Dart SDK from 8ff03ebf7eaa to afbaf4216fc8 (1 revision) (flutter/engine#44259)
2023-08-02 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from 3JSF6hXLRdAK1wUN1... to Hx7ap5qcoqRIknnnG... (flutter/engine#44258)
2023-08-02 skia-flutter-autoroll@skia.org Roll ANGLE from a21631c02e45 to 11cef17b53ac (1 revision) (flutter/engine#44256)
2023-08-02 skia-flutter-autoroll@skia.org Roll Skia from 6807d8b8a9d3 to 1c0bba7c1053 (1 revision) (flutter/engine#44255)
2023-08-02 skia-flutter-autoroll@skia.org Roll ANGLE from 5d4b3645d0dc to a21631c02e45 (1 revision) (flutter/engine#44252)
2023-08-02 skia-flutter-autoroll@skia.org Roll Skia from 6087a5224c6f to 6807d8b8a9d3 (2 revisions) (flutter/engine#44250)
2023-08-02 skia-flutter-autoroll@skia.org Roll Skia from 6cb888889ad8 to 6087a5224c6f (1 revision) (flutter/engine#44249)
2023-08-02 jonahwilliams@google.com [Impeller] Give a fixed timeout for acquireNextImageKHR. (flutter/engine#44241)
2023-08-02 skia-flutter-autoroll@skia.org Roll Dart SDK from 7c03426705da to 8ff03ebf7eaa (2 revisions) (flutter/engine#44243)
2023-08-02 skia-flutter-autoroll@skia.org Roll Skia from 4deae93198f9 to 6cb888889ad8 (1 revision) (flutter/engine#44240)
2023-08-01 hterkelsen@users.noreply.github.com [canvaskit] Enable CanvasKit to compute tight SkPicture bounds (flutter/engine#43361)
2023-08-01 skia-flutter-autoroll@skia.org Roll Dart SDK from 197fc0d7fea2 to 7c03426705da (2 revisions) (flutter/engine#44237)
2023-08-01 john@johnmccutchan.com [Impeller] Support for rendering Android Platform Views into a HardwareBuffer backed texture. (flutter/engine#44087)
2023-08-01 yjbanov@google.com [web:a11y] add platform view role (flutter/engine#44188)
2023-08-01 skia-flutter-autoroll@skia.org Roll Skia from 18cf818e044f to 4deae93198f9 (1 revision) (flutter/engine#44236)
2023-08-01 skia-flutter-autoroll@skia.org Roll Skia from 58c031441cbb to 18cf818e044f (6 revisions) (flutter/engine#44234)
2023-08-01 skia-flutter-autoroll@skia.org Roll Fuchsia Mac SDK from g0dgOL2IlZQJCK4El... to 3JSF6hXLRdAK1wUN1... (flutter/engine#44233)
2023-08-01 skia-flutter-autoroll@skia.org Roll ANGLE from b53d99d87e6a to 5d4b3645d0dc (1 revision) (flutter/engine#44231)
2023-08-01 skia-flutter-autoroll@skia.org Roll Fuchsia Linux SDK from lwCo6le6r0X-Srvx3... to KPSWBhOvG6piddBQG... (flutter/engine#44230)
2023-08-01 skia-flutter-autoroll@skia.org Roll Skia from d53f7b880651 to 58c031441cbb (2 revisions) (flutter/engine#44229)
2023-08-01 58529443+srujzs@users.noreply.github.com Remove extends JSTypedArray from JSUint8Array1 (flutter/engine#44175)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from lwCo6le6r0X- to KPSWBhOvG6pi
  fuchsia/sdk/core/mac-amd64 from g0dgOL2IlZQJ to Hx7ap5qcoqRI

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 jacksongardner@google.com,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://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
gaaclarke pushed a commit to gaaclarke/engine that referenced this pull request Aug 30, 2023
@jonahwilliams @gaaclarke
[Impeller] Give a fixed timeout for acquireNextImageKHR. (flutter#44241)
6b7e196 
Fixes flutter/flutter#131610

Since we're not blocking on presentation, we can have multiple images pending. This is generally good, but for the backpressure from acquireNextImageKHR to be valid it needs to have a finite timeout.

Pick a somewhat arbitrary but fairly long ms duration. See also: flutter/flutter#131698
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chinmaygarde chinmaygarde chinmaygarde approved these changes

Assignees

@jonahwilliams jonahwilliams

Labels
autosubmit Merge PR when tree becomes green via auto submit App e: impeller
Projects
No open projects
Impeller
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Impeller] vkAcquireNextImageKHR: "timeout member of pAcquireInfo must not be UINT64_MAX"
2 participants
@jonahwilliams @chinmaygarde