Don’t depend on an implicit transaction when no external view embedder is present. #17258
Conversation
…r is present. All Metal layers have their presentsWithTransaction property set to true. However, when an external view embedder is not present, there is no mechanism to ensure that the command buffer commit is within transaction scope. This works in most cases as there there is usually an implicit (possibly nested) transaction in place during rendering. However, when there isn’t, rendering will look paused at an incorrect size. This code now works similar to OpenGL but will be refactored for ease of understanding and consistency between the various backends.
|
This edge case was hard to reproduce and isolate and the code around transaction management is quite a mess. I am going to rework this in flutter/flutter#53069. For now, brings consistency with OpenGL. |
chinmaygarde
added
the
waiting for tree to go green
|
uh, the frame size cleanup is not related to this patch and is just cleaning up the redundant accessor to the drawable size. Let me know if you'd rather have me put it in another patch. |
| const auto bounds = layer_.get().bounds.size; | ||
| const auto scale = layer_.get().contentsScale; | ||
| if (bounds.width <= 0.0 || bounds.height <= 0.0 || scale <= 0.0) { | ||
| FML_LOG(ERROR) << "Metal layer bounds were invalid."; | ||
| return nullptr; | ||
| } |
There was a problem hiding this comment.
Why is this not needed anymore? Was it just wrong?
There was a problem hiding this comment.
This was redundant. The frame_size already accounts for these calculation. There is no point in doing them again here.
| [command_buffer.get() commit]; | ||
| [command_buffer.get() waitUntilScheduled]; | ||
| [drawable.get() present]; |
There was a problem hiding this comment.
I'm pretty sure this will break platform views - have you tested this with a platform view? e.g. run the webview example on a local engine built with metal support.
There was a problem hiding this comment.
If anything, this brings the behavior more in line with the condition where there are platform views. When there is a platform view, has_external_view_embedder in the previous code path will be true. Now, both paths do the same thing where they wait for the GPU commands to be flushed in the current transaction.
There was a problem hiding this comment.
And yes. I did test this locally with platform views enabled (specifically the web view example).
There was a problem hiding this comment.
Ahhh duh I misread the old code.
|
This pull request is not suitable for automatic merging in its current state.
|
fluttergithubbot
removed
the
waiting for tree to go green
… embedder is present. (flutter/engine#17258)
… embedder is present. (flutter/engine#17258)
… embedder is present. (flutter/engine#17258)
4 participants
All Metal layers have their presentsWithTransaction property set to true.
However, when an external view embedder is not present, there is no mechanism to
ensure that the command buffer commit is within transaction scope. This works in
most cases as there there is usually an implicit (possibly nested) transaction
in place during rendering. However, when there isn’t, rendering will look paused
at an incorrect size. This code now works similar to OpenGL but will be
refactored for ease of understanding and consistency between the various
backends.