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

Support multi-monitor setups #49

Merged
merged 6 commits into from
Sep 4, 2019
Merged

Conversation

ebeem
Copy link
Collaborator

@ebeem ebeem commented Aug 24, 2019

related to #21 and #46

this was discussed before, but due to the many changes that have been implemented since the previous PR, it needed to be done again. the problems that were listed before are

  • Implement a better way to get thumbnails from monitors other than the primary. Current method seems to work fine, but it's probably going to cause conflicts with some extensions or cause some bugs at some point (best solution is to have GNOME support getting thumbnails from a specific monitor and workspace).
  • Improve the performance, the time needed to display the switchers is almost N times the time needed to display a single switcher, where n is the number of monitors.
  • Sync Switchers hide animation/destroy. Sometimes, a little delay in hiding switchers occurs, that's because each switcher has its own timeout in the current implementation (probably better be shared).

regarding point#1, I tried my best to keep the code as simple as possible. making GNOME properly show thumbnails of workspaces is easy, but it will require more rewriting of methods from the class WorkspaceThumbnail.

I have been trying this for days, and it seems fine. However, I will feel more comfortable if anyone who is interested can test it as well.

@mzur
Copy link
Owner

mzur commented Aug 26, 2019

Current method seems to work fine, but it's probably going to cause conflicts with some extensions or cause some bugs at some point (best solution is to have GNOME support getting thumbnails from a specific monitor and workspace).

Do you mean the workaround with Main.layoutManager.primaryIndex in the WorkspaceThumbnail class or anything else?

I'll see if I can test this. I only have a VM for the newer versions of GNOME and I don't know if I can set it up for multiple monitors.

@ebeem
Copy link
Collaborator Author

ebeem commented Aug 26, 2019

yes, it's this part about changing the primary index and then restoring it after getting the thumbnail

@mzur
Copy link
Owner

mzur commented Aug 27, 2019

I found that it's super easy to emulate multiple monitors with a VM. I'll have a look at this in the next few days.

Copy link
Owner

@mzur mzur left a comment

Choose a reason for hiding this comment

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

I applied some consmetic changes that I liked better. Otherwise this looks good! I think the current workaround with the monitor index is fine, too. I don't believe that we could convince the GNOME developers to implement this natively. Even if they would, we would need to wait for the next GNOME release. I saw that there are some bigger updates to workspaces in the next GNOME version so we need to update this extension for the next GNOME release anyway. You can merge this whenever you're ready.

@ebeem
Copy link
Collaborator Author

ebeem commented Aug 29, 2019

Great job!! thanks.
regarding the GNOME 3.34 update, I have tested the extension under GNOME 3.33.91 and it's working fine until now.

@mzur
Copy link
Owner

mzur commented Sep 4, 2019

@ebeem Will you merge this? Are you even able to? 😄 I don't know what permissions you have got as collaborator in this repo.

@ebeem ebeem merged commit b395025 into mzur:master Sep 4, 2019
@ebeem
Copy link
Collaborator Author

ebeem commented Sep 4, 2019

yes, I was able to merge 😆

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