From 2ba1015ef26486b940c68943e72295329856e0ce Mon Sep 17 00:00:00 2001 From: Ric Lewis Date: Fri, 2 Oct 2026 22:45:39 -0700 Subject: [PATCH] Keep image picker work bounded for large theme collections --- bin/omarchy-menu-images | 27 +++++-- shell/plugins/README.md | 2 + shell/plugins/image-picker/ImagePicker.qml | 76 +++++++++++++------ .../plugins/image-picker/ImagePickerModel.js | 44 ++++++++++- test/shell.d/image-picker-test.sh | 50 +++++++++++- test/shell.d/menu-images-test.sh | 74 ++++++++++++++++++ 6 files changed, 240 insertions(+), 33 deletions(-) diff --git a/bin/omarchy-menu-images b/bin/omarchy-menu-images index 6b41e65a..67370cab 100755 --- a/bin/omarchy-menu-images +++ b/bin/omarchy-menu-images @@ -230,7 +230,7 @@ thumbnail_for() { rows_cacheable=false if [[ $prepare_only != true ]]; then - generate_thumbnail "$image" "$thumbnail" >/dev/null 2>&1 & + printf '%s\0%s\0' "$image" "$thumbnail" >>"$pending_file" fi printf '%s' "$image" @@ -247,17 +247,34 @@ thumbnail_for() { printf '%s' "$thumbnail" } -# Each vips run is single-threaded, so still images can fill every core. +# Each vips run is single-threaded. Eager preparation can fill every core; +# lazy preparation leaves CPU and I/O capacity for the interactive shell. # ffmpegthumbnailer leaves FFmpeg's automatic threading on, so a full-width fan # out of those would put a codec thread pool on every core at once. drain_pending_thumbnails() { - local video_jobs + local video_jobs image_jobs pending_fd export -f generate_thumbnail is_video_path if [[ -s $pending_file ]]; then - xargs -a "$pending_file" -0 -n 2 -P "$(nproc)" \ - bash -c 'generate_thumbnail "$1" "$2"' _ >/dev/null 2>&1 || true + if [[ $lazy_thumbnails == true && $cache_only != true ]]; then + image_jobs=$(( $(nproc) / 2 )) + (( image_jobs > 0 )) || image_jobs=1 + (( image_jobs > 2 )) && image_jobs=2 + # The caller may exit immediately after printing rows. Inherit an open + # queue fd so its EXIT trap can unlink the file without losing any jobs. + exec {pending_fd}<"$pending_file" + ( + exec {worker_lock_fd}>"$rows_cache_file.thumbnails.lock" + flock -n "$worker_lock_fd" || exit 0 + nice -n 10 ionice -c 3 xargs -0 -n 2 -P "$image_jobs" \ + bash -c 'generate_thumbnail "$1" "$2"' _ <&"$pending_fd" + ) >/dev/null 2>&1 & + exec {pending_fd}<&- + else + xargs -a "$pending_file" -0 -n 2 -P "$(nproc)" \ + bash -c 'generate_thumbnail "$1" "$2"' _ >/dev/null 2>&1 || true + fi fi if [[ -s $pending_video_file ]]; then diff --git a/shell/plugins/README.md b/shell/plugins/README.md index 7ef3daa3..092dd20c 100644 --- a/shell/plugins/README.md +++ b/shell/plugins/README.md @@ -77,6 +77,8 @@ clears it without writing a selection. The plugin has `keepLoaded: true` so the layer-shell window survives between summons within a single shell session. +The carousel renders only the slices that fit on the screen plus one prefetch slice per side, capped at 33 cards. It preserves overlapping delegates during navigation, decodes images asynchronously at card size, and loads the selected preview before its neighbors. Lazy thumbnail preparation runs below normal CPU and I/O priority in one shared worker pool per image list: one worker on single/dual-core machines and at most two on larger machines. Opening and navigating the picker does not wait for that queue to finish. + ## Lock screen Session-lock surface using Quickshell's native `WlSessionLock` and two diff --git a/shell/plugins/image-picker/ImagePicker.qml b/shell/plugins/image-picker/ImagePicker.qml index b6939fad..73a4b1c3 100644 --- a/shell/plugins/image-picker/ImagePicker.qml +++ b/shell/plugins/image-picker/ImagePicker.qml @@ -25,6 +25,7 @@ Item { property bool showLabels: false property bool filterable: false property bool layoutSettled: false + property bool neighborImagesEnabled: false property bool requestActive: false property int requestSerial: 0 property int applySerial: 0 @@ -51,6 +52,9 @@ Item { property int sliceSpacing: -30 property int skewOffset: 28 property int bottomChromeHeight: showLabels ? (filterable ? 104 : 74) : (filterable ? 60 : 30) + // Render only what fits on this display, plus one prefetch slice per side. + readonly property int previewRadius: Math.max(1, Math.min(16, Math.ceil((panel.width - expandedWidth) / (2 * (sliceWidth + sliceSpacing))) + 1)) + onPreviewRadiusChanged: updateVisibleItems() onOpenedChanged: if (!opened) layoutSettled = false @@ -100,14 +104,6 @@ Item { return ImagePickerModel.firstMatchingIndex(imageArray, filterText) } - function filteredPosition(index) { - return ImagePickerModel.filteredPosition(imageArray, index, filterText) - } - - function selectedFilteredPosition() { - return ImagePickerModel.selectedFilteredPosition(imageArray, selectedIndex, filterText) - } - function select(index, immediate) { if (imageArray.length === 0) return if (index < 0) index = 0 @@ -215,8 +211,10 @@ Item { root.loadedImageRows = rows root.selectedIndex = root.indexForSelectedImage(newImages) + root.neighborImagesEnabled = false root.imageArray = newImages root.imagesLoaded = true + Qt.callLater(root.enableNeighborsWhenReady) if (reveal !== false) { root.opened = true @@ -273,6 +271,22 @@ Item { } property var imageArray: [] + readonly property var matchingImageIndices: ImagePickerModel.matchingIndices(imageArray, filterText) + onMatchingImageIndicesChanged: updateVisibleItems() + onSelectedIndexChanged: updateVisibleItems() + + function updateVisibleItems() { + ImagePickerModel.syncWindow(visibleImages, ImagePickerModel.visibleWindow(matchingImageIndices, selectedIndex, previewRadius)) + } + + function enableNeighborsWhenReady() { + for (var i = 0; i < imageCards.count; i++) { + var item = imageCards.itemAt(i) + if (item && item.selected && item.previewReady) neighborImagesEnabled = true + } + } + + ListModel { id: visibleImages } function currentThemePreview() { @@ -465,6 +479,8 @@ Item { Item { id: card visible: root.opened && root.imagesLoaded && root.layoutSettled && root.imageArray.length > 0 + opacity: root.layoutSettled ? 1 : 0 + Behavior on opacity { NumberAnimation { duration: 90 } } width: Math.min(parent.width - 80, root.expandedWidth + 13 * (root.sliceWidth + root.sliceSpacing) + 40) height: root.expandedHeight + Style.space(30) + root.bottomChromeHeight anchors.centerIn: parent @@ -515,30 +531,32 @@ Item { Component.onCompleted: forceActiveFocus() Repeater { - model: root.imageArray.length + id: imageCards + model: visibleImages delegate: Item { id: item - required property int index + required property int imageIndex + required property int relativeIndex - readonly property var imageData: root.imageArray[index] + readonly property var imageData: root.imageArray[imageIndex] readonly property string filePath: imageData ? imageData.filePath : "" readonly property string fileName: imageData ? imageData.fileName : "" readonly property string thumbnailPath: imageData ? imageData.thumbnailPath : "" - readonly property bool matched: root.itemMatches(index) - readonly property int relativeIndex: root.filteredPosition(index) - root.selectedFilteredPosition() - readonly property bool selected: matched && index === root.selectedIndex - readonly property bool nearby: matched && Math.abs(relativeIndex) <= 16 - property bool sourceActivated: nearby - onNearbyChanged: if (nearby) sourceActivated = true + readonly property bool selected: imageIndex === root.selectedIndex + readonly property bool previewReady: image.status === Image.Ready || image.status === Image.Error + onSelectedChanged: if (selected && previewReady) root.neighborImagesEnabled = true - visible: nearby x: selected ? carousel.previewX : (relativeIndex < 0 ? carousel.previewX + relativeIndex * carousel.itemStep : carousel.previewX + root.expandedWidth + root.sliceSpacing + (relativeIndex - 1) * carousel.itemStep) width: selected ? root.expandedWidth : root.sliceWidth height: selected ? root.expandedHeight : root.sliceHeight y: selected ? 0 : (root.expandedHeight - root.sliceHeight) / 2 z: selected ? 100 : 50 - Math.min(Math.abs(relativeIndex), 40) + Behavior on x { NumberAnimation { duration: 90; easing.type: Easing.OutCubic } } + Behavior on y { NumberAnimation { duration: 90; easing.type: Easing.OutCubic } } + Behavior on width { NumberAnimation { duration: 90; easing.type: Easing.OutCubic } } + Behavior on height { NumberAnimation { duration: 90; easing.type: Easing.OutCubic } } readonly property real skAbs: Math.abs(root.skewOffset) readonly property real topLeft: root.skewOffset >= 0 ? skAbs : 0 @@ -579,17 +597,25 @@ Item { maskSpreadAtMin: 0.3 } + Rectangle { anchors.fill: parent; color: root.dimColor } + Image { id: image anchors.fill: parent - // Load only the initial/visited nearby images, but keep the - // source once activated so Qt does not tear textures down as - // selection moves through the carousel. - source: item.sourceActivated && item.thumbnailPath ? Util.fileUrl(item.thumbnailPath) : "" + // Even an uncached 6K wallpaper decodes at card size, off the + // GUI thread. Departing cards release their images instead of + // retaining every preview visited in a large collection. + // Queue the selected preview first; neighbors must not delay + // the image the user opened the picker to see. + source: (item.selected || root.neighborImagesEnabled) && item.thumbnailPath ? Util.fileUrl(item.thumbnailPath) : "" + sourceSize: Qt.size(root.expandedWidth, root.expandedHeight) fillMode: Image.PreserveAspectCrop - asynchronous: false - cache: true + asynchronous: true + cache: false smooth: true + opacity: status === Image.Ready ? 1 : 0 + Behavior on opacity { NumberAnimation { duration: 90 } } + onStatusChanged: if (item.selected && (status === Image.Ready || status === Image.Error)) root.neighborImagesEnabled = true } Rectangle { @@ -617,7 +643,7 @@ Item { MouseArea { anchors.fill: parent cursorShape: Qt.PointingHandCursor - onClicked: item.selected ? root.applySelected() : root.select(index) + onClicked: item.selected ? root.applySelected() : root.select(item.imageIndex) } } } diff --git a/shell/plugins/image-picker/ImagePickerModel.js b/shell/plugins/image-picker/ImagePickerModel.js index f34fdcab..86c4b07a 100644 --- a/shell/plugins/image-picker/ImagePickerModel.js +++ b/shell/plugins/image-picker/ImagePickerModel.js @@ -82,6 +82,45 @@ function nextSelectedIndexForFilter(images, selectedIndex, filterText) { return firstMatchingIndex(images, filterText) } +function matchingIndices(images, filterText) { + var indices = [] + for (var i = 0; i < images.length; i++) { + if (itemMatches(images, i, filterText)) indices.push(i) + } + return indices +} + +// Keep the rendered carousel independent of the size of the collection. +function visibleWindow(indices, selectedIndex, radius) { + var position = indices.indexOf(selectedIndex) + if (position < 0) position = 0 + var items = [] + for (var i = Math.max(0, position - radius); i < Math.min(indices.length, position + radius + 1); i++) { + items.push({ imageIndex: indices[i], relativeIndex: i - position }) + } + return items +} + +// Preserve overlapping delegates as selection moves, including their decoded +// images. Replacing the entire model on every keypress makes previews flicker. +function syncWindow(model, items) { + var wanted = {} + for (var i = 0; i < items.length; i++) wanted[items[i].imageIndex] = true + for (var i = model.count - 1; i >= 0; i--) { + if (!wanted[model.get(i).imageIndex]) model.remove(i) + } + for (var i = 0; i < items.length; i++) { + var existing = i + while (existing < model.count && model.get(existing).imageIndex !== items[i].imageIndex) existing++ + if (existing === model.count) { + model.insert(i, items[i]) + } else { + if (existing !== i) model.move(existing, i, 1) + model.setProperty(i, "relativeIndex", items[i].relativeIndex) + } + } +} + if (typeof module !== "undefined") { module.exports = { nameForPath: nameForPath, @@ -92,6 +131,9 @@ if (typeof module !== "undefined") { filteredPosition: filteredPosition, selectedFilteredPosition: selectedFilteredPosition, indexForSelectedImage: indexForSelectedImage, - nextSelectedIndexForFilter: nextSelectedIndexForFilter + nextSelectedIndexForFilter: nextSelectedIndexForFilter, + matchingIndices: matchingIndices, + visibleWindow: visibleWindow, + syncWindow: syncWindow } } diff --git a/test/shell.d/image-picker-test.sh b/test/shell.d/image-picker-test.sh index d061f3bf..31abedda 100644 --- a/test/shell.d/image-picker-test.sh +++ b/test/shell.d/image-picker-test.sh @@ -42,6 +42,45 @@ assertEqual(picker.filteredPosition(images, 2, 'dark'), 1, 'image picker compute assertEqual(picker.selectedFilteredPosition(images, 2, 'dark'), 0, 'image picker selected filtered position falls back when selected is hidden') assertEqual(picker.nextSelectedIndexForFilter(images, 0, 'dark'), 1, 'image picker moves selection to first match when filter hides current item') +class WindowModel { + constructor() { this.items = []; this.insertions = 0 } + get count() { return this.items.length } + get(i) { return this.items[i] } + remove(i) { this.items.splice(i, 1) } + insert(i, item) { this.items.splice(i, 0, { ...item }); this.insertions++ } + move(from, to) { this.items.splice(to, 0, this.items.splice(from, 1)[0]) } + setProperty(i, key, value) { this.items[i][key] = value } +} + +for (const count of [0, 1, 200, 530, 10000]) { + const collection = Array.from({ length: count }, (_, i) => ({ filePath: `/themes/theme-${i}.png` })) + const indices = picker.matchingIndices(collection, '') + const model = new WindowModel() + for (const selected of [0, Math.min(1, count - 1), Math.floor(count / 2), count - 1]) { + const window = picker.visibleWindow(indices, selected, 16) + picker.syncWindow(model, window) + assertDeepEqual(model.items, window, `carousel window reconciles ${count} images at ${selected}`) + assert(model.count <= 33, `carousel bounds delegates for ${count} images`) + if (count) assert(window.some(item => item.imageIndex === selected && item.relativeIndex === 0), `carousel includes selection in ${count} images`) + } + const matches = picker.matchingIndices(collection, 'theme-19') + picker.syncWindow(model, picker.visibleWindow(matches, matches[0], 16)) + assert(model.items.every(item => collection[item.imageIndex].filePath.includes('theme-19')), `carousel filters ${count} images`) + picker.syncWindow(model, picker.visibleWindow([], 0, 16)) + assertEqual(model.count, 0, `carousel clears ${count} images when there are no matches`) +} + +const windowModel = new WindowModel() +const allIndices = Array.from({ length: 200 }, (_, i) => i) +picker.syncWindow(windowModel, picker.visibleWindow(allIndices, 100, 16)) +const retained = windowModel.get(17) +picker.syncWindow(windowModel, picker.visibleWindow(allIndices, 101, 16)) +assertEqual(windowModel.get(16), retained, 'carousel retains overlapping delegates when navigating') +assertEqual(windowModel.insertions, 34, 'carousel creates only one new delegate for an adjacent selection') +for (const radius of [1, 8, 16]) { + assertEqual(picker.visibleWindow(allIndices, 100, radius).length, radius * 2 + 1, `carousel scales its window to radius ${radius}`) +} + const imagePickerQml = fs.readFileSync(path.join(root, 'shell/plugins/image-picker/ImagePicker.qml'), 'utf8') assert( /function preloadRows[\s\S]*if \(opened \|\| requestActive\) return/.test(imagePickerQml), @@ -74,7 +113,14 @@ assert( 'image picker parks on OverlayWindow and takes the keyboard once images load' ) assert( - /source: item\.sourceActivated && item\.thumbnailPath \? Util\.fileUrl\(item\.thumbnailPath\) : ""[\s\S]*asynchronous: false/.test(imagePickerQml), - 'image picker loads activated thumbnails synchronously to avoid carousel flicker' + /model: visibleImages/.test(imagePickerQml) && + /sourceSize: Qt\.size\(root\.expandedWidth, root\.expandedHeight\)/.test(imagePickerQml) && + /asynchronous: true\s*cache: false/.test(imagePickerQml), + 'image picker renders its window with bounded asynchronous decoding' +) +assert( + imagePickerQml.includes('(item.selected || root.neighborImagesEnabled)') && + /onStatusChanged: if \(item.selected && \(status === Image.Ready \|\| status === Image.Error\)\) root.neighborImagesEnabled = true/.test(imagePickerQml), + 'image picker prioritizes the selected preview and releases neighbors on success or failure' ) JS diff --git a/test/shell.d/menu-images-test.sh b/test/shell.d/menu-images-test.sh index 7f162a16..0e79a09f 100644 --- a/test/shell.d/menu-images-test.sh +++ b/test/shell.d/menu-images-test.sh @@ -151,3 +151,77 @@ while IFS=$'\t' read -r row_image row_thumbnail; do fail "image menu prints each image with its generated thumbnail" done <<<"$rows" pass "image menu prints its rows for the shell to hold" + +# Block converters behind a gate: printing lazy rows must neither await them +# nor start one process per image. Repeated refreshes share one worker pool. +lazy_images="$tmp/lazy-images" +lazy_state="$tmp/lazy-state" +mkdir -p "$lazy_images" "$lazy_state" +for (( i = 0; i < 40; i++ )); do + printf 'image' >"$lazy_images/$i.png" +done +printf '0\n' >"$lazy_state/active" +printf '0\n' >"$lazy_state/peak" +cat >"$stub_bin/nproc" <<'EOF' +#!/bin/bash +echo "${FAKE_CORES:-2}" +EOF +cat >"$stub_bin/vipsthumbnail" <<'EOF' +#!/bin/bash +while (( $# > 0 )); do + if [[ $1 == "--path" ]]; then output=${2%%\[*}; break; fi + shift +done +exec 9>"$LAZY_STATE/lock" +priority=$(ps -o ni= -p "$$") +(( priority >= 10 )) || : >"$LAZY_STATE/priority-failed" +[[ $(ionice -p "$$") == "idle" ]] || : >"$LAZY_STATE/priority-failed" +flock 9 +active=$(<"$LAZY_STATE/active") +active=$((active + 1)) +printf '%s\n' "$active" >"$LAZY_STATE/active" +(( active <= $(<"$LAZY_STATE/peak") )) || printf '%s\n' "$active" >"$LAZY_STATE/peak" +flock -u 9 +while [[ ! -f $LAZY_STATE/gate ]]; do sleep 0.02; done +printf 'thumbnail' >"$output" +flock 9 +active=$(<"$LAZY_STATE/active") +printf '%s\n' "$((active - 1))" >"$LAZY_STATE/active" +echo done >>"$LAZY_STATE/completed" +EOF +chmod +x "$stub_bin/nproc" "$stub_bin/vipsthumbnail" +# Also release the gate on failure, so background fixtures cannot outlive us. +trap 'touch "$lazy_state/gate"' EXIT + +for cores in 1 2 8; do + lazy_state="$tmp/lazy-state-$cores" + mkdir -p "$lazy_state" + printf '0\n' >"$lazy_state/active" + printf '0\n' >"$lazy_state/peak" + expected_workers=1 + (( cores < 4 )) || expected_workers=2 + for run in 1 2; do + rows=$(PATH="$stub_bin:$PATH" XDG_CACHE_HOME="$tmp/lazy-cache-$cores" LAZY_STATE="$lazy_state" FAKE_CORES="$cores" \ + timeout 10 "$ROOT/bin/omarchy-menu-images" --lazy-thumbnails --print-rows "$lazy_images") + (( $(wc -l <<<"$rows") == 40 )) || fail "lazy image menu returns all rows before conversion" + done + for attempt in {1..100}; do + (( $(<"$lazy_state/active") == expected_workers )) && break + sleep 0.02 + done + (( $(<"$lazy_state/active") == expected_workers && $(<"$lazy_state/peak") == expected_workers )) || + fail "lazy image menu bounds repeated refreshes to $expected_workers workers on $cores cores" + [[ ! -e $lazy_state/completed ]] || fail "lazy image menu does not wait for conversion" + [[ ! -e $lazy_state/priority-failed ]] || fail "lazy image menu reserves CPU and I/O priority for the UI" + pass "lazy image menu opens with at most $expected_workers workers on $cores cores" + + touch "$lazy_state/gate" + for attempt in {1..500}; do + if [[ -f $lazy_state/completed ]] && (( $(wc -l <"$lazy_state/completed") == 40 )); then break; fi + sleep 0.02 + done + (( $(wc -l <"$lazy_state/completed") == 40 && $(<"$lazy_state/peak") == expected_workers )) || + fail "lazy image menu completes the queue after its parent and queue path are gone" + pass "lazy image menu workers finish every queued thumbnail after the caller exits" +done +trap 'rm -rf "$tmp"' EXIT