saket / telephoto

Building blocks for designing media experiences in Compose UI
https://saket.github.io/telephoto/
Apache License 2.0
996 stars 32 forks source link

Telephoto internal NPE #70

Closed revonateB0T closed 8 months ago

revonateB0T commented 8 months ago
FATAL EXCEPTION: main
Process: moe.tarsin.ehviewer.debug, PID: 13059
java.lang.NullPointerException
    at me.saket.telephoto.zoomable.RealZoomableState.fling-BMRW4eQ$zoomable_release(RealZoomableState.kt:502)
    at me.saket.telephoto.zoomable.ZoomableNode$onTransformStopped$1$1.invokeSuspend(Zoomable.kt:138)
    at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
    at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:104)
    at androidx.compose.ui.platform.AndroidUiDispatcher.performTrampolineDispatch(AndroidUiDispatcher.android.kt:81)
    at androidx.compose.ui.platform.AndroidUiDispatcher.access$performTrampolineDispatch(AndroidUiDispatcher.android.kt:41)
    at androidx.compose.ui.platform.AndroidUiDispatcher$dispatchCallback$1.run(AndroidUiDispatcher.android.kt:57)
    at android.os.Handler.handleCallback(Handler.java:958)
    at android.os.Handler.dispatchMessage(Handler.java:99)
    at android.os.Looper.loopOnce(Looper.java:205)
    at android.os.Looper.loop(Looper.java:294)
    at android.app.ActivityThread.main(ActivityThread.java:8296)
    at java.lang.reflect.Method.invoke(Native Method)
    at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:552)
    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:978)
    Suppressed: kotlinx.coroutines.internal.DiagnosticCoroutineContextException: [androidx.compose.ui.platform.MotionDurationScaleImpl@3a1253d, androidx.compose.runtime.BroadcastFrameClock@f23a132, StandaloneCoroutine{Cancelling}@bf50e83, AndroidUiDispatcher@e154800]
revonateB0T commented 8 months ago

https://github.com/FooIbar/EhViewer/pull/832

revonateB0T commented 8 months ago

https://github.com/saket/telephoto/blob/89bedb575e44e1bb945f6b2283c039f38f3e6181/zoomable/src/commonMain/kotlin/me/saket/telephoto/zoomable/RealZoomableState.kt#L502

saket commented 8 months ago

This is so weird. There are only 2 possible situations when gestureState can be null:

  1. No gesture has been recorded yet
  2. ZoomableState#resetZoom() was called

If a fling was detected then 1 should be impossible. 2 also sounds unlikely because I wasn't able to find any calls to ZoomableState#resetZoom() in your codebase.

revonateB0T commented 8 months ago

ZoomableNode is reused in LazyLayout https://github.com/saket/telephoto/blob/89bedb575e44e1bb945f6b2283c039f38f3e6181/zoomable/src/commonMain/kotlin/me/saket/telephoto/zoomable/Zoomable.kt#L104 We should update its state on update https://github.com/saket/telephoto/blob/89bedb575e44e1bb945f6b2283c039f38f3e6181/zoomable/src/commonMain/kotlin/me/saket/telephoto/zoomable/Zoomable.kt#L167 https://github.com/saket/telephoto/blob/89bedb575e44e1bb945f6b2283c039f38f3e6181/zoomable/src/commonMain/kotlin/me/saket/telephoto/zoomable/Zoomable.kt#L105

fun update(
    state: RealZoomableState,
    enabled: Boolean,
    onClick: ((Offset) -> Unit)?,
    onLongClick: ((Offset) -> Unit)?,
  ) {
+  state = state
    transformableNode.update(
      state = state.transformableState,
      canPan = state::canConsumePanChange,
      lockRotationOnZoomPan = false,
      enabled = enabled,
      onTransformStopped = onTransformStopped,
    )
    tappableAndQuickZoomableNode.update(
      onPress = onPress,
      onTap = onClick,
      onLongPress = onLongClick,
      onDoubleTap = onDoubleTap,
      onQuickZoomStopped = onQuickZoomStopped,
      transformableState = state.transformableState,
      gesturesEnabled = enabled,
    )
  }

Then onTransformStopped can callback correct RealZoomableState https://github.com/saket/telephoto/blob/89bedb575e44e1bb945f6b2283c039f38f3e6181/zoomable/src/commonMain/kotlin/me/saket/telephoto/zoomable/Zoomable.kt#L138 @saket

saket commented 8 months ago

Good catch! Let me fix this. Are you able to consistently reproduce this crash?

saket commented 8 months ago

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

revonateB0T commented 8 months ago

Good catch! Let me fix this. Are you able to consistently reproduce this crash?

Fixed in 0.9.0-SNAPSHOT, 👍