Kamel-Media / Kamel

Kotlin asynchronous media loading and caching library for Compose.
Apache License 2.0
592 stars 23 forks source link

KamelConfig.Default does not load automatically with v1 beta #97

Closed DjuroRad closed 3 months ago

DjuroRad commented 4 months ago

Steps to reproduce:

Use KMP wizard by following this link and create multiplatform project sharing UI for iOS and Android Use version of kamel 1.0.0-beta.1 (ktor client version 2.3.8)

KamelImage(
            resource = asyncPainterResource(data = "https://picsum.photos/200"),
            contentDescription = null,
            modifier = Modifier
                .size(230.dp)
                .background(color = Color.Green),
            onFailure = {
                Text(text = "failed")
            },
            onLoading = {
                Text(text = "loading")
            }
        )

Result: KamelImage's onFailed will be rendered

luca992 commented 4 months ago

did you read the release notes?

https://github.com/Kamel-Media/Kamel/releases/tag/v1.0.0-beta.1

DjuroRad commented 4 months ago

I did, I went over them. Is there something that's obviously wrong with the example I provided?

luca992 commented 4 months ago

no that should be fine.

are you using implementation("media.kamel:kamel-image-default:1.0.0-beta.1")?

I can't really help you without more details or at least the stack trace of the error that is being passed to onFailure

DjuroRad commented 4 months ago

Yes, I am using kamel-image-default name from media.kamel group. I tried it with almost any name available.
I tried with other names from your group also, added kamel-decoder-image-bitmap just in case. Also tried adding implementation( "org.jetbrains.kotlin:kotlin-reflect:1.9.21") explicitly but that didn't help either

Sorry about forgeting the stacktrace, here it is

2024-02-29 09:22:23.611  9646-9646  System.out              org.example.melodyreservation        I  java.lang.IllegalStateException: Unable to find a decoder for interface androidx.compose.ui.graphics.ImageBitmap (Kotlin reflection is not available) 
2024-02-29 09:22:23.611  9646-9646  System.out              org.example.melodyreservation        I   message: Unable to find a decoder for interface androidx.compose.ui.graphics.ImageBitmap (Kotlin reflection is not available)
2024-02-29 09:22:23.611  9646-9646  System.out              org.example.melodyreservation        I  stacktrace: java.lang.IllegalStateException: Unable to find a decoder for interface androidx.compose.ui.graphics.ImageBitmap (Kotlin reflection is not available)
2024-02-29 09:22:23.611  9646-9646  System.out              org.example.melodyreservation        I      at io.kamel.core.ImageLoadingKt$loadImageBitmapResource$$inlined$loadResource$1.invokeSuspend(ImageLoading.kt:107)
2024-02-29 09:22:23.611  9646-9646  System.out              org.example.melodyreservation        I      at io.kamel.core.ImageLoadingKt$loadImageBitmapResource$$inlined$loadResource$1.invoke(Unknown Source:8)
2024-02-29 09:22:23.611  9646-9646  System.out              org.example.melodyreservation        I      at io.kamel.core.ImageLoadingKt$loadImageBitmapResource$$inlined$loadResource$1.invoke(Unknown Source:4)
2024-02-29 09:22:23.611  9646-9646  System.out              org.example.melodyreservation        I      at kotlinx.coroutines.flow.SafeFlow.collectSafely(Builders.kt:61)
2024-02-29 09:22:23.611  9646-9646  System.out              org.example.melodyreservation        I      at kotlinx.coroutines.flow.AbstractFlow.collect(Flow.kt:230)
2024-02-29 09:22:23.611  9646-9646  System.out              org.example.melodyreservation        I      at kotlinx.coroutines.flow.FlowKt__ErrorsKt.catchImpl(Errors.kt:156)
2024-02-29 09:22:23.611  9646-9646  System.out              org.example.melodyreservation        I      at kotlinx.coroutines.flow.FlowKt.catchImpl(Unknown Source:1)
2024-02-29 09:22:23.611  9646-9646  System.out              org.example.melodyreservation        I      at kotlinx.coroutines.flow.FlowKt__ErrorsKt$catch$$inlined$unsafeFlow$1.collect(SafeCollector.common.kt:114)
2024-02-29 09:22:23.611  9646-9646  System.out              org.example.melodyreservation        I      at androidx.compose.runtime.SnapshotStateKt__SnapshotFlowKt$collectAsState$1$2.invokeSuspend(SnapshotFlow.kt:66)
2024-02-29 09:22:23.611  9646-9646  System.out              org.example.melodyreservation        I      at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
2024-02-29 09:22:23.611  9646-9646  System.out              org.example.melodyreservation        I      at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:108)
2024-02-29 09:22:23.611  9646-9646  System.out              org.example.melodyreservation        I      at kotlinx.coroutines.internal.LimitedDispatcher$Worker.run(LimitedDispatcher.kt:115)
2024-02-29 09:22:23.611  9646-9646  System.out              org.example.melodyreservation        I      at kotlinx.coroutines.scheduling.TaskImpl.run(Tasks.kt:103)
2024-02-29 09:22:23.611  9646-9646  System.out              org.example.melodyreservation        I      at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:584)
2024-02-29 09:22:23.611  9646-9646  System.out              org.example.melodyreservation        I      at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:793)
2024-02-29 09:22:23.611  9646-9646  System.out              org.example.melodyreservation        I      at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:697)
2024-02-29 09:22:23.611  9646-9646  System.out              org.example.melodyreservation        I      at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:684)
luca992 commented 4 months ago

reflection shouldn't be required. Did you make you happen to make your own KamelConfig?

If you just need it to work I would suggest using a non-beta v0.9 build for now.

DjuroRad commented 4 months ago

I am using version 0.9.3. No issues with that version. I didn't make my own KamelConfig. It worked out of box in the previous versions.

Now that you mentioned it, I see that LocalKamelConfig defaults to KamelConfig.Core In KamelConfigImage.kt

public var configOverride: KamelConfig? = null

public val LocalKamelConfig: ProvidableCompositionLocal<KamelConfig> =
    staticCompositionLocalOf { configOverride ?: KamelConfig.Core }

And if I update it to KamelConfig.Default

KamelConfig {
    configOverride = KamelConfig.Default
}

it will now work 😃

I also see that KamelConfig.Default uses takeFrom from KamelConfig.Core

public val KamelConfig.Companion.Default: KamelConfig
    get() = KamelConfig {
        takeFrom(KamelConfig.Core)
        imageBitmapDecoder()
        imageVectorDecoder()
        svgDecoder()
        platformSpecificConfig()
    }

and beside decoders adds the expected platformSpecificConfig()

Is this intended behaviour? I think it should be provided somewhere within the README.md or the release notes since this happens out of box when using KMP wizard which is used by a lot of people experimenting with compose multiplatform. Do you intend to keep it like this or maybe change the configOverride to default to KamelConfig.Default instead of KamelConfig.Core?

Wdyt luca?

luca992 commented 4 months ago

configOverride is supposed to be set to KamelConfig.Default automatically here.

It sounds like it's not actually being set automatically, I'll look into it. And the 1.0 build is definitely still a beta. I would hope no one is setting that to be used in a wizard rather than the stable build.

DjuroRad commented 4 months ago
image

Regarding the initializer 😃 (though I always suspect a possible IDE issue)

luca992 commented 4 months ago

That is intended.

Are you having this issue on ios only? That is the only target I am able to duplicate it on

luca992 commented 4 months ago

1.0.0-beta.2 should work on iOS out of the box without setting your own config

DjuroRad commented 4 months ago

@luca992 version beta.2 fixes the issue on iOS but the issues is still present on Android. Same stacktrace and everything. Even if I set the configOverride manually on Android it doesn't fix it now. On version beta.1 setting configOverride = KamelConfig.Default fixes the issue for both iosMain and androidMain targets whereas beta.2 doesn't fix it for Android (iOS works out of box)

Should I create another issue and reference this one or you would like to reopen this one?

luca992 commented 4 months ago

Hmm strange. I couldn't cause the same issue on android. Also,configOverride should not be exposed in beta 2 anymore. But either way you should be able to specify the default config to be used with the method described in the readme.

DjuroRad commented 4 months ago

Yap, but doesn't work even when specifying it

luca992 commented 4 months ago

I was able to duplicate it. But specifying the default config does work as it should.

val kamelConfig = remember {
    KamelConfig {
        takeFrom(KamelConfig.Default)
    }
}
CompositionLocalProvider(LocalKamelConfig provides kamelConfig) {
    // your composeable
}
DjuroRad commented 4 months ago

I checked it now with just using within the main composable called App and adding the following (same as previosuly) made it work

KamelConfig {
        configOverride = KamelConfig.Default
    }

I should've invalidated the caches, my bad

luca992 commented 4 months ago

configOverride is supposed to be internal. Just fyi

luca992 commented 3 months ago

@DjuroRad should be fixed in 1.0.0-beta.3