Support sizing of Compose hosting views on iOS#2984
Support sizing of Compose hosting views on iOS#2984Vendula Švastalová (svastven) wants to merge 30 commits into
Conversation
c3fcdd8 to
9d38020
Compare
|
Marked as draft as I still need to add tests for the UIKit/SwiftUI intrinsic sizing path. |
|
Alexander Maryanovsky (@m-sasha) Adding you as reviewer, too because desktop sources are involved and there is overlap with #2938 |
5b3ada9 to
0ae5bce
Compare
| /** | ||
| * Returns the current content size (in pixels) measured with the provided [constraints]. | ||
| * | ||
| * @throws IllegalStateException when [ComposeScene] content has lazy layouts without maximum | ||
| * size bounds (e.g. LazyColumn without maximum height) and the constraints are unbounded. | ||
| */ |
There was a problem hiding this comment.
This comment is only correct if the relevant dimension is unconstrained (e.g. height=Infinity).
There was a problem hiding this comment.
so "the constraints are unbounded" doesn't reflect the behavior correctly? It seems to me that the doc is the same as for unconstrainedSize() only specifies the unbounded constraints in addition which is exactly what happens in unconstrainedSize()
There was a problem hiding this comment.
Sorry, you're right. I didn't read the comment to the end, though it was just copy/pasted from unconstrainedSize.
I would just clarify in the comment that this applies on each axis separately.
| * platform integrations to avoid retain cycles between a hosting container and | ||
| * the scene/root. | ||
| */ | ||
| fun registerOnLayoutCompletedListener(listener: () -> Unit): AutoCloseable |
There was a problem hiding this comment.
Not a blocker, but I'd slightly prefer a dedicated class instead of AutoCloseable.
Existing examples:
PinnableContainer.PinnedHandleDelegatableNode.RegistrationHandle(maybe even just use this)
There was a problem hiding this comment.
added a custom OnLayoutCompletedListenerHandle
Ivan Matkov (MatkovIvan)
left a comment
There was a problem hiding this comment.
I don't think it's the right way to support it.
The root problem now is that compose stages are not aligned with the platform framework.
We already have an item to do it properly, let's combine the efforts to do it right instead of redoing things again later
| * `sizeThatFits` / intrinsic sizing). | ||
| */ | ||
| @ExperimentalComposeUiApi | ||
| var useSelfSizing: Boolean = false |
There was a problem hiding this comment.
Do we need this option? I assumed that self-sizing (like, providing intrinsicContentSize) should be always ON.
There was a problem hiding this comment.
I added this flag so we don't change behavior for existing users. If we turn it on by default, it will affect what they already have. Do we want that, or keep it opt-in for now?
There was a problem hiding this comment.
Hm.. Previously we didn't have any constraints there - so users must configure them explicitly. I expect that transition should go smooth to the new state in most cases.
It looks like a safe option here is to make useSelfSizing = true by default, with "Breaking Change" release note and also with intent to remove the flag in the next version (1.13).
There was a problem hiding this comment.
I have removed the flag, it is correct that embedding view's using AutoLayout already worked correctly so there should be no significant changes in the current way user's embed Compose into UIKit using AutoLayout.
|
|
||
| rootViewController.view.let { | ||
| if (configuration.useSelfSizing) { | ||
| it.addSubview(hostingView) |
There was a problem hiding this comment.
Not sure I get the point. If we're adding view with the translatesAutoresizingMaskIntoConstraints == true, it means the frame of this view must be manually adjusted somewhere (btw, I didn't find the place where it happened). The idea behind the "Self Sizing" Compose views was to make it work correctly inside Autolayout and in the SwiftUI system. No sure we should manually update the frame.
There was a problem hiding this comment.
The setContent flow remains the same as before. I have only created a few convenience methods to be used in setContent.
4ba42df to
5c9fcde
Compare
07caea6 to
9a0f64b
Compare
733dc31 to
69a4248
Compare
5d675b5 to
79e016d
Compare
| sceneRenderingScope.postponingSceneInvalidations { | ||
| frameRecomposer.performFrame(nanoTime) | ||
| measureAndLayout() | ||
| onLayoutCompleted() |
There was a problem hiding this comment.
Why is it important to do it before drawing?
| } | ||
|
|
||
| private fun measureAndCachePreferredSize(constraints: Constraints): Boolean? { | ||
| val preferredSize = composeSceneSize(constraints) ?: return null |
There was a problem hiding this comment.
It seems that one more unconditional re-measure on each frame might be costly
| sceneRenderingScope.postponingSceneInvalidations { | ||
| frameRecomposer.performFrame(nanoTime) | ||
| measureAndLayout() | ||
| onLayoutCompleted() |
There was a problem hiding this comment.
Should we wrap call it only if there was hasPendingMeasureOrLayout == true before measureAndLayout call?
There was a problem hiding this comment.
I thought about this and if we only did this, then here we are assuming that measureAndLayout calculates with hasPendingMeasureOrLayout internally. It would be great to have an indicator that layout actually took place during the measureAndLayout call. But maybe comparing the values of hasPendingMeasureOrLayout is the right way to do it.
There was a problem hiding this comment.
Actually it seems that invalidation request to a system should be initiated much earlier, in invalidateLayout callback from a scene.
It's supposed to be called when the layout requires recalculation on the next frame and needs to be scheduled. But now the chain is like this (afaiu):
invalidateLayout -> scheduleFrame -> render -> invalidateIntrinsicContentSize
which seems that actual resize will happen with one frame delay (see another comment/thread).
Of course, it will work better once measureAndLayout is actually part of the platform layout, not render.
There was a problem hiding this comment.
It would be great to have an indicator that layout actually took place during the measureAndLayout call
Well, the issue is opposite. The platform does not need to call measureAndLayout without receiving invalidateLayout callback at all.
| val didUpdatePreferredSize = measureAndCachePreferredSize(constraints) ?: return | ||
|
|
||
| if (didUpdatePreferredSize) { | ||
| invalidateComposeSceneContainerSize() |
There was a problem hiding this comment.
It seems it will request another frame and apply changes only there.
It might work for PoC, but for the long term I believe compose should participate in the native layout phase synchronously
|
I've removed "changes in API" label since I don't see any changes in public API here anymore |
This PR adds a sizing integration that lets a Compose-hosting UIView participate in UIKit/SwiftUI sizing loops by reporting Compose’s preferred size for a given sizing proposal.
Fixes CMP-5788 iOS investigate intrinsic sizing of ComposeUIViewController
Fixes CMP-5873 [Epic] iOS intrinsic sizing of interop elements
How it works
sizeThatFits(...)with a proposal (includingUIViewNoIntrinsicMetricfor unbounded axes).Fallback handling uses
super.sizeThatFitsfor non-intrinsic axes to avoid 0 sizes before Compose has produced a measurement.Example usage
(SwiftUI, iOS 16+): sizeThatFits
On the Kotlin side:
On the SwiftUI side, wrap the returned
UIView/UIViewControllerand forward SwiftUI’s proposal to UIKitsizeThatFits:(SwiftUI, iOS <16): sizeThatFits
The same mechanism works via intrinsic sizing alone: SwiftUI reacts to invalidateIntrinsicContentSize() and re-queries layout.
Testing
Adds instrumented tests that simulate the SwiftUI sizing loop (proposal →
sizeThatFits→ apply frame → wait for intrinsic invalidation → repeat) and validate that Compose scene size and hosting view frame converge as Compose content or size proposed by SwiftUI changes.This should be tested by QA
Release Notes
Features - iOS
ComposeHostingView/ComposeHostingViewController) used on the UIKit / SwiftUI side. Enabled by default; opt out viaComposeContainerConfiguration.useSelfSizing.Breaking Changes - iOS