diff --git a/android/src/new/java/com/margelo/nitro/rive/HybridViewModelImageProperty.kt b/android/src/new/java/com/margelo/nitro/rive/HybridViewModelImageProperty.kt index 487a4e99..d8cc31d5 100644 --- a/android/src/new/java/com/margelo/nitro/rive/HybridViewModelImageProperty.kt +++ b/android/src/new/java/com/margelo/nitro/rive/HybridViewModelImageProperty.kt @@ -9,28 +9,43 @@ import com.facebook.proguard.annotations.DoNotStrip import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch +import java.util.concurrent.atomic.AtomicInteger + +// Shared by every wrapper for one instance path, since JS may look the property up again per set. +internal class ImagePropertySequencer { + // Single-threaded so a clear cannot run between the generation check and setImage. + val scope = CoroutineScope(Dispatchers.Default.limitedParallelism(1)) + + // Decoding suspends, so a decode that finishes after a newer set or clear must be dropped. + val generation = AtomicInteger() +} @Keep @DoNotStrip -class HybridViewModelImageProperty( +class HybridViewModelImageProperty internal constructor( private val instance: ViewModelInstance, private val path: String, - private val riveWorker: CommandQueue + private val riveWorker: CommandQueue, + private val sequencer: ImagePropertySequencer ) : HybridViewModelImagePropertySpec(), BaseHybridViewModelProperty by BaseHybridViewModelPropertyImpl() { companion object { private const val TAG = "HybridViewModelImageProperty" } - private val imageScope = CoroutineScope(Dispatchers.Default) - override fun set(image: HybridRiveImageSpec?) { - val hybridImage = image as? HybridRiveImage ?: return - imageScope.launch { + val hybridImage = image as? HybridRiveImage + val gen = sequencer.generation.incrementAndGet() + sequencer.scope.launch { try { + if (hybridImage == null) { + instance.setImage(path, null) + return@launch + } val result = ImageAsset.fromBytes(riveWorker, hybridImage.rawData) if (result is app.rive.Result.Success) { - instance.setImage(path, result.value) + // The property keeps its own ref, so close ours right away. + result.value.use { if (sequencer.generation.get() == gen) instance.setImage(path, it) } } else { Log.e(TAG, "Failed to decode image for path '$path'") } diff --git a/android/src/new/java/com/margelo/nitro/rive/HybridViewModelInstance.kt b/android/src/new/java/com/margelo/nitro/rive/HybridViewModelInstance.kt index c18385f4..b122daa4 100644 --- a/android/src/new/java/com/margelo/nitro/rive/HybridViewModelInstance.kt +++ b/android/src/new/java/com/margelo/nitro/rive/HybridViewModelInstance.kt @@ -7,6 +7,7 @@ import app.rive.ViewModelInstanceSource import app.rive.core.CommandQueue import com.facebook.proguard.annotations.DoNotStrip import com.margelo.nitro.core.Promise +import java.util.concurrent.ConcurrentHashMap @Keep @DoNotStrip @@ -24,6 +25,8 @@ class HybridViewModelInstance( private const val TAG = "HybridViewModelInstance" } + private val imageSequencers = ConcurrentHashMap() + override fun dispose() { if (ownsInstance) { runCatching { viewModelInstance.close() } @@ -78,7 +81,8 @@ class HybridViewModelInstance( override fun imageProperty(path: String): HybridViewModelImagePropertySpec? { return try { - HybridViewModelImageProperty(viewModelInstance, path, riveWorker) + val sequencer = imageSequencers.getOrPut(path) { ImagePropertySequencer() } + HybridViewModelImageProperty(viewModelInstance, path, riveWorker, sequencer) } catch (e: Exception) { Log.e(TAG, "imageProperty failed for path '$path'", e) null diff --git a/example/__tests__/image-property-clear.harness.ts b/example/__tests__/image-property-clear.harness.ts new file mode 100644 index 00000000..38ea430e --- /dev/null +++ b/example/__tests__/image-property-clear.harness.ts @@ -0,0 +1,44 @@ +import { describe, it, expect } from 'react-native-harness'; +import { Platform } from 'react-native'; +import { RiveFileFactory, RiveImages } from '@rive-app/react-native'; + +const DATABINDING_IMAGES = require('../assets/rive/databinding_images.riv'); + +// 1x1 red PNG +const PNG_BASE64 = + 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAIAAACQd1PeAAAADElEQVR4nGP4z8AAAAMBAQDJ/pLvAAAAAElFTkSuQmCC'; + +function pngBytes(): ArrayBuffer { + const binary = atob(PNG_BASE64); + const bytes = new Uint8Array(binary.length); + for (let i = 0; i < binary.length; i++) bytes[i] = binary.charCodeAt(i); + return bytes.buffer; +} + +describe('ViewModelImageProperty', () => { + it('repeated set(image) and set(undefined) do not throw', async () => { + const file = await RiveFileFactory.fromSource( + DATABINDING_IMAGES, + undefined + ); + const instance = file + .viewModelByName('MyViewModel') + ?.createInstanceByIndex(0); + const prop = instance?.imageProperty('bound_image'); + expect(prop).toBeDefined(); + + // The new iOS runtime can't clear image properties yet (#360) + const canClear = + Platform.OS !== 'ios' || RiveFileFactory.getBackend() !== 'experimental'; + + const image = await RiveImages.loadFromBytesAsync(pngBytes()); + for (let i = 0; i < 10; i++) prop!.set(image); + if (canClear) prop!.set(undefined); + prop!.set(image); + if (canClear) prop!.set(undefined); + + prop!.dispose(); + instance!.dispose(); + file.dispose(); + }); +});