EDID matching for output stack - #1325
Merged
Merged
Conversation
Member
Author
|
I guess in truncation it can truncate entries until the one that matches the edid, but still add a new entry if the connector is different. The |
ids1024
force-pushed
the
workspace-output-stack-edid_noble
branch
from
March 28, 2025 22:07
4b8a06d to
02cf8dc
Compare
This is the same as `libdisplay_info::edid::VendorProduct`, but with implementations for `Serialize`, `Eq`, etc.
If the user explicitly moves a workspace to an output, assume that is where the user wants it, so it shouldn't be moved back to a different output in the future. For persistent workspaces, the explicitly set workspace is the one that will be stored, instead of trying to track an unbounded list of outputs persistently.
Matches by edid where present, or by connector name if there is no edid information.
If two displays have the same edid (which shouldn't happen, since edid includes a serial number, but is somewhat common in practice with identical monitors), match output stack by comparing both edid and connector name. If we get the same edid but a different connector, `set_output` truncates but also adds the new one. (Before adding edid matching, this would have just added to the output stack.) `prefers_output()` will requrire a connector name match if the edid of the current output is the same as that of the output being compared.
ids1024
force-pushed
the
workspace-output-stack-edid_noble
branch
from
April 1, 2025 20:48
02cf8dc to
ae60523
Compare
ids1024
marked this pull request as ready for review
April 1, 2025 22:18
Member
Author
|
This should now disambiguate properly if two connected outputs have the same EDID. The last commit should be a fairly simple way to do that. |
Drakulix
approved these changes
Apr 2, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
EDID matching in #1252, but for output stacks.
It seems better to add here first (where it isn't persisted) and overhaul the on-disk format for output configuration along with pinned workspace persisting, which will also need this.
For persistent workspaces, I think it makes sense not to persist the whole output stack, but only the last output a workspace was explicitly placed on (where it was created, or moved to by the user). Though there may be edge cases where this is undesirable and confusing. (Which is also a potential problem for any other alternative.)
So when a workspace is moved explictly by the user, the output stack is cleared. And the earliest output in the stack is where it was placed explictly.
Getting the behaviors right when multiple displays have the same edid information is harder.
I had the idea that
Workspaces::add_outputshould check if the edid matches an existing output on a different connector, then try to move workspaces that were explictly assigned to that output with that connector, though the way.matches()is used for truncating the output stack will need to change too...