Capture GUI - #146
Conversation
Added new configuration options for capture settings and screen recordings, including switches for freezing display, snapping to windows, showing notifications, and custom save locations. Enhanced user interface elements for better usability.
Updated grimblast.sh to enhance screenshot functionality and improve code structure.
|
Thanks, that's a cool feature ! Before I review, I notice that on the setting page, we have 3 sections for the same subnect. I think it would be preferable to have one section (capture settings) with sub sections for screenshot and screen record. I will review the code in more details once this is done |
|
@LeVraiArdox it is done :D 
|
LeVraiArdox
left a comment
LeVraiArdox
left a comment
There was a problem hiding this comment.
A few issues that I often find in your PRs
- You make all the feature into a single file instead of properly split
- You keep useless generated comments
- You don't use enough existing components, and not only GUI. You should search in what exists already (cpp files, services or quickshell docs).
For your future PRs, keep that in mind. And give the full sleex architecture to the AI you use, so it doesn't make spaghetti.
This aside, can you explain the backend logic ? I find it a bit confusing. Thanks anyway
| hl.bind( | ||
| "SUPER+SHIFT+T", | ||
| hl.dsp.exec_cmd('grim -g "$(slurp $SLURP_ARGS)" "tmp.png" && tesseract "tmp.png" - | wl-copy && rm "tmp.png"'), | ||
| hl.dsp.exec_cmd("qs -p /usr/share/sleex/shell.qml ipc call screenshot onOpenOcr"), |
There was a problem hiding this comment.
You could make a global shortcut, it would be cleaner
| hl.bind( | ||
| "SUPER+ALT+R", | ||
| hl.dsp.exec_cmd("/usr/share/sleex/scripts/record-script.sh"), | ||
| hl.dsp.exec_cmd("/usr/share/sleex/scripts/grimblast.sh"), |
There was a problem hiding this comment.
Why using grimblast for record ? It is meant to be for screenshots only
| readonly property string qsHome: Quickshell.env("HOME") | ||
| readonly property string qsRuntimeDir: Quickshell.env("XDG_RUNTIME_DIR") | ||
| readonly property string qsCacheDirBase: qsHome + "/.cache/quickshell" | ||
| readonly property string qsStateDirBase: qsHome + "/.local/state/quickshell" | ||
| readonly property string qsRunDirBase: (qsRuntimeDir !== "" ? qsRuntimeDir : "/tmp") + "/quickshell" |
There was a problem hiding this comment.
Two things:
- Use
Directoriesfor global paths (see modules/common/Directories.qml) - Store it in
sleexinstead ofquickshell
| function getCacheDir(widgetName) { | ||
| var envPath = Quickshell.env("QS_CACHE_" + widgetName.toUpperCase()); | ||
| var finalPath = envPath ? envPath : (qsCacheDirBase + "/" + widgetName); | ||
| Quickshell.execDetached(["mkdir", "-p", finalPath]); | ||
| return finalPath; | ||
| } | ||
| function getStateDir(widgetName) { | ||
| var envPath = Quickshell.env("QS_STATE_" + widgetName.toUpperCase()); | ||
| var finalPath = envPath ? envPath : (qsStateDirBase + "/" + widgetName); | ||
| Quickshell.execDetached(["mkdir", "-p", finalPath]); | ||
| return finalPath; | ||
| } | ||
| function getRunDir(widgetName) { | ||
| var envPath = Quickshell.env("QS_RUN_" + widgetName.toUpperCase()); | ||
| var finalPath = envPath ? envPath : (qsRunDirBase + "/" + widgetName); | ||
| Quickshell.execDetached(["mkdir", "-p", finalPath]); | ||
| return finalPath; | ||
| } |
There was a problem hiding this comment.
Can you explain to be why these are necessary please ?
| Process { | ||
| id: stateLoader | ||
| command: ["bash", "-c", | ||
| "cat '" + scopeRoot.getCacheDir("screenshot") + "/video_mode' 2>/dev/null; echo '---'; " + | ||
| "cat '" + scopeRoot.getStateDir("screenshot") + "/audio_prefs' 2>/dev/null"] | ||
| stdout: StdioCollector { | ||
| onStreamFinished: { | ||
| let parts = this.text.split("---"); | ||
| let modeStr = (parts[0] || "").trim(); | ||
| let audioStr = (parts[1] || "").trim(); | ||
| if (modeStr !== "") scopeRoot.savedModeText = modeStr; | ||
| if (audioStr !== "") { | ||
| let a = audioStr.split(","); | ||
| if (a.length >= 5) { | ||
| scopeRoot.savedDeskVol = parseFloat(a[0]) || 1.0; | ||
| scopeRoot.savedDeskMute = a[1] === "true"; | ||
| scopeRoot.savedMicVol = parseFloat(a[2]) || 1.0; | ||
| scopeRoot.savedMicMute = a[3] === "true"; | ||
| scopeRoot.savedMicDevice = a.slice(4).join(","); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
| property bool captureGPUrendering: false | ||
| property bool freezeOnCapture: false | ||
|
|
||
| property bool screenshotCopyToClipboard: true | ||
| property bool screenshotCompressionEnabled: false | ||
| property int screenshotQuality: 6 | ||
| property bool screenshotSaveDirEnabled: false | ||
| property string screenshotSaveDir: "" | ||
|
|
||
| property int screenRecordingFPS: 60 | ||
| property bool autoBitrate: true | ||
| property int screenRecordingBitrate: 8000 | ||
| property bool recordingSaveDirEnabled: false | ||
| property string recordingSaveDir: "" | ||
|
|
||
| property bool snapToWindows: true | ||
| property bool showCapturedNotifications: true | ||
|
|
||
| property bool autoFps: true | ||
| } | ||
| } |
There was a problem hiding this comment.
Make a new component for that
| IpcHandler { | ||
| target: "config" | ||
|
|
||
| function onGetCaptureGPURendering(): string { | ||
| return configOptionsObj.display.captureGPUrendering ? "true" : "false" | ||
| } | ||
| function onGetFreezeOnCapture(): string { | ||
| return configOptionsObj.display.freezeOnCapture ? "true" : "false" | ||
| } | ||
| function onGetScreenshotCopyToClipboard(): string { | ||
| return configOptionsObj.display.screenshotCopyToClipboard ? "true" : "false" | ||
| } | ||
| function onGetScreenshotCompressionEnabled(): string { | ||
| return configOptionsObj.display.screenshotCompressionEnabled ? "true" : "false" | ||
| } | ||
| function onGetScreenRecordingFPS(): string { | ||
| return String(configOptionsObj.display.screenRecordingFPS) | ||
| } | ||
| function onGetAutoBitrate(): string { | ||
| return configOptionsObj.display.autoBitrate ? "true" : "false" | ||
| } | ||
| function onGetScreenRecordingBitrate(): string { | ||
| return String(configOptionsObj.display.screenRecordingBitrate) | ||
| } | ||
| function onGetScreenshotQuality(): string { | ||
| return String(configOptionsObj.display.screenshotQuality) | ||
| } | ||
| function onGetScreenshotSaveDirEnabled(): string { | ||
| return configOptionsObj.display.screenshotSaveDirEnabled ? "true" : "false" | ||
| } | ||
| function onGetScreenshotSaveDir(): string { | ||
| return configOptionsObj.display.screenshotSaveDir || "" | ||
| } | ||
| function onGetRecordingSaveDirEnabled(): string { | ||
| return configOptionsObj.display.recordingSaveDirEnabled ? "true" : "false" | ||
| } | ||
| function onGetRecordingSaveDir(): string { | ||
| return configOptionsObj.display.recordingSaveDir || "" | ||
| } | ||
| function onGetShowCapturedNotifications(): string { | ||
| return configOptionsObj.display.showCapturedNotifications ? "true" : "false" | ||
| } |
There was a problem hiding this comment.
Don't add an IPC handler here, it's not the place. Add it to the file of the actual feature
| // The old UI kit had a per-section `fullWidth` opt-in for the page's | ||
| // two-column masonry layout; the current SleexUiKit ContentPage has no | ||
| // such support, so the capture sections (which were designed to span | ||
| // the full page width) would get squeezed into half-width columns on | ||
| // wide settings windows. Keeping the page in single-column mode makes | ||
| // every section span the full width, matching the original design. | ||
| forceSingleColumn: true |
| onClicked: { | ||
| if (!checked && !captureSection.gsrAvailable) { | ||
| Quickshell.execDetached(["notify-send", "-u", "critical", "-a", "Screen Recorder", | ||
| "Missing dependency: gpu-screen-recorder", "Install gpu-screen-recorder to enable hardware acceleration."]) |
There was a problem hiding this comment.
Here is a tip. For every external software, add it as dependency or optional dependency to the PKGBUILD
| id: fpsSlider | ||
| Layout.fillWidth: true | ||
| from: 15 | ||
| to: 144 |
There was a problem hiding this comment.
You could set the max to the max refresh rate (https://github.com/AxOS-project/Sleex/blob/main/src/share/sleex/plugins/src/Sleex/services/monitors.hpp)
|
@LeVraiArdox Woahhhhhh Let me rework this entire implementation and in the meantime, I will convert this to a draft. |
|
Porting something from a config to another is chirurgical indeed. Some problems might also have been created on the go |
|
Okay just looked a bit... It's true that the source is a mess |
This might just be my new favourite word |
Yeahh this PR is drafted for now |
2 participants
Capture GUI
A screenshot and screen recording overhaul with comprehensive, modifiable parameters within Sleex settings.
Available Functions
Full Showcase
Main Demo (snap-to-windows ON)
demo.mp4
Scanable QR codes (requires
zbar)demo.mp4
OCR Functionality
demo.mp4
Additional Context
All previous functions remain intact and can be executed using the same keybinds as before. Example, OCR still works using
SUPER+SHIFT+T, but now uses the CaptureOverlay's visual grid. This looks much more aesthetic. Also, functions like screen recordings can still be executed directly, bypassing theCaptureOverlaywith their previous direct keybinds likeSUPER+SHIFT+ALT+R).