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

FvwmPager: moving windows inside FvwmPager is inconsistent #198

Closed
ThomasAdam opened this issue Sep 2, 2020 · 4 comments · Fixed by #536
Closed

FvwmPager: moving windows inside FvwmPager is inconsistent #198

ThomasAdam opened this issue Sep 2, 2020 · 4 comments · Fixed by #536
Assignees
Labels
difficulty:hard Issue needs discussion before implementation help wanted Development help required (see `difficulty:*`) type:bug Something's broken!
Projects
Milestone

Comments

@ThomasAdam
Copy link
Member

When FvwmPager became per-monitor aware, this worked for scrolling around pages. But this no longer works very well when moving windows within FvwmPager to new desks/pages.

@somiaj has the following logic to fix this:

# Definitions
#   nx = Number of Pages in the Horizontal Direction
#   ny = Number of Pages in the Vertical Direction
#
#   px = x-coordinate of window in pager.
#   py = y-coordinate of window in pager.
#
#   PagerWidth = Width of Pager Window
#   PagerHeight = Height of Pager Window
#   (This is for a single desktop)
#
#   DisplayWidth = Width of display (single page)
#   DisplayHeight = Height of display (single page)
#
#   mx = monintor's x-coordinate
#   my = monintor's y-coordinate
#
#   Vx = x-coordinate of current viewport
#   Vy = y-coordinate of current viewport

# Get size of a single page
PageWidth = PagerWidth / nx;
PageHeight = PagerHeight / ny;

# Get current page
Pagex = px / PageWidth;
Pagey = py / PageHeight;

# Get coordinates relative to current page and
# scale coordinates to full display.
cx = (px % PageWidth) * DisplayWidth / PageWidth;
cy = (py % PageHeight) * DisplayHeight / PageHeight;

# Compute display coordinates
x = Pagex * DisplayWidth + cx + mx
y = Pagey * DisplayHeight + cy + my

# Adjust Coordinates to Current Display
t->x = x - Vx
t->y = y - Vy

This is probably all via modules/FvwmPager/x_pager.c:MoveWindow()

@issue-label-bot
Copy link

Issue-Label Bot is automatically applying the label bug to this issue, with a confidence of 0.86. Please mark this comment with 👍 or 👎 to give our bot feedback!

Links: app homepage, dashboard and code for this bot.

@issue-label-bot issue-label-bot bot added the type:bug Something's broken! label Sep 2, 2020
@ThomasAdam ThomasAdam added this to To do in FVWM3 via automation Sep 2, 2020
@ThomasAdam ThomasAdam added this to the post-1.0 milestone Sep 2, 2020
@ThomasAdam ThomasAdam removed this from the post-1.0 milestone Dec 7, 2020
@phintsan
Copy link
Contributor

(I don't know if this is the same issue than this one, but for me this is "inconsistent" so I'm adding this report here.)

Here is another sequence in FvwmPager which causes erratic behaviour:

  1. Start fvwm3 with the default configuration.
  2. Open FvwmConsole.
  3. Issue "Module FvwmPager 0 1" command.
  4. Drag FvwmConsole window to Desk 1 in the pager. Do this so that the window overlaps the desk boundary and release mouse button on top of Desk 1.

Expected behaviour: FvwmConsole is on Desk 1, partially outsize the virtual screen.
Observed behaviour: FvwmConsole is on Desk 0, partially visible on FvwmPager but invisible on the screen.

I have seen similar mess-ups after other moves between desks, but this is probably the easiest way to reproduce the problem.

This is a regression caused by a22beb1. The problem is in macro UPDATE_FVWM_SCREEN, presumerably in CMD_Move function. In screen.h:490 there is an assignment

(fw)->Desk = mnew->virtual_scr.CurrentDesk;

This overrides the correct fw->desk by wrong mnew... desk coming from FindScreenOfXY call at line 483. Removing that assignment fixes the problem, but I don't know if it causes any regressions elsewhere since I'm not really familiar with the code.

ThomasAdam added a commit that referenced this issue Dec 10, 2020
When a window is being moved within the pager, the desk the window is on
was being updated, and then later on, FVWM_UPDATE_SCREEN would further
confuse things.  FvwmPager sends Move/MoveToDesk commands back to Fvwm
to get the window's properties updated.

Let the FVWM_UPDATE_SCREEN logic update the window, since the Move
command FvwmPager sends back will also update the desk the window is on,
so no need to explicitly move the window.

Helps #198
@ThomasAdam
Copy link
Member Author

Hi @phintsan

Thanks for your observations. There's multiple breakages in FvwmPager with the changes made to support DesktopConfiguration per-monitor. This has unfortunately also impacted the default behaviour of DesktopConfiguration global as well. As the notes above hint at, I just need to sit down and fix it. ;)

It's certainly not enough to comment out the (fw)->Desk assignment in UPDATE_FVWM_SCREEN as this would break everything. Better for now, would be to stop FvwmPager inferring the desk the window is on, and let Fvwm update that via the FvwmPager is sending fvwm directly (Move, etc.)

Perhaps ta/fvwmpager-dont-update-desk improves this a little? Either way, although it might help keep the window on screen, there's no magic bullet here -- I need to rewrite this entire logic to truly fix this.

@phintsan
Copy link
Contributor

@ThomasAdam

Thanks for your prompt reply. At a glance ta/fvwmpager-dont-update-desk seems to fix the problem: after moving the window in the pager it stays on Desk 1 (in the pager), but switching to that desk does not bring the window visible on the screen. The window reappears on the screen after returning to Desk 0, but it stays on Desk 1 in the pager.

I'm afraid there is not much more for me to do here until you've sorted it out, after which I can help with testing.
Thanks for your efforts.

@ThomasAdam ThomasAdam added this to the 1.0.3 milestone Dec 20, 2020
@ThomasAdam ThomasAdam added help wanted Development help required (see `difficulty:*`) relates:fvwm3 difficulty:hard Issue needs discussion before implementation and removed relates:fvwm3 labels Dec 20, 2020
@ThomasAdam ThomasAdam removed this from the 1.0.3 milestone Feb 22, 2021
@ThomasAdam ThomasAdam added this to the 1.0.4 milestone May 30, 2021
@ThomasAdam ThomasAdam assigned ThomasAdam and somiaj and unassigned ThomasAdam Jun 11, 2021
somiaj added a commit that referenced this issue Jun 14, 2021
Make it possible to move windows within FvwmPager and have them appear
approximately in the correct position in with global or tracking a
single Monitor. Fixes #198.

Note, this update only affects windowed mode, the Iconified pager
view still needs updated.
somiaj added a commit that referenced this issue Jun 14, 2021
Make it possible to move windows within FvwmPager and have them appear
approximately in the correct position in with global or tracking a
single Monitor. Fixes #198.

Note, this update only affects windowed mode, the Iconified pager
view still needs updated.
somiaj added a commit that referenced this issue Jun 14, 2021
Make it possible to move windows within FvwmPager and have them appear
approximately in the correct position in with global or tracking a
single Monitor. Fixes #198.

Note, this update only affects windowed mode, the Iconified pager
view still needs updated.
ThomasAdam pushed a commit that referenced this issue Jun 14, 2021
Make it possible to move windows within FvwmPager and have them appear
approximately in the correct position in with global or tracking a
single Monitor. Fixes #198.

Note, this update only affects windowed mode, the Iconified pager
view still needs updated.
@ThomasAdam ThomasAdam linked a pull request Jun 14, 2021 that will close this issue
FVWM3 automation moved this from To do to Done Jun 14, 2021
ThomasAdam pushed a commit that referenced this issue Jun 14, 2021
Make it possible to move windows within FvwmPager and have them appear
approximately in the correct position in with global or tracking a
single Monitor. Fixes #198.

Note, this update only affects windowed mode, the Iconified pager
view still needs updated.
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Aug 20, 2021
# Changelog

## [1.0.4](https://github.com/fvwmorg/fvwm3/tree/1.0.4) (2021-07-17)

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

**Breaking changes:**

- Deprecate Label for FvwmPager [\#342](fvwmorg/fvwm3#342)
- Extend \*FvwmIconMan:Resolution to include specific Desktop / Pager [\#455](fvwmorg/fvwm3#455)
- Replace libbson with cJSON [\#408](fvwmorg/fvwm3#408)
- FvwmButtons: Shrink windows when honoring Hints. [\#577](fvwmorg/fvwm3#577) ([somiaj](https://github.com/somiaj))
- Replace libbson with cJSON [\#571](fvwmorg/fvwm3#571) ([ThomasAdam](https://github.com/ThomasAdam))
- Add commands to configure the size/position window. [\#558](fvwmorg/fvwm3#558) ([ThomasAdam](https://github.com/ThomasAdam))
- Retire FvwmTile and FvwmCascade wrappers. [\#515](fvwmorg/fvwm3#515) ([somiaj](https://github.com/somiaj))

**Implemented enhancements:**

- Set min. size for windows shown in pager [\#542](fvwmorg/fvwm3#542)
- Moving windows: allow for "warping" to other windows in a given direction [\#540](fvwmorg/fvwm3#540)
- Update monitor struct to know if monintor edges are OUTSIDE\_EDGE or INSIDE\_EDGE [\#523](fvwmorg/fvwm3#523)
- Two issues with the WindowList [\#151](fvwmorg/fvwm3#151)
- Feature Request: Enable using the 'c' unit with the "MinWindowSize" and "MaxWindowSize" styles [\#145](fvwmorg/fvwm3#145)
- Allow per monitor EdgeCommand and EdgeLeaveCommand [\#582](fvwmorg/fvwm3#582) ([somiaj](https://github.com/somiaj))
- expand: add prev\_{desk,pagex,pagey} [\#579](fvwmorg/fvwm3#579) ([ThomasAdam](https://github.com/ThomasAdam))
- Improve translations [\#568](fvwmorg/fvwm3#568) ([somiaj](https://github.com/somiaj))
- Cleanup SetRCDefaults\(\) [\#567](fvwmorg/fvwm3#567) ([somiaj](https://github.com/somiaj))
- Add local configuration file to the default-config. [\#557](fvwmorg/fvwm3#557) ([somiaj](https://github.com/somiaj))
- Add icon for Run Command. [\#552](fvwmorg/fvwm3#552) ([somiaj](https://github.com/somiaj))
- Add command: Move shuffle \[flags\] direction\(s\) [\#550](fvwmorg/fvwm3#550) ([somiaj](https://github.com/somiaj))
- expand: add desk, pagex, pagey variables [\#539](fvwmorg/fvwm3#539) ([ThomasAdam](https://github.com/ThomasAdam))
- Add a screen option to the Scroll command. [\#531](fvwmorg/fvwm3#531) ([ThomasAdam](https://github.com/ThomasAdam))
- RandR: support RandRFunc for screen changes [\#525](fvwmorg/fvwm3#525) ([ThomasAdam](https://github.com/ThomasAdam))

**Fixed bugs:**

- Pager do not show smalls windows in the correct place when snapped to the edge [\#541](fvwmorg/fvwm3#541)
- EwmhBaseStruts glitch when using screen with different resolutions [\#66](fvwmorg/fvwm3#66)
- Fvwm segfaults parsing module configuration [\#575](fvwmorg/fvwm3#575)
- Swallowing FvwmPager inside FvwmButtons breaks resizing FvwmButtons when aspect ratio resizing is used [\#573](fvwmorg/fvwm3#573)
- Move X Y Warp doesn't move pointer to window. [\#551](fvwmorg/fvwm3#551)
- Maximize fullscreen command does not clear \_NET\_WM\_STATE\_FULLSCREEN when exiting fullscreen [\#545](fvwmorg/fvwm3#545)
- EdgeLeaveCommand don't work or need a page change to work [\#543](fvwmorg/fvwm3#543)
- FvwmPager Icon view fix background color. [\#537](fvwmorg/fvwm3#537)
- FvwmPager with bad aspect ratio on dual-head display [\#522](fvwmorg/fvwm3#522)
- AnimatedMove [\#513](fvwmorg/fvwm3#513)
- Spelling errors found by lintian. [\#511](fvwmorg/fvwm3#511)
- SnapAttraction: take into account individual monitors [\#466](fvwmorg/fvwm3#466)
- FvwmPager not taking into account global screen dimensions when configured with DeskTopScale [\#223](fvwmorg/fvwm3#223)
- FvwmPager: moving windows inside FvwmPager is inconsistent [\#198](fvwmorg/fvwm3#198)
- Add force\_icons\_size kludge to .stalonetrayrc. [\#581](fvwmorg/fvwm3#581) ([somiaj](https://github.com/somiaj))
- Fix pass through binding logic. [\#570](fvwmorg/fvwm3#570) ([somiaj](https://github.com/somiaj))
- Make default-config greyed colorset grey. [\#566](fvwmorg/fvwm3#566) ([somiaj](https://github.com/somiaj))
- Cleanup FvwmScript manual page. [\#565](fvwmorg/fvwm3#565) ([somiaj](https://github.com/somiaj))
- GNOME: remove DO\_IGNORE\_GNOME\_HINTS [\#556](fvwmorg/fvwm3#556) ([lgsobalvarro](https://github.com/lgsobalvarro))
- Unmaximize: restore \_NET\_WM\_STATE [\#546](fvwmorg/fvwm3#546) ([ThomasAdam](https://github.com/ThomasAdam))
- Fix compiler warnings in modules/FvwmScript/Instructions.c [\#544](fvwmorg/fvwm3#544) ([pm215](https://github.com/pm215))
- Rework FvwmPager to move windows easier [\#536](fvwmorg/fvwm3#536) ([ThomasAdam](https://github.com/ThomasAdam))
- Move: disable working area when screen given [\#535](fvwmorg/fvwm3#535) ([ThomasAdam](https://github.com/ThomasAdam))
- Make RightPanel use primary monitor dimensions. [\#530](fvwmorg/fvwm3#530) ([somiaj](https://github.com/somiaj))
- Set base struts only for primary monitor in default-config. [\#528](fvwmorg/fvwm3#528) ([somiaj](https://github.com/somiaj))
- Change FvwmPager Logic for initial window size. [\#527](fvwmorg/fvwm3#527) ([somiaj](https://github.com/somiaj))
- EWMH\_GetWorkArea use monitor dimensions. [\#526](fvwmorg/fvwm3#526) ([somiaj](https://github.com/somiaj))
- Make SnapAttraction snap to edges of all monitors. [\#521](fvwmorg/fvwm3#521) ([somiaj](https://github.com/somiaj))
- Update FvwmCommand to allow multiple args. [\#514](fvwmorg/fvwm3#514) ([somiaj](https://github.com/somiaj))

**Merged pull requests:**

- Extend FvwmIconMan Resolution configuration. [\#561](fvwmorg/fvwm3#561) ([somiaj](https://github.com/somiaj))
- Allow Min/Max WindowSize style to use client size [\#560](fvwmorg/fvwm3#560) ([somiaj](https://github.com/somiaj))
- Add more columns to default-config menu ItemFormat. [\#559](fvwmorg/fvwm3#559) ([somiaj](https://github.com/somiaj))
- Fix broken link. [\#529](fvwmorg/fvwm3#529) ([somiaj](https://github.com/somiaj))
- Spelling error fixes. [\#512](fvwmorg/fvwm3#512) ([somiaj](https://github.com/somiaj))
- Working on 1.0.4 [\#509](fvwmorg/fvwm3#509) ([ThomasAdam](https://github.com/ThomasAdam))
- module expand: don't overwrite previous expansion [\#576](fvwmorg/fvwm3#576) ([ThomasAdam](https://github.com/ThomasAdam))
- conditional: fix whitespace/comma parsing [\#572](fvwmorg/fvwm3#572) ([ThomasAdam](https://github.com/ThomasAdam))
- Configure a colorset for XDGMenu options. [\#564](fvwmorg/fvwm3#564) ([somiaj](https://github.com/somiaj))
- Menu: add option to grey entries out [\#563](fvwmorg/fvwm3#563) ([ThomasAdam](https://github.com/ThomasAdam))
- Remove \*FvwmPager: Label configuration option. [\#562](fvwmorg/fvwm3#562) ([somiaj](https://github.com/somiaj))
- move: Warp: move pointer to centre of window [\#553](fvwmorg/fvwm3#553) ([ThomasAdam](https://github.com/ThomasAdam))
- FvwmPager: Improvments with dealing with tiny windows on tiny pagers. [\#548](fvwmorg/fvwm3#548) ([somiaj](https://github.com/somiaj))
- PanFrame improvements [\#547](fvwmorg/fvwm3#547) ([ThomasAdam](https://github.com/ThomasAdam))
- pager: teach Icon view about colorsets [\#538](fvwmorg/fvwm3#538) ([ThomasAdam](https://github.com/ThomasAdam))
- placement: fix mouse positioning [\#533](fvwmorg/fvwm3#533) ([ThomasAdam](https://github.com/ThomasAdam))
- EdgeScroll: a few improvements [\#524](fvwmorg/fvwm3#524) ([ThomasAdam](https://github.com/ThomasAdam))
- Update manual for Echo/EchoFuncDefinition [\#520](fvwmorg/fvwm3#520) ([somiaj](https://github.com/somiaj))
- release: remove dev-docs from release tarball [\#518](fvwmorg/fvwm3#518) ([ThomasAdam](https://github.com/ThomasAdam))
- configure: remove stale references to BUGADDR [\#517](fvwmorg/fvwm3#517) ([ThomasAdam](https://github.com/ThomasAdam))
- Configuration tweaks [\#516](fvwmorg/fvwm3#516) ([ThomasAdam](https://github.com/ThomasAdam))
- build: add CHANGELOG.md to dist [\#510](fvwmorg/fvwm3#510) ([ThomasAdam](https://github.com/ThomasAdam))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty:hard Issue needs discussion before implementation help wanted Development help required (see `difficulty:*`) type:bug Something's broken!
Projects
Status: Done
FVWM3
  
Done
Development

Successfully merging a pull request may close this issue.

3 participants