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

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

Closed
NsCDE opened this issue Feb 23, 2022 · 13 comments · Fixed by #687 or #695
Closed

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

NsCDE opened this issue Feb 23, 2022 · 13 comments · Fixed by #687 or #695
Assignees
Labels
relates:desktop Issues is in desktop management code type:bug Something's broken!
Milestone

Comments

@NsCDE
Copy link
Contributor

NsCDE commented Feb 23, 2022

  • Fvwm3 version (run: fvwm3 --version)

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

  • Linux distribution or BSD name/version

Debian 11

  • Platform

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

  • Enable FvwmEvent module and configure new_desk to run some verbose/visible function

  • Configure virtual (or real) environment with 4 monitors.

  • Configure FVWM with DesktopConfiguration shared

  • Enable monitors (example): xrandr --output Virtual-1 --mode 1400x1050 --primary --output Virtual-2 --mode 1400x1050 --right-of Virtual-1 --output Virtual-3 --mode 1400x1050 --below Virtual-1 --output Virtual-4 --mode 1400x1050 --below Virtual-2

  • Restart FVWM (FvwmMFL must be enabled in config): FvwmCommand Restart

  • Observe ~/.xsession-errors ~/fvwm.log or equivalent for echo from your function which is run by FvwmEvent on new_desk event

  • Does the problem also happen with Fvwm2?

N/A

Does Fvwm3 crash?

No.

Extra Information

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

@NsCDE NsCDE added the type:bug Something's broken! label Feb 23, 2022
@ThomasAdam ThomasAdam added this to the 1.0.5 milestone Feb 27, 2022
@ThomasAdam ThomasAdam added the relates:desktop Issues is in desktop management code label Feb 27, 2022
ThomasAdam added a commit that referenced this issue Aug 29, 2022
When in DesktopConfiguration 'shared' mode, only flag those desks which
have changed, rather than all screens/desks, as this is misleading.

Fixes #655
@ThomasAdam
Copy link
Member

Hi @NsCDE

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

Thanks!

ThomasAdam added a commit that referenced this issue Sep 4, 2022
When in DesktopConfiguration 'shared' mode, only flag those desks which
have changed, rather than all screens/desks, as this is misleading.

Fixes #655
@NsCDE
Copy link
Contributor Author

NsCDE commented Sep 6, 2022

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
Copy link
Contributor Author

NsCDE commented Sep 6, 2022

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 ThomasAdam reopened this Sep 17, 2022
ThomasAdam added a commit that referenced this issue Sep 17, 2022
When looping round GotoDesk and associated commands (GotoDeskAndPage,
etc.), ensure the matching for which desk and monitor to apply the
request to, doesn't loop on the same value, essentially causing multiple
requests happen.

In the case of DesktopConfiguration shared/global, this would happen per
monitor when it shouldn't.

Fixes #655
@ThomasAdam
Copy link
Member

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
Copy link
Contributor Author

NsCDE commented Sep 17, 2022

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
Copy link
Member

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
Copy link
Member

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
Copy link
Contributor Author

NsCDE commented Sep 17, 2022

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

ThomasAdam added a commit that referenced this issue Sep 17, 2022
When looping round GotoDesk and associated commands (GotoDeskAndPage,
etc.), ensure the matching for which desk and monitor to apply the
request to, doesn't loop on the same value, essentially causing multiple
requests happen.

In the case of DesktopConfiguration shared/global, this would happen per
monitor when it shouldn't.

Fixes #655
@ThomasAdam
Copy link
Member

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

ThomasAdam added a commit that referenced this issue Sep 17, 2022
When looping round GotoDesk and associated commands (GotoDeskAndPage,
etc.), ensure the matching for which desk and monitor to apply the
request to, doesn't loop on the same value, essentially causing multiple
requests happen.

In the case of DesktopConfiguration shared/global, this would happen per
monitor when it shouldn't.

Fixes #655
@ThomasAdam ThomasAdam moved this to To do in FVWM3 Sep 18, 2022
@ThomasAdam ThomasAdam added this to FVWM3 Sep 18, 2022
@ThomasAdam ThomasAdam moved this from To do to In progress in FVWM3 Sep 18, 2022
@ThomasAdam ThomasAdam self-assigned this Sep 18, 2022
@ThomasAdam ThomasAdam moved this from In progress to PRs in FVWM3 Sep 18, 2022
@NsCDE
Copy link
Contributor Author

NsCDE commented Sep 18, 2022

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
Copy link
Contributor Author

NsCDE commented Sep 19, 2022

@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
Copy link
Member

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
Copy link
Contributor Author

NsCDE commented Sep 19, 2022

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. :-\

ThomasAdam added a commit that referenced this issue Sep 19, 2022
When looping round GotoDesk and associated commands (GotoDeskAndPage,
etc.), ensure the matching for which desk and monitor to apply the
request to, doesn't loop on the same value, essentially causing multiple
requests happen.

In the case of DesktopConfiguration shared/global, this would happen per
monitor when it shouldn't.

Fixes #655
Repository owner moved this from PRs to Done in FVWM3 Sep 19, 2022
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Oct 19, 2022
(One of the more significant changes is that locking issues with libX11
1.8.1 are fixed.)

## [1.0.5](https://github.com/fvwmorg/fvwm3/tree/1.0.5) (2022-09-28)

[Full Changelog](fvwmorg/fvwm3@1.0.4...1.0.5)

**Breaking changes:**

- Function parser rewrite & Repeat command deprecation [\#642](fvwmorg/fvwm3#642)
- MapRequest: don't fake map/unmap events [\#703](fvwmorg/fvwm3#703) ([ThomasAdam](https://github.com/ThomasAdam))
- Rewrite function parser and remove the Repeat command [\#643](fvwmorg/fvwm3#643) ([ThomasAdam](https://github.com/ThomasAdam))
- Update and cleanup SnapAttract code. [\#641](fvwmorg/fvwm3#641) ([somiaj](https://github.com/somiaj))
- Doc: split manpages into sections [\#637](fvwmorg/fvwm3#637) ([ThomasAdam](https://github.com/ThomasAdam))
- Remove Efence and Dmalloc support [\#635](fvwmorg/fvwm3#635) ([ThomasAdam](https://github.com/ThomasAdam))

**Implemented enhancements:**

- A better ManualPlacement that allows drawing the geometry of the new window. [\#674](fvwmorg/fvwm3#674)
- expand: add monitor.prev variable [\#699](fvwmorg/fvwm3#699) ([ThomasAdam](https://github.com/ThomasAdam))
- Add AnyScreen to conditional in IconManClick [\#696](fvwmorg/fvwm3#696) ([somiaj](https://github.com/somiaj))
- \_NET\_WM\_NAME: update to fvwm3 [\#609](fvwmorg/fvwm3#609) ([ThomasAdam](https://github.com/ThomasAdam))

**Fixed bugs:**

- Style \* Icon cause Fvwm3 stuck in loading when restart. [\#681](fvwmorg/fvwm3#681)
- Recaptured windows can have a negative offset away from the page they should be on [\#678](fvwmorg/fvwm3#678)
- VLC still decorates its transient window even when explicitly given the NakedTransient style [\#673](fvwmorg/fvwm3#673)
- configuring with `--disable-png` causes builds to fail [\#669](fvwmorg/fvwm3#669)
- Emoji in window titles make FvwmIconMan stop showing window names. [\#654](fvwmorg/fvwm3#654)
- Unable to initialize RandR [\#650](fvwmorg/fvwm3#650)
- PipeRead when called from a function cannot grab pointer [\#610](fvwmorg/fvwm3#610)
- Man Pages Cleanup [\#554](fvwmorg/fvwm3#554)
- Windows from various pages are moved to page 0 0 on fvwm3 restart [\#694](fvwmorg/fvwm3#694)
- Separator in menu gets focus [\#675](fvwmorg/fvwm3#675)
- Unshading a window with WindowShade function sometimes makes the window lose "true input focus". [\#671](fvwmorg/fvwm3#671)
- When configured with `--disable-xft` fvwm3 fails to build. [\#667](fvwmorg/fvwm3#667)
- my fvwm config does not work with recent chromium [\#663](fvwmorg/fvwm3#663)
- FvwmEvent event new\_desk gets triggered multiple times in multi-monitor shared setup [\#655](fvwmorg/fvwm3#655)
- Windows with style "PositionPlacement Center" split between monitors [\#648](fvwmorg/fvwm3#648)
- FVWM branch dv/pager-noaspect crashes with core dump [\#647](fvwmorg/fvwm3#647)
- SnapAttraction prefers wrong window [\#631](fvwmorg/fvwm3#631)
- FvwmPrompt is installed unstripped [\#618](fvwmorg/fvwm3#618)
- DesktopName fails to set desktop name under described circumstances [\#606](fvwmorg/fvwm3#606)
- FvwmEvent event monitor\_focus broken in FVWM3 1.0.4 [\#604](fvwmorg/fvwm3#604)
- Building FvwmPrompt disables FvwmConsole, but still installs manual page. [\#597](fvwmorg/fvwm3#597)
- Wait command in configuration file can cause unexpected issues with GeometryWindow. [\#590](fvwmorg/fvwm3#590)
- "GeometryWindow Hide" doesn't work [\#589](fvwmorg/fvwm3#589)
- Special characters \(umlauts\) are sometimes not displayed correctly in the window title [\#482](fvwmorg/fvwm3#482)
- FvwmEvent: handle previous\_monitor and no longer passthrough ID  [\#701](fvwmorg/fvwm3#701) ([ThomasAdam](https://github.com/ThomasAdam))
- doc: don't build FvwmConsole.1 if FvwmPrompt enabled [\#700](fvwmorg/fvwm3#700) ([ThomasAdam](https://github.com/ThomasAdam))
- DesktopConfiguration shared: keep windows in-situ [\#697](fvwmorg/fvwm3#697) ([ThomasAdam](https://github.com/ThomasAdam))
- desk\_add: fix starting desk/monitor [\#689](fvwmorg/fvwm3#689) ([ThomasAdam](https://github.com/ThomasAdam))
- shared: fix flagging of new\_desk [\#687](fvwmorg/fvwm3#687) ([ThomasAdam](https://github.com/ThomasAdam))
- Fix for lock recusion in handle\_all\_expose\(\) [\#683](fvwmorg/fvwm3#683) ([mherrb](https://github.com/mherrb))
- Asciidoc fixes [\#676](fvwmorg/fvwm3#676) ([topcat001](https://github.com/topcat001))
- grow: ignore transient windows [\#627](fvwmorg/fvwm3#627) ([ThomasAdam](https://github.com/ThomasAdam))
- MoveToScreen: fix NULL-dereference [\#605](fvwmorg/fvwm3#605) ([ThomasAdam](https://github.com/ThomasAdam))
- Bugfix: fvwm-menu-desktop --get-menus [\#593](fvwmorg/fvwm3#593) ([somiaj](https://github.com/somiaj))

**Closed issues:**

- Code Cleanup: Codacy issues list [\#107](fvwmorg/fvwm3#107)

**Merged pull requests:**

- avoid sprintf\(%n\) [\#653](fvwmorg/fvwm3#653) ([omar-polo](https://github.com/omar-polo))
- FvwmPrompt: add GOFLAGS to build stripped [\#619](fvwmorg/fvwm3#619) ([Zirias](https://github.com/Zirias))
- Wait: don't run until windows are captured [\#592](fvwmorg/fvwm3#592) ([ThomasAdam](https://github.com/ThomasAdam))
- CMD\_GeometryWindow: Move NULL check. [\#591](fvwmorg/fvwm3#591) ([somiaj](https://github.com/somiaj))
- cleanup: address warnings [\#705](fvwmorg/fvwm3#705) ([ThomasAdam](https://github.com/ThomasAdam))
- modconf: disable debug [\#698](fvwmorg/fvwm3#698) ([ThomasAdam](https://github.com/ThomasAdam))
- GotoDesk: avoid over-eager matching [\#695](fvwmorg/fvwm3#695) ([ThomasAdam](https://github.com/ThomasAdam))
- update\_fvwm\_monitor: cosmetic change [\#692](fvwmorg/fvwm3#692) ([ThomasAdam](https://github.com/ThomasAdam))
- menuitem: set selectable when not a separator [\#690](fvwmorg/fvwm3#690) ([ThomasAdam](https://github.com/ThomasAdam))
- Windowshade: explicitly set input focus [\#672](fvwmorg/fvwm3#672) ([ThomasAdam](https://github.com/ThomasAdam))
- FvwmPrompt: update core modules [\#665](fvwmorg/fvwm3#665) ([ThomasAdam](https://github.com/ThomasAdam))
- FvwmPrompt: update vendor deps [\#664](fvwmorg/fvwm3#664) ([ThomasAdam](https://github.com/ThomasAdam))
- Fix selectable flag for the Resize window operation menu item [\#656](fvwmorg/fvwm3#656) ([topcat001](https://github.com/topcat001))
- Fix ExitFunction [\#651](fvwmorg/fvwm3#651) ([pghvlaans](https://github.com/pghvlaans))
- DisplayPosition: fix segfault [\#645](fvwmorg/fvwm3#645) ([ThomasAdam](https://github.com/ThomasAdam))
- convert UPDATE\_FVWM\_SCREEN from macro to function [\#644](fvwmorg/fvwm3#644) ([ThomasAdam](https://github.com/ThomasAdam))
- ta/dv logfile [\#640](fvwmorg/fvwm3#640) ([ThomasAdam](https://github.com/ThomasAdam))
- Resize: fix resize bounds [\#638](fvwmorg/fvwm3#638) ([ThomasAdam](https://github.com/ThomasAdam))
- ta/dv2 [\#636](fvwmorg/fvwm3#636) ([ThomasAdam](https://github.com/ThomasAdam))
- ta/dv misc [\#634](fvwmorg/fvwm3#634) ([ThomasAdam](https://github.com/ThomasAdam))
- Reject out of range windows for Move and Resize commands. [\#633](fvwmorg/fvwm3#633) ([ThomasAdam](https://github.com/ThomasAdam))
- FVWMMFL: ignore SIGPIPE [\#632](fvwmorg/fvwm3#632) ([ThomasAdam](https://github.com/ThomasAdam))
- ta/dv ifdev [\#630](fvwmorg/fvwm3#630) ([ThomasAdam](https://github.com/ThomasAdam))
- ta/from dv [\#629](fvwmorg/fvwm3#629) ([ThomasAdam](https://github.com/ThomasAdam))
- DesktopName: don't duplicate entries with same name [\#607](fvwmorg/fvwm3#607) ([ThomasAdam](https://github.com/ThomasAdam))
- Patches from Debian [\#599](fvwmorg/fvwm3#599) ([ThomasAdam](https://github.com/ThomasAdam))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
relates:desktop Issues is in desktop management code type:bug Something's broken!
Projects
Status: Done
2 participants