Subject: Re: FVWM: Bug in 2.1.7 and 2.1.10 ?

From: [email-masked] (Paul D. Smith)
Date: 04 Feb 1999 12:14:17 -0500

%% Dan Espen <[email-masked]> writes:

  >   Depth: 1

  de> The logic in builtins.c that checks for a depth of <2
  de> when creating the shadow and relief GCs is gone.

  de> Any volunteers?

Personally, I just can't see this change making it into 2.2.0 at this
late date :(.  I'm still hopeful that 2.1.11 will be a successful
release candidate.

I think if someone fixes it and posts a patch, then if necessary we can
release it in 2.2.1.

Thoughts?  Opinions?

-- 
-------------------------------------------------------------------------------
 Paul D. Smith <[email-masked]>         Network Management Development
 "Please remain calm...I may be mad, but I am a professional." --Mad Scientist
-------------------------------------------------------------------------------
   These are my opinions---Nortel Networks takes no responsibility for them.
--
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: Dan Espen <[email-masked]>
Date: Thu, 04 Feb 1999 13:50:31 -0500

> I think if someone fixes it and posts a patch, then if necessary we can

I don't know if this fixes all the problems, but so far,
it looks like a safe patch to me:

*** builtins.c.orig	Thu Feb  4 13:43:30 1999
--- builtins.c	Thu Feb  4 13:47:12 1999
***************
*** 1691,1698 ****
      ms->look.MenuActiveColors.back = ms->look.MenuColors.back;
    if (!ms->look.f.hasStippleFore)
      ms->look.MenuStippleColors.fore = ms->look.MenuColors.back;
!   ms->look.MenuRelief.back = GetShadow(ms->look.MenuColors.back);
!   ms->look.MenuRelief.fore = GetHilite(ms->look.MenuColors.back);
    ms->look.MenuStippleColors.back = ms->look.MenuColors.back;
  
    /* make GC's */
--- 1691,1703 ----
      ms->look.MenuActiveColors.back = ms->look.MenuColors.back;
    if (!ms->look.f.hasStippleFore)
      ms->look.MenuStippleColors.fore = ms->look.MenuColors.back;
!   if(Scr.d_depth > 2) {                 /* if not black and white */
!     ms->look.MenuRelief.back = GetShadow(ms->look.MenuColors.back);
!     ms->look.MenuRelief.fore = GetHilite(ms->look.MenuColors.back);
!   } else {                              /* black and white */
!     ms->look.MenuRelief.back = GetColor("black");
!     ms->look.MenuRelief.fore = GetColor("white");
!   }
    ms->look.MenuStippleColors.back = ms->look.MenuColors.back;
  
    /* make GC's */

> -- 
>  Paul D. Smith <[email-masked]>         Network Management Developmen

--
Dan Espen
444 Hoes Lane  Room RRC 1C-214           E-mail: [email-masked]
Piscataway, NJ 08854                     Phone: (732) 699-5570
--
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: [email-masked] (Paul D. Smith)
Date: 04 Feb 1999 15:07:22 -0500

de> !     ms->look.MenuRelief.back = GetColor("black");
  de> !     ms->look.MenuRelief.fore = GetColor("white");

Shouldn't this depend on the foreground/background color?  What if
someone used reverse-video (white as their foreground and black as the
background--blech, but... :)?

Or would this be right anyway?

---

From: Dan Espen <[email-masked]>
Date: Thu, 04 Feb 1999 15:17:26 -0500

> Or would this be right anyway?

Beats me.

What  I did was grab  a little of  the original logic.  There was some
more color  allocation logic which I  blissfully ignored.  Perhaps not
the most intelligent thing...

Heres the original:

  if(Scr.d_depth > 2)
  {
    if(fore != NULL)
    {
      Scr.MenuColors.fore = GetColor(fore);
    }
    if(back != NULL)
    {
      Scr.MenuColors.back = GetColor(back);
    }
    Scr.MenuRelief.back  = GetShadow(Scr.MenuColors.back);
    Scr.MenuRelief.fore  = GetHilite(Scr.MenuColors.back);
    Scr.MenuStippleColors.back = Scr.MenuColors.back;
    Scr.MenuStippleColors.fore = GetColor(stipple); 
  }
  else
  {
    Scr.MenuColors.back  = GetColor(white);
    Scr.MenuColors.fore  = GetColor(black); 
    Scr.MenuRelief.back  = GetColor(black);
    Scr.MenuRelief.fore  = GetColor(white);
    Scr.MenuStippleColors.back = GetColor(white);
    Scr.MenuStippleColors.fore = GetColor(black);
  }

Its the last 6 lines that got lost.

It looks like I missed the stipple, (grayed out items).
I would guess that the menu colors themselves are OK as is,
probably as a side effect, since they weren't mentioned.

I was trying to create as small a patch as possible.

---

From: Sam Krishnabhat <[email-masked]>
Date: Fri, 5 Feb 1999 10:19:27 -0800 (PST)

> it looks like a safe patch to me:

OK. I got the CVS version yesterday, looks like the fix was
already in there. Anyway, it did not work. Change was only in UpdateMenuStyle
function, shouldn't it also be in NewMenStyle or other related routines.
I can see this "depth > 2" in few other places too.

-sam

---

From: [email-masked] (Paul D. Smith)
Date: 05 Feb 1999 14:23:01 -0500

>> it looks like a safe patch to me:

  sk> OK. I got the CVS version yesterday, looks like the fix was
  sk> already in there.

This patch has _NOT_ been committed.  I'm looking at the latest source
and it's not there.

Are you sure you're looking at and building with the right code base?
The file size of the latest builtings.c is 119766 bytes, FYI.  How did
you do your checkout from CVS?

---

From: Sam Krishnabhat <[email-masked]>
Date: Fri, 5 Feb 1999 11:54:04 -0800 (PST)

> you do your checkout from CVS?

I followed the instructions on the fvwm webpage for CVS download.

-rw-rw-rw-   1 samk     eng       119766 Feb  4 12:07 builtins.c

-sam

---

From: Dan Espen <[email-masked]>
Date: Fri, 05 Feb 1999 15:07:07 -0500

> -rw-rw-rw-   1 samk     eng       119766 Feb  4 12:07 builtins.c

The size would be 119974 with the patch I posted applied.

---

From: [email-masked] (Paul D. Smith)
Date: 05 Feb 1999 15:07:57 -0500

>> This patch has _NOT_ been committed.  I'm looking at the latest source

  sk> I followed the instructions on the fvwm webpage for CVS download.

  sk> -rw-rw-rw-   1 samk     eng       119766 Feb  4 12:07 builtins.c

Hmm.  So, can you paste the portion of the code that shows the patch
already in there?

My version of builtins.c around line 1691 still looks like this (the
"before" part of the patch):

    /* make GC's */

Not like this (the "after" part of the patch):

    /* make GC's */