Skip to content

KDE: move source picker to portal - #37

Merged
kalopsia-space merged 2 commits into
FTHR-Community:linuxfrom
LokiLeiche:linux-kde-window-capture
Sep 21, 2026
Merged

kalopsia-space merged 2 commits into
FTHR-Community:linuxfrom
LokiLeiche:linux-kde-window-capture

Conversation

@LokiLeiche

Copy link
Copy Markdown

What does this PR do?

Pressing the source button in the UI while using KDE now opens the screencast portal source picker instead of FTHR's own dropdown menu.

Why?

Since screencast portal manages recording permissions and you allow permissions for one screen/window at a time, the previous implementation didn't work. You picked one monitor when first launching the app, with no way to change it after. The settings in the dropdown menu had no effect, since the portal still only permitted recording the previously picked screen.

Changes

  • While on KDE (kwin compositor) the Source button opens the screencast portal source picker instead of the source picker dropdown

Testing

  • Ran QT_QPA_PLATFORM=offscreen pytest tests/ — all pass (16 skips that were already there previously)
  • Tested manually on Linux

Notes

Preview:

2026-09-20_09-28-28.mp4

@kalopsia-space Please review, since you have way more knowledge on other linux distros than me. I use KDE (Kubuntu) and this works flawlessly for me. Right now I made it so that this behaviour only applies to the kwin compositor (KDE) but this may also be a better approach for other compositors using the same screencast portal, if there are any.

Instead of choosing the source in-app via the dropdown, re-opens the screenportal UI that lets you choose the monitor. For permission reasons
@kalopsia-space

Copy link
Copy Markdown
Collaborator

Review

Verdict: Comment — promising, but verification is still needed.

The KDE/KWin restriction is sensible: it preserves the existing source popup on X11 and non-KDE compositors, and the token-reset/restart flow is directionally aligned with the portal behavior.

Before merge, please add or run coverage for the actual native path:

  • restore-token deletion and permission/error handling;
  • capture-engine restart after the source button is pressed;
  • restored-session negotiation failure and retry without the token;
  • portal/PipeWire startup and cancellation behavior.

The current focused test only verifies Python routing to the handler. GitHub reports no checks for this branch, so please run the repository CI/native Linux checks and include the result here.

No blocking correctness or security defect was confirmed from the diff.

@LokiLeiche

LokiLeiche commented Sep 21, 2026 •

Copy link
Copy Markdown
Author

Added more tests to cover the mentioned cases. CI workflows show 5 passes, 1 skip and 3 fails, just like any other PR here

@kalopsia-space kalopsia-space left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: changes requested

This is a promising KDE/KWin portal-source-picker change, but I cannot approve or merge it yet.

Blocking CI issues

  • The Windows engine job fails during test collection because core.upload_manager imports EXPECTED_UPLOADER_LINUX_BUNDLE_SHA256, but this PR branch does not contain that symbol in core/uploader_bundle_manifest.py. The branch is behind current linux (which contains the merged PR #40 changes), so please rebase/update the PR against current linux and resolve the resulting integration conflicts.
  • Both Windows Python test jobs fail in test_linux_activation_restores_uploader_executable_bit; the test expects Unix executable bits on a Windows filesystem. Please preserve the production chmod behavior while making the contract test platform-portable, or otherwise fix the regression exposed by the current CI.

What looks good

The KDE-only routing is appropriately guarded by Wayland plus KWin detection, token deletion is handled before engine restart, cancellation is not retried, and the new native tests cover restore-token retry and portal cancellation.

After updating the branch, please rerun the full CI and confirm the Windows jobs are green. The Linux jobs currently pass, but the overall PR is not merge-ready while required Windows checks fail.

@kalopsia-space kalopsia-space left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair point — this targets the Linux branch, and the Linux-specific validation is passing. The Windows failures are unrelated to the KDE/KWin portal change and I am not treating them as blockers for this Linux PR.

The Linux AppImage/package, Linux engine, Python-on-Ubuntu, and release-verification checks passed. I am updating my review to approve this PR for the Linux branch. The Windows CI failures should be tracked separately as follow-up maintenance.

@kalopsia-space
kalopsia-space dismissed their stale review September 21, 2026 00:22

Superseded: Windows CI failures are not blockers for this Linux-branch change.

@kalopsia-space

Copy link
Copy Markdown
Collaborator

Correction: this PR targets the Linux branch. The Linux-specific checks pass, and the Windows CI failures are unrelated to this KDE/KWin change. I approve PR #37; the earlier Windows-related concerns should be disregarded.

@kalopsia-space
kalopsia-space merged commit dc12f1c into FTHR-Community:linux Sep 21, 2026
6 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants