search

fvwmorg / fvwm3

FVWM version 3 -- the successor to fvwm2
Other
488 stars 75 forks source link

FvwmEvent event new_desk gets triggered multiple times in multi-monitor shared setup #655

Closed NsCDE closed 1 year ago

NsCDE commented 2 years ago

fvwm3 1.0.5 (1.0.4-116-g8342d9dc) with support for: ReadLine, XPM, PNG, SVG, Shape, XShm, SM, Bidi text, XRandR, XRender, XCursor, XFT, NLS

Debian 11

Linux x86_64

Expected Behaviour

In four monitor setup, and DesktopConfiguration is set to "shared", there should be only one trigger for the new_desk event. However ...

Actual Behaviour

... new_desk event is triggered 7-9 times. Looks like as fast as it can until desk is changed. Reducing number of monitors to two, also reduces this triggering to 2-4.

Enabling logging

Function which is defined for new_desk event - if we put some Echo at the beginning (2 monitors):

[1645631581.505294] CMD_Echo: DESK IS: 2 [1645631581.615735] CMD_Echo: DESK IS: 2 [1645631581.633214] CMD_Echo: DESK IS: 2 [1645631581.654741] CMD_Echo: DESK IS: 2 [1645631583.051600] CMD_Echo: DESK IS: 3 [1645631583.087933] CMD_Echo: DESK IS: 3

Steps to Reproduce

N/A

Does Fvwm3 crash?

No.

Extra Information

This weird event behaviour may be similar to disfuctional monitor_focus in #604

ThomasAdam commented 1 year ago

Hi @NsCDE

Please can you try the ta/fix-655 branch to see if this resolves the reported problem?

Thanks!

NsCDE commented 1 year ago

Hi Thomas,

I'm back from vacation and had a bunch of work to do. I see you merged this into the master, so I just compiled master and tried.

I'm afraid I have to tell you it is still triggering multiple times. That is, NsCDE function f_ChangeDesk called from new_desk produces multiple "CMD_Echo: DESK IS: X" if I put "Echo DESK IS $[desk.n]" in that function.

P. S. I saw you produced couple more fixes, I will test this when I catch some free time.

NsCDE commented 1 year ago

Fvwm Event conf: https://github.com/NsCDE/NsCDE/blob/master/data/fvwm/Event.fvwmconf

Function of new_desk (About line 1546 f_ChangeDesk) https://github.com/NsCDE/NsCDE/blob/master/data/fvwm/Functions.fvwmconf.in First line of function in my tests was added: "+ I Echo DESK IS: $[desk.n]"

ThomasAdam commented 1 year ago

Hi @NsCDE

Hopefully this is working correctly now. I think I've made this function the way it should with 'shared', and also fixed the same type of problem with 'global' wherein, that would cause N number of NEW_DESK events per N monitors detected.

Try the ta/fix-655 branch, and let me know how you get on.

NsCDE commented 1 year ago

Sorry to tell you, but it is still hitting 3 times:

[1663437479.245486] CMD_Echo: DESK IS: 2 [1663437479.321148] CMD_Echo: DESK IS: 2 [1663437479.338521] CMD_Echo: DESK IS: 2

But only when it needs to move/flip desks between monitors, that is, when I choose on one monitor desk that is active on another monitor. For desks which are invisible (2 monitors, 4 desks) this is called only once.

ThomasAdam commented 1 year ago

Hmm.

Here's what I've been using to test this with. Don't worry so much about the bogus values which get printed:

DestroyModuleConfig FET: *
*FET: PassID
*FET: Cmd
*FET: new_desk fnd

DestroyFunc fnd
AddToFunc   fnd
+ I Echo "[$0 - $1]: MONITOR: $[w.screen]: DESK: $[w.desk]"

DesktopConfiguration shared
KillModule FvwmEvent FET
Module FvwmEvent FET

For me, I get the expected number of events.

ThomasAdam commented 1 year ago

As seen here:

[1663438508.324695] CMD_GotoDesk: Swapping HDMI-1/1 with DP-1-2/0
[1663438508.324993] MapDesk: '[0x2e0001e] xterm (3)' is on [DP-1-2 (0)], and should be on: [HDMI-1 (0)]
[1663438508.327464] MapDesk: '[0x2c0001e] FvwmPrompt (1)' is on [DP-1-2 (0)], and should be on: [HDMI-1 (0)]
[1663438508.329924] MapDesk: '[0x1c00002] Inbox - thomas@xteddy.org - xteddy Mail - Google Chrome (1)' is on [DP-1-2 (0)], and should be on: [HDMI-1 (0)]
[1663438508.332384] MapDesk: '[0x1a0001e] xterm (2)' is on [DP-1-2 (0)], and should be on: [HDMI-1 (0)]
[1663438508.334736] MapDesk: '[0x180001e] xterm (1)' is on [DP-1-2 (0)], and should be on: [HDMI-1 (0)]
[1663438508.339727] MapDesk: '[0x1c00004] kivikakk/libpcre.zig: Zig bindings to libpcre - Google Chrome (1)' is on [HDMI-1 (1)], and should be on: [DP-1-2 (1)]
[1663438508.342325] MapDesk: '[0x1c00005] Prize crossword No 28,865 | Crosswords | The Guardian - Google Chrome (1)' is on [HDMI-1 (1)], and should be on: [DP-1-2 (1)]
[1663438508.343123] MapDesk: '[0x1c00006] Cryptic crossword No 28,864 | Crosswords | The Guardian - Google Chrome (1)' is on [HDMI-1 (1)], and should be on: [DP-1-2 (1)]
[1663438508.352101] CMD_Echo: "[0 - ]: MONITOR: $[w.screen]: DESK: $[w.desk]"
[1663438508.356667] CMD_Echo: "[1 - ]: MONITOR: $[w.screen]: DESK: $[w.desk]"

Note the two CMD_Echo lines coming from the above config -- that's the only two from the respective monitors in question.

NsCDE commented 1 year ago

As we noted on IRC, GotoDeskAndPage is currently behaving differently than GotoDesk.

ThomasAdam commented 1 year ago

Branch updated -- please retest when you've time.

NsCDE commented 1 year ago

Hi @ThomasAdam

Now it works ok for flipping desks, that is (in two monitors scenario) when from one visible I try to get to another visible. They will be flipped and new_desk triggered twice. However, when going to some of the desks which were not presented on the monitors, new_desk event function is not triggered at all now ... for example, if on Virtual-0 there is desk 0, and on Virtual-1 desk 1, with pointer on Virtual-0 calling GotoDesk 2 will switch there, but new_desk will not be triggered, which is not the case with GotoDesk 1 which will trigger new_desk twice as (probably?) expected.

NsCDE commented 1 year ago

@ThomasAdam The last commit in ta/fix-655 works!

Just some point about design here:

When desks are flipped, new_desk is called for every monitor. While this may have a sense, on the other hand it can execute user's things from FvwmEvent multiple times, which may cause unwanted behaviour. In combination with some background root setter, it can impact performance while doing unnecessary same thing x. Should maybe some desk_changed new event be introduced here, or what? Maybe there is some reliable way for function called from new_desk to detect this in reliable way and adapt, but currently I cannot imagine such workaround, except some loosy schedule/deschedule or infostore "semaphors", which is also non-reliable in case of fast switching.

ThomasAdam commented 1 year ago

Hi @NsCDE

No... if desks are moved between monitors, only those desks which have moved are triggered via an event. You shouldn't be seeing events for desks which are not impacted by this.

As for a new event -- maybe I'll add that in the future. But for now, you'll have to track this manually.

NsCDE commented 1 year ago

Hi @ThomasAdam

I'm not seeing events for non-impacted desks, but events for imacted deskS - plural. Which means, the more monitors, more new_desk events for one GotoDesk if that includes flipping.

Ok, I will try to hande it, but I couldn't think of anything smarter than escaping function called by new_desk for amount of time where inforstore variable is set and removed with schedule. This is loosy and error prone as it best. :-\