Add additional style for :box

Add additional style for :box

[Pedro Andres Aranda Gutierrez]

[-- Attachment #1: Type: text/plain, Size: 834 bytes --]

Hi,

Working on including emacs-26 and emacs-27 faces in the default themes, a
thing has come to my mind: "modern" GUI styles tend not to have 3D effects.
So the 'released-button' and 'pressed-button' styles don't help creating
clean faces.

In my case, I redefine the tab-line and I make it inherit from fringe.
I normally add some extra space, so I want to
:inherit fringe :box (:line-width 3 :background <bg in fringe>)

It would be much easier if we had something like this:
:inherit fringe :box (:line-width 3 :style modern)

Any change I make in the fringe style (normally way above in the themes)
would automatically trickle to the tab-line, making theme definition much
more comfortable...

Just my 2*10^6 cents,
/Pedro A.
-- 
Fragen sind nicht da um beantwortet zu werden,
Fragen sind da um gestellt zu werden
Georg Kreisler

[-- Attachment #2: Type: text/html, Size: 1436 bytes --]

Re: Add additional style for :box

[Eli Zaretskii]

> From: Pedro Andres Aranda Gutierrez <paaguti@gmail.com>
> Date: Mon, 7 Dec 2020 17:01:13 +0100
> 
> It would be much easier if we had something like this:
> :inherit fringe :box (:line-width 3 :style modern)

Doesn't ':style nil' do what you want?

Re: Add additional style for :box

[Pedro Andres Aranda Gutierrez]

[-- Attachment #1.1: Type: text/plain, Size: 971 bytes --]

Hi Eli,

unfortunately not. :style nil takes the _fore_ground for the box and I'm
looking for the _back_ground

:style nil
[image: Screenshot-20201208071520-101x110.png]

what I want
[image: Screenshot-20201208071702-126x105.png]

In a general case, if you are deriving from another face, you have to write
the background colour twice: in the face you are deriving from and in the
face :inheri ing the definition. This, any change in the parent face
implies a change in the derived face and that's not exactly convenient...

Best, /PA

On Mon, 7 Dec 2020 at 17:21, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Pedro Andres Aranda Gutierrez <paaguti@gmail.com>
> > Date: Mon, 7 Dec 2020 17:01:13 +0100
> >
> > It would be much easier if we had something like this:
> > :inherit fringe :box (:line-width 3 :style modern)
>
> Doesn't ':style nil' do what you want?
>


-- 
Fragen sind nicht da um beantwortet zu werden,
Fragen sind da um gestellt zu werden
Georg Kreisler

[-- Attachment #1.2: Type: text/html, Size: 1821 bytes --]

[-- Attachment #2: Screenshot-20201208071520-101x110.png --]
[-- Type: image/png, Size: 3286 bytes --]

[-- Attachment #3: Screenshot-20201208071702-126x105.png --]
[-- Type: image/png, Size: 3315 bytes --]

Re: Add additional style for :box

[Eli Zaretskii]

> From: Pedro Andres Aranda Gutierrez <paaguti@gmail.com>
> Date: Tue, 8 Dec 2020 07:21:42 +0100
> Cc: emacs-devel@gnu.org
> 
> unfortunately not. :style nil takes the _fore_ground for the box and I'm looking for the _back_ground
> 
> :style nil
> Screenshot-20201208071520-101x110.png
> 
> what I want
> Screenshot-20201208071702-126x105.png

I'm confused: I thought you wanted to get rid of the 3D appearance,
which is why I suggested to use :style.  But now you seem to be
talking about the colors, and I cannot understand what exactly is the
problem with that -- just change the colors to whatever you want.

> In a general case, if you are deriving from another face, you have to write the background colour twice: in the
> face you are deriving from and in the face :inheri ing the definition. This, any change in the parent face
> implies a change in the derived face and that's not exactly convenient...

If the inheriting face overrides the color of the parent face, there's
no need to change the former when the latter changes.  So here, too, I
don't think I follow you, please elaborate.

Re: Add additional style for :box

[Pedro Andres Aranda Gutierrez]

[-- Attachment #1: Type: text/plain, Size: 2595 bytes --]

Hi Eli,

let's see if a couple of lines of code explain more than a thousand lines
of 'literature'

I use :box to make some parts of the GUI bigger than one display line. For
example my mode-line face is

'(mode-line ((t (:background "#4C566A" :foreground "#88C0D0" :weight bold
:box (:line-width 6 :color "#4C566A")))))

and yes, OK, at the beginning, I could accept to repeat the background
definition inside the :box, however inconvenient it may be.

Now, I have some some situations where I want to have a common look and
feel:

    '(fringe ((,class (:foreground "#007fcf" :background "#afaf9f"))))
    '(line-number ((,class (:inherit fringe))))

But when I want an element to be higher that the normal size, I need to
repeat the background definition in the :box
      `(tab-line ((,class (:inherit fringe :box (:line-width 3 :color
"#afaf9f")))))

Which means that when I change the fringe background, I need to change it
both in fringe and in tab-line to keep everything uniform.
You pointed out :style t but that uses the _fore_ground for the box,
whereas I need the _back_ground (same happens with :style nil)

This is why I was proposing a new :style modern that fills the box with the
background color of the face. How feasible would be my proposal?

Best, /PA

On Tue, 8 Dec 2020 at 16:16, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Pedro Andres Aranda Gutierrez <paaguti@gmail.com>
> > Date: Tue, 8 Dec 2020 07:21:42 +0100
> > Cc: emacs-devel@gnu.org
> >
> > unfortunately not. :style nil takes the _fore_ground for the box and I'm
> looking for the _back_ground
> >
> > :style nil
> > Screenshot-20201208071520-101x110.png
> >
> > what I want
> > Screenshot-20201208071702-126x105.png
>
> I'm confused: I thought you wanted to get rid of the 3D appearance,
> which is why I suggested to use :style.  But now you seem to be
> talking about the colors, and I cannot understand what exactly is the
> problem with that -- just change the colors to whatever you want.
>
> > In a general case, if you are deriving from another face, you have to
> write the background colour twice: in the
> > face you are deriving from and in the face :inheri ing the definition.
> This, any change in the parent face
> > implies a change in the derived face and that's not exactly convenient...
>
> If the inheriting face overrides the color of the parent face, there's
> no need to change the former when the latter changes.  So here, too, I
> don't think I follow you, please elaborate.
>


-- 
Fragen sind nicht da um beantwortet zu werden,
Fragen sind da um gestellt zu werden
Georg Kreisler

[-- Attachment #2: Type: text/html, Size: 5333 bytes --]

Re: Add additional style for :box

[Pedro Andres Aranda Gutierrez]

[-- Attachment #1.1: Type: text/plain, Size: 3214 bytes --]

Hi Eli,

attached is a small patch with a proof of concept for the master branch
ONLY TESTED ON LINUX.

With this I can define my tab-line as

'(tab-line ((t (:inherit fringe :box (:line-width (5 . 5) :style
modern-button))))))

And changes in the fringe are taken *automatically* by the tabline.

Best, /Pedro A. Aranda

On Tue, 8 Dec 2020 at 18:09, Pedro Andres Aranda Gutierrez <
paaguti@gmail.com> wrote:

> Hi Eli,
>
> let's see if a couple of lines of code explain more than a thousand lines
> of 'literature'
>
> I use :box to make some parts of the GUI bigger than one display line.
> For example my mode-line face is
>
> '(mode-line ((t (:background "#4C566A" :foreground "#88C0D0" :weight bold
> :box (:line-width 6 :color "#4C566A")))))
>
> and yes, OK, at the beginning, I could accept to repeat the background
> definition inside the :box, however inconvenient it may be.
>
> Now, I have some some situations where I want to have a common look and
> feel:
>
>     '(fringe ((,class (:foreground "#007fcf" :background "#afaf9f"))))
>     '(line-number ((,class (:inherit fringe))))
>
> But when I want an element to be higher that the normal size, I need to
> repeat the background definition in the :box
>       `(tab-line ((,class (:inherit fringe :box (:line-width 3 :color
> "#afaf9f")))))
>
> Which means that when I change the fringe background, I need to change it
> both in fringe and in tab-line to keep everything uniform.
> You pointed out :style t but that uses the _fore_ground for the box,
> whereas I need the _back_ground (same happens with :style nil)
>
> This is why I was proposing a new :style modern that fills the box with
> the background color of the face. How feasible would be my proposal?
>
> Best, /PA
>
> On Tue, 8 Dec 2020 at 16:16, Eli Zaretskii <eliz@gnu.org> wrote:
>
>> > From: Pedro Andres Aranda Gutierrez <paaguti@gmail.com>
>> > Date: Tue, 8 Dec 2020 07:21:42 +0100
>> > Cc: emacs-devel@gnu.org
>> >
>> > unfortunately not. :style nil takes the _fore_ground for the box and
>> I'm looking for the _back_ground
>> >
>> > :style nil
>> > Screenshot-20201208071520-101x110.png
>> >
>> > what I want
>> > Screenshot-20201208071702-126x105.png
>>
>> I'm confused: I thought you wanted to get rid of the 3D appearance,
>> which is why I suggested to use :style.  But now you seem to be
>> talking about the colors, and I cannot understand what exactly is the
>> problem with that -- just change the colors to whatever you want.
>>
>> > In a general case, if you are deriving from another face, you have to
>> write the background colour twice: in the
>> > face you are deriving from and in the face :inheri ing the definition.
>> This, any change in the parent face
>> > implies a change in the derived face and that's not exactly
>> convenient...
>>
>> If the inheriting face overrides the color of the parent face, there's
>> no need to change the former when the latter changes.  So here, too, I
>> don't think I follow you, please elaborate.
>>
>
>
> --
> Fragen sind nicht da um beantwortet zu werden,
> Fragen sind da um gestellt zu werden
> Georg Kreisler
>


-- 
Fragen sind nicht da um beantwortet zu werden,
Fragen sind da um gestellt zu werden
Georg Kreisler

[-- Attachment #1.2: Type: text/html, Size: 6453 bytes --]

[-- Attachment #2: modern-box.diff --]
[-- Type: text/x-patch, Size: 3793 bytes --]

diff --git a/lisp/cus-face.el b/lisp/cus-face.el
index 199a76e5cc..5e30590c6f 100644
--- a/lisp/cus-face.el
+++ b/lisp/cus-face.el
@@ -175,6 +175,7 @@ custom-face-attributes
 		   (choice :tag "Style"
 			   (const :tag "Raised" released-button)
 			   (const :tag "Sunken" pressed-button)
+			   (const :tag "Modern" modern-button)
 			   (const :tag "None" nil))))
      ;; filter to make value suitable for customize
      (lambda (real-value)
diff --git a/src/dispextern.h b/src/dispextern.h
index da51772b37..d23551c0b7 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -1628,7 +1628,11 @@ #define FONT_TOO_HIGH(ft)						\
   /* Boxes with 3D shadows.  Color equals the background color of the
      face.  Width is specified.  */
   FACE_RAISED_BOX,
-  FACE_SUNKEN_BOX
+  FACE_SUNKEN_BOX,
+
+  /* 'Modern box: like simple box,
+     but using the *back*ground colour of the face */
+  FACE_MODERN_BOX
 };
 
 /* Underline type. */
@@ -1709,8 +1713,11 @@ #define FONT_TOO_HIGH(ft)						\
      around text in this face.  A value of FACE_SIMPLE_BOX means a box
      of width box_line_width is drawn in color box_color.  A value of
      FACE_RAISED_BOX or FACE_SUNKEN_BOX means a 3D box is drawn with
-     shadow colors derived from the background color of the face.  */
-  ENUM_BF (face_box_type) box : 2;
+     shadow colors derived from the background color of the face.
+     A value of FACE_MODERN_BOX will create a FACE_SIMPLE_BOX drawn
+     with the background colour of the face.
+  */
+  ENUM_BF (face_box_type) box : 3;
 
   /* Style of underlining. */
   ENUM_BF (face_underline_type) underline : 2;
diff --git a/src/xfaces.c b/src/xfaces.c
index 73a536b19c..4b5289f65f 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -3293,7 +3293,7 @@ DEFUN ("internal-set-lisp-face-attribute", Finternal_set_lisp_face_attribute,
 		}
 	      else if (EQ (k, QCstyle))
 		{
-		  if (!EQ (v, Qpressed_button) && !EQ (v, Qreleased_button))
+		    if (!EQ (v, Qpressed_button) && !EQ (v, Qreleased_button) && !EQ(v, Qmodern_button))
 		    break;
 		}
 	      else
@@ -6031,6 +6031,8 @@ realize_gui_face (struct face_cache *cache, Lisp_Object attrs[LFACE_VECTOR_SIZE]
 		face->box = FACE_RAISED_BOX;
 	      else if (EQ (value, Qpressed_button))
 		face->box = FACE_SUNKEN_BOX;
+	      else if (EQ (value, Qmodern_button))
+		face->box = FACE_MODERN_BOX;
 	    }
 	}
     }
@@ -6919,6 +6921,7 @@ syms_of_xfaces (void)
   DEFSYM (Qwave, "wave");
   DEFSYM (Qreleased_button, "released-button");
   DEFSYM (Qpressed_button, "pressed-button");
+  DEFSYM (Qmodern_button, "modern-button");
   DEFSYM (Qnormal, "normal");
   DEFSYM (Qextra_light, "extra-light");
   DEFSYM (Qlight, "light");
diff --git a/src/xterm.c b/src/xterm.c
index 0d2452de92..10580fa003 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -3005,16 +3005,26 @@ x_draw_glyph_string_box (struct glyph_string *s)
 
   get_glyph_string_clip_rect (s, &clip_rect);
 
-  if (s->face->box == FACE_SIMPLE_BOX)
-    x_draw_box_rect (s, left_x, top_y, right_x, bottom_y, hwidth,
-		     vwidth, left_p, right_p, &clip_rect);
-  else
-    {
+  switch(s->face->box) {
+  case FACE_SIMPLE_BOX:
+      x_draw_box_rect (s, left_x, top_y, right_x, bottom_y, hwidth,
+		       vwidth, left_p, right_p, &clip_rect);
+      break;
+  case FACE_MODERN_BOX:
+      {
+	  unsigned long store_color = s->face->box_color;
+	  s->face->box_color = s->face->background;
+	  x_draw_box_rect (s, left_x, top_y, right_x, bottom_y, hwidth,
+			   vwidth, left_p, right_p, &clip_rect);
+	  s->face->box_color = store_color;
+      }
+      break;
+  default:
       x_setup_relief_colors (s);
       x_draw_relief_rect (s->f, left_x, top_y, right_x, bottom_y, hwidth,
 			  vwidth, raised_p, true, true, left_p, right_p,
 			  &clip_rect);
-    }
+  }
 }