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

ComposeUiViewController with nested scrollable column shrinks Column with UIModalPresentationFormSheet #4850

Open
nilufer32 opened this issue May 22, 2024 · 16 comments
Assignees
Labels
bug Something isn't working ios reproduced scroll

Comments

@nilufer32
Copy link

nilufer32 commented May 22, 2024

Describe the bug
A clear and concise description of what the bug is.
When using ComposeUiViewController, and setting the modalPresentationStyle of the view controller to UIModalPresentationFormSheet, dragging the form sheet updwards/downwards causes shrinking/stretching of the nested composable scrollable column I place inside it. Also im not able to vertically scroll my column.

Affected platforms

IOS

Versions

  • Libraries:
    • Compose Multiplatform version: 1.6.10
  • Kotlin version: 2.0.0-RC1

To reproduce:

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 = {},
        )
                
                
 @Composable
fun VerticalScrollWithIndependentHorizontalRows() {
    Column(
        modifier =
            Modifier.fillMaxSize().verticalScroll(verticalScrollState, 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")
                    }
                }
            }
        }
    }
}

Video depicting the bug :

Screen.Recording.2024-05-22.at.7.33.37.PM.mov

Additional Warning:
When attempting to scroll in my nested vertically scrollable column, I receive the following logs in Xcode:
DidReceiveMemoryWarning

@nilufer32
Copy link
Author

any updates on this issue?

@elijah-semyonov
Copy link
Contributor

I'll have a look at it soon.

@elijah-semyonov
Copy link
Contributor

The nested scrolling issue is a separate one:
#3806

@nilufer32
Copy link
Author

But is the scrolling issue related to the shrinking/stretching of the content?

@elijah-semyonov
Copy link
Contributor

Not really

@elijah-semyonov
Copy link
Contributor

elijah-semyonov commented Jun 5, 2024

Memory warning doesn't seem to be related to the repro you posted. Most likely you have a retain cycle between Kotlin and ObjC with a strong reference inside ObjC API that Kotlin can't break.

@nilufer32
Copy link
Author

@elijah-semyonov I dont really see how could that be the case as the prototype app I used to reproduce the bug uses the standard architecture of a multiplatform app where I simply use a ComposeUIViewController from swift, and to this view controller I add the sample code above

@elijah-semyonov
Copy link
Contributor

Do you mind sharing the whole solution?
"DidReceiveMemoryWarning" means something went terribly wrong.

@nilufer32
Copy link
Author

all I do to trigger the memory warning is attempt to scroll inside the column in my bottom sheet by scrolling multiple times consecutively. this actually does scroll inside my column but at an extremely slow pace as in every scroll event scrolls the column by 1 pixel roughly

@elijah-semyonov
Copy link
Contributor

Can you record it? I fail to reproduce this behavior.

@nilufer32
Copy link
Author

sure ill record it right now

@nilufer32
Copy link
Author

nilufer32 commented Jun 5, 2024

Here's the repo containing the reproduction code:
https://github.com/nilufer32/test

RPReplay_Final1717579567.MOV
Screenshot 2024-06-05 at 12 27 03 PM

@elijah-semyonov
Copy link
Contributor

elijah-semyonov commented Jun 6, 2024

@nilufer32
You've got wrong Compose Version in repro 🌚
compose = "1.5.11"

@elijah-semyonov
Copy link
Contributor

Reproduced memory spike though

@nilufer32
Copy link
Author

the issue is 100% reproducible using compose 1.6.10 and kotlin 2.0.0. I just seem to have forgotten to update the library versions when I was creating a sample app to post for you onto github :)

@elijah-semyonov
Copy link
Contributor

elijah-semyonov commented Jun 7, 2024

A memory spike part of the issue should be fixed in JetBrains/compose-multiplatform-core#1390

MatkovIvan added a commit to JetBrains/compose-multiplatform-core that referenced this issue Jun 7, 2024
Since `owner.size` doesn't really observed during composition setting
scene size might not trigger invalidations

- Manually run `updateRootConstraints` instead of marking size as state
- Pick extra conditions from
[aosp/2598886](https://android-review.googlesource.com/c/platform/frameworks/support/+/2598886)

Fixes stretching reported in
JetBrains/compose-multiplatform#4850

## Testing

Run `MultiLayerComposeSceneTest`

## Release Notes

### Fixes - iOS

- Fix missing invalidations during native view resize
elijah-semyonov added a commit to JetBrains/compose-multiplatform-core that referenced this issue Jun 10, 2024
Explicitly manage lifetime of metal drawables by hiding them from Kotlin
runtime.

Fixes a memory spike when quickly resizing metal layer as in a scenario
reported in [the
issue](JetBrains/compose-multiplatform#4850)

Otherwise associated drawable pools are retained until next GC, which
can happen after inadequate amount of drawables with new sizes are
allocated

###
Before:
<img width="414" alt="Screenshot 2024-06-06 at 13 33 46"
src="https://github.com/JetBrains/compose-multiplatform-core/assets/4167681/81d74dfe-91c0-4362-ad71-93416dbe172f">
###
After:
<img width="574" alt="Screenshot 2024-06-07 at 10 04 18"
src="https://github.com/JetBrains/compose-multiplatform-core/assets/4167681/6574b297-1e81-48db-9aee-d31c2425267c">


## Testing

Resize the popup with `ComposeUIViewController` to different detents
inside Demo `IosBugs/PopupStretching`
Requires either patch:
```
Index: compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/scene/ComposeSceneMediator.uikit.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/scene/ComposeSceneMediator.uikit.kt b/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/scene/ComposeSceneMediator.uikit.kt
--- a/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/scene/ComposeSceneMediator.uikit.kt	(revision 39d436a)
+++ b/compose/ui/ui/src/uikitMain/kotlin/androidx/compose/ui/scene/ComposeSceneMediator.uikit.kt	(date 1717747754590)
@@ -558,6 +558,7 @@
                 asDpRect().toRect()
             }
         }
+        onComposeSceneInvalidate()
         scene.size = IntSize(
             width = boundsInPx.width.roundToInt(),
             height = boundsInPx.height.roundToInt()
```
Or #1387

## Release Notes

### iOS - Fixes
- Fixed a memory spike when continuously resizing the
`ComposeUIViewController` (such as when used in modal sheet presentation
context with different detents)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ios reproduced scroll
Projects
None yet
Development

No branches or pull requests

3 participants