Subject: FVWM: Minor placement.c bugs

From: [email-masked] (Dan Astoorian)
Date: Sun, 4 Aug 1996 23:01:08 -0400 (EDT)

(Warning: horrid ugly poorly-explained details ahead.  Casual fvwm users
probably won't benefit from reading them--this report is intended for
the programmers on the list who are fairly intimate with the source
code.)

placement.c seems to have a subtle bug in the code to avoid placing a
window offscreen.  The behaviour is harder to explain than it is to
fix....

To reproduce the bug: turn on SmartPlacement, and arrange the desktop so
that the next window to be placed should be near (but not touching) the
bottom edge of the screen, where "near" means "within roughly a distance
equal to twice the height of a window's titlebar."

For instance: if your xterm windows are 350 pixels high including
decorations, and your TitleFont is a 16 point font, arrange the desktop
so that there's roughly, say, 370-380 pixels of open space between the
bottom of an existing window and the bottom edge of the screen; then
create another xterm.

The window gets created at the bottom of the screen instead of just
under the bottom edge of the other window. 

(A similar bug exists for horizontal placement, but it's easier to
exhibit for vertical placement.

English is the wrong language to try to explain the bug, so I'll put it
in terms of the source code...

ll. 201-215 of placement.c:

201|  if(tmp_win->frame_x + tmp_win->frame_width +
202|     2*tmp_win->boundary_width> Scr.MyDisplayWidth)
203|    {
204|      tmp_win->attr.x = Scr.MyDisplayWidth -tmp_win->attr.width
205|	- tmp_win->old_bw +tmp_win->bw - 2*tmp_win->boundary_width;
206|      Scr.randomx = 0;
207|    }
208|  if(tmp_win->frame_y + 2*tmp_win->boundary_width+tmp_win->title_height
209|     + tmp_win->frame_height > Scr.MyDisplayHeight)
210|    {
211|      tmp_win->attr.y = Scr.MyDisplayHeight -tmp_win->attr.height
212|	- tmp_win->old_bw +tmp_win->bw - tmp_win->title_height -
213|	  2*tmp_win->boundary_width;;
214|      Scr.randomy = 0;
215|    }

I believe that the problem is that tmp_win->frame_width includes the
2*tmp_win->boundary_width, so they don't need to be re-added into the
calcluation; likewise for tmp_win->frame_height and
2*tmp_win->boundary_width and tmp_win->title_height.  The fix is to
simply remove those terms from the assignments.

Also, at ll.34-37, I think both comparisons should be '<=' instead of '<':

34|  while(((test_y + temp_h) < (Scr.MyDisplayHeight))&&(!loc_ok))
35|    {
36|      test_x = 0;
37|      while(((test_x + temp_w) < (Scr.MyDisplayWidth))&&(!loc_ok))

By disallowing the equality, SmartPlacement isn't allowed to place
windows touching the right side or the bottom edge of the screen.

(The SmartPlacement() routine could also use some optimization.  I'm
thinking about writing a more efficient version of that routine which,
in addition to being smarter about where to try to place windows,
could optionally look for a "pretty good" place to put a window
if it couldn't find a place which didn't overlap any windows at all;
"pretty good" means it would use a combination of strategies such as (a)
overlap the smallest region possible, or a reasonable approximation; (b)
try to avoid overlapping other window's titlebars; (c) try to avoid
positions where it would completely obscure some other window; and/or
(d) try to balance the windows so there aren't some regions covered by
eight overlapping windows and other regions with one.  Some people might
prefer such educated guesses for window placement to simply falling back
to RandomPlacement.  In any case, it's extremely doubtful that such a
patch would make it for 2.1.0, even if there's another beta between now
and then.)

Cheers,

--                          People shouldn't think that it's better to have
Dan Astoorian               loved and lost than never loved at all.  It's
http://www.utopia.csas.com  not, it's better to have loved and won.  All
[email-masked]       the other options really suck.    --Dan Redican
--
Visit the official FVWM web page at <URL:http://www.hpc.uh.edu/fvwm/>.
To unsubscribe from the list, send "unsubscribe fvwm" in the body of a
message to [email-masked].
To report problems, send mail to [email-masked].