Consider an alternate capture result syntax

[X1nto]

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!

[PatilShreyas]

It's good to handle the Exception while awaiting the Deferred<> instead of adding further type checks at the consumer's end. The new API was thought from all such perspectives and doesn't seem like we need to revisit it as of now. Though, thanks for your suggestion on the API design. Closing this.