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

iOS postpone CAMetalDrawable acqusition #820

Merged
merged 9 commits into from Sep 19, 2023

Conversation

elijah-semyonov
Copy link

@elijah-semyonov elijah-semyonov commented Sep 15, 2023

Proposed Changes

Adhere to Apple best practices

Always acquire a drawable as late as possible; preferably, immediately before encoding an on-screen render pass. A frame’s CPU work may include dynamic data updates and off-screen render passes that you can perform before acquiring a drawable.

and postpone drawable acqusition till the actual rendering work starts.

Testing

Test: N/A

Issues Fixed

Fixes: drawable being acquired before rendering is performed causing occasional stalls.

before after
Screenshot 2023-09-15 at 14 51 23 Screenshot 2023-09-18 at 12 19 15

@elijah-semyonov elijah-semyonov changed the title Postpone drawable acqusition iOS postpone CAMetalDrawable acqusition Sep 15, 2023
@elijah-semyonov elijah-semyonov marked this pull request as ready for review September 15, 2023 13:55
* Flush async operations, notify all animations and perform recomposition-layout-draw sequence.
* [Canvas] to draw on is retrieved lazily to postpone acquiring drawable as late as possible.
*/
fun render(retrieveCanvas: () -> Canvas?, nanoTime: Long): Unit = postponeInvalidation {
Copy link
Member

Choose a reason for hiding this comment

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

It's a new public API.

  1. Should we make it experimental at first?
  2. Should we deprecate old one?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Alternative solutions:

  • separate to doFrame and draw
  • make custom LazyCanvas implementation

Copy link
Author

Choose a reason for hiding this comment

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

I marked it internal for now. Public API doesn't change.
The old one should probably be deprecated. It's correct, but most likely unoptimal in terms of swap chain usage. Needs investigation on DirectX and OpenGL to prove or disprove it. Added TODO.

This reverts commit 7f3a50f.
# Conflicts:
#	compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/window/MetalRedrawer.kt
#	compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/window/SkikoUIView.kt
@elijah-semyonov elijah-semyonov merged commit e59bdec into jb-main Sep 19, 2023
3 checks passed
@elijah-semyonov elijah-semyonov deleted the es/postpone-drawable-acqusition branch September 19, 2023 13:55
elijah-semyonov added a commit that referenced this pull request Sep 21, 2023
## Proposed Changes

Adhere to [Apple best
practices](https://developer.apple.com/library/archive/documentation/3DDrawing/Conceptual/MTLBestPracticesGuide/Drawables.html)

> Always acquire a drawable as late as possible; preferably, immediately
before encoding an on-screen render pass. A frame’s CPU work may include
dynamic data updates and off-screen render passes that you can perform
before acquiring a drawable.

and postpone drawable acqusition till the actual rendering work starts.

## Testing

Test: N/A

## Issues Fixed

Fixes: drawable being acquired before rendering is performed causing
occasional stalls.

before | after
--- | ---
<img width="620" alt="Screenshot 2023-09-15 at 14 51 23"
src="https://github.com/JetBrains/compose-multiplatform-core/assets/4167681/d6051c73-833a-45ec-bb5c-7fcc3418bcf9">
| <img width="485" alt="Screenshot 2023-09-18 at 12 19 15"
src="https://github.com/JetBrains/compose-multiplatform-core/assets/4167681/5098a952-a2ac-4e66-be41-6d5d3ffac894">
elijah-semyonov added a commit that referenced this pull request Sep 26, 2023
## Proposed Changes

If no CATransaction sync is needed, perform Picture recorded commands
execution on a separate thread.
Fix a freeze on `waitUntilScheduled` if no transaction is available by
moving synchronization from interop scope (any UIView is present in the
composition) to per-frame scope (any UIView transaction is issued by
composition in current frame).

## Testing

Test: N/A

## Issues Fixed

Removes work from main thread, when possible, allowing Compose to run
there without waiting for CPU to finish encoding GPU commands.
A freeze on `waitUntilScheduled` during interop synchronization if no
transaction is available.

<img width="324" alt="Screenshot 2023-09-19 at 12 13 00"
src="https://github.com/JetBrains/compose-multiplatform-core/assets/4167681/e54d83e3-ca58-43aa-a4a7-4764dd8ce841">
<img width="371" alt="Screenshot 2023-09-19 at 12 13 05"
src="https://github.com/JetBrains/compose-multiplatform-core/assets/4167681/8e4fcf0b-2b64-473e-a930-fae97a086a12">

## Depends on
#820
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants