[Impeller] iOS/macOS: Only wait for command scheduling prior to present

[bdero]

edit: This was updated to just wait for scheduling in a new command buffer. Similar to how we already do in the macOS embedder, but better because we don't even block on the current thread thanks to presentDrawable.

This patch makes us conform to the docs and sync properly for the drawable present, and gets rid of the excessive waitUntilScheduled blocking on iOS sim. Tested on iPhone 12, iPhone 6s, and x64 sim.

Open to ideas for better architecting this. Waiting for the last committed command buffer to be scheduled is the same as waiting for all previously committed command buffers to be scheduled. Command buffers are scheduled in queue order.

Explanation:

Sleuthing through the Metal docs for all the calls we're making, I found one thing that's unsafe about not doing waitUntilScheduled on iOS, but I'm only able to reproduce symptoms of an actual problem on iOS sim. Wondrous on iPhone 12 and iPhone 6s seems to run perfectly fine without waitUntilScheduled, but the simulator is a choppy mess because we're flipping too fast:

Screen.Recording.2023-03-29.at.10.37.33.PM.mov

The problem is that immediately after we're done committing our buffers to the root texture, we present by calling MTLDrawable.present().

The docs for present say:

When you call this method, the drawable presents its contents as soon as possible after all scheduled render or write requests for that drawable are complete.

The scheduled part is key here -- committing the buffer signals to the queue that it's ready to be scheduled. MTLCommandBuffer.commit() docs:

The commit() method sends the command buffer to the MTLCommandQueue instance that owns it, which then schedules it to run on the GPU.

This is slightly ambiguous, but we know this call doesn't block on scheduling; scheduling happens in the background sometime after commit() is called, hence the existence of waitUntilScheduled().

[flutter-dashboard]

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.

[bdero]

Gonna change this up to do the trick that we do in the macOS embedder and just create an empty command buffer/wait for it: https://github.com/flutter/engine/blob/main/shell/platform/darwin/macos/framework/Source/FlutterSurfaceManager.mm#L127

[chinmaygarde]

The lock can fail in case of context loss. We need to check and return false from this method.

[chinmaygarde]

Oh, derp. You got rid of the weak ptr. I think the weak ptr does make more sense though. We just need to be able to survive context loss.

[bdero]

I just switched to hanging onto a shared_ptr instead.

[chinmaygarde]

I slightly favor the weak hold on the context. As a habit, I think we should not hold onto the context via a strong reference from objects derived from the context itself. Its easy to get into a case where we cause reference cycles. I could image surfaces being held by swapchains and such.

Having said that though, I this this use is probably safe. But again, weak ptrs with a lock checks I like better.

[bdero]

Alrighty, seems reasonable. Added the weak ptr back with a check.

[flutter-dashboard]

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

Changes reported for pull request #40781 at sha f0fd1a7