-
Notifications
You must be signed in to change notification settings - Fork 70
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
Hide Metal drawables from Kotlin runtime #1390
Changes from 4 commits
4601d2c
39d436a
f8b1a73
2427719
f327ccc
f5b20dd
07172b1
2d7baca
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,114 @@ | ||
/* | ||
* Copyright 2024 The Android Open Source Project | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
package androidx.compose.mpp.demo.bugs | ||
|
||
import androidx.compose.foundation.background | ||
import androidx.compose.foundation.horizontalScroll | ||
import androidx.compose.foundation.layout.Box | ||
import androidx.compose.foundation.layout.Column | ||
import androidx.compose.foundation.layout.Row | ||
import androidx.compose.foundation.layout.Spacer | ||
import androidx.compose.foundation.layout.fillMaxSize | ||
import androidx.compose.foundation.layout.height | ||
import androidx.compose.foundation.layout.padding | ||
import androidx.compose.foundation.layout.size | ||
import androidx.compose.foundation.rememberScrollState | ||
import androidx.compose.foundation.verticalScroll | ||
import androidx.compose.material.Text | ||
import androidx.compose.material3.Button | ||
import androidx.compose.mpp.demo.Screen | ||
import androidx.compose.runtime.Composable | ||
import androidx.compose.ui.Modifier | ||
import androidx.compose.ui.graphics.Color | ||
import androidx.compose.ui.interop.LocalUIViewController | ||
import androidx.compose.ui.unit.dp | ||
import androidx.compose.ui.window.ComposeUIViewController | ||
import platform.UIKit.UIModalPresentationFormSheet | ||
import platform.UIKit.UISheetPresentationControllerDetent | ||
import platform.UIKit.sheetPresentationController | ||
|
||
//val bottomSheetUIViewController = | ||
// ComposeUIViewController { | ||
// VerticalScrollWithIndependentHorizontalRows() | ||
// } | ||
//bottomSheetUIViewController.modalPresentationStyle = UIModalPresentationFormSheet | ||
//bottomSheetUIViewController.sheetPresentationController?.setDetents( | ||
//listOf( | ||
//UISheetPresentationControllerDetent.mediumDetent(), | ||
//UISheetPresentationControllerDetent.largeDetent(), | ||
//) | ||
//) | ||
// | ||
//UIApplication.sharedApplication.keyWindow?.rootViewController?.presentViewController( | ||
//viewControllerToPresent = bottomSheetUIViewController, | ||
//animated = true, | ||
//completion = {}, | ||
//) | ||
|
||
val PopupStretching = Screen.Example("Popup stretching") { | ||
val viewController = LocalUIViewController.current | ||
|
||
Button(onClick = { | ||
val bottomSheetController = ComposeUIViewController { | ||
VerticalScrollWithIndependentHorizontalRows() | ||
} | ||
|
||
bottomSheetController.modalPresentationStyle = UIModalPresentationFormSheet | ||
bottomSheetController.sheetPresentationController?.setDetents( | ||
listOf( | ||
UISheetPresentationControllerDetent.mediumDetent(), | ||
UISheetPresentationControllerDetent.largeDetent(), | ||
) | ||
) | ||
|
||
viewController.presentViewController(bottomSheetController, animated = true, completion = {}) | ||
}) { | ||
Text("Show popup") | ||
} | ||
} | ||
|
||
|
||
@Composable | ||
fun VerticalScrollWithIndependentHorizontalRows() { | ||
Column( | ||
modifier = | ||
Modifier.fillMaxSize().verticalScroll(rememberScrollState(), enabled = true), | ||
) { | ||
repeat(10) { rowIndex -> | ||
val horizontalScrollState = rememberScrollState() | ||
|
||
Spacer(Modifier.height(30.dp).background(Color.DarkGray)) | ||
Row( | ||
modifier = | ||
Modifier | ||
.padding(start = 16.dp, end = 16.dp) | ||
.horizontalScroll(horizontalScrollState), | ||
) { | ||
repeat(5) { | ||
Box( | ||
modifier = | ||
Modifier | ||
.size(100.dp) | ||
.background(Color.Gray), | ||
) { | ||
Text("Item $it in row $rowIndex") | ||
} | ||
} | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
// | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please check copyright There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed! |
||
// CMPMetalDrawablesHandler.h | ||
// CMPUIKitUtils | ||
// | ||
// Created by Ilia.Semenov on 07/06/2024. | ||
// | ||
|
||
#import <Foundation/Foundation.h> | ||
#import <QuartzCore/QuartzCore.h> | ||
#import <Metal/Metal.h> | ||
|
||
#import "CMPMacros.h" | ||
|
||
NS_ASSUME_NONNULL_BEGIN | ||
|
||
@interface CMPMetalDrawablesHandler : NSObject | ||
MatkovIvan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
- (instancetype)initWithMetalLayer:(CAMetalLayer *)metalLayer; | ||
|
||
- (void * CMP_OWNED)nextDrawable; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just thinking about slightly more generic solution. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried to model this approach in my head, but didn't succeed. Do you have proof-of-concept available? |
||
- (void)releaseDrawable:(void * CMP_CONSUMED)drawablePtr; | ||
- (void * CMP_BORROWED)drawableTexture:(void * CMP_BORROWED)drawablePtr; | ||
- (void)presentDrawable:(void * CMP_CONSUMED)drawablePtr; | ||
- (void)scheduleDrawablePresentation:(void * CMP_CONSUMED)drawablePtr onCommandBuffer:(id <MTLCommandBuffer>)commandBuffer; | ||
|
||
|
||
@end | ||
|
||
NS_ASSUME_NONNULL_END |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,63 @@ | ||
// | ||
// CMPMetalDrawablesHandler.m | ||
// CMPUIKitUtils | ||
// | ||
// Created by Ilia.Semenov on 07/06/2024. | ||
// | ||
|
||
#import "CMPMetalDrawablesHandler.h" | ||
|
||
@implementation CMPMetalDrawablesHandler { | ||
CAMetalLayer *_metalLayer; | ||
} | ||
|
||
- (instancetype)initWithMetalLayer:(CAMetalLayer *)metalLayer { | ||
self = [super init]; | ||
if (self) { | ||
_metalLayer = metalLayer; | ||
} | ||
return self; | ||
} | ||
|
||
- (void * CMP_OWNED)nextDrawable { | ||
id <CAMetalDrawable> drawable = [_metalLayer nextDrawable]; | ||
|
||
if (drawable) { | ||
void *ptr = (__bridge_retained void *)drawable; | ||
return ptr; | ||
} else { | ||
return NULL; | ||
} | ||
} | ||
|
||
- (void)releaseDrawable:(void * CMP_CONSUMED)drawablePtr { | ||
assert(drawablePtr != NULL); | ||
|
||
id <CAMetalDrawable> drawable __unused = (__bridge_transfer id <CAMetalDrawable>)drawablePtr; | ||
// drawable will be released by ARC | ||
} | ||
|
||
- (void * CMP_BORROWED)drawableTexture:(void * CMP_BORROWED)drawablePtr { | ||
assert(drawablePtr != NULL); | ||
|
||
id <CAMetalDrawable> drawable = (__bridge id <CAMetalDrawable>)drawablePtr; | ||
id <MTLTexture> texture = drawable.texture; | ||
void *texturePtr = (__bridge void *)texture; | ||
return texturePtr; | ||
} | ||
|
||
- (void)presentDrawable:(void * CMP_CONSUMED)drawablePtr { | ||
assert(drawablePtr != NULL); | ||
|
||
id <CAMetalDrawable> drawable = (__bridge_transfer id <CAMetalDrawable>)drawablePtr; | ||
[drawable present]; | ||
} | ||
|
||
- (void)scheduleDrawablePresentation:(void * CMP_CONSUMED)drawablePtr onCommandBuffer:(id <MTLCommandBuffer>)commandBuffer { | ||
assert(drawablePtr != NULL); | ||
|
||
id <CAMetalDrawable> drawable = (__bridge_transfer id <CAMetalDrawable>)drawablePtr; | ||
[commandBuffer presentDrawable:drawable]; | ||
} | ||
|
||
@end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,6 +20,7 @@ import androidx.compose.ui.interop.UIKitInteropState | |
import androidx.compose.ui.interop.UIKitInteropTransaction | ||
import androidx.compose.ui.interop.doLocked | ||
import androidx.compose.ui.interop.isNotEmpty | ||
import androidx.compose.ui.uikit.utils.CMPMetalDrawablesHandler | ||
import androidx.compose.ui.util.fastForEach | ||
import androidx.compose.ui.util.trace | ||
import kotlin.math.roundToInt | ||
|
@@ -145,6 +146,7 @@ internal class MetalRedrawer( | |
private val metalLayer: CAMetalLayer, | ||
private val callbacks: MetalRedrawerCallbacks | ||
) { | ||
private val metalDrawableHandler = CMPMetalDrawablesHandler(metalLayer) | ||
MatkovIvan marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// Workaround for KN compiler bug | ||
// Type mismatch: inferred type is objcnames.protocols.MTLDeviceProtocol but platform.Metal.MTLDeviceProtocol was expected | ||
@Suppress("USELESS_CAST") | ||
|
@@ -316,7 +318,7 @@ internal class MetalRedrawer( | |
} | ||
|
||
val metalDrawable = trace("MetalRedrawer:draw:nextDrawable") { | ||
metalLayer.nextDrawable() | ||
metalDrawableHandler.nextDrawable() | ||
} | ||
|
||
if (metalDrawable == null) { | ||
|
@@ -328,7 +330,7 @@ internal class MetalRedrawer( | |
} | ||
|
||
val renderTarget = | ||
BackendRenderTarget.makeMetal(width, height, metalDrawable.texture.objcPtr()) | ||
BackendRenderTarget.makeMetal(width, height, metalDrawableHandler.drawableTexture(metalDrawable).rawValue) | ||
|
||
val surface = Surface.makeFromBackendRenderTarget( | ||
context, | ||
|
@@ -344,6 +346,7 @@ internal class MetalRedrawer( | |
// Logger.warn { "'Surface.makeFromBackendRenderTarget' returned null. Skipping the frame." } | ||
picture.close() | ||
renderTarget.close() | ||
metalDrawableHandler.releaseDrawable(metalDrawable) | ||
dispatch_semaphore_signal(inflightSemaphore) | ||
return@autoreleasepool | ||
} | ||
|
@@ -372,7 +375,9 @@ internal class MetalRedrawer( | |
commandBuffer.label = "Present" | ||
|
||
if (!presentsWithTransaction) { | ||
commandBuffer.presentDrawable(metalDrawable) | ||
// scheduleDrawablePresentation consumes metalDrawable | ||
// don't use metalDrawable after this call | ||
Comment on lines
+384
to
+385
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we just nullify it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's a smart cast |
||
metalDrawableHandler.scheduleDrawablePresentation(metalDrawable, commandBuffer) | ||
} | ||
|
||
commandBuffer.addCompletedHandler { | ||
|
@@ -388,7 +393,9 @@ internal class MetalRedrawer( | |
commandBuffer.waitUntilScheduled() | ||
} | ||
|
||
metalDrawable.present() | ||
// presentDrawable consumes metalDrawable | ||
// don't use metalDrawable after this call | ||
metalDrawableHandler.presentDrawable(metalDrawable) | ||
|
||
interopTransaction.actions.fastForEach { | ||
it.invoke() | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comments?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, removed