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