Skip to content

Don't make a seperate call to disable outputs#20

Closed
RasmusWL wants to merge 1 commit intophillipberndt:masterfrom
RasmusWL:master
Closed

Don't make a seperate call to disable outputs#20
RasmusWL wants to merge 1 commit intophillipberndt:masterfrom
RasmusWL:master

Conversation

@RasmusWL
Copy link
Contributor

@RasmusWL RasmusWL commented Mar 5, 2015

Having no enabled outputs crashed my WM, so I have grouped the
first enable xrandr call with the disable call

Having no enabled outputs crashed my WM, so I have grouped the
first enable xrandr call with the disable call
Copy link
Owner

Choose a reason for hiding this comment

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

This overrides import copy from above, so ll copy.deepcopy calls below won't work anymore. Please use copy.copy instead.

@phillipberndt
Copy link
Owner

I am not sure how this will interact with wertarbyte/autorandr#18 and tachylatus/autorandr@83355f4. @tachylatus, are you still affected by this problem / can you test if the approach from this pull request works for you?

If it doesn't, we could first disable all but one of the unused outputs, then enable the first used output (that's the one at 0x0) in the same call that disables the last one, and then enable the remainder in pairs of two. This would require us to change the logic of add_unused_outputs such that outputs that are connected, unused and continue to be unused in the profile that is to be loaded are removed entirely from the configuration. (Otherwise we could still end up with all outputs being disabled at some point.)

@RasmusWL
Copy link
Contributor Author

RasmusWL commented Mar 6, 2015

Okay, so to sum up, there are problems when:

  1. At some point no output is enabled (this pull request) [WM problem]
  2. Making changes to more than two outputs at the same time (change each output separately wertarbyte/autorandr#18) [Driver problem]
  3. Exceeding maximum number of outputs (tachylatus/autorandr@83355f4) [xrandr problem?]

Solution could be:

  • Enable one output and disable one output in the first call to xrandr
  • Disable rest of unused outputs in pairwise calls to xrandr
  • Enable rest of outputs in pairwise calls to xrandr

@phillipberndt
Copy link
Owner

There's a fourth point to it: The first screen to be enabled must be the one at 0x0, because xrandr will move the first screen to that position no matter what. (autorandr.py already sorts that screen to the beginning of the list.)

phillipberndt added a commit that referenced this pull request Mar 7, 2015
@phillipberndt
Copy link
Owner

Here's an implementation of what I've suggested above:
0bb7e07

I don't know if this works yet.

@RasmusWL
Copy link
Contributor Author

RasmusWL commented Mar 8, 2015

Yes, this works for me. But there is an edge case where things go wrong. Lets say 4 outputs are enabled, and you want to disable them all and enable an other one.

In the normal case, the first call to xrandr will disable 3 of the outputs, and a second call will be made to disable the last output while enabling the new output.

However, if the first call to xrandr fails (because the driver can only work on max 2 outputs at a time), then you group all 5 outputs changes in pairs of two, meaning the second call will disable 2 outputs, the next call will disable 2 outputs, and then your WM will crash because there are no enabled outputs 😞

phillipberndt added a commit that referenced this pull request Mar 8, 2015
@phillipberndt
Copy link
Owner

Right, thanks. (Though the only reported case of problems with changing multiple screens in one call concerns enabling screens, not disabling them.) 29cd564 should fix that.

@phillipberndt
Copy link
Owner

Apparently no one else is interested in this right now. Shall I merge my version, or do you prefer a different solution?

@RasmusWL
Copy link
Contributor Author

No just merge your version, I think that should be fine :)

@phillipberndt
Copy link
Owner

Very well.

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