Subject: fixes for menus

From: Dominik Vogt <[email-masked]>
Date: Wed, 27 Jan 1999 02:03:03 +0100

--- ChangeLog.orig	Wed Jan 27 01:53:44 1999
+++ ChangeLog	Wed Jan 27 01:51:28 1999
@@ -1,3 +1,31 @@
+1999-01-27  Dominik Vogt  <[email-masked]>
+
+	* fvwm/menus.c (MenuInteraction): check for expose events first to
+	ensure the menu is drawn properly before navigating
+	(FPopupMenu): fixed *badly* broken code to avoid overlapping menus
+	(with and without animation).
+	(DoMenusOverlap): fixed broken overlapping algorithm
+
+	* fvwm/menus.h (MenuRoot): removed 'redrawn' flag
+
+	* fvwm/menus.c (DoMenusOverlap): fixed gradient menu drawing bug
+	(MenuInteraction):
+	(PopDownAndRepaintParent):
+	(PaintMenu):
+	(SetMenuItemSelected): removed 'redrawn' flag. Use flush_expose
+	instead: simpler and fixes a drawing bug

1999-01-20  Paul D. Smith  <[email-masked]>

	* INSTALL.fvwm: Document new install-strip issue and workaround.

	* Makefile.am (AUTOMAKE_OPTIONS): Require automake 1.4.  Needed
	now that the VERSION and PACKAGE undefs are removed.

Wed Jan 20 01:13:38 1999  Steve Robbins  <[email-masked]>

	* INSTALL.fvwm: Removed doubled paragraph.

 1999-01-19  Paul D. Smith  <[email-masked]>
 
 	* README: Update new URLs; a few spelling corrections; specify
@@ -5,7 +33,6 @@
 
 1999-01-19  Steven Michael ROBBINS  <[email-masked]>
 
-	* configure.in:
 	* README: Changed "care and feeding" info from Brady Montz to the
 	Fvwm Workers.
 
--- fvwm/menus.h.orig	Tue Jan 12 22:07:43 1999
+++ fvwm/menus.h	Wed Jan 27 00:08:12 1999
@@ -216,7 +216,6 @@
       struct
       {
 	unsigned painted : 1;
-	unsigned redrawn : 1;
 	unsigned is_left : 1;   /* menu direction relative to parent menu */
 	unsigned is_right : 1;
 	unsigned is_up : 1;
--- fvwm/menus.c.orig	Wed Jan 20 02:50:15 1999
+++ fvwm/menus.c	Wed Jan 27 01:51:06 1999
@@ -630,28 +630,31 @@
 	Event.xmotion.time = lastTimestamp;
 	fFakedMotion = TRUE;
 	fForceReposition = FALSE;
-      } else if (Scr.menus.PopupDelay10ms > 0) {
-        while (XCheckMaskEvent(dpy,
-                               ButtonPressMask|ButtonReleaseMask|
-                               ExposureMask|KeyPressMask|VisibilityChangeMask|
-                               ButtonMotionMask,
-                               &Event) == FALSE) {
-	  usleep(MICRO_S_FOR_10MS);
- 	  if (c10msDelays++ == Scr.menus.PopupDelay10ms) {
-	    DBUG("MenuInteraction","Faking motion");
-	    /* fake a motion event, and set fDoPopupNow */
-	    Event.type = MotionNotify;
-	    Event.xmotion.time = lastTimestamp;
-	    fFakedMotion = TRUE;
-	    fDoPopupNow = TRUE;
-	    break;
+      } else if (!XCheckMaskEvent(dpy,ExposureMask,&Event)) {
+	/* handle exposure events first */
+	if (Scr.menus.PopupDelay10ms > 0) {
+	  while (XCheckMaskEvent(dpy,
+				 ButtonPressMask|ButtonReleaseMask|
+				 ExposureMask|KeyPressMask|
+				 VisibilityChangeMask|ButtonMotionMask,
+				 &Event) == FALSE) {
+	    usleep(MICRO_S_FOR_10MS);
+	    if (c10msDelays++ == Scr.menus.PopupDelay10ms) {
+	      DBUG("MenuInteraction","Faking motion");
+	      /* fake a motion event, and set fDoPopupNow */
+	      Event.type = MotionNotify;
+	      Event.xmotion.time = lastTimestamp;
+	      fFakedMotion = TRUE;
+	      fDoPopupNow = TRUE;
+	      break;
+	    }
 	  }
+	} else {  /* block until there is an event */
+	  XMaskEvent(dpy,
+		     ButtonPressMask|ButtonReleaseMask|ExposureMask |
+		     KeyPressMask|VisibilityChangeMask|ButtonMotionMask,
+		     &Event);
 	}
-      } else {  /* block until there is an event */
-	XMaskEvent(dpy,
-		   ButtonPressMask|ButtonReleaseMask|ExposureMask |
-		   KeyPressMask|VisibilityChangeMask|ButtonMotionMask,
-		   &Event);
       }
       /*DBUG("MenuInteraction","mrPopup=%s",mrPopup?mrPopup->name:"(none)");*/
 
@@ -730,13 +733,7 @@
 	  /* grab our expose events, let the rest go through */
 	  if((XFindContext(dpy, Event.xany.window,MenuContext,
 			   (caddr_t *)&mrNeedsPainting)!=XCNOENT)) {
-	    /* don't redraw multiple times in a row */
-	    if (mrNeedsPainting == menu) {
-	      if (menu->flags.f.redrawn == 1)
-		continue;
-	      else
-		menu->flags.f.redrawn = 1;
-	    }
+	    flush_expose(Event.xany.window);
 	    PaintMenu(mrNeedsPainting,&Event);
 	  }
 	  /* continue; */ /* instead of continuing, we want to
@@ -864,7 +861,6 @@
 	      }
 	      FPopupMenu(mrPopup, menu, x, y, fPopupAndWarp, &mops,
 			 &fSubmenuOverlaps);
-	      menu->flags.f.redrawn = 0;
 	    }
 	    if (mrPopup->mrDynamicPrev == menu) {
 	      mi = FindEntry(NULL);
@@ -1068,7 +1064,7 @@
       tolerance1 = 3;
       if (mr->ms->feel.PopupOffsetAdd < 0)
 	tolerance1 -= mr->ms->feel.PopupOffsetAdd;
-      tolerance2 = tolerance1 + 4;
+      tolerance2 = 4;
     }
   else
     {
@@ -1078,11 +1074,14 @@
   XGetGeometry(dpy,mr->w,&JunkRoot,&prior_x,&prior_y,
 	       &prior_width,&prior_height,&JunkBW,&JunkDepth);
   x_overlap = 0;
-  prior_width = prior_width * mr->ms->feel.PopupOffsetPercent / 100;
+  if (fTolerant) {
+    /* Don't use multiplier if doing an intolerant check */
+    prior_width *= mr->ms->feel.PopupOffsetPercent / 100;
+  }
   if (y <= prior_y + prior_height - tolerance2 &&
-      prior_y <= y + height - tolerance1 &&
-      x <= prior_x + prior_width - tolerance2 &&
-      prior_x <= x + width - tolerance1) {
+      prior_y <= y + height - tolerance2 &&
+      x <= prior_x + prior_width - tolerance1 &&
+      prior_x <= x + width - tolerance2) {
     x_overlap = x - prior_x;
     if (x <= prior_x) {
       x_overlap--;
@@ -1118,7 +1117,6 @@
   }
   menu->mrDynamicPrev = menuPrior;
   menu->flags.f.painted = 0;
-  menu->flags.f.redrawn = 0;
   menu->flags.f.is_left = 0;
   menu->flags.f.is_right = 0;
   menu->flags.f.is_up = 0;
@@ -1155,26 +1153,34 @@
 	    menu->width +1;
       }
   } /* if (pops->flags.f.has_poshints) */
-  x_overlap = DoMenusOverlap(menuPrior, x, y,menu->width,menu->height,True);
+fprintf(stderr,"\n\nx=%d-%d, y=%d-%d\n",x,x+menu->width,y,y+menu->height);
+  x_overlap = DoMenusOverlap(menuPrior, x, y, menu->width, menu->height, True);
+fprintf(stderr,"clipping\n");
   /* clip to screen */
   if (x + menu->width > Scr.MyDisplayWidth - 2)
     x = Scr.MyDisplayWidth - 2 - menu->width;
   if (y + menu->height > Scr.MyDisplayHeight)
     y = Scr.MyDisplayHeight - menu->height;
-  if (x < 0) x = 0;
-  if (y < 0) y = 0;
+  if (x < 0)
+    x = 0;
+  if (y < 0)
+    y = 0;
+fprintf(stderr,"x=%d-%d, y=%d-%d\n",x,x+menu->width,y,y+menu->height);
 
   if (menuPrior != NULL) {
     int prev_x, prev_y, left_x, right_x;
     unsigned int prev_width, prev_height;
+    int x_offset;
 
+fprintf(stderr,"has prior\n");
     /* try to find a better place */
     XGetGeometry(dpy,menuPrior->w,&JunkRoot,&prev_x,&prev_y,
 		 &prev_width,&prev_height,&JunkBW,&JunkDepth);
 
     /* check if menus overlap */
-    x_clipped_overlap = DoMenusOverlap(menuPrior, x, y,
-				       menu->width, menu->height, True);
+    x_clipped_overlap = DoMenusOverlap(menuPrior, x, y, menu->width,
+				       menu->height, True);
+fprintf(stderr,"xo=%d, xco=%d\n",x_overlap,x_clipped_overlap);
     if (x_clipped_overlap &&
 	(!(pops->flags.f.has_poshints) ||
 	 pops->pos_hints.fRelative == FALSE || x_overlap == 0)) {
@@ -1183,12 +1189,14 @@
       Bool fDefaultLeft;
       Bool fEmergencyLeft;
 
+fprintf(stderr,"conflict!\n");
+      x_offset = prev_width * menu->ms->feel.PopupOffsetPercent / 100 +
+	menu->ms->feel.PopupOffsetAdd;
       left_x = prev_x - menu->width + 2;
-      right_x = prev_x + prev_width * menu->ms->feel.PopupOffsetPercent / 100;
-      if (menu->ms->feel.PopupOffsetPercent >= 100)
+      right_x = prev_x + x_offset;
+      if (x_offset > prev_width - 2)
 	right_x = prev_x + prev_width - 2;
-      if (x+menu->width < (prev_x + right_x)/2 ||
-	  (x < (prev_x + right_x)/2 && -x > menu->width/2))
+      if (x + menu->width < prev_x + right_x)
 	fDefaultLeft = TRUE;
       else
 	fDefaultLeft = FALSE;
@@ -1197,9 +1205,11 @@
       if (menu->ms->feel.f.Animated) {
 	/* animate previous out of the way */
 	int left_x, right_x, end_x;
-	left_x = x - prev_width * menu->ms->feel.PopupOffsetPercent / 100;
-	if (menu->ms->feel.PopupOffsetPercent >= 100)
-	  left_x = x - prev_width + 3;
+
+fprintf(stderr,"will animate\n");
+	left_x = x - x_offset;
+	if (x_offset >= prev_width)
+	  left_x = x - x_offset + 3;
 	right_x = x + menu->width;
 	if (fDefaultLeft) {
 	  /* popup menu is left of old menu, try to move prior menu right */
@@ -1222,7 +1232,7 @@
 	AnimatedMoveOfWindow(menuPrior->w,prev_x,prev_y,end_x,prev_y,
 			     TRUE, -1, NULL);
       } /* if (menu->ms->feel.f.Animated) */
-      else if ((menu->ms->feel.PopupOffsetPercent >= 100) &&
+      else if (prev_x + x_offset > x + 3 && x_offset + 3 > -menu->width &&
 	       !(pops->flags.f.fixed)) {
 	Bool fLeftIsOK = FALSE;
 	Bool fRightIsOK = FALSE;
@@ -1239,6 +1249,7 @@
 	else
 	  fUseLeft = FALSE;
 	x = (fUseLeft) ? left_x : right_x;
+fprintf(stderr,"lok=%d, rok=%d, ul=%d, fel=%d, lx=%d, rx=%d, x=%d\n", fLeftIsOK,fRightIsOK, fUseLeft, fEmergencyLeft, left_x, right_x, x);
 	/* force the menu onto the screen; prefer to have the left border
 	 * visible if the menu is wider than the screen. But leave at least
 	 * 20 pixels of the parent menu visible */
@@ -1290,10 +1301,11 @@
   /* popup the menu */
   XMoveWindow(dpy, menu->w, x, y);
   XMapRaised(dpy, menu->w);
-  if (ret_overlap)
+  if (ret_overlap) {
     *ret_overlap =
       DoMenusOverlap(menuPrior, x, y, menu->width, menu->height, False) ?
       True : False;
+  }
 
   if (!fWarpItem) {
     mi = FindEntry(NULL);
@@ -1357,7 +1369,7 @@
       if (!mi->mr->flags.f.painted)
       {
 	PaintMenu(mi->mr, NULL);
-	mi->mr->flags.f.redrawn = 1;
+	flush_expose(mi->mr->w);
       }
       iy = mi->y_offset - 2;
       ih = mi->y_height + 4;
@@ -1509,7 +1521,7 @@
     event.xexpose.y = mr_y - parent_y;
     event.xexpose.height = mr_height;
     PaintMenu(parent, &event);
-    parent->flags.f.redrawn = 1;
+    flush_expose(parent->w);
   }
   else
   {

---

From: Dominik Vogt <[email-masked]>
Date: Wed, 27 Jan 1999 02:16:00 +0100

On Wed, Jan 27, 1999 at 02:03:03AM +0100, Dominik Vogt wrote:
> The patch is against the CVS code I checked out
> on 23-Jan, but I don't think that matters since the menu
> code didn't change.

Sorry, not quite correct but sti probably doesn't matter:

I started patching against said CVS code, but the diff
is against the 2.1.10 file (forgot to backup the original
file).

Bye

Dominik

--
Dominik Vogt, [email-masked]
Reply-To: [email-masked]

--
Visit the official FVWM web page at <URL:http://fvwm.math.uh.edu/>.
To unsubscribe from the list, send "unsubscribe fvwm-workers" in the
body of a message to [email-masked].
To report problems, send mail to [email-masked].

---

From: Tim Phipps <[email-masked]>
Date: Wed, 27 Jan 1999 12:27:46 +0000

Dominik Vogt wrote:
>    menus.patchName: menus.patch
>               Type: Plain Text (text/plain)

This fixes the popupoffset/anim problem (bug 147) but something is very
wrong with the menu painting of D/BGradient, symptoms: Dgradients are
not painted at all; I get a mini-icon that belongs to the focussed
window in the top left of the window (not the actual pixmap but the
shape of it drawn in the first colour of the DGradient or the last
colour of the BGradient).

I had to apply the patch by hand as my patch program complained so maybe
I've missed something.  If you can't figure out what's going wrong with
me could you post your complete menus.c (or email it to me).

Anyway, nice work on the overlap stuff.
Cheers,
Tim.

---

From: Dominik Vogt <[email-masked]>
Date: Wed, 27 Jan 1999 18:54:36 +0100

> colour of the BGradient).

Tim, whatever you do, make a copy of that screwed up source tree first.
This is the first time one of the developers sees this. Can you
try to hunt down this bug?

> me could you post your complete menus.c (or email it to me).

Tar file attached. It contains menus.h too since I removed a
now unused flag there.

> Anyway, nice work on the overlap stuff.

That cost me quite some time. I had completely forgotten
to update this for the MenuStyle changes and didn't
understand my logic behind that anymore.

Bye

Dominik

---

From: Tim Phipps <[email-masked]>
Date: Thu, 28 Jan 1999 10:18:50 +0000

> try to hunt down this bug?

No need to make a copy, it happening with today's CVS.  I though the
problem had been seen and fixed. I haven't been paying attention to
gradient stuff as I'm stuck with an 8 bit card and was running
PseudoColor while the Gradients were being developed.  It's only in the
last fortnight that I figured out how to get my apps to work with
TrueColor. I'll try to find out the problem. It smells like the
D/BGradient is being drawn into a pixmap that has accidentally got a
shape from the last mini-icon to be drawn. DGradient 100 black white =
black mini-icon, BGradient 100 black white = grey mini-icon.

Cheers,
Tim.

---

From: Tim Phipps <[email-masked]>
Date: Thu, 28 Jan 1999 10:41:18 +0000

This looks a likely suspect. Line 2054 of menus.c

        else if ( type == DGradMenu )
        {
	  register int i = 0, dc;
	  float ds;
	  int cindex = 0;

	  if( mr->width > mr->height )
          {
	    ds = (float) mr->height / mr->width;
	    dc = mr->width * 2 / ms->look.face.u.grad.npixels + 1;
	    for(i = 0; i < mr->width; i++)
	    {
	      if( i % dc == 0 )
		XSetForeground(dpy, Scr.TransMaskGC,
			       ms->look.face.u.grad.pixels[cindex++]);
	      XDrawLine(dpy, mr->w, Scr.TransMaskGC, 0, i*ds, i, 0);
	    }

What is TransMaskGC?

Cheers,
Tim.

---

From: Tim Phipps <[email-masked]>
Date: Thu, 28 Jan 1999 10:48:39 +0000

A couple of XSetClipMask90s fixes this.  Should I commit?

Cheers,
Tim.

---

From: Tim Phipps <[email-masked]>
Date: Thu, 28 Jan 1999 14:08:25 +0000

I've been looking at the drawing code in menus.c and I'm confused.
What's the consensus for how lines are drawn?  Does XDrawLine(x,y1,x,y2)
draw the pixels at (x,y1) and (x,y2)?  My server doesn't unless I turn
on draft mode. Without draft mode the last pixel in the line is not
drawn.

I've looked at the man page for XCreateGC and I can't work out what it's
supposed to be.  We don't ever specify the CapStyle in a GC so I assume
it defaults to CapNotLast (defined as 0 in X.h). We also use zero width
lines. If so then this bit of the man page looks like it specifies it.

     The cap-style defines how the endpoints of a path are drawn:

     CapNotLast: This is equivalent CapButt except that for a
     line-width of zero the final endpoint is not drawn.

If this is true then I think we have some missing pixel in the menu 3d
code.  I also notice that the Motif widgets are drawn wrong. You'd they
Motif would get it right so maybe my Xserver setting is screwed.

Cheers,
Tim.

---

From: Dominik Vogt <[email-masked]>
Date: Thu, 28 Jan 1999 22:30:58 +0100

On Thu, Jan 28, 1999 at 10:48:39AM +0000, Tim Phipps wrote:
> A couple of XSetClipMask90s fixes this.

What's an XSetClipMask90? A typo?

>  Should I commit?

Of course.

Bye

Dominik

---

From: Tim Phipps <[email-masked]>
Date: Fri, 29 Jan 1999 09:16:19 +0000

> What's an XSetClipMask90? A typo?
Yeah, supposed to be ...().
> Of course.

Done, I applied the cast to int patch too as it's in the same area and
at least one person reports it fixing their problem.  It's the only
place in fvwm that a float is implicitly cast to an int, I didn't check
the modules.  The other conversion warnings are double -> float,
unsigned <-> signed and some different widths.

Cheers,
Tim.