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

EdgeScroll: implement per monitor #916

Merged
merged 3 commits into from
Oct 22, 2023
Merged

Conversation

ThomasAdam
Copy link
Member

Bring the EdgeScroll command inline with other command, such as
EwmhBaseStruts whereby the dimensions can be specified per-monitor, such
as:

EdgeScroll 0 0
EdgeScroll screen DP-1 100 100

@ThomasAdam ThomasAdam added this to the 1.0.9 milestone Oct 20, 2023
@ThomasAdam ThomasAdam self-assigned this Oct 20, 2023
@somiaj
Copy link
Collaborator

somiaj commented Oct 21, 2023

Tested this out and it works as described. I am able to control EdgeScroll on a per monitor basis.

It would also be nice to have a way to turn panframes on/off on a per-monitor basis as well. I use EdgeThickness 0 and EdgeThickness 2 to turn them off/on. One thing I noticed is that just setting EdgeScroll screen DP-1 0 0 disables EdgeScroll on that monitor, but I can still move windows over the window boundary, as opposed to EdgeThickness 0 disables the panframes and I cannot scroll or move windows over page boundaries.

@ThomasAdam
Copy link
Member Author

One thing I noticed is that just setting EdgeScroll screen DP-1 0 0 disables EdgeScroll on that monitor, but I can still move windows over the window boundary, as opposed to EdgeThickness 0 disables the panframes and I cannot scroll or move windows over page boundaries.

Yeah -- OK, that's a good point. Because the panframes still exist with setting EdgeScroll 0 0, I'll have to make EdgeThickness also per-monitor aware.

I'll add that in with this PR.

Note that the semantics of all of this is getting weird, with EdgeResistance, and EdgeMoveDelay -- which is a Style option. But never mind, that's not something I want to address.

So I think what I'm suggesting is to achieve what you want, it is going to have to be a combination of a few different commands, but you can always put them in a function, perhaps passing in the monitor name to toggle these things on/off.

@ThomasAdam
Copy link
Member Author

ThomasAdam commented Oct 21, 2023

Right, this PR now allows for:

EdgeThickness screen DP-1 0
EdgeScroll screen DP-1 0 0

Which will completely disable the ability to switch pages and drag windows on the specified monitor. Not specifying the screen XXX option to those commands still works in the global way it currently does.

Please do test it, and let me know how you get on.

I'll update the man page bits in due course.

@somiaj
Copy link
Collaborator

somiaj commented Oct 21, 2023

I notice a few issues with this, first EdgeThickness is broken if no screen is supplied, and second EdgeThickness screeen DP-1 only seems to apply to my primary monitor, no matter what identifier I use for the screen.

@somiaj
Copy link
Collaborator

somiaj commented Oct 21, 2023

Actually EdgeThickness without a screen setting is working, it is just only applying to the primary monitor, and not my secondary one.

@ThomasAdam
Copy link
Member Author

Actually EdgeThickness without a screen setting is working, it is just only applying to the primary monitor, and not my secondary one.

Huh. Yeah, OK. Fix incoming...

@ThomasAdam
Copy link
Member Author

Huh. Yeah, OK. Fix incoming...

Try that, when you get a mo...

@somiaj
Copy link
Collaborator

somiaj commented Oct 21, 2023

There is still some oddness going on, but harder to describe. In short it starts out working just fine, but after toggling EdgeThickness 0 and EdgeThickness 2 a few times, the pan frames seem to just vanish and all scrolling stops even with EdgeThickness 2 set, and sometimes only one monitor is affected, sometimes only one panframe (so I can scroll up but not down).

@ThomasAdam
Copy link
Member Author

There is still some oddness going on, but harder to describe. In short it starts out working just fine, but after toggling EdgeThickness 0 and EdgeThickness 2 a few times, the pan frames seem to just vanish and all scrolling stops even with EdgeThickness 2 set, and sometimes only one monitor is affected, sometimes only one panframe (so I can scroll up but not down).

Odd -- thanks for checking. I'll add some logging to see where it's getting stuck.

@somiaj
Copy link
Collaborator

somiaj commented Oct 21, 2023

Some more tests, I run EdgeThickness screen DP-4 0, then EdgeThickness screen DP-4 2, then EdgeThickness screen DP-4 0, at this moment the bottom panframe is removed but the top panframe is still there. I then scroll up using the panframe, but at that point all panframes are gone and I can't get them back with EdgeThickness screen DP-4 2.

@somiaj
Copy link
Collaborator

somiaj commented Oct 21, 2023

For what it is worth, I have two monitors with same resolution positioned left/right and the following setup, so I can scroll up and down on each monitor.

# Desktop Setup
DesktopConfiguration per-monitor
DesktopSize 1x3

@ThomasAdam
Copy link
Member Author

ThomasAdam commented Oct 21, 2023

Thanks, @somiaj

I think this might have been due to a former check which tried to be clever about how to map/unmap the pan frames. Now that Edge{Scroll,Thickness} can always change the state of these, it's not too expensive to recreate them on-the-fly.

So I've removed this check, just to see if that makes a difference. From what I can tell here, it seems to.

@somiaj
Copy link
Collaborator

somiaj commented Oct 21, 2023

My initial tests seem to confirm it is now working, I will use this for a few days to see if I have any more issues.

@somiaj
Copy link
Collaborator

somiaj commented Oct 21, 2023

Again so far, so good. Now scroll lock can toggle scrolling on a per monitor basis.

DestroyFunc ToggleScroll
AddToFunc ToggleScroll
+ I Test (EnvMatch infostore.DisableScroll$[monitor.current] True) SetEdgeThickness 2 False
+ I TestRc (NoMatch) SetEdgeThickness 0 True

DestroyFunc SetEdgeThickness
AddToFunc SetEdgeThickness
+ I EdgeThickness screen $[monitor.current] $0
+ I InfoStoreAdd DisableScroll$[monitor.current] $1

Key Pause A A ToggleScroll

Copy link
Collaborator

@somiaj somiaj left a comment

Choose a reason for hiding this comment

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

Been using this for a little bit now, and it seems to work. I say it is good to go (though maybe rename the WIP in the commit name).

Bring the EdgeScroll command inline with other command, such as
EwmhBaseStruts whereby the dimensions can be specified per-monitor, such
as:

    EdgeScroll 0 0
    EdgeScroll screen DP-1 100 100
Rather than having a global value to represent the state of
edgethickness, make this a part of the monitor struct, so it can be
changed per-monitor.
This check was an optimisation, but now that Edge{Scroll,Thickness} can
change this, we should let the commands figure out for themselves when
to map/unmap.
@ThomasAdam
Copy link
Member Author

Thanks, @somiaj -- merging this now to main -- let me know if you experience any other issues.

@ThomasAdam ThomasAdam merged commit 058b6ab into main Oct 22, 2023
5 checks passed
@ThomasAdam ThomasAdam deleted the ta/edgescroll-per-monitor branch October 22, 2023 21:42
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