Background and previous improvements by Ethran · Pull Request #236 · Ethran/notable

Ethran · 2026-04-11T20:21:50Z

No description provided.

…date drawing operations. ### Editor Logic - **PageView**: - Introduced `drawBgToCanvas` to centralize background resource resolution (bitmaps, PDFs, and internal resources like `iris`). - Updated `onScrollChanged` and `redrawFullCanvas` to use the new unified drawing method. - **Background Rendering**: - Simplified `drawBg` in `backgrounds.kt` by removing direct dependency on `Context` and `PageView`, now accepting a pre-resolved `resourceBitmap`. - Removed redundant `drawBackgroundImages` and `drawPdfPage` helper functions, moving their logic into the calling context or the simplified `drawBg`. - Refactored `BackgroundType` handling to separate native drawing (colors/dots/lines) from bitmap-based drawing. ### Clean up - Improved logging in `renderFromFile.kt` for image decoding failures. - Temporarily disabled background drawing in `PageContentRenderer` and updated `pageDrawing.kt` to use the new `PageView` drawing flow.

- **ThumbnailGenerator**: Temporarily force `isThumbnailStale` to always return `true` to ensure thumbnails are refreshed. - **PageContentRenderer**: - Updated `renderPage` and `renderContent` to resolve the background type internally instead of accepting it as a parameter. - Added logic to load background bitmaps, including specific support for resource-based backgrounds (e.g., "iris"). - Re-enabled background drawing in `renderContent` and integrated it with images and strokes. - **ExportEngine**: Simplified calls to `renderContent` by removing the redundant `backgroundType` parameter.

… format and improve naming consistency. ### Core Changes - **Format Update**: Switched from PNG/JPEG to WEBP for full-page previews and thumbnails, including version-specific support for `WEBP_LOSSY`. - **API Renaming**: Standardized persistence functions for better clarity: - `persistBitmapFull` → `savePageFull` - `loadPersistBitmap` → `loadPageFull` - `loadPreview` → `loadPagePreviewOrFallback` - `persistBitmapThumbnail` → `savePageThumbnail` ### Preview & Thumbnail Management - **Preview Storage**: Updated file naming to include `.webp` extensions and simplified the removal of legacy preview files. - **Loading Logic**: Refactored `loadPageFull` to focus on WEBP candidates and improved the fallback mechanism in `loadPagePreviewOrFallback` to better handle missing or mismatched previews. - **Thumbnail Generation**: Simplified `ThumbnailGenerator` by removing the redundant `isThumbnailStale` check and renaming `generateIfNeeded` to `generate`. ### Miscellaneous - **Logging**: Improved log messages across the rendering and persistence pipeline for better traceability. - **Bug Fix**: Fixed a typo in `isEqApprox` utility function.

- **PreviewBitmapStore.kt**: - Reduced `PREVIEW_QUALITY` from 90 to 85. - Suppressed the unused variable in the `savePagePreview` catch block. - Fixed string template formatting in the `loadPagePreview` failure log.

…storage for E-ink devices. ### System Information - **SystemInformationViewModel**: Added comprehensive device metadata fields (kernel, CPU serial, resolution, etc.) using `DeviceInfoUtil` and `DeviceCompat`. - **SystemInformation View**: Integrated new device information fields into the UI display and updated the mock snapshot for previews. ### Storage & Rendering Optimization - **PreviewBitmapStore**: - Introduced `optimizeBitmapForStorage` to reduce memory and disk footprint on monochrome and E-ink devices (e.g., using `RGB_565` for thumbnails or stripping saturation). - Renamed `savePageFull` to `saveHQPagePreview` and `loadPageFull` to `loadHQPagePreview` for clarity. - Updated storage logic to recycle temporary optimized bitmaps. - **PageContentRenderer**: Updated `RenderTarget.Thumbnail` to support optional dimensions, defaulting to a screen-ratio-based height if not provided. - **ThumbnailGenerator**: Updated to use the new flexible `maxHeightPx` in `RenderTarget`. ### Device Compatibility - **DeviceCompat**: Added `isColorDevice()` helper and a `delayBeforeResumingDrawing()` method to handle refresh rate differences between color and monochrome E-ink screens. - **CanvasObserverRegistry**: Removed the `requireExactMatch` parameter when loading bitmap previews to allow more flexible fallback to thumbnails. ### General Clean up - Improved code formatting for `AppEvent.ActionHint` calls across `PageDataManager`. - Simplified bitmap loading logic in `PageView` to use the renamed high-quality preview methods.

…tic thumbnail generation. ### Core Logic - **ImportEngine**: - Updated `import`, `handleImportXopp`, and `handleImportPDF` to return `AppResult<List<String>, DomainError>` instead of raw strings or throwing exceptions. - Improved error handling by accumulating persistent errors during the import process. - Now returns a list of successfully imported page IDs. - **ThumbnailBackfillQueue**: - Updated the processing queue to accept a `Pair<String, PreviewSaveMode>`, allowing specific rendering modes for backfilled thumbnails. - Modified `enqueue` to support an optional `PreviewSaveMode` parameter (defaults to `REGULAR`). ### UI & Integration - **LibraryViewModel**: - Updated PDF and XOPP import flows to handle `AppResult` via `fold`. - Integrated `ThumbnailBackfillQueue` to automatically trigger high-contrast (STRICT_BW) thumbnail generation for newly imported PDF pages. - Standardized snackbar error messaging using `DomainError.userMessage`.

Copilot

Pull request overview

This PR extends the app’s diagnostics and import/preview pipeline by adding Onyx DeviceInfoUtil fields to the system info screen, refactoring import to return a typed AppResult, and reworking thumbnail/preview persistence and background rendering paths.

Changes:

Added DeviceInfoUtil-derived device fields to the system information snapshot/UI.
Changed ImportEngine.import() to return AppResult<List<String>, DomainError> and wired import results into thumbnail backfill (with a stricter preview save mode for PDFs).
Replaced legacy preview/thumbnail persistence with a new WebP-based PreviewBitmapStore, and refactored thumbnail/background rendering code to use it.

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 11 comments.

Show a summary per file

File	Description
app/src/main/java/com/ethran/notable/ui/views/SystemInformation.kt	Displays newly collected `DeviceInfoUtil` fields in the system info UI.
app/src/main/java/com/ethran/notable/ui/viewmodels/SystemInformationViewModel.kt	Collects additional device/kernel/vcom/resolution fields into `DeviceSnapshot`.
app/src/main/java/com/ethran/notable/ui/viewmodels/LibraryViewModel.kt	Handles `AppResult` import outcomes and enqueues thumbnail backfill with mode.
app/src/main/java/com/ethran/notable/io/renderFromFile.kt	Reformats error logging for null decode path.
app/src/main/java/com/ethran/notable/io/importPdf.kt	Updates import callback payload to `PageWithData`.
app/src/main/java/com/ethran/notable/io/XoppFile.kt	Updates import callback payload to `PageWithData`.
app/src/main/java/com/ethran/notable/io/ThumbnailGenerator.kt	Adds mode-aware thumbnail generation and new staleness logic (currently broken).
app/src/main/java/com/ethran/notable/io/ThumbnailBackfillQueue.kt	Passes generation mode through the queue to the generator.
app/src/main/java/com/ethran/notable/io/PageContentRenderer.kt	Refactors rendering/background loading and thumbnail sizing behavior.
app/src/main/java/com/ethran/notable/io/ImportEngine.kt	Returns `AppResult` with accumulated `DomainError`s and tracks imported page IDs.
app/src/main/java/com/ethran/notable/io/ExportEngine.kt	Adapts to `PageContentRenderer.drawPage` signature changes.
app/src/main/java/com/ethran/notable/editor/utils/einkHelper.kt	Adds `DeviceCompat.isColorDevice()` and a color-dependent resume delay helper.
app/src/main/java/com/ethran/notable/editor/utils/PreviewBitmapStore.kt	New WebP-based preview/thumbnail persistence utilities and save modes.
app/src/main/java/com/ethran/notable/editor/drawing/pageDrawing.kt	Switches background drawing path during partial redraws (currently incorrect).
app/src/main/java/com/ethran/notable/editor/drawing/backgrounds.kt	Refactors `drawBg` to require a provided bitmap resource for non-native backgrounds.
app/src/main/java/com/ethran/notable/editor/canvas/CanvasObserverRegistry.kt	Uses new preview loading helper and removes/changes refresh wait behavior.
app/src/main/java/com/ethran/notable/editor/PageView.kt	Adds helper to draw backgrounds via new `drawBg` signature (currently has logic bugs).
app/src/main/java/com/ethran/notable/data/PageDataManager.kt	Migrates preview/thumbnail persistence calls to `PreviewBitmapStore`.
app/src/main/java/com/ethran/notable/editor/utils/persistBitmap.kt	Removes legacy persistence implementation.

Copilot · 2026-04-18T13:23:35Z

+        val pageNumber = currentPageNumber
+        val scale = zoomLevel.value
+        val bgImage: Bitmap? =
+            when (backgroundType) {
+                BackgroundType.Image, BackgroundType.CoverImage, BackgroundType.AutoPdf,
+                is BackgroundType.Pdf, BackgroundType.ImageRepeating -> {
+                    if (backgroundType is BackgroundType.Image && bg == "iris") {
+                        val resId = R.drawable.iris
+                        ImageBitmap.imageResource(context.resources, resId).asAndroidBitmap()
+                    } else {
+                        getOrLoadBackground(bg, pageNumber, scale)
+                    }


drawBgToCanvas always uses pageNumber = currentPageNumber when loading backgrounds. For BackgroundType.Pdf, the correct page index is backgroundType.page, not the current on-screen page number (which is used for AutoPdf). As written, fixed-PDF backgrounds can render the wrong PDF page.

Copilot · 2026-04-18T13:23:36Z

+//        drawBg(page.context, this, backgroundType, background, page.scroll, zoomLevel, page, page.currentPageNumber)
+        page.drawBgToCanvas(null)


drawOnCanvasFromPage draws the background via page.drawBgToCanvas(null), which renders onto PageView.windowedCanvas rather than the canvas passed into this function (the one inside withClip). This means callers that provide a different canvas won’t get the background drawn, and even for the default path it bypasses the clip bounds. Consider drawing the background onto this (the clipped canvas) or refactoring drawBgToCanvas to accept a target canvas.

Suggested change

// drawBg(page.context, this, backgroundType, background, page.scroll, zoomLevel, page, page.currentPageNumber)

page.drawBgToCanvas(null)

drawBg(page.context, this, backgroundType, background, page.scroll, zoomLevel, page, page.currentPageNumber)

Copilot · 2026-04-18T13:23:36Z

        withContext(Dispatchers.Default) {
            canvas.scale(scaleFactor, scaleFactor)
-            val scaledScroll = scroll / scaleFactor
+
            drawBg(
-                context = context,
                canvas = canvas,
-                backgroundType = backgroundType,
+                backgroundType = resolvedBackgroundType,
                background = data.page.background,
-                scroll = scaledScroll,
-                scale = scaleFactor
+                scroll = scroll,
+                resourceBitmap = bgImage,
+                scale = scaleFactor,
+                repeat = resolvedBackgroundType is BackgroundType.ImageRepeating
            )
-            data.images.forEach { drawImage(context, canvas, it, -scaledScroll) }
-            data.strokes.forEach { drawStroke(canvas, it, -scaledScroll) }
+
+            data.images.forEach { drawImage(context, canvas, it, -scroll) }
+            data.strokes.forEach { drawStroke(canvas, it, -scroll) }


drawPage scales the canvas (canvas.scale(scaleFactor, scaleFactor)) but then uses the unscaled scroll for background and content offsets (scroll = scroll, -scroll). This will over-apply the scroll by scaleFactor when exporting/paginating, causing incorrect cropping/positioning. Either divide scroll by scaleFactor before drawing, or avoid scaling the canvas and instead apply scaling consistently in the draw helpers.

Copilot · 2026-04-18T13:23:37Z

+        val resolvedBackgroundType = resolveExportBackgroundType(data)
+


drawPage calls resolveExportBackgroundType(data) internally, which (via appRepository.getPageNumber) can perform IO. ExportEngine calls drawPage in a loop during pagination, so this can repeat the same IO many times per export. Consider resolving the background type once per page (e.g., in ExportEngine) and passing it into drawPage, or caching the resolved value within the export flow.

Ethran added 10 commits April 11, 2026 21:09

clean up code

fa86295

remove waitForEpdRefresh

1eec939

rename to PreviewBitmapStore.kt

caff133

Adjust preview quality and clean up logging in PreviewBitmapStore

76a0814

- **PreviewBitmapStore.kt**: - Reduced `PREVIEW_QUALITY` from 90 to 85. - Suppressed the unused variable in the `savePagePreview` catch block. - Fixed string template formatting in the `loadPagePreview` failure log.

rename image decoding utility.

1462c4d

Ethran requested a review from Copilot April 18, 2026 13:17

Copilot started reviewing on behalf of Ethran April 18, 2026 13:17 View session

Copilot AI reviewed Apr 18, 2026

View reviewed changes

apply copilot suggestions

a41b6a6

Ethran merged commit a82876e into main Apr 18, 2026
2 of 3 checks passed

Ethran deleted the dev branch April 18, 2026 13:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Background and previous improvements #236

Background and previous improvements #236
Ethran merged 11 commits intomainfrom
dev

Ethran commented Apr 11, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 18, 2026

Uh oh!

Copilot AI Apr 18, 2026

Uh oh!

Uh oh!

Copilot AI Apr 18, 2026

Uh oh!

Copilot AI Apr 18, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		// drawBg(page.context, this, backgroundType, background, page.scroll, zoomLevel, page, page.currentPageNumber)
		page.drawBgToCanvas(null)

	// drawBg(page.context, this, backgroundType, background, page.scroll, zoomLevel, page, page.currentPageNumber)
	page.drawBgToCanvas(null)
	drawBg(page.context, this, backgroundType, background, page.scroll, zoomLevel, page, page.currentPageNumber)

		val resolvedBackgroundType = resolveExportBackgroundType(data)

Uh oh!

Conversation

Ethran commented Apr 11, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants