Zoomable modifier looses state after ZoomableState gets disposed and recreated

[CalamityDeadshot]

I'm using Modifier.zoomable inside a HorizontalPager - a pretty standard use case.
My issue is that after a page is composed for a second time RealZoomableState looses its gestureState synchronization for some reason, so when ZoomableNode's onTransformStopped calls state.isZoomOutsideRange() its val userZoomFactor = gestureState?.userZoomFactor pretty much equals 1 for me, so state.smoothlySettleZoomOnGestureEnd() is not called whatsoever, and no zoom bounds are enforced.
A minimal repro with coil:

HorizontalPager(
    modifier = Modifier.fillMaxSize(),
    state = rememberPagerState(
        pageCount = { contents.size }
    ),
    beyondBoundsPageCount = 1
) { page ->
    val content = contents[page]
    val painter = rememberAsyncImagePainter(content.url)
    val zoomableState = rememberZoomableState(
        zoomSpec = ZoomSpec(
            maxZoomFactor = 3f,
            preventOverOrUnderZoom = false
        )
    )
    LaunchedEffect(painter.intrinsicSize) {
        if (painter.intrinsicSize.isSpecified) {
            zoomableState.setContentLocation(
                ZoomableContentLocation.scaledToFitAndCenterAligned(painter.intrinsicSize)
            )
        }
    }
    val contentModifier = Modifier
        .fillMaxSize()
        .zoomable(
            state = zoomableState
        )
    Image(
        painter = painter,
        contentDescription = null,
        contentScale = ContentScale.Fit,
        modifier = contentModifier,
    )
}

Here's how it looks for users:

video5417946174109795186.mp4

The same thing happens with pages that are composed for the first time if I scroll enough pages forwards/backwards from initialPage. Is my usage incorrect in some way?

Versions:

    kotlinCompilerExtensionVersion = "1.5.1"

    implementation(platform("androidx.compose:compose-bom:2024.02.00")) // So, androidx.compose.foundation:foundation is 1.6.2
    implementation("io.coil-kt:coil-compose:2.1.0")
    implementation("me.saket.telephoto:zoomable:0.8.0")

[CalamityDeadshot]

Also, flings no longer work in this situation either

[saket]

Is this no longer an issue?

[CalamityDeadshot]

It is. I honestly don't know why it's closed. I don't remember closing it. Maybe a missclick

[CalamityDeadshot]

Ah, I see. I referenced this issue in a commit to a private repo with a message containing Still need to fix https://github.com/saket/telephoto/issues/72. I guess GitHub tried to be clever. Funny

[kvn-stgl]

I think I have the same problem with the SubSamplingImage (with the ZoomableAsyncImage everything works as expected, sadly I can't use it for reasons).

I've changed the sample app a bit to reproduce the issue: kvn-stgl@b1b74ac
You only have to swipe the pager forward and backward. After the backward swipe, the double tab and back fling do not work anymore. Only the multitouch zoom.

Here is also a short video:

Screen_recording_20240306_105618.mp4

[saket]

@kvn-stgl I was able to reproduce the problem using your sample, thank you! This is related to #70.

@kvn-stgl and @CalamityDeadshot I'm also curious to learn your reasons for not using ZoomableAsyncImage(). Are both of you writing a multiplatform app?

[saket]

1749177 was deployed to maven. Can you please try out 0.9.0-SNAPSHOT?

[CalamityDeadshot]

reasons for not using ZoomableAsyncImage()

My gallery displays multiple types of content, like photos and videos, so it just makes more sense to use Modifier.zoomable for different types of content rather than implement a different solution for each one.

[CalamityDeadshot]

Can you please try out 0.9.0-SNAPSHOT?

I no longer experience this issue with me.saket.telephoto:zoomable:0.9.0-SNAPSHOT. Thank you for a quick fix!

[saket]

@CalamityDeadshot glad to hear!

My gallery displays multiple types of content, like photos and videos, so it just makes more sense to use Modifier.zoomable for different types of content rather than implement a different solution for each one.

This is exactly the usecase I wanted to support with having a decoupled ZoomableState. You shouldn't need to touch the lower level APIs for sharing your zoomable code between images and videos:

val zoomableState = rememberZoomableState()

when (media) {
 is Image -> {
    ZoomableAsyncImage(
     model = media.imageUrl,
     state = rememberZoomableImageState(zoomableState),
    )
  }
  is Video -> {
    ZoomableVideoPlayer(
      model = media.videoUrl,
      state = rememberZoomableExoState(zoomableState),
    )
  }
}

I'm going ahead and closing this issue, but let's continue this discussion because I want to nudge developers away from maintaining their own implementation of ZoomableAsyncImage().

[CalamityDeadshot]

You shouldn't need to touch the lower level APIs for sharing your zoomable code between images and videos

It's more of a matter of interoperability for me, as my gallery uses Painters as models for images and generic composable content models for videos.

val contentModifier = Modifier
    .fillMaxSize()
    .zoomable(
        state = zoomableState,
        onClick = { onClick() }
    )
when (model) {
    is Painter -> {
        // ...
        Image(
            painter = model,
            contentDescription = null,
            contentScale = ContentScale.Fit,
            modifier = contentModifier,
        )
    }
    is ComposeModel -> {
        // ...
        Box(
            modifier = contentModifier,
            contentAlignment = Alignment.Center
        ) {
            model.content()
        }
    }
}

This comes from a fork of this gallery which had its own transform gestures implementation, but it broke horribly (with a fatal crash) after I updated androidx.compose.foundation:foundation version to gain access to essential new features. It happened at such an unfortunate time, too - release deadline is around the corner, and I could not spare time to find the bug and fix it (this lib's code is, um... hard to reason about). I'm lucky I found Telephoto :)
So, basically, you are probably right, but I don't have the resources to refactor this at the moment, as this gallery API is currently used all over the app, - but I most certainly intend to! Sub-sampling alone is a very cool feature.

[saket]

Got it. Thanks for explaining!