Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update MonitorArrangement widget to match parameter settings after screen configuration change #1405

Merged
merged 1 commit into from Jan 10, 2022

Conversation

samhed
Copy link
Member

@samhed samhed commented Jan 7, 2022

If the options dialog was open when a screen configuration happened the widget could get out of sync from the settings. A scenario when this happened was:

  1. 3 monitors, fullscreen selected on the two right-most screens
  2. disconnect the left-most screen (the one not selected)

In this case, using GNOME, vncviewer would appear in fullscreen on the right of the two remaining monitors, but the widget would show both monitors selected. The reason was that the MonitorArragement index doesn't work the same way as FLTK's screen index.

It's debatable how vncviewer should behave here, but the GUI should at least match the actual setting.

@samhed samhed added the bug Something isn't working label Jan 7, 2022
@samhed
Copy link
Member Author

samhed commented Jan 7, 2022

Tested on Fedora 35, macOS 12.1, and Windows 10.

@CendioOssman
Copy link
Member

Did you look at how this would race with the event handler in the monitor widget?

@samhed
Copy link
Member Author

samhed commented Jan 10, 2022

I had considered it, but now that I look closer I see that we're not safe here. That event handler could cause problems if it fires after the handler in this PR since the map monitors isn't cleared in the new handler.

I see two options:

  • making refresh() of the MonitorArrangement class public
  • removing the handler from MonitorArrangement and making this stuff the responsibility of the implementor

@CendioOssman
Copy link
Member

  • Use an idle handler/timeout to make sure this is run after every other handler is done

If the options dialog was open when a screen configuration happened the
widget could get out of sync from the settings. A scenario when this
happened was:

 1) 3 monitors, fullscreen selected on the two right-most screens
 2) disconnect the left-most screen (the one not selected)

In this case, using GNOME, vncviewer would appear in fullscreen on the
right of the two remaining monitors, but the widget would show both
monitors selected. The reason was that the MonitorArragement index
doesn't work the same way as FLTK's screen index.

It's debatable how vncviewer should behave here, but the GUI should at
least match the actual setting.
@samhed
Copy link
Member Author

samhed commented Jan 10, 2022

Good point regarding the timeout. Updated the commit.

I had some trouble testing this, but looking at the code at https://github.com/fltk/fltk/blob/branch-1.3/src/Fl.cxx indicates that timeouts with time set to zero should work as expected.

I verified that things still work on Windows 11 and Fedora 35.

@samhed samhed merged commit eee6638 into master Jan 10, 2022
@samhed samhed deleted the monitorArrangementSync branch January 10, 2022 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants