Skip to content

feat(linux/xdgportal): Simplify display matching logic#5053

Merged
ReenigneArcher merged 1 commit intoLizardByte:masterfrom
Kishi85:update-portalgrab-display-matching
Apr 27, 2026
Merged

feat(linux/xdgportal): Simplify display matching logic#5053
ReenigneArcher merged 1 commit intoLizardByte:masterfrom
Kishi85:update-portalgrab-display-matching

Conversation

@Kishi85
Copy link
Copy Markdown
Contributor

@Kishi85 Kishi85 commented Apr 26, 2026

Description

This simplifies the display name matching logic and also allows for wl_output names to be used directly for config parameter output_name to select the initial display. The latter I've noticed was possible due to a side-note in #5049.

Note: If someone already figured out that output_name can be used with portalgrab by prefixing the wayland output name with 'n' (DP-1 getting selected by output_name=nDP-1) this will break as it will now require the output_name to be equal to the wayland output name (DP-1 being selected with output_name=DP-1 in config) which is a lot more self-explaining.

Additionally the log message for found display explicitly states the id/name parameter now so it somewhat matches the documented behaviour.

Screenshot

Issues Fixed or Closed

Roadmap Issues

Type of Change

  • feat: New feature (non-breaking change which adds functionality)
  • fix: Bug fix (non-breaking change which fixes an issue)
  • docs: Documentation only changes
  • style: Changes that do not affect the meaning of the code (white-space, formatting, missing semicolons, etc.)
  • refactor: Code change that neither fixes a bug nor adds a feature
  • perf: Code change that improves performance
  • test: Adding missing tests or correcting existing tests
  • build: Changes that affect the build system or external dependencies
  • ci: Changes to CI configuration files and scripts
  • chore: Other changes that don't modify src or test files
  • revert: Reverts a previous commit
  • BREAKING CHANGE: Introduces a breaking change (can be combined with any type above)

Checklist

  • Code follows the style guidelines of this project
  • Code has been self-reviewed
  • Code has been commented, particularly in hard-to-understand areas
  • Code docstring/documentation-blocks for new or existing methods/components have been added or updated
  • Unit tests have been added or updated for any new or modified functionality

AI Usage

  • None: No AI tools were used in creating this PR
  • Light: AI provided minor assistance (formatting, simple suggestions)
  • Moderate: AI helped with code generation or debugging specific parts
  • Heavy: AI generated most or all of the code changes

@Kishi85
Copy link
Copy Markdown
Contributor Author

Kishi85 commented Apr 26, 2026

This should also serve as a potential workaround for #221 for xdgportal by setting output_name to the desired primary display connector name.

@psyke83 Do you think we should also push the output_name from config to the top of the display_name list in that case (so it's always on CTRL+ALT+SHIFT+F1) or should we just let video.cpp select the display by configured name and leave the display name order (and therefore keyboard shortcuts) by position as it is currently?

@Kishi85 Kishi85 force-pushed the update-portalgrab-display-matching branch from 1266efd to 4d32cb1 Compare April 26, 2026 13:12
@psyke83
Copy link
Copy Markdown
Contributor

psyke83 commented Apr 26, 2026

I think that leaving the order by position seems logical but I really don't have a strong preference.

This simplifies the display name matching logic and also allows for
wl_output names to be used directly for config parameter output_name to
select the initial display.
@ReenigneArcher ReenigneArcher force-pushed the update-portalgrab-display-matching branch from 4d32cb1 to 306f146 Compare April 27, 2026 04:08
@ReenigneArcher ReenigneArcher added this to the xdg portal grab milestone Apr 27, 2026
@sonarqubecloud
Copy link
Copy Markdown

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 27, 2026

Codecov Report

❌ Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 18.16%. Comparing base (a458b20) to head (306f146).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/platform/linux/portalgrab.cpp 0.00% 6 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           master    #5053    +/-   ##
========================================
  Coverage   18.15%   18.16%            
========================================
  Files         109      109            
  Lines       23582    23564    -18     
  Branches    10406    10396    -10     
========================================
- Hits         4282     4280     -2     
- Misses      16046    16856   +810     
+ Partials     3254     2428   -826     
Flag Coverage Δ
Archlinux 11.53% <0.00%> (+0.01%) ⬆️
FreeBSD-14.3-aarch64 ?
FreeBSD-14.3-amd64 13.77% <0.00%> (+0.01%) ⬆️
Homebrew-ubuntu-22.04 13.91% <0.00%> (+0.01%) ⬆️
Linux-AppImage 12.49% <0.00%> (+0.01%) ⬆️
Windows-AMD64 14.86% <ø> (ø)
Windows-ARM64 13.25% <ø> (+<0.01%) ⬆️
macOS-arm64 19.07% <ø> (ø)
macOS-x86_64 18.42% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/platform/linux/portalgrab.cpp 18.57% <0.00%> (+0.73%) ⬆️

... and 51 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a458b20...306f146. Read the comment docs.

@Kishi85
Copy link
Copy Markdown
Contributor Author

Kishi85 commented Apr 27, 2026

I'll leave the current behaviour as it is then (sorting key order by position). This should be good to go then unless anyone finds an issue with it.

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 27, 2026

Bundle Report

Bundle size has no change ✅

@ReenigneArcher ReenigneArcher merged commit 2f00cb3 into LizardByte:master Apr 27, 2026
124 of 127 checks passed
@RevengeRip RevengeRip mentioned this pull request Apr 27, 2026
2 tasks
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.

3 participants