Capturable icon indicating copy to clipboard operation
Capturable copied to clipboard

Consider an alternate capture result syntax

Open X1nto opened this issue 1 year ago • 0 comments

The v2 syntax for capturing uses Kotlin's Deferred interface to return the captured ImageBitmap. While this allows for async execution, I think it has a major flaw, and that is its ability to properly catch errors. Exceptions in Kotlin aren't exactly type-checked, meaning that it's very easy to forget the try-catch clause. Hence, my first proposal - custom return value based on a sealed interface:

sealed interface CaptureResult {

    @JvmInline
    value class Success(val bitmap: ImageBitmap) : CaptureResult

    @JvmInline
    value class Error(val error: Throwable) : CaptureResult
}

The idea is simple, if the operation was successful, return CaptureResult.Success, else return CaptureResult.Error.

Next, I'd like to discuss the actual capture syntax. I have 3 proposals, each with their pros and cons:

Option 1

class CaptureController internal constructor(internal val onCapture: (CaptureResult.Success) -> Unit) {
    private val _captureRequests = MutableSharedFlow<CaptureRequest>(extraBufferCapacity = 1)
    internal val captureRequests = _captureRequests.asSharedFlow()

    fun capture(config: Bitmap.Config = Bitmap.Config.ARGB_8888) {
        _captureRequests.tryEmit(CaptureRequest(config, onCapture))
    }

    internal class CaptureRequest(
        val config: Bitmap.Config
    )
}

@Composable
fun rememberCaptureController(onCapture: (CaptureResult.Success)): CaptureController {
    return remember(onCapture) { CaptureController(onCapture) }
}

private class CapturableModifierNode(
    var controller: CaptureController
) : DelegatingNode(), DelegatableNode {

    ...

    override fun onAttach() {
        super.onAttach()
        coroutineScope.launch {
            controller.captureRequests.collect { request ->
                try {
                    val bitmap = withContext(Dispatchers.Default) {
                        picture.asBitmap(request.config)
                    }
                    controller.onCapture(CaptureResult.Success(bitmap.asImageBitmap()))
                } catch (error: Throwable) {
                    controller.onCapture(CaptureResult.Error(error))
                }
            }
        }
    }
}

@Composable
fun TestScreen() {
	val controller = rememberCapturableController { result -> 
		when (result) {
            is CaptureResult.Success -> { /*TODO*/ }
            is CaptureResult.Error -> { /*TODO*/ }
        }
	}
	ShouldBeCaptured(modifier = Modifier.capturable(controller))
	Button(onClick = { controller.capture() }) {
		Text("Capture")
	}
}
✅ Pros ❌ Cons
Easy to assign actions to captures Hard to migrate from the Capturable composable, as the library wouldn't be able to migrate onCaptured: (ImageBitmap?, Throwable?) -> Unit to the new controller syntax.
Elegant syntax

Option 2

class CaptureController internal constructor() {
    private val _captureRequests = MutableSharedFlow<CaptureRequest>(extraBufferCapacity = 1)
    internal val captureRequests = _captureRequests.asSharedFlow()

    fun capture(
        config: Bitmap.Config = Bitmap.Config.ARGB_8888,
        onCapture: (CaptureResult) -> Unit
    ) {
        _captureRequests.tryEmit(CaptureRequest(config, onCapture))
    }

    suspend fun capture(config: Bitmap.Config = Bitmap.Config.ARGB_8888): CaptureResult {
        return suspendCoroutine { continuation ->
            val request = CaptureRequest(config) {
                continuation.resume(it)
            }
            _captureRequests.tryEmit(request)
        }
    }

    internal class CaptureRequest(
        val config: Bitmap.Config,
        val onCapture: (CaptureResult) -> Unit
    )
}

private class CapturableModifierNode(
    var controller: CaptureController
) : DelegatingNode(), DelegatableNode {

    ...

    override fun onAttach() {
        super.onAttach()
        coroutineScope.launch {
            controller.captureRequests.collect { request ->
                try {
                    val bitmap = withContext(Dispatchers.Default) {
                        picture.asBitmap(request.config)
                    }
                    request.onCapture(CaptureResult.Success(bitmap.asImageBitmap()))
                } catch (error: Throwable) {
                    request.onCapture(CaptureResult.Error(error))
                }
            }
        }
    }
}

@Composable
fun TestScreen() {
	val coroutineScope = rememberCoroutineScope()
	val controller = rememberCapturableController()
	ShouldBeCaptured(modifier = Modifier.capturable(controller))
	Button(onClick = { 
		controller.capture { result ->
			when (result) {
                is CaptureResult.Success -> { /*TODO*/ }
                is CaptureResult.Error -> { /*TODO*/ }
             }
		}
	 }) {
		Text("Capture Callback")
	}
	Button(onClick = { 
		coroutineScope.launch {
			val result = controller.capture()
			when (result) {
                is CaptureResult.Success -> { /*TODO*/ }
                is CaptureResult.Error -> { /*TODO*/ }
             }
		}
	 }) {
		Text("Capture Suspending")
	}
}
✅ Pros ❌ Cons
Allows for different logic per different capture Hard to migrate from the Capturable composable, as the library wouldn't be able to migrate onCaptured: (ImageBitmap?, Throwable?) -> Unit to the new controller syntax.
Allows for both suspendable and callback-based methods Can get repetitive if you have multiple capture handlers

Option 3

class CaptureController internal constructor() {
    private val _captureRequests = MutableSharedFlow<CaptureRequest>(extraBufferCapacity = 1)
    internal val captureRequests = _captureRequests.asSharedFlow()

    fun capture(config: Bitmap.Config = Bitmap.Config.ARGB_8888) {
        _captureRequests.tryEmit(CaptureRequest(config, onCapture))
    }

    internal class CaptureRequest(
        val config: Bitmap.Config
    )
}

fun Modifier.capturable(
	controller: CaptureController,
	onCapture: (CaptureResult) -> Unit
): Modifier {
    return this then CapturableModifierNodeElement(controller, onCapture)
}

private data class CapturableModifierNodeElement(
    private val controller: CaptureController,
	private val onCapture: (CaptureResult) -> Unit
) : ModifierNodeElement<CapturableModifierNode>() {
    override fun create(): CapturableModifierNode {
        return CapturableModifierNode(controller, onCapture)
    }

    override fun update(node: CapturableModifierNode) {
        node.controller = controller
		node.onCapture = onCapture
    }
}

private class CapturableModifierNode(
    var controller: CaptureController,
 	var onCapture: (CaptureResult) -> Unit
) : DelegatingNode(), DelegatableNode {

    ...

    override fun onAttach() {
        super.onAttach()
        coroutineScope.launch {
            controller.captureRequests.collect { request ->
                try {
                    val bitmap = withContext(Dispatchers.Default) {
                        picture.asBitmap(request.config)
                    }
                    onCapture(CaptureResult.Success(bitmap.asImageBitmap()))
                } catch (error: Throwable) {
                    onCapture(CaptureResult.Error(error))
                }
            }
        }
    }
}

@Composable
fun TestScreen() {
	val coroutineScope = rememberCoroutineScope()
	val controller = rememberCapturableController()
	ShouldBeCaptured(
		modifier = Modifier.capturable(
			controller = controller,
			onCapture = { result -> 
				when (result) {
					is CaptureResult.Success -> { /*TODO*/ }
				    is CaptureResult.Error -> { /*TODO*/ }
              	}
			}
		)
	)
	Button(onClick = { controller.capture() }) {
		Text("Capture")
	}
}
✅ Pros ❌ Cons
Matches the style of other foundation modifiers, such as `.clickable() Probably not as intuitive as previous 2
Allows for easy migration from Capturable, a onCapture(CaptureResult) from the modifier can invoke onCaptured: (ImageBitmap?, Throwable?) -> Unit

Personally, I like the 3rd approach the most, as it minimizes the migration hassle and comes in-line with the foundation APIs. Also, Kotlin's Result class would fit as a return type too, I just think it's a bit of a heavy class for tasks as simple as this. Please, let me know what you think!

X1nto avatar Dec 27 '23 22:12 X1nto