Skip to content

Conversation

ids1024
Copy link
Member

@ids1024 ids1024 commented Mar 26, 2025

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.

  • We want to ignore connectors, and match by edid information when possible
  • But if two monitors with same edid information are connected at the same time, they should be distinguished

I had the idea that Workspaces::add_output should 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...

@ids1024
Copy link
Member Author

ids1024 commented Mar 26, 2025

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 PATH property should help with consistent connector names for mst displays, though I'm not sure exactly how consistent it is in practice. (I wonder if we could just change connector names to use the parent connector, then an additional dash and number...)

Copy link
Member

@Drakulix Drakulix left a comment

Choose a reason for hiding this comment

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

Seems good to me.

@ids1024 ids1024 force-pushed the workspace-output-stack-edid_noble branch from 4b8a06d to 02cf8dc Compare March 28, 2025 22:07
ids1024 added 4 commits April 1, 2025 11:19
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 ids1024 force-pushed the workspace-output-stack-edid_noble branch from 02cf8dc to ae60523 Compare April 1, 2025 20:48
@ids1024 ids1024 marked this pull request as ready for review April 1, 2025 22:18
@ids1024
Copy link
Member Author

ids1024 commented Apr 1, 2025

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.

@ids1024 ids1024 requested a review from a team April 1, 2025 22:19
@Drakulix Drakulix merged commit 0c95871 into master Apr 2, 2025
6 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