Skip to content

Commit 1db9292

Browse files
authored
Replace ClickOverlay enterMonitor reactive actor approach with much simpler implementation (#751)
@Lythenas, @Thesola10, I've been thinking about this one for a while: PaperWM uses a very curious approach to implement a "one space per monitor" paradigm (see [my response](peterfajdiga/karousel#3 (comment)) to a question of how PaperWM does this). Unfortunately this approach is quite complex and exhibits many side-effects that have required many workarounds (which often causes further issues). This PR replaces this approach with a simpler `PointerWatcher` implementation (a Gnome supported implementation to monitor mouse pointer movements). Moving to this approach provides the following advantages: - switch to a Gnome supported feature to detect when mouse pointer moves to another monitor (instead of an unsupported, custom, error-prone implementation); - significantly simplifies the codebase for PaperWM's multimonitor support; - replaces around ~164 lines of (now) unneeded code with ~31 lines; - removes a complex part of multimonitor support and would allow us to add simple behaviours (based on user preference) like #389; _Note: we actually already use the `PointerWatcher` approach for multi-monitor drag/drop support - so this PR is essentially removing the other approach and adding a "activate space" method to the `PointerWatcher`._ @Lythenas and @Thesola10 - could you give this PR branch a test? I'll set it as a draft atm given it's quite a large change and I'm sure there might be some side-effects from this. @Thesola10 - I note that this may impact some of your touch signals, although, we no longer have the ClickOverlay element blocking actions so it may turn out better (just a hypothesis).
2 parents 71c9a03 + a99ce0d commit 1db9292

File tree

5 files changed

+41
-174
lines changed

5 files changed

+41
-174
lines changed

grab.js

-4
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,6 @@ export class MoveGrab {
5353
this.dispatcher = new Navigator.getActionDispatcher(Clutter.GrabState.POINTER);
5454
this.actor = this.dispatcher.actor;
5555

56-
for (let [monitor, $] of Tiling.spaces.monitors) {
57-
monitor.clickOverlay.deactivate();
58-
}
59-
6056
let metaWindow = this.window;
6157
let actor = metaWindow.get_compositor_private();
6258
let clone = metaWindow.clone;

navigator.js

-3
Original file line numberDiff line numberDiff line change
@@ -356,9 +356,6 @@ class NavigatorClass {
356356
let visible = [];
357357
for (let monitor of Main.layoutManager.monitors) {
358358
visible.push(Tiling.spaces.monitors.get(monitor));
359-
if (monitor === this.monitor)
360-
continue;
361-
monitor.clickOverlay.activate();
362359
}
363360

364361
if (!visible.includes(space) && this.monitor !== this.space.monitor) {

scratch.js

+1-3
Original file line numberDiff line numberDiff line change
@@ -134,9 +134,7 @@ export function makeScratch(metaWindow) {
134134
}
135135
}
136136

137-
let monitor = Tiling.focusMonitor();
138-
if (monitor.clickOverlay)
139-
monitor.clickOverlay.hide();
137+
Tiling.focusMonitor()?.clickOverlay?.hide();
140138
}
141139

142140
export function unmakeScratch(metaWindow) {

stackoverlay.js

+39-153
Original file line numberDiff line numberDiff line change
@@ -43,43 +43,69 @@ import { Settings, Utils, Tiling, Navigator, Grab, Scratch } from './imports.js'
4343
restack loops)
4444
*/
4545

46-
let pointerWatch;
46+
let pointerWatch, lastSpace;
4747
export function enable(extension) {
4848

4949
}
5050

5151
export function disable() {
52-
disableMultimonitorDragDropSupport();
52+
disableMultimonitorSupport();
53+
lastSpace = null;
5354
}
5455

5556
/**
5657
* Checks for multiple monitors and if so, then enables multimonitor
57-
* drag/drop support in PaperWM.
58+
* support in PaperWM.
5859
*/
59-
export function multimonitorDragDropSupport() {
60+
export function multimonitorSupport() {
6061
// if only one monitor, return
6162
if (Tiling.spaces.monitors?.size > 1) {
62-
enableMultimonitorDragDropSupport();
63+
enableMultimonitorSupport();
6364
}
6465
else {
65-
disableMultimonitorDragDropSupport();
66+
disableMultimonitorSupport();
6667
}
6768
}
6869

69-
export function enableMultimonitorDragDropSupport() {
70+
export function enableMultimonitorSupport() {
7071
pointerWatch = PointerWatcher.getPointerWatcher().addWatch(100,
7172
() => {
72-
Tiling.spaces?.clickOverlays?.forEach(c => {
73-
c.monitorActiveCheck();
74-
});
73+
const monitor = Utils.monitorAtCurrentPoint();
74+
const space = Tiling.spaces.monitors.get(monitor);
75+
76+
// same space
77+
if (space === lastSpace) {
78+
return;
79+
}
80+
// update to space
81+
lastSpace = space;
82+
83+
// check if in the midst of a window resize action
84+
if (Tiling.inGrab &&
85+
Tiling.inGrab instanceof Grab.ResizeGrab) {
86+
const window = global.display?.focus_window;
87+
if (window) {
88+
Scratch.makeScratch(window);
89+
}
90+
return;
91+
}
92+
93+
// if drag/grabbing window, do simple activate
94+
if (Tiling.inGrab) {
95+
space?.activate(false, false);
96+
return;
97+
}
98+
99+
const selected = space?.selectedWindow;
100+
space?.activateWithFocus(selected, false, false);
75101
});
76-
console.debug('paperwm multimonitor drag/drop support is ENABLED');
102+
console.debug('paperwm multimonitor support is ENABLED');
77103
}
78104

79-
export function disableMultimonitorDragDropSupport() {
105+
export function disableMultimonitorSupport() {
80106
pointerWatch?.remove();
81107
pointerWatch = null;
82-
console.debug('paperwm multimonitor drag/drop support is DISABLED');
108+
console.debug('paperwm multimonitor support is DISABLED');
83109
}
84110

85111
export function createAppIcon(metaWindow, size) {
@@ -102,139 +128,6 @@ export class ClickOverlay {
102128
this.onlyOnPrimary = onlyOnPrimary;
103129
this.left = new StackOverlay(Meta.MotionDirection.LEFT, monitor);
104130
this.right = new StackOverlay(Meta.MotionDirection.RIGHT, monitor);
105-
106-
let enterMonitor = new Clutter.Actor({ reactive: true });
107-
this.enterMonitor = enterMonitor;
108-
enterMonitor.set_position(monitor.x, monitor.y);
109-
110-
// Uncomment to debug the overlays
111-
// enterMonitor.background_color = Clutter.color_from_string('green')[1];
112-
// enterMonitor.opacity = 100;
113-
114-
Main.uiGroup.add_actor(enterMonitor);
115-
Main.layoutManager.trackChrome(enterMonitor);
116-
117-
this.signals = new Utils.Signals();
118-
119-
this._lastPointer = [];
120-
this._lastPointerTimeout = null;
121-
122-
this.signals.connect(enterMonitor, 'touch-event', () => {
123-
if (Tiling.inPreview)
124-
return;
125-
this.select();
126-
});
127-
128-
this.signals.connect(enterMonitor, 'enter-event', () => {
129-
if (Tiling.inPreview)
130-
return;
131-
this.select();
132-
});
133-
134-
this.signals.connect(enterMonitor, 'button-press-event', () => {
135-
if (Tiling.inPreview)
136-
return;
137-
this.select();
138-
return Clutter.EVENT_STOP;
139-
});
140-
141-
this.signals.connect(Main.overview, 'showing', () => {
142-
this.deactivate();
143-
this.hide();
144-
});
145-
146-
this.signals.connect(Main.overview, 'hidden', () => {
147-
this.activate();
148-
this.show();
149-
});
150-
151-
/**
152-
* Handle grabbed (drag & drop) windows in ClickOverlay. If a window is
153-
* grabbed-dragged-and-dropped on a monitor, then select() on this ClickOverlay
154-
* (which deactivates ClickOverlay and immediately activates/selects the dropped window.
155-
*/
156-
this.signals.connect(global.display, 'grab-op-end', (display, mw, type) => {
157-
if (this.monitor === this.mouseMonitor) {
158-
this.select();
159-
}
160-
});
161-
}
162-
163-
/**
164-
* Returns the space of this ClickOverlay instance.
165-
*/
166-
get space() {
167-
return Tiling.spaces.monitors.get(this.monitor);
168-
}
169-
170-
/**
171-
* Returns the monitor the mouse is currently on.
172-
*/
173-
get mouseMonitor() {
174-
return Utils.monitorAtCurrentPoint();
175-
}
176-
177-
monitorActiveCheck() {
178-
// if clickoverlay not active (space is already selected), then nothing to do
179-
if (!this.active) {
180-
return;
181-
}
182-
183-
if (Main.overview.visible || Tiling.inPreview) {
184-
return;
185-
}
186-
187-
// if mouse on me, select
188-
if (this.monitor === this.mouseMonitor) {
189-
this.select();
190-
}
191-
}
192-
193-
select() {
194-
// if clickoverlay not active (space is already selected), then nothing to do
195-
if (!this.active) {
196-
return;
197-
}
198-
199-
// check if in the midst of a window resize action
200-
if (Tiling.inGrab && Tiling.inGrab instanceof Grab.ResizeGrab) {
201-
const window = global.display?.focus_window;
202-
if (window) {
203-
Scratch.makeScratch(window);
204-
}
205-
}
206-
207-
/**
208-
* stop navigation before activating workspace. Avoids an issue
209-
* in multimonitors where workspaces can get snapped to another monitor.
210-
*/
211-
Navigator.finishDispatching();
212-
Navigator.finishNavigation(true);
213-
this.deactivate();
214-
let selected = this.space.selectedWindow;
215-
this.space.activateWithFocus(selected, false, false);
216-
}
217-
218-
activate() {
219-
if (this.onlyOnPrimary || Main.overview.visible)
220-
return;
221-
222-
let spaces = Tiling.spaces;
223-
let active = global.workspace_manager.get_active_workspace();
224-
let monitor = this.monitor;
225-
// Never activate the clickoverlay of the active monitor
226-
if (spaces && spaces.monitors.get(monitor) === spaces.get(active))
227-
return;
228-
229-
this.active = true;
230-
this.space?.setSelectionInactive();
231-
this.enterMonitor.set_position(monitor.x, monitor.y);
232-
this.enterMonitor.set_size(monitor.width, monitor.height);
233-
}
234-
235-
deactivate() {
236-
this.active = false;
237-
this.enterMonitor.set_size(0, 0);
238131
}
239132

240133
reset() {
@@ -255,10 +148,6 @@ export class ClickOverlay {
255148
}
256149

257150
destroy() {
258-
Utils.timeout_remove(this._lastPointerTimeout);
259-
this._lastPointerTimeout = null;
260-
this.signals.destroy();
261-
this.signals = null;
262151
for (let overlay of [this.left, this.right]) {
263152
let actor = overlay.overlay;
264153
overlay.signals.destroy();
@@ -269,9 +158,6 @@ export class ClickOverlay {
269158
}
270159
actor.destroy();
271160
}
272-
273-
Main.layoutManager.untrackChrome(this.enterMonitor);
274-
this.enterMonitor.destroy();
275161
}
276162
}
277163

tiling.js

+1-11
Original file line numberDiff line numberDiff line change
@@ -1955,7 +1955,6 @@ export const Spaces = class Spaces extends Map {
19551955
for (let monitor of monitors) {
19561956
let overlay = new ClickOverlay(monitor, this.onlyOnPrimary);
19571957
monitor.clickOverlay = overlay;
1958-
overlay.activate();
19591958
this.clickOverlays.push(overlay);
19601959
}
19611960

@@ -1977,10 +1976,9 @@ export const Spaces = class Spaces extends Map {
19771976
});
19781977

19791978
this.spaceContainer.show();
1980-
activeSpace.monitor.clickOverlay.deactivate();
19811979
Topbar.refreshWorkspaceIndicator();
19821980
this.setSpaceTopbarElementsVisible();
1983-
Stackoverlay.multimonitorDragDropSupport();
1981+
Stackoverlay.multimonitorSupport();
19841982
};
19851983

19861984
if (this.onlyOnPrimary) {
@@ -2295,14 +2293,6 @@ export const Spaces = class Spaces extends Map {
22952293
fromSpace,
22962294
doAnimate);
22972295

2298-
toSpace.monitor?.clickOverlay.deactivate();
2299-
2300-
for (let monitor of Main.layoutManager.monitors) {
2301-
if (monitor === toSpace.monitor)
2302-
continue;
2303-
monitor.clickOverlay.activate();
2304-
}
2305-
23062296
// Update panel to handle target workspace
23072297
signals.disconnect(Main.panel, this.touchSignal);
23082298
this.touchSignal = signals.connect(Main.panel, "captured-event", Gestures.horizontalTouchScroll.bind(toSpace));

0 commit comments

Comments
 (0)