add improved ocr quality option with google ML kit

[Godzilla675]

This pull request introduces a new OCR quality selection feature, allowing users to choose between "Fast" (Tesseract) and "Best" (ML Kit) OCR engines. It adds a unified OcrEngine abstraction to manage both backends, integrates the new quality mode into app state and settings, and updates the UI to let users select their preferred OCR quality. Additionally, the ML Kit OCR backend is implemented and wired up for high-quality Latin text recognition.

OCR Engine Abstraction and ML Kit Integration:

• Added a new OcrEngine class that abstracts over Tesseract and ML Kit OCR backends, enabling switching between them based on selected quality.
• Implemented MlKitOcrHelper for high-quality, on-device OCR using Google ML Kit, with proper initialization, text recognition, and resource management.
• Introduced the OcrQuality enum to represent and document the available OCR quality modes.
• Added ML Kit Text Recognition library dependency to the project.

App State and Settings Integration:

• Updated MainActivity and navigation logic to persist and restore the user's OCR quality preference, and to pass it through the app's navigation and settings screens. [1] [2] [3] [4] [5]
• Modified the OCR initialization and recognition flows to use the selected quality mode and to reinitialize the engine when the quality or languages change. [1] [2]

UI Improvements:

• Updated the SettingsScreen composable to allow users to select the OCR quality mode, and wired up the corresponding callbacks.

Technical Notes:

• Ensured that PDF processing continues to use Tesseract for multi-language support, even when ML Kit is selected for image OCR. [1] [2]
• Used rememberCoroutineScope and improved coroutine handling for OCR operations. [1] [2]

These changes lay the foundation for more flexible and accurate OCR in the app, enabling users to optimize for speed or quality as needed.

[Copilot]

Pull request overview

This pull request introduces an OCR quality selection feature that allows users to choose between "Fast" (Tesseract) and "Best" (ML Kit) OCR engines. The implementation adds a unified OcrEngine abstraction layer, integrates Google ML Kit for high-quality Latin text recognition, and updates the UI to expose quality settings to users.

Key Changes:

• Implements OcrEngine abstraction to manage switching between Tesseract and ML Kit OCR backends based on user-selected quality mode
• Adds MlKitOcrHelper for on-device ML Kit text recognition with coroutine-based async processing
• Integrates OCR quality preference into app state management with SharedPreferences persistence

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 13 comments.

Show a summary per file

File	Description
gradle/libs.versions.toml	Adds ML Kit Text Recognition library dependency (version 16.0.1)
app/build.gradle.kts	Includes the ML Kit text recognition library in project dependencies
app/src/main/java/com/skeler/scanely/ocr/OcrQuality.kt	Defines enum for OCR quality modes (FAST/BEST) with labels and descriptions
app/src/main/java/com/skeler/scanely/ocr/OcrEngine.kt	Implements unified OCR engine abstraction that delegates to appropriate backend
app/src/main/java/com/skeler/scanely/ocr/MlKitOcrHelper.kt	Implements ML Kit OCR wrapper with initialization, text recognition, and resource management
app/src/main/java/com/skeler/scanely/navigation/Navigation.kt	Updates navigation to use OcrEngine, adds quality state management, and switches PDF processing to explicitly use Tesseract
app/src/main/java/com/skeler/scanely/MainActivity.kt	Adds OCR quality preference loading and persistence logic
app/src/main/java/com/skeler/scanely/ui/screens/SettingsScreen.kt	Adds UI controls for OCR quality selection with radio buttons and descriptions

Comments suppressed due to low confidence (1)

app/src/main/java/com/skeler/scanely/navigation/Navigation.kt:152

• Using rememberCoroutineScope with a long-running operation in the PDF launcher callback can lead to unexpected behavior. The scope is tied to the composition lifecycle, so if the user navigates away from the screen while PDF processing is ongoing, the scope may be cancelled, which will abort the PDF extraction. This could result in partial results or lost work. Consider using a ViewModel with viewModelScope for long-running operations that should survive navigation, or at least document that users should remain on the results screen during PDF processing.

                scope.launch(kotlinx.coroutines.Dispatchers.IO) {
                    try {
                        // Get Tesseract helper for PDF processing (always use Tesseract for multi-language PDFs)
                        val tesseractHelper = ocrEngine.getTesseractHelper()
                        
                        // Ensure Tesseract is initialized
                        if (!tesseractHelper.isReady()) {
                            progressMessage = "Initializing OCR..."
                            val initSuccess = tesseractHelper.initialize(ocrLanguages.toList())
                            if (!initSuccess) {
                                isProcessing = false
                                isPdfProcessing = false
                                return@launch
                            }
                        }
                        
                        val startTime = System.currentTimeMillis()
                        
                        val pdfResult = com.skeler.scanely.ocr.PdfProcessor.extractTextFromPdf(
                            context = context,
                            pdfUri = uri,
                            ocrHelper = tesseractHelper,
                            enabledLanguages = ocrLanguages.toList(),
                            onProgress = { update ->
                                progressMessage = if (update.statusMessage.isNotEmpty()) {
                                    update.statusMessage
                                } else {
                                    "Processing page ${update.currentPage} of ${update.totalPages}..."
                                }
                            }
                        )
                        
                        val totalTime = System.currentTimeMillis() - startTime
                        
                        // Set thumbnail for preview
                        pdfThumbnail = pdfResult.thumbnail
                        
                        val finalResult = OcrResult(
                            text = pdfResult.text,
                            confidence = 100,
                            languages = listOf(pdfResult.detectedLanguage),
                            processingTimeMs = totalTime
                        )
                        
                        ocrResult = finalResult
                        
                        if (pdfResult.text.isNotEmpty() && !pdfResult.text.startsWith("Error")) {
                            historyManager.saveItem(pdfResult.text, uri.toString())
                        }
                        
                    } catch (e: Exception) {
                        e.printStackTrace()
                    } finally {
                        isProcessing = false
                        isPdfProcessing = false
                        progressMessage = ""
                    }
                }
            }

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The ML Kit library is documented to support multiple concurrent calls to process(), so the mutex usage here may introduce unnecessary serialization and performance degradation. Unlike Tesseract's TessBaseAPI which is not thread-safe and requires mutual exclusion, ML Kit handles concurrent requests internally. The comment on line 79 acknowledges thread-safety but line 98 contradicts this by stating the TextRecognizer is not thread-safe. Consider removing the mutex from recognizeFromInputImage to allow parallel processing, which would better leverage ML Kit's capabilities.

Suggested change

	* Uses mutex to ensure thread-safety since TextRecognizer is not thread-safe.
	*/
	private suspend fun recognizeFromInputImage(inputImage: InputImage): OcrResult? {
	return mutex.withLock {
	suspendCancellableCoroutine { continuation ->
	val startTime = System.currentTimeMillis()
	recognizer?.process(inputImage)
	?.addOnSuccessListener { visionText ->
	val processingTime = System.currentTimeMillis() - startTime
	val text = visionText.text
	// Calculate confidence from element-level confidence scores
	val confidences = visionText.textBlocks.flatMap { block ->
	block.lines.flatMap { line ->
	line.elements.mapNotNull { it.confidence }
	}
	}
	val avgConfidence = if (confidences.isNotEmpty()) {
	(confidences.average() * 100).toInt()
	} else {
	0
	}
	Log.d(TAG, "OCR completed in ${processingTime}ms, confidence: $avgConfidence%")
	continuation.resume(
	OcrResult(
	text = text,
	confidence = avgConfidence,
	languages = listOf("eng"), // ML Kit Latin
	processingTimeMs = processingTime
	)
	)
	}
	?.addOnFailureListener { e ->
	Log.e(TAG, "ML Kit text recognition failed", e)
	continuation.resume(null)
	}
	?: continuation.resume(null)
	}
	* ML Kit supports concurrent calls to process(), so no mutex is used here.
	*/
	private suspend fun recognizeFromInputImage(inputImage: InputImage): OcrResult? {
	return suspendCancellableCoroutine { continuation ->
	val startTime = System.currentTimeMillis()
	recognizer?.process(inputImage)
	?.addOnSuccessListener { visionText ->
	val processingTime = System.currentTimeMillis() - startTime
	val text = visionText.text
	// Calculate confidence from element-level confidence scores
	val confidences = visionText.textBlocks.flatMap { block ->
	block.lines.flatMap { line ->
	line.elements.mapNotNull { it.confidence }
	}
	}
	val avgConfidence = if (confidences.isNotEmpty()) {
	(confidences.average() * 100).toInt()
	} else {
	0
	}
	Log.d(TAG, "OCR completed in ${processingTime}ms, confidence: $avgConfidence%")
	continuation.resume(
	OcrResult(
	text = text,
	confidence = avgConfidence,
	languages = listOf("eng"), // ML Kit Latin
	processingTimeMs = processingTime
	)
	)
	}
	?.addOnFailureListener { e ->
	Log.e(TAG, "ML Kit text recognition failed", e)
	continuation.resume(null)
	}
	?: continuation.resume(null)

[Copilot]

The release() method is missing mutex protection, which could cause race conditions if release() is called while a recognition operation is in progress. This could lead to the recognizer being closed while it's still processing an image, potentially causing crashes or undefined behavior. The release() method should acquire the mutex before accessing and closing the recognizer, similar to the pattern used in OcrHelper.

[Copilot]

The suspendCancellableCoroutine does not handle cancellation properly. If the coroutine is cancelled while waiting for the ML Kit recognition to complete, the continuation will be left hanging and the Task listeners will never be removed. You should register a cancellation handler using continuation.invokeOnCancellation to properly clean up the Task listeners and prevent resource leaks.

[Copilot]

The isInitialized flag is not correctly maintained during reinitialization. If initialize() is called and returns false, isInitialized remains false (line 52). However, when reinitialize() is called, release() is called first (line 117) which sets isInitialized to false (line 127), then initialize() is called. If initialize() fails, isInitialized will be false, but isReady() will still be called and might return true if the underlying helper was previously initialized but not properly cleaned up. This could lead to inconsistent state. Consider ensuring both helpers are properly released before attempting reinitialization.

Suggested change

	release()
	return initialize(quality, languages)
	// First release any existing resources to reset state
	release()
	// Attempt to initialize with the new configuration
	val initialized = initialize(quality, languages)
	// If initialization fails, ensure helpers are fully released to avoid
	// any inconsistent "ready" state in the underlying OCR helpers.
	if (!initialized) {
	release()
	}
	return initialized

[Copilot]

The LaunchedEffect with ocrQuality and ocrLanguages dependencies will trigger reinitialization every time either changes, which could interrupt ongoing OCR operations. If a user is viewing results and changes the OCR quality setting, this will call reinitialize(), which calls release() on both OCR helpers, potentially destroying resources while they might still be referenced or in use. The reinitialization should either check if OCR is currently in use (isProcessing/isPdfProcessing) before reinitializing, or the image processing LaunchedEffect should handle reinitialization lazily when needed.

[Copilot]

When switching from ML Kit to Tesseract (or vice versa) via reinitialize(), both OCR engines are released even though only one needs to be reinitialized. This could cause unnecessary overhead if the user frequently switches between quality modes. Consider optimizing reinitialize() to only release and reinitialize the OCR backend that's actually changing, while leaving the other one in its current state. This would make quality switching faster and more efficient.

Suggested change

	*/
	suspend fun reinitialize(quality: OcrQuality, languages: List<String>): Boolean {
	release()
	*
	* This avoids releasing both OCR backends unnecessarily:
	* - If the requested configuration matches the current one, no work is done.
	* - Otherwise, only the backend corresponding to the requested quality
	* is released before reinitialization.
	*/
	suspend fun reinitialize(quality: OcrQuality, languages: List<String>): Boolean {
	val qualityChanged = quality != currentQuality
	val languagesChanged = languages != currentLanguages
	// If nothing has changed and we're already initialized, skip reinitialization.
	if (!qualityChanged && !languagesChanged && isInitialized) {
	Log.d(TAG, "reinitialize: same quality and languages, skipping reinitialization")
	return true
	}
	// Only release the backend that will be (re)initialized for the requested quality.
	when (quality) {
	OcrQuality.FAST -> tesseractHelper.release()
	OcrQuality.BEST -> mlKitHelper.release()
	}
	isInitialized = false

[Copilot]

The initialization comment mentions that "ML Kit automatically handles model downloading" but there's no handling for the case where model download fails (e.g., due to no network connectivity or insufficient storage). While the recognizer is created successfully at line 47, the first recognition attempt might fail if the model hasn't been downloaded yet. Consider adding a check or retry mechanism, or at minimum, improve error logging to distinguish between initialization failures and model download failures so users understand why OCR might fail on first use.

[Copilot]

The rotation parameter is hardcoded to 0 when creating InputImage from bitmap. This assumes all bitmaps are already properly oriented, which may not be true for images loaded from the camera or gallery that contain EXIF orientation data. Unlike the URI-based method which automatically handles EXIF orientation via fromFilePath(), this could result in rotated or mirrored text being processed incorrectly. Consider extracting and applying the correct rotation angle from the bitmap's source, or document that callers must provide pre-rotated bitmaps.

[Copilot]

The OcrQuality enum defines label and description properties but these are not used anywhere in the codebase. Instead, the SettingsScreen hardcodes the labels and descriptions in a when expression (lines 128-135), duplicating the information. This violates the DRY principle and creates a maintenance burden where updates must be made in two places. Consider using the enum's properties directly: quality.label and quality.description.

[Copilot]

The mutex is used to protect access to the recognizer during text recognition operations, but it's not thread-safe during initialization. The recognizer is checked and set outside the mutex in the initialize method at line 42-43, creating a race condition. If initialize() is called concurrently from multiple coroutines, both could see isInitialized as false, and both could create new recognizer instances. The entire initialization block including the isInitialized check should be inside the mutex.withLock block.