Fix initial Skiko popup positioning

[Vendula Švastalová (svastven)]

Fixes selection handles briefly shown at an offset.

Popup anchoring relied on the next snapshot apply (removed in #3272) during drawing to make parent bounds written during layout visible to the separate popup scene. As a result, the first popup layout used the initial IntRect.Zero anchor causing the issue.

We now compose the Popup content only after a real parent anchor is available which prevents drawing the first zero-anchor frame. Similar approach is used on Android

compose-multiplatform-core/compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/window/AndroidPopup.android.kt

Line 635 in 9e18f11

/** Track parent coordinates and content size; only show popup once we have both. */

Bug	Fix
Bug.mov	Fix.mov

Fixes CMP-10684 Selection handles blink on the view top
Fixes CMP-10714 [iOS] Dismissing a Popup leaves stale content when another Popup layer remains

Testing

This should be tested by QA

Release Notes

Fixes - iOS

• (prerelease fix) Fix an issue where text-selection handles could briefly appear in the wrong position when selecting text.

[Ivan Matkov (MatkovIvan)]

It seems it breaks "showing in the same frame" behavior. Test failures seems to be related

[Vendula Švastalová (svastven)]

yes, wrote a comment about that: #3321 (comment) we need to choose which is preferred.

[Ivan Matkov (MatkovIvan)]

Popup does not always depend on parent bounds and can have absolute positioning. In this case it seems we shouldn't delay showing it.
But I'm not sure if we can detect it here since it's just custom positioning logic from PoV of this code

[Vendula Švastalová (svastven)]

testLayerContentOnFirstRender currently expects that a Popup that is created in a frame will also be rendered in the same frame, but the fix in this PR is not in line with this expectation. Now the popup scene is empty up until it has a valid parent anchor.

The core problem is that the main scene's placement phase and the popup layer layout phase are independent, the frame timing is shared but there is no guarantee in the dependency between layers so that we can expect main onPlaced -> popup layout -> popup draw.

now there are two options if we don't want to tackle the dependency issue ATM:

1. Wrap the parent bounds write in Snapshot.withMutableSnapshot

Snapshot.withMutableSnapshot {
    parentBoundsInWindow.value = IntRect(layoutPosition, layoutSize)
}

This would restore the local snapshot apply during the main scene layout call (removed in #3272), and the popup scene can then consume the real anchor and render in the first frame.

2. Keep the current state, wait with first popup frame until parent bounds are ready. This avoids applying snapshot changes during layout. But it would require losening the expecations of testLayerContentOnFirstRender. Android also waits for when parent bounds are ready but it makes the popup content transparent until then rather than leaving it empty.

[Alexander Maryanovsky (m-sasha)]

The original code (and comments) assumed the popup and the parent are in the same scene. When that's not true, then indeed I don't think we can expect the popup to be shown in the same frame.

I wouldn't completely remove my comments or tests though, but instead update them to document the two cases (same scene, different scenes).

[Vendula Švastalová (svastven)]

I updated the comments and tests to document both cases based on the updated behavior. Alexander Maryanovsky (@m-sasha) can you please review?

[Alexander Maryanovsky (m-sasha)]

This PR breaks a needed functionality: that popups are shown immediately.
This is needed for example, for initial sizing of a window that has a popup in it. Without this, it can't be sized to take into account the popup size.

I understand that this is not possible when the popup is in a separate native layer (maybe will be possible again after Ivan's planned shared recomposer change?), but can we keep it when the popup is in the same layer?

[Alexander Maryanovsky (m-sasha)]

By checking the 100x100 size you're validating that the popup isn't composed until the dialog is rendered. This is the (new) behavior, but it's not a required behavior, so we shouldn't check for it.

[Alexander Maryanovsky (m-sasha)]

This test wants to verify that a dialog/window with a popup accomodates the popup's size when being shown. The PR seems to break this.

[Vendula Švastalová (svastven)]

I see, got it. I didn't realize that some popups are still part of the scene they are shown from and can be shown immediately. We could distinguish this by introducing a flag in ComposeSceneContext, like layersShareCurrentScene, do you see a problem with this approach?

[Alexander Maryanovsky (m-sasha)]

Can we do it without a flag?

I guess what I don't understand is how this PR breaks the existing functionality when the popup is in the same scene. It seems that if indeed EmptyLayout(Modifier.onPlaced) is called before the popup measure policy, the popup should be shown immediately. What am I missing?

[Ivan Matkov (MatkovIvan)]

do you see a problem with this approach?

Yes. Scene is designed to hide this implementation detail.

[Vendula Švastalová (svastven)]

Flag is not needed. We can handle it all in Popup. For some reason I didn't realize it at first and imagined it more complicated. Alexander Maryanovsky (@m-sasha) thanks for pointing it out.