Subject: FvwmButtons patch + Suggestion

From: Remko Troncon <[email-masked]>
Date: Thu, 13 Mar 2003 19:50:51 +0100

Hello FVWM workers,

Enclosed is a patch i proposed a few weeks ago, but which wasn't accepted at
the time because of another bug in FvwmButtons. So, i am reposting it for
revision, and would like it committed. The patch adds a new option to 
FvwmButtons 'Relief <reliefmode>' (with <reliefmode> one of Up, Down, Both),
specifying in which state of the button a relief should be drawn.

Once this is done, I would like to implement two new options for Panel in 
FvwmButtons.
The first one is 'OpenCreates': When a button controlling a panel is
clicked, and the panel does not exist yet, the command of the Panel
is executed to create the panel.
This new option induces another possible option, 'NoStart': When FvwmButtons 
is started or restarted, the 'command' of the panel is _not_ executed. 

I personally have a need for both options, both for a respawning and a non-
respawning panel. Does anyone have any comments on this ? 
The names of the options might not be very good, so suggestions on those 
are also very welcome.

cheers,
Remko

PS: I am not on the fvwm-workers list.

diff -urw fvwm-cvs/modules/FvwmButtons/FvwmButtons.1 fvwm-cvs.patched/modules/FvwmButtons/FvwmButtons.1
--- fvwm-cvs/modules/FvwmButtons/FvwmButtons.1	Sat Nov  9 16:18:52 2002
+++ fvwm-cvs.patched/modules/FvwmButtons/FvwmButtons.1	Mon Feb 10 20:40:10 2003
@@ -169,6 +169,11 @@
 Specifies a background pixmap to use.  Specify "none" (without the
 double quotes) for a transparent background.
 
+.IP "*FvwmButtons: Relief \fImode\fP"
+Specifies when to draw relief around the button. \fImode\fP is either
+\fIUp\fP, \fIDown\fP or \fIBoth\fP for drawing relief only when
+the button is up, down, or in both cases.
+
 .IP "*FvwmButtons: Rows \fIrows\fP"
 Specifies the number of rows of buttons to be created. The default
 is 2 rows.
diff -urw fvwm-cvs/modules/FvwmButtons/FvwmButtons.c fvwm-cvs.patched/modules/FvwmButtons/FvwmButtons.c
--- fvwm-cvs/modules/FvwmButtons/FvwmButtons.c	Sat Mar  8 17:27:23 2003
+++ fvwm-cvs.patched/modules/FvwmButtons/FvwmButtons.c	Sat Mar  8 17:28:31 2003
@@ -780,6 +780,7 @@
 
   UberButton->title   = MyName;
   UberButton->swallow = 1; /* the panel is shown */
+  UberButton->relief  = RELIEF_BOTH; /* By default, show both reliefs */
 
   /* parse module options */
   ParseConfiguration(UberButton);
diff -urw fvwm-cvs/modules/FvwmButtons/FvwmButtons.h fvwm-cvs.patched/modules/FvwmButtons/FvwmButtons.h
--- fvwm-cvs/modules/FvwmButtons/FvwmButtons.h	Tue Oct  1 09:03:29 2002
+++ fvwm-cvs.patched/modules/FvwmButtons/FvwmButtons.h	Mon Feb 10 20:40:10 2003
@@ -89,6 +89,7 @@
 		     0x02000000 /* By default this only done on Popup */
 #define b_Id         0x04000000 /* Has a user defined id for referencing */
 #define b_IconAlpha  0x08000000 /* Icon has an alpha chanel */
+#define b_Relief     0x10000000 /* When to use relief */
 
 /* Flags for b->swallow */
 #define b_Count       0x0003 /* Init counter for swallowing */
@@ -139,6 +140,7 @@
   Pixel bc,hc,sc;          /* b_Back && !b_IconBack */
   FvwmPicture *backicon;   /* b_Back && b_IconBack */
   ushort minx,miny;        /* b_Size */
+  byte relief;             /* b_Relief */
 };
 
 typedef struct
@@ -169,6 +171,10 @@
   char *back;              /* b_Back */
   char *fore;              /* b_Fore */
   int colorset;            /* b_Colorset */
+#define RELIEF_UP 0x1      /* Use relief only in up state */
+#define RELIEF_DOWN 0x2    /* Use relief only in down state */
+#define RELIEF_BOTH 0x3    /* use relief in both states */
+  byte relief;             /* b_Relief */
   byte xpad,ypad;          /* b_Padding */
   signed char framew;      /* b_Frame */
   byte justify;            /* b_Justify */
diff -urw fvwm-cvs/modules/FvwmButtons/button.c fvwm-cvs.patched/modules/FvwmButtons/button.c
--- fvwm-cvs/modules/FvwmButtons/button.c	Tue Oct  1 09:03:29 2002
+++ fvwm-cvs.patched/modules/FvwmButtons/button.c	Mon Feb 10 20:40:10 2003
@@ -309,6 +309,24 @@
   return -1;
 }
 
+byte buttonRelief(button_info *b)
+{
+    button_info *bb = b;
+    if (bb->flags & b_Relief) 
+        return bb->relief;
+    else if (bb->flags & b_Container && bb->c->flags & b_Colorset)
+        return bb->relief;
+    while ((bb = bb->parent))
+    {
+        if (bb->c->flags & b_Relief)
+            return bb->c->relief;
+    }
+#ifdef DEBUG
+    fprintf(stderr,"%s: BUG: No relief definition?\n",MyName);
+#endif
+    return RELIEF_BOTH;
+}
+
 int buttonBackgroundButton(button_info *b, button_info **r_b)
 {
 	button_info *tmpb, *pb;
@@ -515,6 +533,7 @@
     b->c->back=safestrdup("rgb:90/80/90");
     b->c->fore=safestrdup("black");
     b->c->framew=2;
+    b->c->relief=RELIEF_BOTH;
   }
 
 }
diff -urw fvwm-cvs/modules/FvwmButtons/button.h fvwm-cvs.patched/modules/FvwmButtons/button.h
--- fvwm-cvs/modules/FvwmButtons/button.h	Tue Oct  1 09:03:29 2002
+++ fvwm-cvs.patched/modules/FvwmButtons/button.h	Mon Feb 10 20:40:10 2003
@@ -42,6 +42,7 @@
 Pixel buttonHilite(button_info*);
 Pixel buttonShadow(button_info*);
 int buttonColorset(button_info *b);
+byte buttonRelief(button_info *b);
 int buttonBackgroundButton(button_info *b, button_info **r_b);
 byte buttonSwallow(button_info*);
 byte buttonJustify(button_info*);
diff -urw fvwm-cvs/modules/FvwmButtons/draw.c fvwm-cvs.patched/modules/FvwmButtons/draw.c
--- fvwm-cvs/modules/FvwmButtons/draw.c	Fri Feb 14 09:10:13 2003
+++ fvwm-cvs.patched/modules/FvwmButtons/draw.c	Mon Feb 10 20:41:05 2003
@@ -61,30 +61,32 @@
 *** RelieveButton()
 *** Draws the relief pattern around a window.
 **/
-void RelieveButton(Window wn,int width,int x,int y,int w,int h,Pixel relief,
-		   Pixel shadow,int rev)
+void RelieveButton(Window wn,int width,int x,int y,int w,int h,Pixel backgr,
+           Pixel relief, Pixel shadow,int rev,byte relief_flags)
 {
   XGCValues gcv;
-  GC swapGC, reliefGC, shadowGC;
+  GC reliefGC, shadowGC, swapGC;
 
   if(!width)
     return;
 
-  gcv.foreground=relief;
+  if (rev) 
+    gcv.foreground=(relief_flags&0x2 ? shadow : backgr);
+  else 
+    gcv.foreground=(relief_flags&0x1 ? relief : backgr);
   XChangeGC(Dpy,NormalGC,GCForeground,&gcv);
   reliefGC=NormalGC;
 
-  gcv.foreground=shadow;
+  if(rev)
+    gcv.foreground=(relief_flags & 0x2 ? relief : backgr);
+  else
+    gcv.foreground=(relief_flags & 0x1 ? shadow : backgr);
   XChangeGC(Dpy,ShadowGC,GCForeground,&gcv);
   shadowGC=ShadowGC;
 
   if(width<0)
   {
     width=-width;
-    rev = !rev;
-  }
-  if(rev)
-  {
     swapGC=reliefGC;
     reliefGC=shadowGC;
     shadowGC=swapGC;
@@ -353,6 +355,7 @@
 	}
 
 	/* ------------------------------------------------------------------ */
+
 	of = f;
 	f=abs(f);
 
@@ -618,7 +621,8 @@
 	}
 
 	/* relief */
-	RelieveButton(MyWindow,of,x,y,BW,BH,hc,sc,rev ^ rev_xor);
+	RelieveButton(MyWindow,of,x,y,BW,BH,bc,hc,sc,rev ^ rev_xor, 
+                  buttonRelief(b));
 }
 
 /**
diff -urw fvwm-cvs/modules/FvwmButtons/draw.h fvwm-cvs.patched/modules/FvwmButtons/draw.h
--- fvwm-cvs/modules/FvwmButtons/draw.h	Tue Nov  5 13:30:29 2002
+++ fvwm-cvs.patched/modules/FvwmButtons/draw.h	Mon Feb 10 20:40:10 2003
@@ -33,7 +33,7 @@
 #define DRAW_FORCE       3
 #define DRAW_DESK_RELIEF 4
 
-void RelieveButton(Window,int,int,int,int,int,Pixel,Pixel,int);
+void RelieveButton(Window,int,int,int,int,int,Pixel,Pixel,Pixel,int,byte);
 void MakeButton(button_info*);
 void RedrawButton(button_info*, int draw, XEvent *pev);
 void DrawTitle(
diff -urw fvwm-cvs/modules/FvwmButtons/output.c fvwm-cvs.patched/modules/FvwmButtons/output.c
--- fvwm-cvs/modules/FvwmButtons/output.c	Wed Jun 26 15:42:45 2002
+++ fvwm-cvs.patched/modules/FvwmButtons/output.c	Mon Feb 10 20:40:10 2003
@@ -66,6 +66,14 @@
     fprintf(stderr,"Title(%s) ",b->title);
   if(b->flags&b_Icon)
     fprintf(stderr,"Icon(%s,%i) ",b->icon_file,(int)b->IconWin);
+  if(b->flags&b_Relief) {
+    switch(b->relief) {
+        case RELIEF_UP: fprintf(stderr,"Relief(Up) "); break;
+        case RELIEF_DOWN: fprintf(stderr,"Relief(Down) "); break;
+        case RELIEF_BOTH: fprintf(stderr,"Relief(Both) "); break;
+        default: fprintf(stderr,"Relief(ILLEGAL) "); /* Should not happen */
+    }
+  }
   if(b->flags&b_Icon)
     fprintf(stderr,"Panelw(%i) ",(int)b->PanelWin);
   if(b->flags&b_Action)
@@ -128,6 +136,13 @@
     fprintf(stderr,"Frame %i ",b->framew);
   if(b->flags&b_Padding)
     fprintf(stderr,"Padding %i %i ",b->xpad,b->ypad);
+  if(b->flags&b_Relief) {
+    switch(b->relief) {
+        case RELIEF_UP: fprintf(stderr,"Relief Up "); break;
+        case RELIEF_DOWN: fprintf(stderr,"Relief Down "); break;
+        case RELIEF_BOTH: fprintf(stderr,"Relief Both "); break;
+    }
+  }
   if(b->flags&b_Title)
   {
     fprintf(stderr,"Title ");
diff -urw fvwm-cvs/modules/FvwmButtons/parse.c fvwm-cvs.patched/modules/FvwmButtons/parse.c
--- fvwm-cvs/modules/FvwmButtons/parse.c	Thu Mar 13 19:42:19 2003
+++ fvwm-cvs.patched/modules/FvwmButtons/parse.c	Thu Mar 13 19:42:36 2003
@@ -242,6 +242,43 @@
   *ss=s;
 }
 
+static void ParseRelief(char **ss, byte *relief, unsigned long *flags)
+{
+    char *opts[] =
+    {
+      "up",
+      "down",
+      "both",
+      NULL
+    };
+    char *t,*s=*ss;
+    
+    s = trimleft(s);
+    switch (GetTokenIndex(s,opts,0,&s)) 
+    {
+        case 0: /* Up */
+            *relief = RELIEF_UP;
+            *flags |= b_Relief;
+            break;
+        case 1: /* Down */
+            *relief = RELIEF_DOWN;
+            *flags |= b_Relief;
+            break;
+        case 2: /* Both */
+            *relief = RELIEF_BOTH;
+            *flags |= b_Relief;
+            break;
+        default:
+            t = seekright(&s);
+            fprintf(stderr,"%s: Illegal relief option \"%s\"\n",
+                                                        MyName,(t)?t:""); 
+            if(t) 
+                free (t);
+    }
+    if (*s) s++;
+    *ss=s;
+}
+
 /**
 *** ParseSwallow()
 *** Parses the options possible to Swallow
@@ -576,6 +613,7 @@
     "size",
     "boxsize",
     "colorset",
+    "relief",
     NULL
   };
   char *t,*o,*s=*ss;
@@ -748,6 +786,10 @@
       }
       break;
 
+    case 13: /* Relief */
+        ParseRelief(&s,&b->c->relief,&b->c->flags);
+        break;
+
     default:
       t=seekright(&s);
       fprintf(stderr,"%s: Illegal container option \"%s\"\n",MyName,
@@ -1348,6 +1390,7 @@
   return;
 }
 
+
 /**
 *** ParseConfigLine
 **/
@@ -1369,6 +1412,7 @@
     "pixmap",
     "boxsize",
     "colorset",
+    "relief",
     NULL
   };
   int i,j,k;
@@ -1492,6 +1536,11 @@
       ub->c->flags &= ~b_Colorset;
     }
     break;
+ 
+  case 13: /* Relief */ 
+    ParseRelief(&s,&ub->c->relief,&ub->c->flags);
+    break;
+  
   default:
     s = trimleft(s);
     ParseButton(ubb,s);
--
Visit the official FVWM web page at <URL:http://www.fvwm.org/>.
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: [email-masked]
Date: Thu, 13 Mar 2003 22:43:22 +0100

> The names of the options might not be very good, so suggestions on those 

Because we are in feature freeze, no new features are accepted at
the whim of one developer.  However, I agree to apply this patch
if a) someone else proof reads and applies it, and b) my fellow
developers agree.  I thought the patch already went into the code.

[snip]

Bye

Dominik ^_^  ^_^

 --
Dominik Vogt, [email-masked]
Reply-To: [email-masked]
--
Visit the official FVWM web page at <URL:http://www.fvwm.org/>.
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: Mikhael Goikhman <[email-masked]>
Date: Fri, 14 Mar 2003 12:25:03 +0000

> > specifying in which state of the button a relief should be drawn.

I hoped you will redo this using UpRelief and DownRelief options... :)

> > The names of the options might not be very good, so suggestions on those 

I may suggest to do it a bit differently, new options for Panel: Respawn
and SpawnOnOpen. This should achieve the functionality you want, but in a
bit different (and I hope a more intuitive) way,

> developers agree.  I thought the patch already went into the code.

I want to have the functionality in the patch, but the way it is done now
it is not very extendible and as it is now it should be named
"DrawReliefOn <state>", not "Relief <reliefmode>".

Ideally I would like to have more relief styles in the future, like:

  Relief <state> <style>   # state = up/down/both, style=clean/shadow/edge

Or to use "UpRelief/DownRelief <state>" syntax for this.
Additionaly I would like not to encourage a special meaning of negative
Frame value and do it like FvwmIconMan does it. I.e. Frame is taken only
to be positive and we have style=raised/sunk/raisededge/sunkedge values.
By default style for "up" is "raised" and for down is "sunk", like now.

So although I want to have this functionality, I can't vote "yes" yet
for the patch. This needs more discussion so we don't need to change the
syntax in the future.

Regards,
Mikhael.

---

From: Remko Troncon <[email-masked]>
Date: Fri, 14 Mar 2003 14:22:34 +0100

> bit different (and I hope a more intuitive) way,

This is indeed a better alternative. I thought about this alternative
yesterday, but leaving out the Respawn option (since Respawn is default
behaviour, and NoRespawn is an existing option). At the time, i found a
problem with this, but i can't seem to find it anymore. So, is the
Respawn option still needed ?

> "DrawReliefOn <state>", not "Relief <reliefmode>".

I agree.

>   Relief <state> <style>   # state = up/down/both, style=clean/shadow/edge

I'm not sure i understand 'style' completely. I would say style (at 
the moment) would be something of 'raisededge/sunkedge/flat' ?

> for the patch. This needs more discussion so we don't need to change the

Fair enough. I wasn't aware of a similar option in FvwmIconMan. It would
indeed be better to have similar options. If i understand your 'Relief' 
approach above correctly, i will implement it that way.

By the way, can you estimate how long the feature freeze will last ?

cheers,
Remko

---

From: Mikhael Goikhman <[email-masked]>
Date: Fri, 14 Mar 2003 19:32:03 +0000

> Respawn option still needed ?

Yes, Panel already has Respawn (a default), so it is not needed.

As for the second option I don't really worry about the exact name,
SpawnOnOpen seems better to me, but LazyStart or NoStart may do too.

> > I want to have the functionality in the patch, but the way it is done
> > now it is not very extendible and as it is now it should be named

It would be "raisededge/sunkedge" if not the negative Frame values.
With negative frame it only makes sence to have simply "edge", because
negativeness means "sunk".

It may be called "flat", but I used "clean" to make it clear that the N
pixels defined in Frame _are_ used, but bg color is used not sh/hi.

I used "shadow" to name the current plain-3d-effect. If we remove the
special meaning of negative values of Frame, it should be "raised" and
"sunk" instead.

Theoretically there may be a lot of styles, oval-like, gtk-like, qt-like,
sgi-like and so on, but some of them require more colors than just
bg/sh/hi. I would also like to have a relative shadow where the real
image pixels on the border are dimmed/brighten instead of hi/sh (that is
a dinned/brightened version of one color only bg). All these shadow
styles should be done as a library and be shared among modules.

What others think, should we remove the meaning of negative Frame values?
It is not critical to do this change, a user simply gets a raised button;
to get a sunk button he should use this new Relief option(s).

> By the way, can you estimate how long the feature freeze will last ?

Until 2.6.0 is released. :) But there are good chances your features are
approved if you convince developers that they are very wanted/needed and
you tested possible cases throughly.

Regards,
Mikhael.