From fcf9eeb5c454739f3f23cdfd7d57d77b12961025 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Fri, 9 Oct 2026 12:21:45 +0200 Subject: [PATCH] Show the image picker instantly, with no backdrop (#14709) Remove the carousel's fade and geometry transitions, hold the reveal until every visible preview has decoded and two frames have pre-rendered (falling back to revealing anyway after 400ms), and drop the fullscreen scrim and its theme tokens so the picker sits directly on the desktop. Only Escape is accepted before the picker is visible. --- default/themed/shell.toml.tpl | 8 +- shell/Commons/Color.qml | 6 +- shell/plugins/image-picker/ImagePicker.qml | 91 ++++++++++++++++------ test/shell.d/image-picker-test.sh | 84 ++++++++++++++++++++ 4 files changed, 157 insertions(+), 32 deletions(-) diff --git a/default/themed/shell.toml.tpl b/default/themed/shell.toml.tpl index b0a68d8b..fda55cec 100644 --- a/default/themed/shell.toml.tpl +++ b/default/themed/shell.toml.tpl @@ -224,12 +224,10 @@ selection = "{{ accent }}" selection-alpha = 0.45 [image-picker] -# Carousel-style picker. The picker has no card surface, so `scrim` is -# the full-screen wash. Per-slice dim overlays and text outlines on top -# of the scrim track the foundational background color directly. +# Carousel-style picker shown directly over the desktop, with no card +# surface or backdrop wash. Per-slice dim overlays and text outlines +# track the foundational background color directly. # unselected-border-alpha softens carousel slices that aren't selected. -scrim = "{{ background }}" -scrim-alpha = 0.5 text = "{{ foreground }}" selected-border = "{{ accent }}" selected-border-alpha = 1.0 diff --git a/shell/Commons/Color.qml b/shell/Commons/Color.qml index 4fd2452b..0dbcbf67 100644 --- a/shell/Commons/Color.qml +++ b/shell/Commons/Color.qml @@ -128,11 +128,9 @@ QtObject { property color borderError: root.composed("lock.border-error", "lock.border-alpha", root.urgent, 1.0) property color selection: root.composed("lock.selection", "lock.selection-alpha", root.accent, 0.45) } - // The image picker has no card surface; `scrim` is the full-screen dim - // wash, and per-slice dim overlays / text outlines use the foundational - // `background` color directly. + // The image picker has no card surface or backdrop; per-slice dim + // overlays / text outlines use the foundational `background` color directly. readonly property QtObject imagePicker: QtObject { - property color scrim: root.composed("image-picker.scrim", "image-picker.scrim-alpha", root.background, 0.5) property color text: root.pick("image-picker.text", root.foreground) property color selectedBorder: root.composed("image-picker.selected-border", "image-picker.selected-border-alpha", root.accent, 1.0) property color unselectedBorder: root.composed("image-picker.unselected-border", "image-picker.unselected-border-alpha", root.foreground, 0.28) diff --git a/shell/plugins/image-picker/ImagePicker.qml b/shell/plugins/image-picker/ImagePicker.qml index d4ec95ec..e3947e78 100644 --- a/shell/plugins/image-picker/ImagePicker.qml +++ b/shell/plugins/image-picker/ImagePicker.qml @@ -27,6 +27,7 @@ Item { property bool filterable: false property bool layoutSettled: false property bool neighborImagesEnabled: false + property int renderedFrames: 0 property bool requestActive: false property int requestSerial: 0 property int applySerial: 0 @@ -39,11 +40,11 @@ Item { property bool themeMode: false property bool themeOpenPending: false // Bound to the central [image-picker] section in shell.toml via Commons.Color.qml. - // `dimColor` tints unselected slices and text outlines on top of the scrim; - // it intentionally tracks the foundational background, not a surface role. + // `dimColor` tints unselected slices and text outlines against the desktop + // behind the picker; it intentionally tracks the foundational background, + // not a surface role. property color dimColor: Commons.Color.background property color foreground: Commons.Color.imagePicker.text - property color scrim: Commons.Color.imagePicker.scrim property color selectedBorder: Commons.Color.imagePicker.selectedBorder property color unselectedBorder: Commons.Color.imagePicker.unselectedBorder property int expandedWidth: 768 @@ -57,7 +58,7 @@ Item { 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 + onOpenedChanged: if (!opened) { layoutSettled = false; renderedFrames = 0 } function scriptPath(name) { return omarchyPath + "/shell/plugins/image-picker/" + name @@ -70,10 +71,35 @@ Item { function revealWhenSettled(serial) { Qt.callLater(function() { - if (serial === root.requestSerial && root.opened && root.imagesLoaded && root.imageArray.length > 0) { - root.layoutSettled = true - root.focusPicker() - } + if (serial === root.requestSerial) root.maybeReveal() + }) + } + + // The card renders at opacity 0 until every visible preview has decoded + // and its masked layers have presented, so the picker lands in one + // complete frame instead of flashing its images in as each asynchronous + // decode and layer upload finishes. + function allPreviewsSettled() { + if (!opened || !imagesLoaded || layoutSettled || imageArray.length === 0 || imageCards.count === 0) return false + for (var i = 0; i < imageCards.count; i++) { + var item = imageCards.itemAt(i) + if (!item || !item.previewSettled) return false + } + return true + } + + function settleReveal() { + if (!opened || !imagesLoaded || layoutSettled || imageArray.length === 0) return + layoutSettled = true + focusPicker() + } + + function maybeReveal() { + if (!allPreviewsSettled() || renderedFrames < 2) return + // Confirm on the next tick: delegates inserted later in the same window + // sync must not pop in after an early all-ready reading. + Qt.callLater(function() { + if (root.allPreviewsSettled() && root.renderedFrames >= 2) root.settleReveal() }) } @@ -450,7 +476,7 @@ Item { // Theme/background set hooks can warm selector rows after a picker was // dismissed. Ignore those preloads while a user-visible request is open; // otherwise the preload resets layoutSettled without revealing again, - // leaving only the fullscreen scrim. + // leaving the picker invisible behind its own open overlay. if (opened || requestActive) return requestSerial += 1 @@ -488,28 +514,39 @@ Item { shownKeyboardFocus: root.imagesLoaded ? WlrKeyboardFocus.Exclusive : WlrKeyboardFocus.None WlrLayershell.namespace: "omarchy-image-selector" - Rectangle { - anchors.fill: parent - visible: root.opened && root.imagesLoaded - color: root.scrim - } - MouseArea { anchors.fill: parent enabled: root.opened && root.imagesLoaded onClicked: root.cancel() } + // Count presented frames while the card pre-renders below; two frames + // prove its masked layers uploaded on this fresh surface. + FrameAnimation { + running: root.opened && !root.layoutSettled && root.renderedFrames < 2 + onTriggered: { + root.renderedFrames += 1 + root.maybeReveal() + } + } + + // A preview stuck decoding (slow disk, huge original) must not hold an + // invisible picker with the keyboard grabbed: reveal anyway shortly. + Timer { + interval: 400 + running: root.opened && root.imagesLoaded && !root.layoutSettled && root.imageArray.length > 0 + onTriggered: root.settleReveal() + } + Item { id: card - visible: root.opened && root.imagesLoaded && root.layoutSettled && root.imageArray.length > 0 + visible: root.opened && root.imagesLoaded && 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 - MouseArea { anchors.fill: parent; onClicked: {} } + MouseArea { anchors.fill: parent; enabled: root.layoutSettled; onClicked: {} } Item { id: carousel @@ -527,6 +564,15 @@ Item { Keys.priority: Keys.BeforeItem Keys.onPressed: function(event) { + // Nothing is visible before the reveal: only let the user back + // out, never filter into a dead end or apply an unseen pick. + if (!root.layoutSettled) { + if (event.key === Qt.Key_Escape) { + root.cancel() + event.accepted = true + } + return + } if (event.key === Qt.Key_Escape) { if (root.filterText) { root.updateFilter("") @@ -557,6 +603,7 @@ Item { Repeater { id: imageCards model: visibleImages + onCountChanged: root.maybeReveal() delegate: Item { id: item @@ -570,6 +617,8 @@ Item { readonly property bool selected: imageIndex === root.selectedIndex readonly property bool previewReady: image.status === Image.Ready || image.status === Image.Error + readonly property bool previewSettled: !thumbnailPath || previewReady + onPreviewSettledChanged: if (previewSettled) root.maybeReveal() onSelectedChanged: if (selected && previewReady) root.neighborImagesEnabled = true x: selected ? carousel.previewX : (relativeIndex < 0 ? carousel.previewX + relativeIndex * carousel.itemStep : carousel.previewX + root.expandedWidth + root.sliceSpacing + (relativeIndex - 1) * carousel.itemStep) @@ -577,10 +626,6 @@ Item { 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 @@ -640,7 +685,6 @@ Item { 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 } @@ -668,6 +712,7 @@ Item { MouseArea { anchors.fill: parent + enabled: root.layoutSettled cursorShape: Qt.PointingHandCursor onClicked: item.selected ? root.applySelected() : root.select(item.imageIndex) } diff --git a/test/shell.d/image-picker-test.sh b/test/shell.d/image-picker-test.sh index a18ac870..3867c651 100644 --- a/test/shell.d/image-picker-test.sh +++ b/test/shell.d/image-picker-test.sh @@ -174,4 +174,88 @@ assert( /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' ) +assert( + !/Behavior on (opacity|x|y|width|height)/.test(imagePickerQml) && !imagePickerQml.includes('NumberAnimation'), + 'image picker appears instantly with no fade or geometry transitions' +) +assertEqual((imagePickerQml.match(/layoutSettled = true/g) || []).length, 1, 'image picker settles its layout in exactly one place') +assert( + /readonly property bool previewSettled: !thumbnailPath \|\| previewReady/.test(imagePickerQml) && + /onPreviewSettledChanged: if \(previewSettled\) root\.maybeReveal\(\)/.test(imagePickerQml) && + /function allPreviewsSettled\(\) \{[\s\S]*?for \(var i = 0; i < imageCards\.count; i\+\+\) \{[\s\S]*?if \(!item \|\| !item\.previewSettled\) return false[\s\S]*?return true/.test(imagePickerQml) && + /function maybeReveal\(\) \{\s*if \(!allPreviewsSettled\(\) \|\| renderedFrames < 2\) return[\s\S]*?Qt\.callLater\(function\(\) \{\s*if \(root\.allPreviewsSettled\(\) && root\.renderedFrames >= 2\) root\.settleReveal\(\)/.test(imagePickerQml) && + /function settleReveal\(\) \{\s*if \(!opened \|\| !imagesLoaded \|\| layoutSettled \|\| imageArray\.length === 0\) return\s*layoutSettled = true\s*focusPicker\(\)/.test(imagePickerQml), + 'image picker reveals only once every visible preview has loaded' +) +assert( + /id: card\s*visible: root\.opened && root\.imagesLoaded && root\.imageArray\.length > 0\s*opacity: root\.layoutSettled \? 1 : 0/.test(imagePickerQml) && + /FrameAnimation \{\s*running: root\.opened && !root\.layoutSettled && root\.renderedFrames < 2\s*onTriggered: \{\s*root\.renderedFrames \+= 1\s*root\.maybeReveal\(\)/.test(imagePickerQml) && + /if \(!allPreviewsSettled\(\) \|\| renderedFrames < 2\) return/.test(imagePickerQml) && + /MouseArea \{ anchors\.fill: parent; enabled: root\.layoutSettled; onClicked: \{\} \}/.test(imagePickerQml), + 'image picker pre-renders hidden for two frames so its first visible frame is complete' +) +assert( + !imagePickerQml.includes('color: root.scrim') && !/property color scrim:/.test(imagePickerQml), + 'image picker shows no fullscreen scrim wash behind the carousel' +) +const colorQml = fs.readFileSync(path.join(root, 'shell/Commons/Color.qml'), 'utf8') +const shellTpl = fs.readFileSync(path.join(root, 'default/themed/shell.toml.tpl'), 'utf8') +const pickerSection = shellTpl.slice(shellTpl.indexOf('[image-picker]'), shellTpl.indexOf('\n[', shellTpl.indexOf('[image-picker]') + 1)) +assert( + !colorQml.includes('image-picker.scrim') && !pickerSection.includes('scrim'), + 'image picker scrim theme tokens are gone with the backdrop' +) +assert( + /Timer \{\s*interval: 400\s*running: root\.opened && root\.imagesLoaded && !root\.layoutSettled && root\.imageArray\.length > 0\s*onTriggered: root\.settleReveal\(\)/.test(imagePickerQml), + 'image picker reveals anyway after a short wait if a preview stalls' +) +assert( + /if \(!root\.layoutSettled\) \{\s*if \(event\.key === Qt\.Key_Escape\) \{\s*root\.cancel\(\)\s*event\.accepted = true\s*\}\s*return\s*\}/.test(imagePickerQml) && + imagePickerQml.indexOf('if (!root.layoutSettled) {') < imagePickerQml.indexOf('if (root.filterText) {'), + 'image picker only accepts Escape before it is visible' +) + +// Run the QML reveal guards themselves, the way the loadRows handler above +// runs, against mocked delegates in each state the gate must decide. +const revealFns = [ + imagePickerQml.match(/function allPreviewsSettled\(\) \{[\s\S]*?\n \}/)[0], + imagePickerQml.match(/function settleReveal\(\) \{[\s\S]*?\n \}/)[0], + imagePickerQml.match(/function maybeReveal\(\) \{[\s\S]*?\n \}/)[0] +].join('\n') +function revealContext(settledFlags, frames) { + const ctx = { + opened: true, + imagesLoaded: true, + layoutSettled: false, + renderedFrames: frames, + imageArray: settledFlags.map(() => ({})), + focused: 0, + Qt: { callLater(callback) { callback() } } + } + const items = settledFlags.map(previewSettled => ({ previewSettled })) + ctx.imageCards = { count: items.length, itemAt(i) { return items[i] } } + ctx.focusPicker = () => { ctx.focused += 1 } + ctx.root = ctx + require('vm').runInNewContext(revealFns, ctx) + return ctx +} +let reveal = revealContext([true, true], 2) +reveal.maybeReveal() +assert(reveal.layoutSettled && reveal.focused === 1, 'reveal settles and focuses once every preview is settled and frames presented') +reveal = revealContext([true, false], 2) +reveal.maybeReveal() +assert(!reveal.layoutSettled, 'an unsettled preview holds the reveal') +reveal = revealContext([true, true], 1) +reveal.maybeReveal() +assert(!reveal.layoutSettled, 'reveal waits for presented frames') +reveal = revealContext([], 2) +reveal.maybeReveal() +assert(!reveal.layoutSettled, 'reveal waits while no cards are rendered') +reveal = revealContext([false], 0) +reveal.settleReveal() +assert(reveal.layoutSettled, 'fallback settle reveals despite a stalled preview') +reveal = revealContext([true], 2) +reveal.opened = false +reveal.settleReveal() +assert(!reveal.layoutSettled, 'settle does nothing once the picker is closed') JS