GNU bug report logs - #35468
[PATCH] Refactor draw_glyph_string on X and w32

Please note: This is a static page, with minimal formatting, updated once a day.
Click here to see this page with the latest information and nicer formatting.

Package: emacs; Reported by: Alex Gramiak <agrambot@HIDDEN>; Keywords: patch; dated Sun, 28 Apr 2019 01:48:03 UTC; Maintainer for emacs is bug-gnu-emacs@HIDDEN.

Message received at 35468 <at> debbugs.gnu.org:


Received: (at 35468) by debbugs.gnu.org; 4 May 2019 08:17:46 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Sat May 04 04:17:46 2019
Received: from localhost ([127.0.0.1]:50319 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1hMprd-0006K3-Pg
	for submit <at> debbugs.gnu.org; Sat, 04 May 2019 04:17:46 -0400
Received: from eggs.gnu.org ([209.51.188.92]:54518)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <eliz@HIDDEN>) id 1hMprb-0006Jr-DY
 for 35468 <at> debbugs.gnu.org; Sat, 04 May 2019 04:17:44 -0400
Received: from fencepost.gnu.org ([2001:470:142:3::e]:47233)
 by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from <eliz@HIDDEN>)
 id 1hMprW-0006fN-7e; Sat, 04 May 2019 04:17:38 -0400
Received: from [176.228.60.248] (port=2248 helo=home-c4e4a596f7)
 by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256)
 (Exim 4.82) (envelope-from <eliz@HIDDEN>)
 id 1hMprV-0000tS-LH; Sat, 04 May 2019 04:17:38 -0400
Date: Sat, 04 May 2019 11:17:21 +0300
Message-Id: <838svmmy5q.fsf@HIDDEN>
From: Eli Zaretskii <eliz@HIDDEN>
To: Alex Gramiak <agrambot@HIDDEN>
In-reply-to: <878svomynv.fsf@HIDDEN> (message from Alex Gramiak on Thu, 02
 May 2019 13:41:56 -0600)
Subject: Re: bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
References: <877ebeor2d.fsf@HIDDEN> <83tveit5ph.fsf@HIDDEN>
 <87pnp5oqu1.fsf@HIDDEN> <877ebcogg4.fsf@HIDDEN>
 <83sgu0rsue.fsf@HIDDEN> <8736lznzjf.fsf@HIDDEN>
 <83zho6ox5u.fsf@HIDDEN> <878svomynv.fsf@HIDDEN>
X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]
X-Spam-Score: -2.3 (--)
X-Debbugs-Envelope-To: 35468
Cc: 35468 <at> debbugs.gnu.org
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -3.3 (---)

> From: Alex Gramiak <agrambot@HIDDEN>
> Cc: 35468 <at> debbugs.gnu.org
> Date: Thu, 02 May 2019 13:41:56 -0600

Sorry, it took slightly more than "a few hours".

> >> As for the color manipulation, I went low-level as you said before:
> >> 
> >>   unsigned long foreground, background;
> >>   gdif->get_context_foreground (gc, &foreground);
> >>   gdif->get_context_background (gc, &background);
> >>   gdif->set_context_foreground (gc, background);
> >>   gdif->fill_rectangle (s, gc, x, y, w, h);
> >>   gdif->set_context_foreground (gc, foreground);
> >> 
> >> which replaces the XGetGCValues section in x_draw_stretch_glyph_string.
> >> This unfortunately is more work in the w32 case as it manipulates s->gc
> >> instead of just using the calculated gc->background.
> >
> > I don't think I understand what you mean by "manipulates s->gc".  Can
> > you show the code which does that?
> 
> I just meant that it does something like:
> 
>   unsigned long foreground = s->gc->foreground;
>   unsigned long background = s->gc->background;
>   s->gc->foreground = background;
>   fill_rectangle (...)
>   s->gc->foreground = foreground;
> 
> Where previously the foreground didn't need to be saved/restored in the
> w32 implementation.

First, it should be possible to have just 1 interface for getting both
colors, with the semantics that if one of the pointers is NULL, the
corresponding color is not required.  I'd call this interface
select_colors.  If w32 doesn't need that, we could either (1) omit
implementing such an interface for w32 (if it's _never_ needed there),
or (2) not call it when the frame is a w32 GUI frame.  So I see no
problems in this part.

> >> If that is unsatisfactory), then instead I could do something like:
> >> 
> >>   gdif->fill_rectangle_with_color (s, gc->background, gc, x, y, w, h);
> >> 
> >> Which wouldn't touch s->gc for the w32 version and would do the whole
> >> XGetGCValues dance for the X version.
> >
> > So fill_rectangle will fill with the current background and
> > fill_rectangle_with_color will fill with the specified color?  It's
> > possible, yes.
> 
> Possible, yes, but would you prefer that over the above?

I think so, yes.  And it sounds like Mituharu-san prefers something
like that as well.

> >> 4) The w32 versions of some procedures need to save the font around the
> >> calls to font->driver->draw; is this necessary?
> >
> > Yes.  The X 'draw' methods accept the font as an argument, but the w32
> > implementation relies on setting the font outside of the 'draw' method
> > because the 'draw' method draws using the "currently selected font".
> > Then we need to restore the original font.
> 
> Where do the X 'draw' methods accept the font as an argument? Looking
> at, e.g., *_draw_glyph_string_foreground, font->driver->draw takes the
> same arguments.

The way I wrote it was confusing: by the 'draw' method I actually
meant the external APIs called by the 'draw' method, like
XftDrawGlyphs.  Compare that with w32's ExtTextOutW in w32font_draw.

> Since font->driver->draw takes in the glyph string, why can't the 'draw'
> methods use SelectObject (s->hdc, FONT_HANDLER (s->font)) and
> SelectObject (s->hdc, old_font)?
> 
> If they can't, is there any other way to do it inside the draw methods?

The draw method doesn't know if it's the last call with a given font.
Only the caller knows that.  So the caller knows when it needs to
restore the previous font.

> I'm having trouble with *_draw_image_foreground -- they just seem too
> different to nicely abstract. Would it be okay if some generic
> constructs leak into it (namely: s->img->mask)? Otherwise the common
> setup that w32 does would be problematic.

I don't think I understand the difficulties, sorry.  Why is
s->img->mask a problem?

In any case, it's not 100% "verboten" for platform-specific code to
look at the internals of 'struct glyph_string', if the interface needs
many different members of that struct.  Avoiding this is just a
general rule, which makes it easier to implement generic interfaces
that will fit future uses.  However draw_image (which, btw, I'd call
draw_image_foreground) looks specialized enough to be exempt of that
rule.

> Does the RestoreDC/SaveDC call need to be around both toplevel branches
> (i.e., also around the w32_draw_rectangle), or just the s->img->pixmap
> branch?

It doesn't look like the call to w32_draw_rectangle needs that, as the
code doesn't do that elsewhere.

> For reference, this is what I have right now for
> gui_draw_image_foreground:
> 
>   static void
>   gui_draw_image_foreground (struct glyph_string *s)
>   {
>     struct graphical_drawing_interface *gdif = FRAME_GDIF (s->f);
>     int x = s->x;
>     int y = s->ybase - image_ascent (s->img, s->face, &s->slice);
> 
>     /* If first glyph of S has a left box line, start drawing it to the
>        right of that line.  */
>     if (s->face->box != FACE_NO_BOX
>         && s->first_glyph->left_box_line_p
>         && s->slice.x == 0)
>       x += eabs (s->face->box_line_width);
> 
>     /* If there is a margin around the image, adjust x- and y-position
>        by that margin.  */
>     if (s->slice.x == 0)
>       x += s->img->hmargin;
>     if (s->slice.y == 0)
>       y += s->img->vmargin;
> 
>     if (gdif->save_secondary_context)
>       gdif->save_secondary_context (s, CON_ALL); // SaveDC (s->hdc);
> 
>     if (gdif->glyph_has_image (s))

What details does glyph_has_image hide?  Is that just to test
s->img->pixmap?

>       {
>         gdif->draw_image (s, s->img->width, s->img->height,
>                           s->slice.x, s->slice.y, s->slice.width, s->slice.height,
>                           x, y, true);
>         if (!gdif->glyph_image_uses_mask (s))

And what does glyph_image_uses_mask hide?  AFAIU, the current code
simply looks at s->img->mask, and if so, why do we need an interface
for that?

Thanks.




Information forwarded to bug-gnu-emacs@HIDDEN:
bug#35468; Package emacs. Full text available.

Message received at 35468 <at> debbugs.gnu.org:


Received: (at 35468) by debbugs.gnu.org; 4 May 2019 04:00:22 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Sat May 04 00:00:22 2019
Received: from localhost ([127.0.0.1]:50149 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1hMlqW-0006OQ-VP
	for submit <at> debbugs.gnu.org; Sat, 04 May 2019 00:00:22 -0400
Received: from mathmail.math.s.chiba-u.ac.jp ([133.82.132.2]:61528)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <mituharu@HIDDEN>) id 1hMlqS-0006OA-0C
 for 35468 <at> debbugs.gnu.org; Sat, 04 May 2019 00:00:16 -0400
Received: from weber.math.s.chiba-u.ac.jp (weber [192.168.32.4])
 by mathmail.math.s.chiba-u.ac.jp (Postfix) with ESMTP id 3FF10F08E7;
 Sat,  4 May 2019 13:00:14 +0900 (JST)
 (envelope-from mituharu@HIDDEN)
Received: from 180.3.209.86 (SquirrelMail authenticated user mituharu)
 by weber.math.s.chiba-u.ac.jp with HTTP;
 Sat, 4 May 2019 13:00:14 +0900
Message-ID: <db923d34f59cc702c37729cd130b2076.squirrel@HIDDEN>
In-Reply-To: <1e1cbd7fdb42983a334d32e6675e31f0.squirrel@HIDDEN>
References: <877ebeor2d.fsf@HIDDEN> <83tveit5ph.fsf@HIDDEN>
 <87pnp5oqu1.fsf@HIDDEN> <877ebcogg4.fsf@HIDDEN>
 <83sgu0rsue.fsf@HIDDEN> <8736lznzjf.fsf@HIDDEN>
 <c560b2f02aea5b22d56d3989e3588508.squirrel@HIDDEN>
 <877eb7l5vl.fsf@HIDDEN>
 <1e1cbd7fdb42983a334d32e6675e31f0.squirrel@HIDDEN>
Date: Sat, 4 May 2019 13:00:14 +0900
Subject: Re: bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
From: mituharu@HIDDEN
To: "Alex Gramiak" <agrambot@HIDDEN>,
 35468 <at> debbugs.gnu.org
User-Agent: SquirrelMail/1.4.22-5.el6
MIME-Version: 1.0
Content-Type: text/plain;charset=iso-2022-jp
Content-Transfer-Encoding: 8bit
X-Priority: 3 (Normal)
Importance: Normal
X-Spam-Score: 0.0 (/)
X-Debbugs-Envelope-To: 35468
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -1.0 (-)

> Because of the reasons I mentioned above, I'd prefer
> erase_rectangle to fill_rectangle_with_color/stipple.  Also, many
> use cases in the Mac port involve erasure of multiple rectangles,
> so erase_rectangles (multi-rectangle version like
> XFillRectangles) would eliminate some overhead of setting up
> colors and the clipping area.

Sorry, there weren't so many occurrence of multi-rectangle
filling/erasing.  Maybe I counted them wrong previously.

				     YAMAMOTO Mitsuharu
				mituharu@HIDDEN






Information forwarded to bug-gnu-emacs@HIDDEN:
bug#35468; Package emacs. Full text available.

Message received at 35468 <at> debbugs.gnu.org:


Received: (at 35468) by debbugs.gnu.org; 3 May 2019 21:33:09 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Fri May 03 17:33:09 2019
Received: from localhost ([127.0.0.1]:49801 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1hMfno-0005mT-Vx
	for submit <at> debbugs.gnu.org; Fri, 03 May 2019 17:33:09 -0400
Received: from mathmail.math.s.chiba-u.ac.jp ([133.82.132.2]:61735)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <mituharu@HIDDEN>) id 1hMfnm-0005mH-A4
 for 35468 <at> debbugs.gnu.org; Fri, 03 May 2019 17:33:07 -0400
Received: from weber.math.s.chiba-u.ac.jp (weber [192.168.32.4])
 by mathmail.math.s.chiba-u.ac.jp (Postfix) with ESMTP id 38CA3F08E1;
 Sat,  4 May 2019 06:33:00 +0900 (JST)
 (envelope-from mituharu@HIDDEN)
Received: from 180.3.209.86 (SquirrelMail authenticated user mituharu)
 by weber.math.s.chiba-u.ac.jp with HTTP;
 Sat, 4 May 2019 06:33:05 +0900
Message-ID: <1e1cbd7fdb42983a334d32e6675e31f0.squirrel@HIDDEN>
In-Reply-To: <877eb7l5vl.fsf@HIDDEN>
References: <877ebeor2d.fsf@HIDDEN> <83tveit5ph.fsf@HIDDEN>
 <87pnp5oqu1.fsf@HIDDEN> <877ebcogg4.fsf@HIDDEN>
 <83sgu0rsue.fsf@HIDDEN> <8736lznzjf.fsf@HIDDEN>
 <c560b2f02aea5b22d56d3989e3588508.squirrel@HIDDEN>
 <877eb7l5vl.fsf@HIDDEN>
Date: Sat, 4 May 2019 06:33:05 +0900
Subject: Re: bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
From: mituharu@HIDDEN
To: "Alex Gramiak" <agrambot@HIDDEN>
User-Agent: SquirrelMail/1.4.22-5.el6
MIME-Version: 1.0
Content-Type: text/plain;charset=iso-2022-jp
Content-Transfer-Encoding: 8bit
X-Priority: 3 (Normal)
Importance: Normal
X-Spam-Score: 0.0 (/)
X-Debbugs-Envelope-To: 35468
Cc: Eli Zaretskii <eliz@HIDDEN>, 35468 <at> debbugs.gnu.org
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -1.0 (-)

> mituharu@HIDDEN writes:
>
>> Unlike the NS port, the terminal code in the Mac port is much like the
>> X11
>> version.
>
> I looked at your code and fortunately it indeed looks like the same
> abstractions for glyph drawing that would work for the X11 version would
> work for the Mac port.

I deliberately made the Mac port code aligned to the X11 version
for the consistency in behavior as well as the ease of catching
up upstream changes.

> Why does it use the background color of GC? It appears that the
> Cairo version uses the foreground color of GC; is that because the Cairo
> version doesn't handle stippling?

I wrote both Mac and cairo versions, and I used the foreground
color for cairo (although I already used the background color on
the Mac port) to minimize the changes from the Xlib version
because it was incorporated into xterm.c.

Actually the stipple bitmap support for the Mac port was added
very recently.  It had been missing in the Mac port for more than
a decade because I also thought it is an outdated thing and not
worth implemented.  But I changed my mind by looking at an
unexpected use of stipple bitmaps:

  https://lists.gnu.org/r/emacs-devel/2019-03/msg00502.html

So the use of the background color of GC on the Mac port is not
for stipple bitmaps (at least historically) but mainly for the
following reasons:

  1. It can avoid XGetGCValues dance you mentioned without the
     drastic changes in the code that may make catching up
     mainstream difficult.
  2. There are multiple cases that "filling with the background
     color" is useful.
  3. The Mac port supports "vibrancy" (a frosted-glass-like
     translucency effect) only for the background color.  So
     filling with the background color is implemented differently
     from doing that with the foreground color on the Mac
     port (see the macro CG_CONTEXT_FILL_RECT_WITH_GC_BACKGROUND
     in macterm.h).  It is regarded as "special stipples" and
     actually controlled by the stipple attibute of faces:
     https://bitbucket.org/mituharu/emacs-mac/src/df827786d7a7fb0a0e2f27577af67e32d9a888a9/doc/emacs/macport.texi#lines-519
     In retrospect, it is natural to associate "stipple"
     operations with "erasing" (filling with the background
     color) in this sense.

>> I guess introducing the erase_rectagle handler makes things simpler and
>> efficient.
>
> Looking over the code, and considering that stippling is quite uncommon
> nowadays (GTK 3 removed it, AFAIK), I think the best approach would be
> to define 3 separate interface procedures:
>
> 1) fill_rectangle: handles x_fill_rectangle/mac_fill_rectangle
>
> 2) fill_rectangle_with_color: handles (1), but also with the temporary
> overriding of the GC that's done frequently for mac_erase_rectangle.
>
> 3) fill_rectangle_with_stipple: handles the temporary XFillStyle
> overriding of the GC.
>
> A third procedure wouldn't be inelegant in comparison to the
> alternatives, since otherwise I would need a set_context_stipple
> procedure to indicate stippling.
>
> Then mac_fill_rectangle can be assigned to fill_rectangle, and
> mac_erase_rectangle can be assigned to fill_rectangle_with_stipple.

Because of the reasons I mentioned above, I'd prefer
erase_rectangle to fill_rectangle_with_color/stipple.  Also, many
use cases in the Mac port involve erasure of multiple rectangles,
so erase_rectangles (multi-rectangle version like
XFillRectangles) would eliminate some overhead of setting up
colors and the clipping area.

				     YAMAMOTO Mitsuharu
				mituharu@HIDDEN





Information forwarded to bug-gnu-emacs@HIDDEN:
bug#35468; Package emacs. Full text available.

Message received at 35468 <at> debbugs.gnu.org:


Received: (at 35468) by debbugs.gnu.org; 3 May 2019 21:12:48 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Fri May 03 17:12:48 2019
Received: from localhost ([127.0.0.1]:49781 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1hMfU7-0005IA-R4
	for submit <at> debbugs.gnu.org; Fri, 03 May 2019 17:12:48 -0400
Received: from mail-wm1-f48.google.com ([209.85.128.48]:37452)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <athird@HIDDEN>) id 1hMfU5-0005Hx-Md
 for 35468 <at> debbugs.gnu.org; Fri, 03 May 2019 17:12:46 -0400
Received: by mail-wm1-f48.google.com with SMTP id y5so8213697wma.2
 for <35468 <at> debbugs.gnu.org>; Fri, 03 May 2019 14:12:45 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=googlemail.com; s=20161025;
 h=sender:date:from:to:cc:subject:message-id:references:mime-version
 :content-disposition:content-transfer-encoding:in-reply-to
 :user-agent; bh=RNyLLxlPhr9G1kqnkxklq3dW8kuKN9Y0L/4cuj/VpJg=;
 b=t7BZugmCCxbeEYKjMzOWuChfXRDSE0BGN6MwJ/9TbjklGXPYDhCq0uVvDe8jTZQnwt
 L5pUxae/TzwcsbFRXKRbwqIzRyc8uDz6fvl+KniNmcVlzABRfhvAd9Q0jYkNSX6D5/uJ
 vd3wynndAyaeZMGi/lCSKz4yk0wGiAlyySkhmJ1XuBb3MOVdC5Nvg6rUzmtJGL+s8Jv0
 QKJs7AGu/iEkdwY3twF0rvQfPCxlXq2rOcDE41Q9ofIJG5G1E//1zEYZjRoA9YEu4A3g
 I/iv85ZsgEm8zNZfuroGmIHVe/IxfVdpTWBHeHZTdUvfnR3VZmt+QYdBnwGpDEgECWM0
 XLtg==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20161025;
 h=x-gm-message-state:sender:date:from:to:cc:subject:message-id
 :references:mime-version:content-disposition
 :content-transfer-encoding:in-reply-to:user-agent;
 bh=RNyLLxlPhr9G1kqnkxklq3dW8kuKN9Y0L/4cuj/VpJg=;
 b=jF6z1H1AdCLXQZupuQIkD/IkJffXhy2dMhi47FQw/DD7bxZeenujWqAgcm+Ccu7vPX
 tdWr73Q641gcU9t8bjirWrSEDdxG26lz5iGkRiTVbqYTcaTmRBYs6M0EIqQCXVeQVlA4
 2QKvyjyEELB/UqwstXY2Fb+ixY6F7qH7qvoZQU0tOyZSK2hnX2hHn2idhnc6RL5/lev7
 FZUn0syxBqZHRvEIrYLs2A3RPbouqhPicMNwc1BD0qTO9JMX8pxXvaBQoUPDRd+xjV8a
 MGhZeZaJ6V6ekT5JiAnPrxHtRtXhqtl4t9xX34AV6BzDFDSLJzVE2C+vja9CBVU2zmks
 9aHw==
X-Gm-Message-State: APjAAAUlg6X2O8S0xWZcOo6NRRCX+KYKHEhukNcNTGVpT3KytwvK3T8n
 pgZkkjTj9EuQ1N6351z7bkc=
X-Google-Smtp-Source: APXvYqw1p7OEUmODvNW6nYg3sgIGUw5dyDQclfpbGuCaZBV25lbevH879gE5YOFPXlDP8DNLy7Bskw==
X-Received: by 2002:a7b:cb4e:: with SMTP id v14mr7423888wmj.52.1556917959824; 
 Fri, 03 May 2019 14:12:39 -0700 (PDT)
Received: from breton.holly.idiocy.org
 (ip6-2001-08b0-03f8-8129-c8db-4b16-8787-1926.holly.idiocy.org.
 [2001:8b0:3f8:8129:c8db:4b16:8787:1926])
 by smtp.gmail.com with ESMTPSA id x81sm498093wmg.8.2019.05.03.14.12.38
 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);
 Fri, 03 May 2019 14:12:38 -0700 (PDT)
Date: Fri, 3 May 2019 22:12:36 +0100
From: Alan Third <alan@HIDDEN>
To: Alex Gramiak <agrambot@HIDDEN>
Subject: Re: bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
Message-ID: <20190503211236.GB77011@HIDDEN>
References: <877ebeor2d.fsf@HIDDEN> <83tveit5ph.fsf@HIDDEN>
 <87pnp5oqu1.fsf@HIDDEN> <877ebcogg4.fsf@HIDDEN>
 <83sgu0rsue.fsf@HIDDEN>
 <20190430201129.GA73973@HIDDEN>
 <837ebaqdme.fsf@HIDDEN>
 <20190501210847.GA74837@HIDDEN>
 <87ef5gn2q5.fsf@HIDDEN>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <87ef5gn2q5.fsf@HIDDEN>
User-Agent: Mutt/1.11.3 (2019-02-01)
X-Spam-Score: 0.1 (/)
X-Debbugs-Envelope-To: 35468
Cc: Eli Zaretskii <eliz@HIDDEN>, 35468 <at> debbugs.gnu.org
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -0.9 (/)

On Thu, May 02, 2019 at 12:14:10PM -0600, Alex Gramiak wrote:
> Alan Third <alan@HIDDEN> writes:
> 
> > It looks exactly the same to me with or without the code. Perhaps I’d
> > see something different if I was using some theme which defined mouse
> > face differently.
> >
> > I really don’t see any reason why NS should behave differently from
> > the other terms here.
> 
> There's a similar block in ns_maybe_dumpglyphs_background. Could you
> test with that block removed as well? Also, there's a conditional check
> (s->hl != DRAW_CURSOR) there that's not present in the other versions;
> could you see if that's necessary?

Removing the block in ns_maybe_dumpglyphs_background seems to make no
difference here. Removing the DRAW_CURSOR check results in the cursor
being drawn as a blank (background colour) rectangle on occasion.

I’m not sure what the difference is, but there are a couple of
comments in nsterm.m which reference NS drawing the cursor differently
to other terms, i.e.

  /* We draw the cursor (with NSRectFill), then draw the glyph on top
     (other terminals do it the other way round).  We must set
     w->phys_cursor_width to the cursor width.  For bar cursors, that
     is CURSOR_WIDTH; for box cursors, it is the glyph width.  */

and 

  /* NOTE: under NS this is NOT used to draw cursors, but we must avoid
     overwriting cursor (usually when cursor on a tab).  */

and I suspect this check is related.
-- 
Alan Third




Information forwarded to bug-gnu-emacs@HIDDEN:
bug#35468; Package emacs. Full text available.

Message received at 35468 <at> debbugs.gnu.org:


Received: (at 35468) by debbugs.gnu.org; 3 May 2019 19:01:26 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Fri May 03 15:01:26 2019
Received: from localhost ([127.0.0.1]:49671 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1hMdR0-0002Ba-E2
	for submit <at> debbugs.gnu.org; Fri, 03 May 2019 15:01:26 -0400
Received: from mail-pg1-f171.google.com ([209.85.215.171]:36022)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <agrambot@HIDDEN>) id 1hMdQy-0002BN-Q5
 for 35468 <at> debbugs.gnu.org; Fri, 03 May 2019 15:01:25 -0400
Received: by mail-pg1-f171.google.com with SMTP id 85so3142177pgc.3
 for <35468 <at> debbugs.gnu.org>; Fri, 03 May 2019 12:01:24 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;
 h=from:to:cc:subject:references:date:in-reply-to:message-id
 :user-agent:mime-version;
 bh=Qey9XNZXA29HJ4q1ubc7hQaeY6l7CqRUnBfHZA5Itm4=;
 b=uw55Fq6llW3KN+S87p0ptJZNEli51kMQGblSaCAS9+05o8v2eVPIgfI2WB10h5qbwN
 PvUBcA7HO6C8yhk3u5TM28sggG/csOg1HwWCb1gqCT7phLhHNQuKo1utojXLXXq3DPzj
 dxZE4Gm0lYitjkmRii0IIyQhR7Z/9L23Hsu9mdjQI2NiDJXEyeq2FXuFoCsbfzDF4hMG
 1tDry6wl5MtoSZAASrobcyfLB2Tv63RJxBa7SrbQYTgBBgDYal+S4zD+yBbemkUvrwtW
 NMqmXQ5GDYb+W/zuN/NDztHyA+YIsffMP4NyRhcUSVempiZZL4o8L7b6OlrPAPI4h26w
 Xy7A==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20161025;
 h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to
 :message-id:user-agent:mime-version;
 bh=Qey9XNZXA29HJ4q1ubc7hQaeY6l7CqRUnBfHZA5Itm4=;
 b=reOY7c5RDU5DtCeRc0/z3oRAyzkpJ7uqzhtMMClpPqYt3YWL2vqnx4WKJycKcNlKIe
 ZExsB4KHZ0x6XImvbivMssgDt98J58c1JvkLcIbNiV5BDB7f0vJhSD0ekX9MlpcBL7W3
 BH64cGNqpGPgg1HnR1hPiicrS9Dcf9EBbLqvAn/lDArT5KmnMybE0g+KMl2AKdGTdJB1
 BPHJtZU+IknO1lOXMYD7bvTEHETPf/wfEKCb8Fgl9fDYnKjUIXzt+aztTnTUrZsy+m6R
 grAgvUMnJyvvnaQNJSGekSKuYJHHvy4HfFxBFbeodfgU9pGyCJ26heJS83EEJgTxL/C1
 Xj4w==
X-Gm-Message-State: APjAAAX5r6hRkrUY+MaavWqFZbef3dlzOFrHbBnims5mxwYN9U8bMyBy
 gdJKRmdU7JOmi07Cl4Jwi0wn0ZGK
X-Google-Smtp-Source: APXvYqy39BjK8heARRF7p8ojZ/OGJZJ+clry3VVEbs/wVHlZvxGIIfNAZAUeHhg/tC+I9Hk8LSEPkw==
X-Received: by 2002:a62:a503:: with SMTP id v3mr13278764pfm.32.1556910078450; 
 Fri, 03 May 2019 12:01:18 -0700 (PDT)
Received: from lylat ([2604:3d09:e37f:1500:1a72:4878:e793:7302])
 by smtp.gmail.com with ESMTPSA id e29sm3343515pgb.37.2019.05.03.12.01.16
 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);
 Fri, 03 May 2019 12:01:17 -0700 (PDT)
From: Alex Gramiak <agrambot@HIDDEN>
To: mituharu@HIDDEN
Subject: Re: bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
References: <877ebeor2d.fsf@HIDDEN> <83tveit5ph.fsf@HIDDEN>
 <87pnp5oqu1.fsf@HIDDEN> <877ebcogg4.fsf@HIDDEN>
 <83sgu0rsue.fsf@HIDDEN> <8736lznzjf.fsf@HIDDEN>
 <c560b2f02aea5b22d56d3989e3588508.squirrel@HIDDEN>
Date: Fri, 03 May 2019 13:01:18 -0600
In-Reply-To: <c560b2f02aea5b22d56d3989e3588508.squirrel@HIDDEN>
 (mituharu@HIDDEN's message of "Wed, 1 May 2019 09:14:50
 +0900")
Message-ID: <877eb7l5vl.fsf@HIDDEN>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.2 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
X-Spam-Score: 0.0 (/)
X-Debbugs-Envelope-To: 35468
Cc: Eli Zaretskii <eliz@HIDDEN>, 35468 <at> debbugs.gnu.org
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -1.0 (-)

mituharu@HIDDEN writes:

> Unlike the NS port, the terminal code in the Mac port is much like the X11
> version.

I looked at your code and fortunately it indeed looks like the same
abstractions for glyph drawing that would work for the X11 version would
work for the Mac port.

> But there is one notable difference in the above respect.
> It defines mac_erase_rectangle, which is like mac_fill_rectangle (the
> XFillRectangle counterpart)
> but uses the background color of GC and also handles stipples:
>
> /* Mac replacement for XFillRectangle.  */
>
> static void
> mac_fill_rectangle (struct frame *f, GC gc, int x, int y, int width, int
> height)
> {
>   MAC_BEGIN_DRAW_TO_FRAME (f, gc, context);
>   CGContextSetFillColorWithColor (context, gc->cg_fore_color);
>   {
>     CGRect rect = mac_rect_make (f, x, y, width, height);
>
>     CGContextFillRects (context, &rect, 1);
>   }
>   MAC_END_DRAW_TO_FRAME (f);
> }
>
> static void
> mac_erase_rectangle (struct frame *f, GC gc, int x, int y,
> 		     int width, int height)
> {
>   MAC_BEGIN_DRAW_TO_FRAME (f, gc, context);
>   {
>     CGRect rect = mac_rect_make (f, x, y, width, height);
>
>     CG_CONTEXT_FILL_RECT_WITH_GC_BACKGROUND (f, context, rect, gc);
>     if (gc->xgcv.fill_style == FillOpaqueStippled && gc->xgcv.stipple)
>       {
> 	CGContextClipToRects (context, &rect, 1);
> 	CGContextSetFillColorWithColor (context, gc->cg_fore_color);
> 	int scale = CFArrayGetCount (gc->xgcv.stipple);
> 	if (FRAME_BACKING_SCALE_FACTOR (f) < scale)
> 	  scale = FRAME_BACKING_SCALE_FACTOR (f);
> 	CGImageRef image_mask =
> 	  (CGImageRef) CFArrayGetValueAtIndex (gc->xgcv.stipple, scale - 1);
> 	rect = CGRectMake (0, 0, CGImageGetWidth (image_mask) / (CGFloat) scale,
> 			   CGImageGetHeight (image_mask) / (CGFloat) scale);
> 	CGContextScaleCTM (context, 1, -1);
> 	CGContextSetInterpolationQuality (context, kCGInterpolationNone);
> 	CGContextDrawTiledImage (context, rect, image_mask);
>       }
>   }
>   MAC_END_DRAW_TO_FRAME (f);
> }

Why does it use the background color of GC? It appears that the
Cairo version uses the foreground color of GC; is that because the Cairo
version doesn't handle stippling?

> I guess introducing the erase_rectagle handler makes things simpler and
> efficient.

Looking over the code, and considering that stippling is quite uncommon
nowadays (GTK 3 removed it, AFAIK), I think the best approach would be
to define 3 separate interface procedures:

1) fill_rectangle: handles x_fill_rectangle/mac_fill_rectangle

2) fill_rectangle_with_color: handles (1), but also with the temporary
overriding of the GC that's done frequently for mac_erase_rectangle.

3) fill_rectangle_with_stipple: handles the temporary XFillStyle
overriding of the GC.

A third procedure wouldn't be inelegant in comparison to the
alternatives, since otherwise I would need a set_context_stipple
procedure to indicate stippling.

Then mac_fill_rectangle can be assigned to fill_rectangle, and
mac_erase_rectangle can be assigned to fill_rectangle_with_stipple.




Information forwarded to bug-gnu-emacs@HIDDEN:
bug#35468; Package emacs. Full text available.

Message received at 35468 <at> debbugs.gnu.org:


Received: (at 35468) by debbugs.gnu.org; 3 May 2019 15:26:17 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Fri May 03 11:26:17 2019
Received: from localhost ([127.0.0.1]:49342 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1hMa4n-0003Fj-GQ
	for submit <at> debbugs.gnu.org; Fri, 03 May 2019 11:26:17 -0400
Received: from mail-ed1-f51.google.com ([209.85.208.51]:34747)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <contovob@HIDDEN>) id 1hMa4l-0003FU-Sx
 for 35468 <at> debbugs.gnu.org; Fri, 03 May 2019 11:26:16 -0400
Received: by mail-ed1-f51.google.com with SMTP id w35so4795289edd.1
 for <35468 <at> debbugs.gnu.org>; Fri, 03 May 2019 08:26:15 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=tcd-ie.20150623.gappssmtp.com; s=20150623;
 h=from:to:cc:subject:references:date:in-reply-to:message-id
 :user-agent:mime-version;
 bh=mLNnD0eOjS7x6hxVHkRSVRS4ZJ+RTBBz8yLNkoNr9Mg=;
 b=2PLK09CdKyQuWpHxSRZExGyPfMj2+bYdGlJBMfSly2FS4RgmxtIh+JTe/26wiIJImk
 prChnc9nz7XA8Kc3XpYu+Ds0EBrDp2FZtwahXrcuR9zcu/IoGIV0YnOKVRH70yAyi442
 E+mwzDbC0q2cGRHucynn77JRR4QGHblNsiqXuHeLtvVnJ12z6iGbiEKxMyms6hiBNq7K
 jRtQrl5X18USuqZfsiHt/iqjnzpReILCT6WDUw//clPmoC5Ensxyyc5aCg6s3/3++5It
 PquabmnmbQdTyJ35I9mRnR4JG+wDjz+DQLWl+X4Si0moYvYjYoR8YvVjWI1D2jKBS0x1
 X8Lw==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20161025;
 h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to
 :message-id:user-agent:mime-version;
 bh=mLNnD0eOjS7x6hxVHkRSVRS4ZJ+RTBBz8yLNkoNr9Mg=;
 b=mxm148WGfUT0F32UjF4ROMRxbKawIh06he94DEKJJUYPWpH6h80hvYo9uuZngDR189
 k6iTPmh5PKy+JYLFbb5cmOu8fdSWyexPjZ5YAOifS5S+u0KlGHXr4wean+YVGuXz72JF
 hHIlHQTHQsB6Vl7qDfWFAJXM8Vm3SflOozJW2S/rHmn7mu4M+6tc9gcC4RK/YhpMSr9E
 PYNopdcZH8YhQqPFkZzNMHS2phaaizySww2n1SofS5Cw5YN79S3JCCU0M1HcZP8nK6Go
 jUwjlZGrqApWk98QjjfEaWHuR1TKydD0BtZWXclPcqzBTC80FC0LebU8LUlBTctIfa7v
 aYmQ==
X-Gm-Message-State: APjAAAVPki7z488yk/HuJ5GOH5j/ZsKTVM4yOQNW10ZXTclrKTjRUBYq
 K9LL5cEKPyU5jpNwo7eo8gNdMQ==
X-Google-Smtp-Source: APXvYqzzuM3P4dHJUb+dFAxSBANfkSp0Plh664Ea5TDqUJvHYxi3Oc3y1G9ADKeTIBYHWXeQl3Uzyg==
X-Received: by 2002:a50:8a8b:: with SMTP id j11mr8979543edj.212.1556897170129; 
 Fri, 03 May 2019 08:26:10 -0700 (PDT)
Received: from localhost ([89.101.223.218])
 by smtp.gmail.com with ESMTPSA id e43sm653589edb.38.2019.05.03.08.26.09
 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256);
 Fri, 03 May 2019 08:26:09 -0700 (PDT)
From: "Basil L. Contovounesios" <contovob@HIDDEN>
To: Eli Zaretskii <eliz@HIDDEN>
Subject: Re: bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
References: <877ebeor2d.fsf@HIDDEN> <83tveit5ph.fsf@HIDDEN>
 <87pnp5oqu1.fsf@HIDDEN> <877ebcogg4.fsf@HIDDEN>
 <83sgu0rsue.fsf@HIDDEN> <8736lznzjf.fsf@HIDDEN>
 <83zho6ox5u.fsf@HIDDEN> <878svomynv.fsf@HIDDEN>
 <83o94kobpc.fsf@HIDDEN>
Date: Fri, 03 May 2019 16:26:08 +0100
In-Reply-To: <83o94kobpc.fsf@HIDDEN> (Eli Zaretskii's message of "Thu, 2 May
 2019 23:14:55 +0300")
Message-ID: <874l6beezz.fsf@HIDDEN>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.0.50 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
X-Spam-Score: 0.0 (/)
X-Debbugs-Envelope-To: 35468
Cc: 35468 <at> debbugs.gnu.org, Alex Gramiak <agrambot@HIDDEN>
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -1.0 (-)

Eli Zaretskii <eliz@HIDDEN> writes:

> It's late down here, and I had a bad week and an even worse day.  So
> just a few quick answers for now, and the rest will follow in a few
> hours.

I hope things look up sooner than later, and thank you for your efforts
regardless.

-- 
Basil




Information forwarded to bug-gnu-emacs@HIDDEN:
bug#35468; Package emacs. Full text available.

Message received at 35468 <at> debbugs.gnu.org:


Received: (at 35468) by debbugs.gnu.org; 2 May 2019 20:15:29 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Thu May 02 16:15:29 2019
Received: from localhost ([127.0.0.1]:47093 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1hMI77-0007LU-37
	for submit <at> debbugs.gnu.org; Thu, 02 May 2019 16:15:29 -0400
Received: from eggs.gnu.org ([209.51.188.92]:36709)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <eliz@HIDDEN>) id 1hMI73-0007LF-2U
 for 35468 <at> debbugs.gnu.org; Thu, 02 May 2019 16:15:25 -0400
Received: from fencepost.gnu.org ([2001:470:142:3::e]:51432)
 by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from <eliz@HIDDEN>)
 id 1hMI6w-0002zB-4k; Thu, 02 May 2019 16:15:19 -0400
Received: from [176.228.60.248] (port=4181 helo=home-c4e4a596f7)
 by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256)
 (Exim 4.82) (envelope-from <eliz@HIDDEN>)
 id 1hMI6v-0004vj-3R; Thu, 02 May 2019 16:15:17 -0400
Date: Thu, 02 May 2019 23:14:55 +0300
Message-Id: <83o94kobpc.fsf@HIDDEN>
From: Eli Zaretskii <eliz@HIDDEN>
To: Alex Gramiak <agrambot@HIDDEN>
In-reply-to: <878svomynv.fsf@HIDDEN> (message from Alex Gramiak on Thu, 02
 May 2019 13:41:56 -0600)
Subject: Re: bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
References: <877ebeor2d.fsf@HIDDEN> <83tveit5ph.fsf@HIDDEN>
 <87pnp5oqu1.fsf@HIDDEN> <877ebcogg4.fsf@HIDDEN>
 <83sgu0rsue.fsf@HIDDEN> <8736lznzjf.fsf@HIDDEN>
 <83zho6ox5u.fsf@HIDDEN> <878svomynv.fsf@HIDDEN>
X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]
X-Spam-Score: -2.3 (--)
X-Debbugs-Envelope-To: 35468
Cc: 35468 <at> debbugs.gnu.org
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -3.3 (---)

> From: Alex Gramiak <agrambot@HIDDEN>
> Cc: 35468 <at> debbugs.gnu.org
> Date: Thu, 02 May 2019 13:41:56 -0600

It's late down here, and I had a bad week and an even worse day.  So
just a few quick answers for now, and the rest will follow in a few
hours.

> > It is slightly inelegant, but it certainly isn't a catastrophe.
> > Unused arguments that are there for the sake of a function signature
> > compatibility is not an uncommon technique in C.
> 
> I suppose. It's only because of the w32 version that uses s->HDC that
> `s' needs to be passed in at all for the glyph string version.

Why can't you pass HDC itself?  It's just a pointer.

> > But please note that gc->foreground and background used in some cases
> > are exactly the color numbers used directly in other cases, so I think
> > you could always pass colors or always pass a GC.
> 
> Do you mean that I could leave out either the color or the GC argument?

Yes, that was my impression from looking at the code.  We will just
need to pack the color into a GC in some cases.

> >> 4) The w32 versions of some procedures need to save the font around the
> >> calls to font->driver->draw; is this necessary?
> >
> > Yes.  The X 'draw' methods accept the font as an argument, but the w32
> > implementation relies on setting the font outside of the 'draw' method
> > because the 'draw' method draws using the "currently selected font".
> > Then we need to restore the original font.
> 
> Where do the X 'draw' methods accept the font as an argument? Looking
> at, e.g., *_draw_glyph_string_foreground, font->driver->draw takes the
> same arguments.

Look at the driver's 'draw' method itself, e.g. xftfont_draw.

> Since font->driver->draw takes in the glyph string, why can't the 'draw'
> methods use SelectObject (s->hdc, FONT_HANDLER (s->font)) and
> SelectObject (s->hdc, old_font)?

It could, but that would be less efficient, since we many times call
the draw method several times in a row.

> If they can't, is there any other way to do it inside the draw methods?
> It seems like it would simplify the code on the w32 side and make it
> more in line with the other backends.

I would leave this for another rainy day, and for now just have an
interface which just w32 will implement.

> > You could define a set_font interface, with only a w32 implementation.
> 
> Did you have something else in mind besides the
> save/restore_secondary_context_font I posted?

Not besides, instead.

> >>         if (gdif->save_secondary_context_font)
> >
> > This name is misleading: this is not a "secondary" font.  We are
> > selecting the font to be used for drawing by the font driver's 'draw'
> > method.
> 
> The "secondary" here applies to "context", not "context font". I used
> the name since the code changes s->hdc's (what I dubbed to be the
> "secondary context") font. Would you prefer just
> {save,restore}_context_font?

There's no context here, either.  That call selects a different font,
and returns a handle of the previous font so that it could be restored
later.

Stay tuned for the rest.

Thanks.




Information forwarded to bug-gnu-emacs@HIDDEN:
bug#35468; Package emacs. Full text available.

Message received at 35468 <at> debbugs.gnu.org:


Received: (at 35468) by debbugs.gnu.org; 2 May 2019 19:42:01 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Thu May 02 15:42:01 2019
Received: from localhost ([127.0.0.1]:47052 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1hMHai-0006X4-Tn
	for submit <at> debbugs.gnu.org; Thu, 02 May 2019 15:42:01 -0400
Received: from mail-pg1-f175.google.com ([209.85.215.175]:45615)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <agrambot@HIDDEN>) id 1hMHah-0006Wp-Bx
 for 35468 <at> debbugs.gnu.org; Thu, 02 May 2019 15:42:00 -0400
Received: by mail-pg1-f175.google.com with SMTP id i21so1505328pgi.12
 for <35468 <at> debbugs.gnu.org>; Thu, 02 May 2019 12:41:59 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;
 h=from:to:cc:subject:references:date:in-reply-to:message-id
 :user-agent:mime-version;
 bh=uP266WGI6k98nZTLvbjKNHVsGHCCCLd+TX8eFfQZqE8=;
 b=R+biJmGOyWy98iNaeWznNmMe8xZczDtZGCShrd0LiAViVmvi1JEzIF7rm3CErkyE85
 QtrWzU8LKIuYpQdYm/pL729M6evh/upixG0tslzGeOkf6dF6VVjRpnoEQg+cRQhjEExa
 wD/2CLxYXlGRZykjwQs8rCSNVWP8x1JGc3f4akM87Gd8vYJpolOArSVi73qLgb4IDK/v
 T2gmUKT/E7KW0t8b8+L2DhC9pqHSASpgQK+6VtbQOu3JA4FLnCJDDXiWwWD6HD6hqWjS
 CdJ3PAX27GOVyk3jFdeju5J6Qb07nRV0zQhVjfklJMUQRDa899ah68Ch6TZY/AgV9O2z
 05FA==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20161025;
 h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to
 :message-id:user-agent:mime-version;
 bh=uP266WGI6k98nZTLvbjKNHVsGHCCCLd+TX8eFfQZqE8=;
 b=jBLvrU4XLICqLP3kVFc7bG/Rqbm52rnyVvGZ5oZzudf2lmMb5PyKBl6UsP302zwPaS
 YERCxxPONXU/b1CADiD/PlNB1RJPbBouNUFXYIxtr6VSFGy+jgwCPcH+KZfHssvopXTQ
 7Eewwj1jTJ6sMThAbC05qiyZyAoAGP25kIyLBSO8dgKMbyzn42l+CbLe2kRovWFSCAFB
 Hdu33hjxnwp3ML76bIPNnDYt5O8DjmvQWYW6mdZkV8YrO/rTdqMwwt8/7vomuKXoTYwY
 fa4lXoaKke+opbLZQsyBEXkyQBoHOqDvystR4CwHVm5xPLHNIWKepeq4smuSoHgPxVON
 z75w==
X-Gm-Message-State: APjAAAXGnl/ePHoHaSLdwxTxdH26TlamD730wsCjGG2eSe1luZaaw2P9
 OmB2b0YdIEdtvCK8hhqyMbFra0gQ
X-Google-Smtp-Source: APXvYqwP4qJE4MdFRxADOvXVeAZddoMOZ/C0TwdxJJSIVFLBZ1TJaEaRf/P72I0n3AxSr56cL/kqRQ==
X-Received: by 2002:a62:6845:: with SMTP id d66mr6142853pfc.21.1556826112986; 
 Thu, 02 May 2019 12:41:52 -0700 (PDT)
Received: from lylat ([2604:3d09:e37f:1500:1a72:4878:e793:7302])
 by smtp.gmail.com with ESMTPSA id d38sm60348pgd.40.2019.05.02.12.41.51
 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);
 Thu, 02 May 2019 12:41:52 -0700 (PDT)
From: Alex Gramiak <agrambot@HIDDEN>
To: Eli Zaretskii <eliz@HIDDEN>
Subject: Re: bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
References: <877ebeor2d.fsf@HIDDEN> <83tveit5ph.fsf@HIDDEN>
 <87pnp5oqu1.fsf@HIDDEN> <877ebcogg4.fsf@HIDDEN>
 <83sgu0rsue.fsf@HIDDEN> <8736lznzjf.fsf@HIDDEN>
 <83zho6ox5u.fsf@HIDDEN>
Date: Thu, 02 May 2019 13:41:56 -0600
In-Reply-To: <83zho6ox5u.fsf@HIDDEN> (Eli Zaretskii's message of "Wed, 01 May
 2019 21:19:09 +0300")
Message-ID: <878svomynv.fsf@HIDDEN>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.2 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
X-Spam-Score: 0.0 (/)
X-Debbugs-Envelope-To: 35468
Cc: 35468 <at> debbugs.gnu.org
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -1.0 (-)

Eli Zaretskii <eliz@HIDDEN> writes:

>> From: Alex Gramiak <agrambot@HIDDEN>
>> Cc: 35468 <at> debbugs.gnu.org
>> Date: Tue, 30 Apr 2019 12:00:52 -0600
>> 
>> I think that there can be a single interface for both glyph_string
>> filling (e.g., in *_clear_glyph_string_rect), and for non-glyph_string
>> filling (e.g., in *_draw_bar_cursor), but I'm not sure it would be worth
>> it. I would think that having two separate interfaces, one taking in a
>> glyph string and the other taking in a frame, would be cleaner:
>> 
>>  fill_rectangle_glyph (struct glyph_string *, GC, int, int, int, int);
>>  fill_rectangle (struct frame *, GC, int, int, int, int);
>> 
>> There needs to be a glyph_string argument in one version so that the w32
>> version can use s->HDC. If there was only one interface, then one would
>> have to supply NULL to the glyph_string argument whenever not dealing
>> with glyph strings, which IMO is unclean.
>
> It is slightly inelegant, but it certainly isn't a catastrophe.
> Unused arguments that are there for the sake of a function signature
> compatibility is not an uncommon technique in C.

I suppose. It's only because of the w32 version that uses s->HDC that
`s' needs to be passed in at all for the glyph string version.

> What is needed in fill_rectangle_glyph from the glyph_string except
> the frame pointer?

It depends, sometimes it's just the frame and GC (and HDC), and
sometimes it's other attributes like x/y/width/height. AFAICT, all
arguments except the frame (it's always s->f) are needed for the glyph
string filling.

> (The name is also sub-optimal: what is a "rectangle glyph"?)

I meant it as "fill rectangle from glyph", but I guess I was too terse.

>> So is having to have two versions, but I think it's the best option
>> unless you know of a way to embed HDC in w32's version of GC.
>
> I see no reason to add HDC to the w32 GC, they can remain separate at
> this point.

I'm not sure how it would affect the w32 side, but I just mentioned
combining the two since it would make "more sense" from the generic POV.

> But please note that gc->foreground and background used in some cases
> are exactly the color numbers used directly in other cases, so I think
> you could always pass colors or always pass a GC.

Do you mean that I could leave out either the color or the GC argument?
The X/Cairo side at least need the GC (since it's not always s->gc), and
outside of the {get,set}_context_* way I posted, I don't see how I could
leave out the color argument.

>> As for the color manipulation, I went low-level as you said before:
>> 
>>   unsigned long foreground, background;
>>   gdif->get_context_foreground (gc, &foreground);
>>   gdif->get_context_background (gc, &background);
>>   gdif->set_context_foreground (gc, background);
>>   gdif->fill_rectangle (s, gc, x, y, w, h);
>>   gdif->set_context_foreground (gc, foreground);
>> 
>> which replaces the XGetGCValues section in x_draw_stretch_glyph_string.
>> This unfortunately is more work in the w32 case as it manipulates s->gc
>> instead of just using the calculated gc->background.
>
> I don't think I understand what you mean by "manipulates s->gc".  Can
> you show the code which does that?

I just meant that it does something like:

  unsigned long foreground = s->gc->foreground;
  unsigned long background = s->gc->background;
  s->gc->foreground = background;
  fill_rectangle (...)
  s->gc->foreground = foreground;

Where previously the foreground didn't need to be saved/restored in the
w32 implementation.

>> If that is unsatisfactory), then instead I could do something like:
>> 
>>   gdif->fill_rectangle_with_color (s, gc->background, gc, x, y, w, h);
>> 
>> Which wouldn't touch s->gc for the w32 version and would do the whole
>> XGetGCValues dance for the X version.
>
> So fill_rectangle will fill with the current background and
> fill_rectangle_with_color will fill with the specified color?  It's
> possible, yes.

Possible, yes, but would you prefer that over the above?

>> 1) Why does w32_set_mouse_face_gc use GCFont in its mask when the X
>> version doesn't?
>
> It's a remnant of old code which was deleted from the X version.  We
> should remove that.

Okay, I did that and removed all usages of GCFont in the W32 version
(and an unused #define GCFont in nsgui.h).

>> 4) The w32 versions of some procedures need to save the font around the
>> calls to font->driver->draw; is this necessary?
>
> Yes.  The X 'draw' methods accept the font as an argument, but the w32
> implementation relies on setting the font outside of the 'draw' method
> because the 'draw' method draws using the "currently selected font".
> Then we need to restore the original font.

Where do the X 'draw' methods accept the font as an argument? Looking
at, e.g., *_draw_glyph_string_foreground, font->driver->draw takes the
same arguments.

Since font->driver->draw takes in the glyph string, why can't the 'draw'
methods use SelectObject (s->hdc, FONT_HANDLER (s->font)) and
SelectObject (s->hdc, old_font)?

If they can't, is there any other way to do it inside the draw methods?
It seems like it would simplify the code on the w32 side and make it
more in line with the other backends.

> You could define a set_font interface, with only a w32 implementation.

Did you have something else in mind besides the
save/restore_secondary_context_font I posted?

>>         if (gdif->save_secondary_context_font)
>
> This name is misleading: this is not a "secondary" font.  We are
> selecting the font to be used for drawing by the font driver's 'draw'
> method.

The "secondary" here applies to "context", not "context font". I used
the name since the code changes s->hdc's (what I dubbed to be the
"secondary context") font. Would you prefer just
{save,restore}_context_font?

>> Currently this requires save_secondary_context_font to allocate memory,
>> which is unideal. One could avoid this by introducing a new union type
>> (backend_font or somesuch) and using that instead of a void*. WDYT?
>
> There's no need: the HFONT data type is just a pointer, so you can
> store it in a 'void *' without allocating anything.  If you want to be
> "more Catholic than the Pope", make that variable a unitptr_t.

Okay, I was thinking about other backends, but as long as the only
backends using that interface just pass pointers void * works just fine.



I'm having trouble with *_draw_image_foreground -- they just seem too
different to nicely abstract. Would it be okay if some generic
constructs leak into it (namely: s->img->mask)? Otherwise the common
setup that w32 does would be problematic.

Does the RestoreDC/SaveDC call need to be around both toplevel branches
(i.e., also around the w32_draw_rectangle), or just the s->img->pixmap
branch?

For reference, this is what I have right now for
gui_draw_image_foreground:

  static void
  gui_draw_image_foreground (struct glyph_string *s)
  {
    struct graphical_drawing_interface *gdif = FRAME_GDIF (s->f);
    int x = s->x;
    int y = s->ybase - image_ascent (s->img, s->face, &s->slice);

    /* If first glyph of S has a left box line, start drawing it to the
       right of that line.  */
    if (s->face->box != FACE_NO_BOX
        && s->first_glyph->left_box_line_p
        && s->slice.x == 0)
      x += eabs (s->face->box_line_width);

    /* If there is a margin around the image, adjust x- and y-position
       by that margin.  */
    if (s->slice.x == 0)
      x += s->img->hmargin;
    if (s->slice.y == 0)
      y += s->img->vmargin;

    if (gdif->save_secondary_context)
      gdif->save_secondary_context (s, CON_ALL); // SaveDC (s->hdc);

    if (gdif->glyph_has_image (s))
      {
        gdif->draw_image (s, s->img->width, s->img->height,
                          s->slice.x, s->slice.y, s->slice.width, s->slice.height,
                          x, y, true);
        if (!gdif->glyph_image_uses_mask (s))
          {
            /* When the image has a mask, we can expect that at
               least part of a mouse highlight or a block cursor will
               be visible.  If the image doesn't have a mask, make
               a block cursor visible by drawing a rectangle around
               the image.  I believe it's looking better if we do
               nothing here for mouse-face.  */
            if (s->hl == DRAW_CURSOR)
              {
                const int relief = eabs (s->img->relief);
                gdif->draw_rectangle (s,
                                      x - relief,
                                      y - relief,
                                      s->slice.width + relief*2 - 1,
                                      s->slice.height + relief*2 - 1);
              }
          }
      }
    else
      /* Draw a rectangle if image could not be loaded.  */
      gdif->draw_rectangle (s, x, y, s->slice.width - 1, s->slice.height - 1);

    if (gdif->restore_secondary_context)
      gdif->restore_secondary_context (s); // RestoreDC (s->hdc, -1);
  }




Information forwarded to bug-gnu-emacs@HIDDEN:
bug#35468; Package emacs. Full text available.

Message received at 35468 <at> debbugs.gnu.org:


Received: (at 35468) by debbugs.gnu.org; 2 May 2019 18:14:19 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Thu May 02 14:14:19 2019
Received: from localhost ([127.0.0.1]:46975 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1hMGDr-0002OP-3E
	for submit <at> debbugs.gnu.org; Thu, 02 May 2019 14:14:19 -0400
Received: from mail-pl1-f178.google.com ([209.85.214.178]:43221)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <agrambot@HIDDEN>) id 1hMGDp-0002OD-Ej
 for 35468 <at> debbugs.gnu.org; Thu, 02 May 2019 14:14:18 -0400
Received: by mail-pl1-f178.google.com with SMTP id n8so1386717plp.10
 for <35468 <at> debbugs.gnu.org>; Thu, 02 May 2019 11:14:17 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;
 h=from:to:cc:subject:references:date:in-reply-to:message-id
 :user-agent:mime-version:content-transfer-encoding;
 bh=vzTfdR+Ij1EbAkMAMUuoqhx7RdaIBCB6gtuihRGd2kI=;
 b=nxYNAv9IyJB43Ku5qSUmhU5Iq6ThIE3SMry7mBd6VxJENvEghDHeZwHoBMrVPCjoXn
 o/1kSsmjLZKxL7ZuECojt6I1TFJ4DtjTg9aZZOGMVXGVejvUnR6edfwEzDedg5B/mA+T
 qjutNbug9h/c/FZnV4m4nIvRmDnJI3qJJxAQlpTJej0SnrlU2c20952neU9Gw2Dk1bCS
 Iy3q5/1EULV0DIJvcNG6bXS+/Vr6JMSmyUYYuez1zbmDa852EUTWPkkuFVuHERT5UBMA
 EyLsMDmD19Qu7hSv+qP5RKD742V2hBsABNTZlHSg9nq9GZz41jjFy7at4VYhsSgf9TG0
 O3QA==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20161025;
 h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to
 :message-id:user-agent:mime-version:content-transfer-encoding;
 bh=vzTfdR+Ij1EbAkMAMUuoqhx7RdaIBCB6gtuihRGd2kI=;
 b=L9mJcc7FvDHyMqOKPEk1sj95y2ZyyglCjKVlmj8DdfrCiXtEbovyQBsM6babc99Frl
 2Hp8Fah8mM7pQbqBLsaQ2mW6nSFIQpdXDeZnmhvS+LZtCl9OGQI2DOyV7JW0c9enAMbE
 NV1VvaH0a0qDMN5Hkl5uB7pPsfcQ9l4/ygEGD2JjOcOS5AsFvo8RXDkjBEXKtWKyc8ts
 82GhZe8PaC3BGV9E7SNbX1ZPc2nuDSBSKp+z8KxCfme+0n/SUteAWuWr22bz5dCfI37k
 0bEA1FdJcMqqdu9/QzJnpjW+UoobOaFdi+RsfXOl4nrT2g9u1sOD1arRIkg+dbRuscn7
 74Jg==
X-Gm-Message-State: APjAAAVoAQ+jml/142OXXEntNn/BNCTvzrxwNtqGXi7B7V+6jt1QJgy8
 6hgDSKrcnFGQ0DizyHuECX5yxgLP
X-Google-Smtp-Source: APXvYqxi0vZeEcfoK38l+ZqfbnI6oljJCckMSvmFttqZw1ylm1XL+l5yNtpq3CRIfs7O9yPBCJNGzg==
X-Received: by 2002:a17:902:7b97:: with SMTP id
 w23mr5313734pll.335.1556820851096; 
 Thu, 02 May 2019 11:14:11 -0700 (PDT)
Received: from lylat ([2604:3d09:e37f:1500:1a72:4878:e793:7302])
 by smtp.gmail.com with ESMTPSA id a18sm4918990pff.6.2019.05.02.11.14.09
 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);
 Thu, 02 May 2019 11:14:09 -0700 (PDT)
From: Alex Gramiak <agrambot@HIDDEN>
To: Alan Third <alan@HIDDEN>
Subject: Re: bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
References: <877ebeor2d.fsf@HIDDEN> <83tveit5ph.fsf@HIDDEN>
 <87pnp5oqu1.fsf@HIDDEN> <877ebcogg4.fsf@HIDDEN>
 <83sgu0rsue.fsf@HIDDEN>
 <20190430201129.GA73973@HIDDEN>
 <837ebaqdme.fsf@HIDDEN>
 <20190501210847.GA74837@HIDDEN>
Date: Thu, 02 May 2019 12:14:10 -0600
In-Reply-To: <20190501210847.GA74837@HIDDEN> (Alan Third's
 message of "Wed, 1 May 2019 22:08:47 +0100")
Message-ID: <87ef5gn2q5.fsf@HIDDEN>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.2 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
X-Spam-Score: 0.0 (/)
X-Debbugs-Envelope-To: 35468
Cc: Eli Zaretskii <eliz@HIDDEN>, 35468 <at> debbugs.gnu.org
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -1.0 (-)

Alan Third <alan@HIDDEN> writes:

> It looks exactly the same to me with or without the code. Perhaps I=E2=80=
=99d
> see something different if I was using some theme which defined mouse
> face differently.
>
> I really don=E2=80=99t see any reason why NS should behave differently fr=
om
> the other terms here.

There's a similar block in ns_maybe_dumpglyphs_background. Could you
test with that block removed as well? Also, there's a conditional check
(s->hl !=3D DRAW_CURSOR) there that's not present in the other versions;
could you see if that's necessary?




Information forwarded to bug-gnu-emacs@HIDDEN:
bug#35468; Package emacs. Full text available.

Message received at 35468 <at> debbugs.gnu.org:


Received: (at 35468) by debbugs.gnu.org; 1 May 2019 21:08:59 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Wed May 01 17:08:59 2019
Received: from localhost ([127.0.0.1]:45059 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1hLwTK-0001DO-T7
	for submit <at> debbugs.gnu.org; Wed, 01 May 2019 17:08:59 -0400
Received: from mail-wr1-f50.google.com ([209.85.221.50]:41203)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <athird@HIDDEN>) id 1hLwTI-0001D9-Pf
 for 35468 <at> debbugs.gnu.org; Wed, 01 May 2019 17:08:57 -0400
Received: by mail-wr1-f50.google.com with SMTP id c12so213634wrt.8
 for <35468 <at> debbugs.gnu.org>; Wed, 01 May 2019 14:08:56 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=googlemail.com; s=20161025;
 h=sender:date:from:to:cc:subject:message-id:references:mime-version
 :content-disposition:content-transfer-encoding:in-reply-to
 :user-agent; bh=Qpx5Ukui1/lSCZpTRkjKdwJ5YM0LJ8LXDUB0ISmyQEg=;
 b=lpt1fJtGZDqgCnsi7VcQv6Fm6w2idMUOe6gLNapFFZGTG1nePc2UgXNYQNMqiE+ey+
 loas+J7KQqa4YwlGJRDVzOrxzfX481GOSlRUlhDUGXItie6xFlkhIz1pKo5peyNBBQht
 yLiAIDe/5hfHEpzqLvcaVej2y167Uhag8XdWDGVKVYK7b1VgMhkzzMPf02C8hTDCC6QQ
 06O/UWnTmCZLRUWK0RmUpCfZ5fLeoEaTTqXyH8e0tfYOaD7OxIG56nyrdrJMFcRH3zKH
 Mcq0ahSsZeuVv4TLfXQxAyF49o5V+bXKaQ6ZhtY2bsshJtVcYxHDPPX6buYFIzv0cQ7Z
 hJbQ==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20161025;
 h=x-gm-message-state:sender:date:from:to:cc:subject:message-id
 :references:mime-version:content-disposition
 :content-transfer-encoding:in-reply-to:user-agent;
 bh=Qpx5Ukui1/lSCZpTRkjKdwJ5YM0LJ8LXDUB0ISmyQEg=;
 b=AaLFK9cgylBZjIvq7d1+eXcDkvAbQoZBMQumRxY19NLszr6XADnSVMKc2hS92D4Xwj
 m465KO0mLzyLgGYZHNL0CYTEsbvOBzmRKTbau3jwhX0VGzuTCJBOzSBQSKlfJJl3cM0h
 FRK01po/18RwbgKsIjmv71tVgvvRfjrZ1vWKt+UWG//1YnDBqxMaRvMGxeU3+//wsdMm
 817skix0XwCpuKcKBez+UZiza1NmCCbSV4s+5N7rXOxVJnp7DWCoELAEV36ZDUquuJJI
 RLVpAH5tUEcfWGiTlfuzLqw60OgfP3RCxbq8vzMKLDA4l4rqqyN5gmbKxsac/WfJp2k9
 QkqA==
X-Gm-Message-State: APjAAAVRp0Y0ssQVRVEqkARykJKkw12K/JX62qxW3pgyJoI41wkpQiqH
 Sle8G/CAG/rl3c3bgAvJ4MY=
X-Google-Smtp-Source: APXvYqwCb+StU9zM57NwhakKidD1gA9DCep1uk7Rvh3OeSQxp3LZCLhji4shx7Gf037lNV3b2exNgg==
X-Received: by 2002:adf:fcc8:: with SMTP id f8mr126515wrs.250.1556744930832;
 Wed, 01 May 2019 14:08:50 -0700 (PDT)
Received: from breton.holly.idiocy.org
 (ip6-2001-08b0-03f8-8129-c8db-4b16-8787-1926.holly.idiocy.org.
 [2001:8b0:3f8:8129:c8db:4b16:8787:1926])
 by smtp.gmail.com with ESMTPSA id z5sm7090058wre.70.2019.05.01.14.08.49
 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);
 Wed, 01 May 2019 14:08:49 -0700 (PDT)
Date: Wed, 1 May 2019 22:08:47 +0100
From: Alan Third <alan@HIDDEN>
To: Eli Zaretskii <eliz@HIDDEN>
Subject: Re: bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
Message-ID: <20190501210847.GA74837@HIDDEN>
References: <877ebeor2d.fsf@HIDDEN> <83tveit5ph.fsf@HIDDEN>
 <87pnp5oqu1.fsf@HIDDEN> <877ebcogg4.fsf@HIDDEN>
 <83sgu0rsue.fsf@HIDDEN>
 <20190430201129.GA73973@HIDDEN>
 <837ebaqdme.fsf@HIDDEN>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <837ebaqdme.fsf@HIDDEN>
User-Agent: Mutt/1.11.3 (2019-02-01)
X-Spam-Score: 0.1 (/)
X-Debbugs-Envelope-To: 35468
Cc: 35468 <at> debbugs.gnu.org, agrambot@HIDDEN
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -0.9 (/)

On Wed, May 01, 2019 at 08:38:17PM +0300, Eli Zaretskii wrote:
> > Date: Tue, 30 Apr 2019 21:11:29 +0100
> > From: Alan Third <alan@HIDDEN>
> > Cc: Alex Gramiak <agrambot@HIDDEN>, 35468 <at> debbugs.gnu.org
> > 
> > > >   if (s->hl == DRAW_MOUSE_FACE)
> > > >     {
> > > >       face = FACE_FROM_ID_OR_NULL (s->f,
> > > > 				   MOUSE_HL_INFO (s->f)->mouse_face_face_id);
> > > >       if (!face)
> > > >         face = FACE_FROM_ID (s->f, MOUSE_FACE_ID);
> > > >     }
> > > >   else
> > > >     face = s->face;
> > > 
> > > I don't know why this is TRT, either.  We could ask Alan to look into
> > > this, or we could simply remove that, as other terminals don't use
> > > box_line_width from the mouse face, they use s->face instead.
> > 
> > A quick test doesn’t turn up any immediate issues with removing this,
> > but I’m unsure in what circumstances DRAW_MOUSE_FACE would be in use.
> 
> E.g., when you hover the mouse pointer above the mouse-sensitive
> portions of the mode line.

It looks exactly the same to me with or without the code. Perhaps I’d
see something different if I was using some theme which defined mouse
face differently.

I really don’t see any reason why NS should behave differently from
the other terms here.
-- 
Alan Third




Information forwarded to bug-gnu-emacs@HIDDEN:
bug#35468; Package emacs. Full text available.

Message received at 35468 <at> debbugs.gnu.org:


Received: (at 35468) by debbugs.gnu.org; 1 May 2019 18:19:25 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Wed May 01 14:19:25 2019
Received: from localhost ([127.0.0.1]:44875 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1hLtpD-0003bQ-7h
	for submit <at> debbugs.gnu.org; Wed, 01 May 2019 14:19:25 -0400
Received: from eggs.gnu.org ([209.51.188.92]:60549)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <eliz@HIDDEN>) id 1hLtpB-0003bB-Nc
 for 35468 <at> debbugs.gnu.org; Wed, 01 May 2019 14:19:22 -0400
Received: from fencepost.gnu.org ([2001:470:142:3::e]:58254)
 by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from <eliz@HIDDEN>)
 id 1hLtp6-0005rH-Hp; Wed, 01 May 2019 14:19:16 -0400
Received: from [176.228.60.248] (port=4057 helo=home-c4e4a596f7)
 by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256)
 (Exim 4.82) (envelope-from <eliz@HIDDEN>)
 id 1hLtp5-0006nr-PB; Wed, 01 May 2019 14:19:16 -0400
Date: Wed, 01 May 2019 21:19:09 +0300
Message-Id: <83zho6ox5u.fsf@HIDDEN>
From: Eli Zaretskii <eliz@HIDDEN>
To: Alex Gramiak <agrambot@HIDDEN>
In-reply-to: <8736lznzjf.fsf@HIDDEN> (message from Alex Gramiak on Tue, 30
 Apr 2019 12:00:52 -0600)
Subject: Re: bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
References: <877ebeor2d.fsf@HIDDEN> <83tveit5ph.fsf@HIDDEN>
 <87pnp5oqu1.fsf@HIDDEN> <877ebcogg4.fsf@HIDDEN>
 <83sgu0rsue.fsf@HIDDEN> <8736lznzjf.fsf@HIDDEN>
X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]
X-Spam-Score: -2.3 (--)
X-Debbugs-Envelope-To: 35468
Cc: 35468 <at> debbugs.gnu.org
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -3.3 (---)

> From: Alex Gramiak <agrambot@HIDDEN>
> Cc: 35468 <at> debbugs.gnu.org
> Date: Tue, 30 Apr 2019 12:00:52 -0600
> 
> I think that there can be a single interface for both glyph_string
> filling (e.g., in *_clear_glyph_string_rect), and for non-glyph_string
> filling (e.g., in *_draw_bar_cursor), but I'm not sure it would be worth
> it. I would think that having two separate interfaces, one taking in a
> glyph string and the other taking in a frame, would be cleaner:
> 
>  fill_rectangle_glyph (struct glyph_string *, GC, int, int, int, int);
>  fill_rectangle (struct frame *, GC, int, int, int, int);
> 
> There needs to be a glyph_string argument in one version so that the w32
> version can use s->HDC. If there was only one interface, then one would
> have to supply NULL to the glyph_string argument whenever not dealing
> with glyph strings, which IMO is unclean.

It is slightly inelegant, but it certainly isn't a catastrophe.
Unused arguments that are there for the sake of a function signature
compatibility is not an uncommon technique in C.

What is needed in fill_rectangle_glyph from the glyph_string except
the frame pointer?  (The name is also sub-optimal: what is a
"rectangle glyph"?)

> So is having to have two versions, but I think it's the best option
> unless you know of a way to embed HDC in w32's version of GC.

I see no reason to add HDC to the w32 GC, they can remain separate at
this point.  But please note that gc->foreground and background used
in some cases are exactly the color numbers used directly in other
cases, so I think you could always pass colors or always pass a GC.

> As for the color manipulation, I went low-level as you said before:
> 
>   unsigned long foreground, background;
>   gdif->get_context_foreground (gc, &foreground);
>   gdif->get_context_background (gc, &background);
>   gdif->set_context_foreground (gc, background);
>   gdif->fill_rectangle (s, gc, x, y, w, h);
>   gdif->set_context_foreground (gc, foreground);
> 
> which replaces the XGetGCValues section in x_draw_stretch_glyph_string.
> This unfortunately is more work in the w32 case as it manipulates s->gc
> instead of just using the calculated gc->background.

I don't think I understand what you mean by "manipulates s->gc".  Can
you show the code which does that?

> If that is unsatisfactory), then instead I could do something like:
> 
>   gdif->fill_rectangle_with_color (s, gc->background, gc, x, y, w, h);
> 
> Which wouldn't touch s->gc for the w32 version and would do the whole
> XGetGCValues dance for the X version.

So fill_rectangle will fill with the current background and
fill_rectangle_with_color will fill with the specified color?  It's
possible, yes.

> 1) Why does w32_set_mouse_face_gc use GCFont in its mask when the X
> version doesn't?

It's a remnant of old code which was deleted from the X version.  We
should remove that.

> 2) Why does w32_draw_glyphless_glyph_string_foreground have the boolean
> `with_background' instead of using false unconditionally like the X
> version does? Should the X version be updated?

I don't know, but the w32 code was added in a dedicated commit.
Unfortunately, I cannot find any trace of discussing this change or
any bug reports that led to it.  We could leave this code under the
FRAME_W32_P condition.

> 3) Why does w32_draw_glyph_string for FACE_UNDER_LINE use a thickness of
> 1 for w32_fill_area instead of using s->underline_thickness like X/NS
> do? This seems like a bug.

It's a bug of sorts, but you will never see any buggy behavior as
result of it, because there are no Windows fonts that specify
thickness other than zero or 1.  I'm okay with using the X code for
w32 as well, it cannot do any harm.

> 4) The w32 versions of some procedures need to save the font around the
> calls to font->driver->draw; is this necessary?

Yes.  The X 'draw' methods accept the font as an argument, but the w32
implementation relies on setting the font outside of the 'draw' method
because the 'draw' method draws using the "currently selected font".
Then we need to restore the original font.

You could define a set_font interface, with only a w32 implementation.

>         if (gdif->save_secondary_context_font)

This name is misleading: this is not a "secondary" font.  We are
selecting the font to be used for drawing by the font driver's 'draw'
method.

> Currently this requires save_secondary_context_font to allocate memory,
> which is unideal. One could avoid this by introducing a new union type
> (backend_font or somesuch) and using that instead of a void*. WDYT?

There's no need: the HFONT data type is just a pointer, so you can
store it in a 'void *' without allocating anything.  If you want to be
"more Catholic than the Pope", make that variable a unitptr_t.

Thanks.




Information forwarded to bug-gnu-emacs@HIDDEN:
bug#35468; Package emacs. Full text available.

Message received at 35468 <at> debbugs.gnu.org:


Received: (at 35468) by debbugs.gnu.org; 1 May 2019 17:38:34 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Wed May 01 13:38:34 2019
Received: from localhost ([127.0.0.1]:44831 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1hLtBi-0002ZN-Bc
	for submit <at> debbugs.gnu.org; Wed, 01 May 2019 13:38:34 -0400
Received: from eggs.gnu.org ([209.51.188.92]:52497)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <eliz@HIDDEN>) id 1hLtBg-0002ZA-FR
 for 35468 <at> debbugs.gnu.org; Wed, 01 May 2019 13:38:32 -0400
Received: from fencepost.gnu.org ([2001:470:142:3::e]:57072)
 by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from <eliz@HIDDEN>)
 id 1hLtBb-00067x-7V; Wed, 01 May 2019 13:38:27 -0400
Received: from [176.228.60.248] (port=1529 helo=home-c4e4a596f7)
 by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256)
 (Exim 4.82) (envelope-from <eliz@HIDDEN>)
 id 1hLtBV-0003zb-Ut; Wed, 01 May 2019 13:38:23 -0400
Date: Wed, 01 May 2019 20:38:17 +0300
Message-Id: <837ebaqdme.fsf@HIDDEN>
From: Eli Zaretskii <eliz@HIDDEN>
To: Alan Third <alan@HIDDEN>
In-reply-to: <20190430201129.GA73973@HIDDEN> (message from
 Alan Third on Tue, 30 Apr 2019 21:11:29 +0100)
Subject: Re: bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
References: <877ebeor2d.fsf@HIDDEN> <83tveit5ph.fsf@HIDDEN>
 <87pnp5oqu1.fsf@HIDDEN> <877ebcogg4.fsf@HIDDEN>
 <83sgu0rsue.fsf@HIDDEN> <20190430201129.GA73973@HIDDEN>
MIME-version: 1.0
Content-type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]
X-Spam-Score: -2.3 (--)
X-Debbugs-Envelope-To: 35468
Cc: 35468 <at> debbugs.gnu.org, agrambot@HIDDEN
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -3.3 (---)

> Date: Tue, 30 Apr 2019 21:11:29 +0100
> From: Alan Third <alan@HIDDEN>
> Cc: Alex Gramiak <agrambot@HIDDEN>, 35468 <at> debbugs.gnu.org
> 
> > >   if (s->hl == DRAW_MOUSE_FACE)
> > >     {
> > >       face = FACE_FROM_ID_OR_NULL (s->f,
> > > 				   MOUSE_HL_INFO (s->f)->mouse_face_face_id);
> > >       if (!face)
> > >         face = FACE_FROM_ID (s->f, MOUSE_FACE_ID);
> > >     }
> > >   else
> > >     face = s->face;
> > 
> > I don't know why this is TRT, either.  We could ask Alan to look into
> > this, or we could simply remove that, as other terminals don't use
> > box_line_width from the mouse face, they use s->face instead.
> 
> A quick test doesn’t turn up any immediate issues with removing this,
> but I’m unsure in what circumstances DRAW_MOUSE_FACE would be in use.

E.g., when you hover the mouse pointer above the mouse-sensitive
portions of the mode line.




Information forwarded to bug-gnu-emacs@HIDDEN:
bug#35468; Package emacs. Full text available.

Message received at 35468 <at> debbugs.gnu.org:


Received: (at 35468) by debbugs.gnu.org; 1 May 2019 00:14:55 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Tue Apr 30 20:14:55 2019
Received: from localhost ([127.0.0.1]:42823 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1hLcti-0000PZ-CI
	for submit <at> debbugs.gnu.org; Tue, 30 Apr 2019 20:14:55 -0400
Received: from mathmail.math.s.chiba-u.ac.jp ([133.82.132.2]:64464)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <mituharu@HIDDEN>) id 1hLctf-0000PO-JW
 for 35468 <at> debbugs.gnu.org; Tue, 30 Apr 2019 20:14:52 -0400
Received: from weber.math.s.chiba-u.ac.jp (weber [192.168.32.4])
 by mathmail.math.s.chiba-u.ac.jp (Postfix) with ESMTP id 5C3ACF08EE;
 Wed,  1 May 2019 09:14:50 +0900 (JST)
 (envelope-from mituharu@HIDDEN)
Received: from 180.3.209.86 (SquirrelMail authenticated user mituharu)
 by weber.math.s.chiba-u.ac.jp with HTTP;
 Wed, 1 May 2019 09:14:50 +0900
Message-ID: <c560b2f02aea5b22d56d3989e3588508.squirrel@HIDDEN>
In-Reply-To: <8736lznzjf.fsf@HIDDEN>
References: <877ebeor2d.fsf@HIDDEN> <83tveit5ph.fsf@HIDDEN>
 <87pnp5oqu1.fsf@HIDDEN> <877ebcogg4.fsf@HIDDEN>
 <83sgu0rsue.fsf@HIDDEN> <8736lznzjf.fsf@HIDDEN>
Date: Wed, 1 May 2019 09:14:50 +0900
Subject: Re: bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
From: mituharu@HIDDEN
To: "Alex Gramiak" <agrambot@HIDDEN>
User-Agent: SquirrelMail/1.4.22-5.el6
MIME-Version: 1.0
Content-Type: text/plain;charset=iso-2022-jp
Content-Transfer-Encoding: 8bit
X-Priority: 3 (Normal)
Importance: Normal
X-Spam-Score: 0.0 (/)
X-Debbugs-Envelope-To: 35468
Cc: Eli Zaretskii <eliz@HIDDEN>, 35468 <at> debbugs.gnu.org
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -1.0 (-)

> As for the color manipulation, I went low-level as you said before:
>
>   unsigned long foreground, background;
>   gdif->get_context_foreground (gc, &foreground);
>   gdif->get_context_background (gc, &background);
>   gdif->set_context_foreground (gc, background);
>   gdif->fill_rectangle (s, gc, x, y, w, h);
>   gdif->set_context_foreground (gc, foreground);
>
> which replaces the XGetGCValues section in x_draw_stretch_glyph_string.
> This unfortunately is more work in the w32 case as it manipulates s->gc
> instead of just using the calculated gc->background. I'm not sure how
> one would make a no-op version of setting the context foreground work in
> all fill calls.
>
> If that is unsatisfactory), then instead I could do something like:
>
>   gdif->fill_rectangle_with_color (s, gc->background, gc, x, y, w, h);
>
> Which wouldn't touch s->gc for the w32 version and would do the whole
> XGetGCValues dance for the X version.

Unlike the NS port, the terminal code in the Mac port is much like the X11
version.
But there is one notable difference in the above respect.
It defines mac_erase_rectangle, which is like mac_fill_rectangle (the
XFillRectangle counterpart)
but uses the background color of GC and also handles stipples:

/* Mac replacement for XFillRectangle.  */

static void
mac_fill_rectangle (struct frame *f, GC gc, int x, int y, int width, int
height)
{
  MAC_BEGIN_DRAW_TO_FRAME (f, gc, context);
  CGContextSetFillColorWithColor (context, gc->cg_fore_color);
  {
    CGRect rect = mac_rect_make (f, x, y, width, height);

    CGContextFillRects (context, &rect, 1);
  }
  MAC_END_DRAW_TO_FRAME (f);
}

static void
mac_erase_rectangle (struct frame *f, GC gc, int x, int y,
		     int width, int height)
{
  MAC_BEGIN_DRAW_TO_FRAME (f, gc, context);
  {
    CGRect rect = mac_rect_make (f, x, y, width, height);

    CG_CONTEXT_FILL_RECT_WITH_GC_BACKGROUND (f, context, rect, gc);
    if (gc->xgcv.fill_style == FillOpaqueStippled && gc->xgcv.stipple)
      {
	CGContextClipToRects (context, &rect, 1);
	CGContextSetFillColorWithColor (context, gc->cg_fore_color);
	int scale = CFArrayGetCount (gc->xgcv.stipple);
	if (FRAME_BACKING_SCALE_FACTOR (f) < scale)
	  scale = FRAME_BACKING_SCALE_FACTOR (f);
	CGImageRef image_mask =
	  (CGImageRef) CFArrayGetValueAtIndex (gc->xgcv.stipple, scale - 1);
	rect = CGRectMake (0, 0, CGImageGetWidth (image_mask) / (CGFloat) scale,
			   CGImageGetHeight (image_mask) / (CGFloat) scale);
	CGContextScaleCTM (context, 1, -1);
	CGContextSetInterpolationQuality (context, kCGInterpolationNone);
	CGContextDrawTiledImage (context, rect, image_mask);
      }
  }
  MAC_END_DRAW_TO_FRAME (f);
}

BTW, the code between MAC_BEGIN_DRAW_TO_FRAME and MAC_END_DRAW_TO_FRAME
is executed in the dedicated drawing thread with the help of Grand Central
Dispatch.
See https://bitbucket.org/mituharu/emacs-mac/src/master/src/macterm.c for
the full source code.

I guess introducing the erase_rectagle handler makes things simpler and
efficient.

					YAMAMOTO Mitsuharu
				mituharu@HIDDEN





Information forwarded to bug-gnu-emacs@HIDDEN:
bug#35468; Package emacs. Full text available.

Message received at 35468 <at> debbugs.gnu.org:


Received: (at 35468) by debbugs.gnu.org; 30 Apr 2019 20:11:41 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Tue Apr 30 16:11:41 2019
Received: from localhost ([127.0.0.1]:42457 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1hLZ6K-0002qf-QG
	for submit <at> debbugs.gnu.org; Tue, 30 Apr 2019 16:11:41 -0400
Received: from mail-wr1-f42.google.com ([209.85.221.42]:44155)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <athird@HIDDEN>) id 1hLZ6I-0002qS-TU
 for 35468 <at> debbugs.gnu.org; Tue, 30 Apr 2019 16:11:39 -0400
Received: by mail-wr1-f42.google.com with SMTP id c5so22425944wrs.11
 for <35468 <at> debbugs.gnu.org>; Tue, 30 Apr 2019 13:11:38 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=googlemail.com; s=20161025;
 h=sender:date:from:to:cc:subject:message-id:references:mime-version
 :content-disposition:content-transfer-encoding:in-reply-to
 :user-agent; bh=2yBRKYIq6i9wS7EgfQbxL75jE6e3mlRfkLgSfOhZ9bQ=;
 b=PjaAYtlKugqiSA9Dz9sSctWMsw3PjybxA7PkJSfdC6Pbc+I0/xqg9iv1WQvP72FzUm
 4geD7x4iDKzFh/zppjQKT3t2zX9mLdJLGPRSQyy7twrLpJ+fRPuGt6Ui++eNPHIbVCLW
 MdBbmcwsA43y5GciU0ndrO0PVAtULKvqjOlIvtOFR9rF2SKX2XjrnJq18xW0Q3NOQlqm
 ICeyTvHHl2q5l63SQnFxbmWe7YeXwoERU9gUbj3hoTFccr/80M39eAxNi3UCBOmakgQY
 4RKSV/hfNdFhLidWKUN5Lx25M6EdyriQpi/iHf1TbzDbqs72gLxJkbFlO5FHQ2+IDBZU
 2P4g==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20161025;
 h=x-gm-message-state:sender:date:from:to:cc:subject:message-id
 :references:mime-version:content-disposition
 :content-transfer-encoding:in-reply-to:user-agent;
 bh=2yBRKYIq6i9wS7EgfQbxL75jE6e3mlRfkLgSfOhZ9bQ=;
 b=dS2QWplhlr6qvPrAsNIdleu+j5p49SmzMAifyqqc+9LFzizAfqBnc/f4JbDxBj8aF0
 zXFe9GLGunGxpAO8ZhNOss6cVfpaKqpT3WBx2MSZUV1gMmpZBRqes4gUfGBiFircLGap
 pkrN4gviODtyYp6OEfP2BXwFokg8vh5jxLR2H4psmt+vknHwOV/s8M2OfPq9LyEp1o17
 6TCwSEnXnVOw2QIn2RaQRj6NySVih9XvZoTE0QFKRW9FpRRQgj/P2YYMq9QoYduwRbK9
 V1DUgpQHgvW6GbF7mQB0KA/9OFEwfYVY7V8TkZAH7CGugfEcfrLOcocSLS8orQU6Kev6
 k9aA==
X-Gm-Message-State: APjAAAXmQn2R6z2E6o7N77uw6Z8OnPLWb74xs4Zjeu6njvrZCaSx3hJB
 kSQI6PmPUcfVW2kMWF57G+4=
X-Google-Smtp-Source: APXvYqyhANqB368q3XYnex8zU1qFVf5gig++trzYVyjjy6DAiAaGefyhLBZp2pDxdKhCPoiLc+V6iQ==
X-Received: by 2002:a5d:4cd1:: with SMTP id c17mr11292886wrt.231.1556655093003; 
 Tue, 30 Apr 2019 13:11:33 -0700 (PDT)
Received: from breton.holly.idiocy.org
 (ip6-2001-08b0-03f8-8129-c8db-4b16-8787-1926.holly.idiocy.org.
 [2001:8b0:3f8:8129:c8db:4b16:8787:1926])
 by smtp.gmail.com with ESMTPSA id 67sm4904057wmz.41.2019.04.30.13.11.31
 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);
 Tue, 30 Apr 2019 13:11:31 -0700 (PDT)
Date: Tue, 30 Apr 2019 21:11:29 +0100
From: Alan Third <alan@HIDDEN>
To: Eli Zaretskii <eliz@HIDDEN>
Subject: Re: bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
Message-ID: <20190430201129.GA73973@HIDDEN>
References: <877ebeor2d.fsf@HIDDEN> <83tveit5ph.fsf@HIDDEN>
 <87pnp5oqu1.fsf@HIDDEN> <877ebcogg4.fsf@HIDDEN>
 <83sgu0rsue.fsf@HIDDEN>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <83sgu0rsue.fsf@HIDDEN>
User-Agent: Mutt/1.11.3 (2019-02-01)
X-Spam-Score: 0.1 (/)
X-Debbugs-Envelope-To: 35468
Cc: 35468 <at> debbugs.gnu.org, Alex Gramiak <agrambot@HIDDEN>
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -0.9 (/)

On Tue, Apr 30, 2019 at 07:59:37AM +0300, Eli Zaretskii wrote:
> > From: Alex Gramiak <agrambot@HIDDEN>
> > Cc: 35468 <at> debbugs.gnu.org
> > Date: Mon, 29 Apr 2019 11:43:23 -0600
> > 
> > It doesn't work for NS partially because of the following section only
> > present in the NS equivalent of gui_draw_glyph_string_box. Would you be
> > okay with putting this in the generic version inside a FRAME_NS_P (s->f)
> > check? I don't know why only NS has this, though...
> > 
> >   if (s->hl == DRAW_MOUSE_FACE)
> >     {
> >       face = FACE_FROM_ID_OR_NULL (s->f,
> > 				   MOUSE_HL_INFO (s->f)->mouse_face_face_id);
> >       if (!face)
> >         face = FACE_FROM_ID (s->f, MOUSE_FACE_ID);
> >     }
> >   else
> >     face = s->face;
> 
> I don't know why this is TRT, either.  We could ask Alan to look into
> this, or we could simply remove that, as other terminals don't use
> box_line_width from the mouse face, they use s->face instead.

A quick test doesn’t turn up any immediate issues with removing this,
but I’m unsure in what circumstances DRAW_MOUSE_FACE would be in use.
-- 
Alan Third




Information forwarded to bug-gnu-emacs@HIDDEN:
bug#35468; Package emacs. Full text available.

Message received at 35468 <at> debbugs.gnu.org:


Received: (at 35468) by debbugs.gnu.org; 30 Apr 2019 18:01:03 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Tue Apr 30 14:01:03 2019
Received: from localhost ([127.0.0.1]:42370 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1hLX3v-0007yc-AY
	for submit <at> debbugs.gnu.org; Tue, 30 Apr 2019 14:01:03 -0400
Received: from mail-pf1-f195.google.com ([209.85.210.195]:44620)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <agrambot@HIDDEN>) id 1hLX3t-0007y1-3s
 for 35468 <at> debbugs.gnu.org; Tue, 30 Apr 2019 14:01:01 -0400
Received: by mail-pf1-f195.google.com with SMTP id y13so7426143pfm.11
 for <35468 <at> debbugs.gnu.org>; Tue, 30 Apr 2019 11:01:01 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;
 h=from:to:cc:subject:references:date:in-reply-to:message-id
 :user-agent:mime-version;
 bh=ezKfEvfgubBMpX7/tObcxQOtzQSg3cWOg3IMxt9/KQI=;
 b=RWN404FSyf/BQJeRCTtW1QZ1QDgZu9dxSbkZtnqq6rynO7GR48xra6PBGfTS/p/hRJ
 lh62D8yqG+7NZjQTeEstIb9MhyfHfu9xZggKyGPckiT5hL8qLbt7ATweJjzKmjkAqIB0
 p390NilqIwd2Dyr6nbezdqZfPHsuiZ4npETs8V7fhrwpzjUlJySis9q+jbW9+0JWgfml
 XCy9evT9BGKtMVPrWQXO10HB0aMNqmtRk4sDKmiLnK1kw1Jycoq8aAcwGpJ8EmOFRk0s
 msZS1QwWs5h3nJ5eJH7+Zp88cPBBYPfF7Hjz/2FvlwFET3bm8sPWpow6iTpgCojasFp8
 PzzQ==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20161025;
 h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to
 :message-id:user-agent:mime-version;
 bh=ezKfEvfgubBMpX7/tObcxQOtzQSg3cWOg3IMxt9/KQI=;
 b=HE90HCjoqXpZwwoRxAJR492dOIp7rqXnf8NKAQcfACXVxM7WB1IlBdCFnIjZK4pXTc
 jU/4nY35+FQ/976J0XOEe0d+fezG4ndfhLKZKP//mwZTUcbnmVXdfEbpzdVCsdthPpw4
 736L6S8q7Ln/lwup+i7p9rh0xDAXgdKi98j/vGNvJhY7EzRelXo5mLWm5fr+bebMHvJ7
 lyt39+7Pw4D6HbkQdhWGUGd+UiMGOb8YKgW7rddrkBBTvCF0QSHg3UWYGoEOVhP1oBap
 eBSL89gDjf0br8dLJ8jc5os3iQ53M1rB9EH7WMwWVKNEHp3euDbRkG/t4U973i6pHP8a
 GC+Q==
X-Gm-Message-State: APjAAAXqEAo+kCwo4cLRfuq2Ea8XaRYZSHOAVHIn3SeojsZJ23pJfPWG
 3V2UHYiHKEitq97RsVe7J70NaypG
X-Google-Smtp-Source: APXvYqzkYr4lYSzqTg3JHRCnWSNkpGa/ewn8xcQIbOBtv0dT4llS6U4mCkRVCDFZSj9IQxRpX6Xnzg==
X-Received: by 2002:a63:f503:: with SMTP id w3mr64621606pgh.60.1556647254366; 
 Tue, 30 Apr 2019 11:00:54 -0700 (PDT)
Received: from lylat ([2604:3d09:e37f:1500:1a72:4878:e793:7302])
 by smtp.gmail.com with ESMTPSA id c28sm27401657pgm.42.2019.04.30.11.00.51
 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);
 Tue, 30 Apr 2019 11:00:52 -0700 (PDT)
From: Alex Gramiak <agrambot@HIDDEN>
To: Eli Zaretskii <eliz@HIDDEN>
Subject: Re: bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
References: <877ebeor2d.fsf@HIDDEN> <83tveit5ph.fsf@HIDDEN>
 <87pnp5oqu1.fsf@HIDDEN> <877ebcogg4.fsf@HIDDEN>
 <83sgu0rsue.fsf@HIDDEN>
Date: Tue, 30 Apr 2019 12:00:52 -0600
In-Reply-To: <83sgu0rsue.fsf@HIDDEN> (Eli Zaretskii's message of "Tue, 30 Apr
 2019 07:59:37 +0300")
Message-ID: <8736lznzjf.fsf@HIDDEN>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.2 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
X-Spam-Score: 0.0 (/)
X-Debbugs-Envelope-To: 35468
Cc: 35468 <at> debbugs.gnu.org
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -1.0 (-)

Eli Zaretskii <eliz@HIDDEN> writes:

>> From: Alex Gramiak <agrambot@HIDDEN>
>> Cc: 35468 <at> debbugs.gnu.org
>> Date: Mon, 29 Apr 2019 11:43:23 -0600
>> 
>> I'm in {x,w32}_draw_box_rect right now, trying to generalize both
>> versions. The issue is that the fill command in each accepts different
>> arguments; specifically the w32 version takes in the color explicitly
>> and uses s->hdc instead of s->gc. So I think there will have to be 2
>> different fill_rectangle interface procedures: one for glyph strings (so
>> that the w32 version can access s->hdc), and another for other
>> procedures like *_draw_bar_cursor.
>
> Would it be possible to generalize this into a single interface?  The
> GC vs HDC part can be generalized by passing both (HDC will be NULL in
> the X and NS cases), and the color will be passed as in the w32
> version, with the X version doing its thing with GCForeground
> internally.  Alternatively, we could have a set_foreground interface
> that will do nothing for w32 and for X call XGetGCValues and
> XSetForeground, to set up s->gc.  The second alternative will probably
> be more efficient.
>
> If this doesn't work, can you tell why?

I think that there can be a single interface for both glyph_string
filling (e.g., in *_clear_glyph_string_rect), and for non-glyph_string
filling (e.g., in *_draw_bar_cursor), but I'm not sure it would be worth
it. I would think that having two separate interfaces, one taking in a
glyph string and the other taking in a frame, would be cleaner:

 fill_rectangle_glyph (struct glyph_string *, GC, int, int, int, int);
 fill_rectangle (struct frame *, GC, int, int, int, int);

There needs to be a glyph_string argument in one version so that the w32
version can use s->HDC. If there was only one interface, then one would
have to supply NULL to the glyph_string argument whenever not dealing
with glyph strings, which IMO is unclean. So is having to have two
versions, but I think it's the best option unless you know of a way to
embed HDC in w32's version of GC.

As for the color manipulation, I went low-level as you said before:

  unsigned long foreground, background;
  gdif->get_context_foreground (gc, &foreground);
  gdif->get_context_background (gc, &background);
  gdif->set_context_foreground (gc, background);
  gdif->fill_rectangle (s, gc, x, y, w, h);
  gdif->set_context_foreground (gc, foreground);

which replaces the XGetGCValues section in x_draw_stretch_glyph_string.
This unfortunately is more work in the w32 case as it manipulates s->gc
instead of just using the calculated gc->background. I'm not sure how
one would make a no-op version of setting the context foreground work in
all fill calls.

If that is unsatisfactory), then instead I could do something like:

  gdif->fill_rectangle_with_color (s, gc->background, gc, x, y, w, h);

Which wouldn't touch s->gc for the w32 version and would do the whole
XGetGCValues dance for the X version.

Alternatively, if you don't want another version, there could be a
mandatory color argument that one could supply with a pre-defined
invalid color to instead use the GC color.


I have some more questions regarding w32 incompatibilities that I ran
into:

1) Why does w32_set_mouse_face_gc use GCFont in its mask when the X
version doesn't?

2) Why does w32_draw_glyphless_glyph_string_foreground have the boolean
`with_background' instead of using false unconditionally like the X
version does? Should the X version be updated?

3) Why does w32_draw_glyph_string for FACE_UNDER_LINE use a thickness of
1 for w32_fill_area instead of using s->underline_thickness like X/NS
do? This seems like a bug.

4) The w32 versions of some procedures need to save the font around the
calls to font->driver->draw; is this necessary? Specifically, see
w32_draw_glyph_string_foreground. Assuming it's necessary, I generalized
it as:

  static void
  gui_draw_glyph_string_foreground (struct glyph_string *s)
  {
    int i, x;
    struct graphical_drawing_interface *gdif = FRAME_GDIF (s->f);

    /* If first glyph of S has a left box line, start drawing the text
       of S to the right of that box line.  */
    if (s->face->box != FACE_NO_BOX
        && s->first_glyph->left_box_line_p)
      x = s->x + eabs (s->face->box_line_width);
    else
      x = s->x;

    /* Currently just used by the w32 port to update S->hdc.  */
    if (gsif->update_secondary_context)
      gsif->update_secondary_context (s, CON_BACK | CON_FORE | CON_ALIGN);

    /* Draw characters of S as rectangles if S's font could not be
       loaded.  */
    if (s->font_not_found_p)
      {
        for (i = 0; i < s->nchars; ++i)
          {
            struct glyph *g = s->first_glyph + i;
            gdif->draw_rectangle (s, x, s->y, g->pixel_width - 1, s->height - 1);
            x += g->pixel_width;
          }
      }
    else
      {
        struct font *font = s->font;
        int boff = font->baseline_offset;
        int y;
        void *old_font;


        if (gdif->save_secondary_context_font)
          old_font = gdif->save_secondary_context_font (s);

        if (font->vertical_centering)
          boff = VCENTER_BASELINE_OFFSET (font, s->f) - boff;

        y = s->ybase - boff;
        if (s->for_overlaps
            || (s->background_filled_p && s->hl != DRAW_CURSOR))
          font->driver->draw (s, 0, s->nchars, x, y, false);
        else
          font->driver->draw (s, 0, s->nchars, x, y, true);
        if (s->face->overstrike)
          font->driver->draw (s, 0, s->nchars, x + 1, y, false);

        if (gdif->set_secondary_context_font)
          gdif->set_secondary_context_font (s, old_font);
      }
  }

Currently this requires save_secondary_context_font to allocate memory,
which is unideal. One could avoid this by introducing a new union type
(backend_font or somesuch) and using that instead of a void*. WDYT?




Information forwarded to bug-gnu-emacs@HIDDEN:
bug#35468; Package emacs. Full text available.

Message received at 35468 <at> debbugs.gnu.org:


Received: (at 35468) by debbugs.gnu.org; 30 Apr 2019 04:59:59 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Tue Apr 30 00:59:59 2019
Received: from localhost ([127.0.0.1]:40452 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1hLKs3-0007Ol-CR
	for submit <at> debbugs.gnu.org; Tue, 30 Apr 2019 00:59:59 -0400
Received: from eggs.gnu.org ([209.51.188.92]:48451)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <eliz@HIDDEN>) id 1hLKs1-0007OW-KJ
 for 35468 <at> debbugs.gnu.org; Tue, 30 Apr 2019 00:59:57 -0400
Received: from fencepost.gnu.org ([2001:470:142:3::e]:39902)
 by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from <eliz@HIDDEN>)
 id 1hLKrs-00057x-1v; Tue, 30 Apr 2019 00:59:51 -0400
Received: from [176.228.60.248] (port=1303 helo=home-c4e4a596f7)
 by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256)
 (Exim 4.82) (envelope-from <eliz@HIDDEN>)
 id 1hLKrq-0003AP-Aa; Tue, 30 Apr 2019 00:59:47 -0400
Date: Tue, 30 Apr 2019 07:59:37 +0300
Message-Id: <83sgu0rsue.fsf@HIDDEN>
From: Eli Zaretskii <eliz@HIDDEN>
To: Alex Gramiak <agrambot@HIDDEN>
In-reply-to: <877ebcogg4.fsf@HIDDEN> (message from Alex Gramiak on Mon, 29
 Apr 2019 11:43:23 -0600)
Subject: Re: bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
References: <877ebeor2d.fsf@HIDDEN> <83tveit5ph.fsf@HIDDEN>
 <87pnp5oqu1.fsf@HIDDEN> <877ebcogg4.fsf@HIDDEN>
X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]
X-Spam-Score: -2.3 (--)
X-Debbugs-Envelope-To: 35468
Cc: 35468 <at> debbugs.gnu.org
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -3.3 (---)

> From: Alex Gramiak <agrambot@HIDDEN>
> Cc: 35468 <at> debbugs.gnu.org
> Date: Mon, 29 Apr 2019 11:43:23 -0600
> 
> I'm in {x,w32}_draw_box_rect right now, trying to generalize both
> versions. The issue is that the fill command in each accepts different
> arguments; specifically the w32 version takes in the color explicitly
> and uses s->hdc instead of s->gc. So I think there will have to be 2
> different fill_rectangle interface procedures: one for glyph strings (so
> that the w32 version can access s->hdc), and another for other
> procedures like *_draw_bar_cursor.

Would it be possible to generalize this into a single interface?  The
GC vs HDC part can be generalized by passing both (HDC will be NULL in
the X and NS cases), and the color will be passed as in the w32
version, with the X version doing its thing with GCForeground
internally.  Alternatively, we could have a set_foreground interface
that will do nothing for w32 and for X call XGetGCValues and
XSetForeground, to set up s->gc.  The second alternative will probably
be more efficient.

If this doesn't work, can you tell why?

> It doesn't work for NS partially because of the following section only
> present in the NS equivalent of gui_draw_glyph_string_box. Would you be
> okay with putting this in the generic version inside a FRAME_NS_P (s->f)
> check? I don't know why only NS has this, though...
> 
>   if (s->hl == DRAW_MOUSE_FACE)
>     {
>       face = FACE_FROM_ID_OR_NULL (s->f,
> 				   MOUSE_HL_INFO (s->f)->mouse_face_face_id);
>       if (!face)
>         face = FACE_FROM_ID (s->f, MOUSE_FACE_ID);
>     }
>   else
>     face = s->face;

I don't know why this is TRT, either.  We could ask Alan to look into
this, or we could simply remove that, as other terminals don't use
box_line_width from the mouse face, they use s->face instead.

Thanks.




Information forwarded to bug-gnu-emacs@HIDDEN:
bug#35468; Package emacs. Full text available.

Message received at 35468 <at> debbugs.gnu.org:


Received: (at 35468) by debbugs.gnu.org; 29 Apr 2019 17:43:34 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Mon Apr 29 13:43:33 2019
Received: from localhost ([127.0.0.1]:39641 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1hLAJR-00036L-JQ
	for submit <at> debbugs.gnu.org; Mon, 29 Apr 2019 13:43:33 -0400
Received: from mail-pf1-f182.google.com ([209.85.210.182]:45135)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <agrambot@HIDDEN>) id 1hLAJP-000368-Dj
 for 35468 <at> debbugs.gnu.org; Mon, 29 Apr 2019 13:43:32 -0400
Received: by mail-pf1-f182.google.com with SMTP id e24so5661636pfi.12
 for <35468 <at> debbugs.gnu.org>; Mon, 29 Apr 2019 10:43:31 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;
 h=from:to:cc:subject:references:date:in-reply-to:message-id
 :user-agent:mime-version;
 bh=k1jcwuhyWT7dGgnKVfiyXuaw6E4J2JwEMB8NTEfFCAg=;
 b=e6uvhYN8q/jNdIHArDPVV9U54chsh+qSM2gnGFgwH18rQEfIEhDk9OM63C8w8LYYEl
 2pvLqr3xtjb0vKC79kRw4GwxveQ5Fi3D8A767HG4HIAJhYF5oJna0hrToIHDGTO5LxtE
 SLXNIlE4emQgZXrOqQ/2rnFpd25wbdSvOve+DujCmYZNgwffFJzqHWvxEyCVWsQTlU/a
 ZqQIREgGVlFJx0pYY3IzPG2SI2yhsw1NtGHqGZFPCKxxLJCjbhg8KADsNOUrLKD5dwrJ
 kocCsHbhW7lBf5b5eBy/8zA76NmNRapYESCTgJC+EAdVKHSyLrHpEUflVxLUJN6DNY6u
 oqcw==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20161025;
 h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to
 :message-id:user-agent:mime-version;
 bh=k1jcwuhyWT7dGgnKVfiyXuaw6E4J2JwEMB8NTEfFCAg=;
 b=RFJo39TgeIH4w8y05OMf+nG6Wqwc5iSb7FRcD1r8SHQCGKSgFES1n1dqj4l437OqcR
 53I6lKPPHRDq/PsNrrA/5cx4yls66JvymCQ1DGBvKy6TElVt3br7tPWlo1e15qez9k1G
 syG+X2Vsx00Rsqm0WlA1YuawKNuG2SQ77w++9CcR2ZBTdEW1xyEIj9e1aFjgAZ0ZGE/4
 blJKChavNjM9pWhSfTLUAx6cqZ/pCTLm4sO/Th604nBXKFYYDWRS4CYQ8IoAMl4Hkye7
 xCxp0XSfxeYfvtDU8x3pNvUjW1kvPu3Oj6ch2ezL0OJpmn/h5E5IP9BrSfMO3kEykE31
 LpYg==
X-Gm-Message-State: APjAAAW4b3hgtuwrUVBFSBI41IIeE2vGNsvFjl+cRwwY5BBcqz/Ck7eV
 JSnq4ipXVq+LSnBBk/WOpu+EColw
X-Google-Smtp-Source: APXvYqyBMvzM/sZrfggDCZUkchgEo3HzcarbuFYcKf0ZhESoejJ3CGUWsErZIL4tOq+PO3/4RHJLEw==
X-Received: by 2002:a63:42:: with SMTP id 63mr1929790pga.337.1556559805229;
 Mon, 29 Apr 2019 10:43:25 -0700 (PDT)
Received: from lylat ([2604:3d09:e37f:1500:1a72:4878:e793:7302])
 by smtp.gmail.com with ESMTPSA id j16sm44275767pfi.58.2019.04.29.10.43.23
 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);
 Mon, 29 Apr 2019 10:43:24 -0700 (PDT)
From: Alex Gramiak <agrambot@HIDDEN>
To: Eli Zaretskii <eliz@HIDDEN>
Subject: Re: bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
References: <877ebeor2d.fsf@HIDDEN> <83tveit5ph.fsf@HIDDEN>
 <87pnp5oqu1.fsf@HIDDEN>
Date: Mon, 29 Apr 2019 11:43:23 -0600
In-Reply-To: <87pnp5oqu1.fsf@HIDDEN> (Alex Gramiak's message of "Sun, 28
 Apr 2019 13:46:46 -0600")
Message-ID: <877ebcogg4.fsf@HIDDEN>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.2 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
X-Spam-Score: 0.0 (/)
X-Debbugs-Envelope-To: 35468
Cc: 35468 <at> debbugs.gnu.org
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -1.0 (-)

>> The result of this refactoring should be more low-level and more
>> primitive interfaces, and they should each one make sense, not be
>> ad-hoc.  It means the job becomes more complex, and you will probably
>> need to ask questions regarding the GUI systems you are less familiar
>> with.  But the result will IMO much better and future-proof.
>
> I'll see what I can do.

Alright, for the first question may I ask if you're okay with the
following abstraction (which may be the start of a pattern)?

I'm in {x,w32}_draw_box_rect right now, trying to generalize both
versions. The issue is that the fill command in each accepts different
arguments; specifically the w32 version takes in the color explicitly
and uses s->hdc instead of s->gc. So I think there will have to be 2
different fill_rectangle interface procedures: one for glyph strings (so
that the w32 version can access s->hdc), and another for other
procedures like *_draw_bar_cursor.

  void
  gui_draw_glyph_string_box (struct glyph_string *s)
  {
    int width, left_x, right_x, top_y, bottom_y, last_x;
    bool raised_p, left_p, right_p;
    struct glyph *last_glyph;

    last_x = ((s->row->full_width_p && !s->w->pseudo_window_p)
              ? WINDOW_RIGHT_EDGE_X (s->w)
              : window_box_right (s->w, s->area));

    /* The glyph that may have a right box line.  */
    last_glyph = (s->cmp || s->img
                  ? s->first_glyph
                  : s->first_glyph + s->nchars - 1);

    width = eabs (s->face->box_line_width);
    raised_p = s->face->box == FACE_RAISED_BOX;
    left_x = s->x;
    right_x = (s->row->full_width_p && s->extends_to_end_of_line_p
               ? last_x - 1
               : min (last_x, s->x + s->background_width) - 1);
    top_y = s->y;
    bottom_y = top_y + s->height - 1;

    left_p = (s->first_glyph->left_box_line_p
              || (s->hl == DRAW_MOUSE_FACE
                  && (s->prev == NULL
                      || s->prev->hl != s->hl)));
    right_p = (last_glyph->right_box_line_p
               || (s->hl == DRAW_MOUSE_FACE
                   && (s->next == NULL
                       || s->next->hl != s->hl)));

    if (s->face->box == FACE_SIMPLE_BOX)
      FRAME_GDIF (s->f)->draw_box_rect (s,
                                        left_x,
                                        top_y,
                                        right_x,
                                        bottom_y,
                                        width,
                                        left_p,
                                        right_p);
    else
      {
        FRAME_GDIF (s->f)->setup_relief_colors (s);
        FRAME_GDIF (s->f)->draw_relief_rect (s->f,
                                             left_x,
                                             top_y,
                                             right_x,
                                             bottom_y,
                                             width,
                                             raised_p,
                                             true,
                                             true,
                                             left_p,
                                             right_p);
      }
  }

  void
  gui_draw_box_rect (struct glyph_string *s,
                     int left_x, int top_y, int right_x, int bottom_y, int width,
                     bool left_p, bool right_p)
  {
    struct graphical_drawing_interface *gdif = FRAME_GDIF (s->f);

    gdif->save_context (s);

    gdif->set_context_clip (s);
    gdif->set_context_foreground (s, s->face->box_color);

    /* Top.  */
    gdif->fill_rectangle (s, s->gc, left_x, top_y, right_x - left_x + 1, width);

    /* Left.  */
    if (left_p)
      {
        gdif->fill_rectangle (s, s->gc,
                              left_x, top_y,
                              width, bottom_y - top_y + 1);
      }

    /* Bottom.  */
    gdif->fill_rectangle (s, s->gc,
                          left_x, bottom_y - width + 1,
                          right_x - left_x + 1, width);

    /* Right.  */
    if (right_p)
      {
        gdif->fill_rectangle (s, s->gc,
                              right_x - width + 1, top_y,
                              width, bottom_y - top_y + 1);
      }

    gdif->reset_context (s);
  }

It doesn't work for NS partially because of the following section only
present in the NS equivalent of gui_draw_glyph_string_box. Would you be
okay with putting this in the generic version inside a FRAME_NS_P (s->f)
check? I don't know why only NS has this, though...

  if (s->hl == DRAW_MOUSE_FACE)
    {
      face = FACE_FROM_ID_OR_NULL (s->f,
				   MOUSE_HL_INFO (s->f)->mouse_face_face_id);
      if (!face)
        face = FACE_FROM_ID (s->f, MOUSE_FACE_ID);
    }
  else
    face = s->face;




Information forwarded to bug-gnu-emacs@HIDDEN:
bug#35468; Package emacs. Full text available.

Message received at 35468 <at> debbugs.gnu.org:


Received: (at 35468) by debbugs.gnu.org; 28 Apr 2019 19:46:54 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Sun Apr 28 15:46:54 2019
Received: from localhost ([127.0.0.1]:37842 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1hKplF-00029L-PL
	for submit <at> debbugs.gnu.org; Sun, 28 Apr 2019 15:46:54 -0400
Received: from mail-pg1-f172.google.com ([209.85.215.172]:33098)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <agrambot@HIDDEN>) id 1hKplD-000297-Bm
 for 35468 <at> debbugs.gnu.org; Sun, 28 Apr 2019 15:46:51 -0400
Received: by mail-pg1-f172.google.com with SMTP id k19so4141819pgh.0
 for <35468 <at> debbugs.gnu.org>; Sun, 28 Apr 2019 12:46:51 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;
 h=from:to:cc:subject:references:date:in-reply-to:message-id
 :user-agent:mime-version;
 bh=2TFJW3W/u+Z4Hb+A01WKvKy13Ypg36Aso/+c192xkjo=;
 b=cK7XTJxvWQbZvrPj9e/VjH6uN912DOAc64dkGoajBzwvh0i/03Ye241LvTSbjTrQqK
 YlLkiPTgdzTNfyqK3vgVQkPapGHujCD3m3ok4q3FJyXH81hSKbwY95NdmdrZJb1V9wBg
 5bQgjLdToFkGkES0hFifHww9fYRqq22A68zp6rSVQUF97e/OIXEKNXfvKkZn1vxibAM6
 nj95jAvrp8bJkpVogCe5XUEcYpWhEgcuO4yijOv13IDy4B0fp9Cg/tmJwNZhXnnnE+n+
 0NrpbZK5O/icBOr3ktDSXzJAq+Db4KcXkiJk0Qlabb5AN/pfxx7+BxNVx/k9m++EXTdq
 zn7Q==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20161025;
 h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to
 :message-id:user-agent:mime-version;
 bh=2TFJW3W/u+Z4Hb+A01WKvKy13Ypg36Aso/+c192xkjo=;
 b=q+K+MFkerkMQzj5x1d7aFvsmp/ZWW/QO/uwFvVnC+8BT6DxRP2tI47PkhL+GN6O9rs
 Xh6QSZhaRhwEprN3oabgBCSBErYr9tBdYjOQghXWTn2RP4TABX+UQ3xpzzZRTErMD8Xb
 PZyYDPY/bqk13KR0o0rO/vImrF6ma/hvLbLB5qeAevRByo8vVF2pjUJzbZA/8tMr9L6Z
 tuDsYO7oFlZOgf9BhYNrNqnKGAjo2Fyw72rWqO3skCyX6ySo2Pz3xC5mZ59V5B2b9TnY
 PZZbeTqKpSo2kauX47wbLvt1C2Yrb5n+RT4/h5+637DTDwhKTD71t8ey9wuW/rQHZQwL
 1CoA==
X-Gm-Message-State: APjAAAU90RxS2ejWTlSfdduuNZYAIczRShcKNTxU6l0CQ/wG2nb5I9qA
 L3+xzgwWB5KHkIvJo9ePd0HfsvIK
X-Google-Smtp-Source: APXvYqxL2W4oxxTRu/j/rABKLYsBZQtNE2JD+dDZejXcS+q8b4XSeBXgRuaTyrWeCrB29xWjDU8HsA==
X-Received: by 2002:aa7:8e14:: with SMTP id c20mr27416773pfr.14.1556480805038; 
 Sun, 28 Apr 2019 12:46:45 -0700 (PDT)
Received: from lylat ([2604:3d09:e37f:1500:1a72:4878:e793:7302])
 by smtp.gmail.com with ESMTPSA id u62sm50587215pfa.124.2019.04.28.12.46.43
 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);
 Sun, 28 Apr 2019 12:46:44 -0700 (PDT)
From: Alex Gramiak <agrambot@HIDDEN>
To: Eli Zaretskii <eliz@HIDDEN>
Subject: Re: bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
References: <877ebeor2d.fsf@HIDDEN> <83tveit5ph.fsf@HIDDEN>
Date: Sun, 28 Apr 2019 13:46:46 -0600
In-Reply-To: <83tveit5ph.fsf@HIDDEN> (Eli Zaretskii's message of "Sun, 28 Apr
 2019 20:11:54 +0300")
Message-ID: <87pnp5oqu1.fsf@HIDDEN>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.2 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
X-Spam-Score: 0.0 (/)
X-Debbugs-Envelope-To: 35468
Cc: 35468 <at> debbugs.gnu.org
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -1.0 (-)

Eli Zaretskii <eliz@HIDDEN> writes:

>> From: Alex Gramiak <agrambot@HIDDEN>
>> Date: Sat, 27 Apr 2019 19:29:30 -0600
>> 
>> This merges x_draw_glyph_string and w32_draw_glyph_string together to
>> remove duplicated code.
>> 
>> I wanted to do it for NS as well, but it seems just a bit too different
>> to make it easily work.
>
> If we don't include NS in this, we won't actually gain enough to
> justify the reshuffle.

I still think that removing duplication between 2 out of 3 is worth it
considering the size of the removal, but I started to do this because
I'm working on a new backend (GTK without depending on X) that would
benefit from this refactoring.

> What prevented you from including NS, it looks to me the code is very
> similar?

Mostly the differences in clipping behaviour that seemed just slightly
incompatible with the way gui_draw_glyphs_string does it. I'll look at
it a bit harder.

It also doesn't have the two sections (the s->prev and s->next
conditionals) at the end, which I'm not sure is a bug or unnecessary
there. Those could just be surrounded by a (!FRAME_NS_P (s->f))
conditional though.

>> +struct draw_glyph_string_interface
>
> I'd prefer draw_glyphs_interface, it's shorter.  And given the
> comments below, maybe the name should be gui_interface.

draw_glyphs_interface doesn't seem bad, as long as glyph strings are the
only types of glyphs that are drawn (otherwise draw_glyphs would be too
broad).

I think gui_interface is too broad of a description, since many of the
terminal hooks and frame parameter handlers (setting GUI window
properties, accessing color structures, setting WM hints, etc.) could
also be considered as belonging to a GUI interface. Perhaps
gui_drawing_interface or graphical_drawing_interface?

> The result of this refactoring should be more low-level and more
> primitive interfaces, and they should each one make sense, not be
> ad-hoc.  It means the job becomes more complex, and you will probably
> need to ask questions regarding the GUI systems you are less familiar
> with.  But the result will IMO much better and future-proof.

I'll see what I can do.




Information forwarded to bug-gnu-emacs@HIDDEN:
bug#35468; Package emacs. Full text available.

Message received at 35468 <at> debbugs.gnu.org:


Received: (at 35468) by debbugs.gnu.org; 28 Apr 2019 17:12:14 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Sun Apr 28 13:12:14 2019
Received: from localhost ([127.0.0.1]:37718 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1hKnLZ-0006fP-Po
	for submit <at> debbugs.gnu.org; Sun, 28 Apr 2019 13:12:14 -0400
Received: from eggs.gnu.org ([209.51.188.92]:50414)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <eliz@HIDDEN>) id 1hKnLY-0006fC-PA
 for 35468 <at> debbugs.gnu.org; Sun, 28 Apr 2019 13:12:13 -0400
Received: from fencepost.gnu.org ([2001:470:142:3::e]:38449)
 by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from <eliz@HIDDEN>)
 id 1hKnLT-0007GL-Ms; Sun, 28 Apr 2019 13:12:07 -0400
Received: from [176.228.60.248] (port=3612 helo=home-c4e4a596f7)
 by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256)
 (Exim 4.82) (envelope-from <eliz@HIDDEN>)
 id 1hKnLS-0006Xh-R9; Sun, 28 Apr 2019 13:12:07 -0400
Date: Sun, 28 Apr 2019 20:11:54 +0300
Message-Id: <83tveit5ph.fsf@HIDDEN>
From: Eli Zaretskii <eliz@HIDDEN>
To: Alex Gramiak <agrambot@HIDDEN>
In-reply-to: <877ebeor2d.fsf@HIDDEN> (message from Alex Gramiak on Sat, 27
 Apr 2019 19:29:30 -0600)
Subject: Re: bug#35468: [PATCH] Refactor draw_glyph_string on X and w32
References: <877ebeor2d.fsf@HIDDEN>
X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]
X-Spam-Score: -2.3 (--)
X-Debbugs-Envelope-To: 35468
Cc: 35468 <at> debbugs.gnu.org
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -3.3 (---)

> From: Alex Gramiak <agrambot@HIDDEN>
> Date: Sat, 27 Apr 2019 19:29:30 -0600
> 
> This merges x_draw_glyph_string and w32_draw_glyph_string together to
> remove duplicated code.
> 
> I wanted to do it for NS as well, but it seems just a bit too different
> to make it easily work.

If we don't include NS in this, we won't actually gain enough to
justify the reshuffle.  What prevented you from including NS, it looks
to me the code is very similar?  But maybe wait with the answer until
you read the rest of the comments below.

> +struct draw_glyph_string_interface

I'd prefer draw_glyphs_interface, it's shorter.  And given the
comments below, maybe the name should be gui_interface.

> -x_draw_glyph_string (struct glyph_string *s)
> +x_draw_underwave (struct glyph_string *s)
>  {
> -  bool relief_drawn_p = false;
> -
> -  /* If S draws into the background of its successors, draw the
> -     background of the successors first so that S can draw into it.
> -     This makes S->next use XDrawString instead of XDrawImageString.  */
> -  if (s->next && s->right_overhang && !s->for_overlaps)
> +  if (s->face->underline_defaulted_p)
> +    x_draw_underwave_1 (s);
> +  else
>      {
> -      int width;
> -      struct glyph_string *next;
> -
> -      for (width = 0, next = s->next;
> -	   next && width < s->right_overhang;
> -	   width += next->width, next = next->next)
> -	if (next->first_glyph->type != IMAGE_GLYPH)
> -	  {
> -	    x_set_glyph_string_gc (next);
> -	    x_set_glyph_string_clipping (next);
> -	    if (next->first_glyph->type == STRETCH_GLYPH)
> -	      x_draw_stretch_glyph_string (next);
> -	    else
> -	      x_draw_glyph_string_background (next, true);
> -	    next->num_clips = 0;
> -	  }
> +      XGCValues xgcv;
> +      XGetGCValues (s->display, s->gc, GCForeground, &xgcv);
> +      XSetForeground (s->display, s->gc, s->face->underline_color);
> +      x_draw_underwave_1 (s);
> +      XSetForeground (s->display, s->gc, xgcv.foreground);
>      }
> +}

I think this stops short of the goal.  The goal is to refactor the
code into terminal-independent code and terminal-dependent code, and
then arrange the latter in meaningful interfaces that will be useful
henceforth for writing display code.  By contrast, what you did leaves
in terminal-specific parts code that is completely independent of the
window-system, and OTOH doesn't try to identify terminal-specific
parts which are different implementations of the same general idea.
For example, all GUI terminals have code that sets foreground and
background colors, code that clears a rectangle, code that fills a
rectangle with a given color, code that draws a line between 2 points,
etc.  We should identify such pieces of code, give them names, and
make them the methods called via the redisplay interface pointers.  As
a rule, no terminal-independent code should ever appear in the
window-system specific part, not even logic that looks at
terminal-independent data structures, like this, for example:

  +      if (s->face->strike_through_color_defaulted_p)
  +        {
  +          w32_fill_area (s->f, s->hdc, s->gc->foreground, s->x,
  +                         y, s->width, height);
  +        }
  +      else
  +        {
  +          w32_fill_area (s->f, s->hdc, s->face->strike_through_color, s->x,
  +                         y, s->width, height);
  +        }

The meaning of s->face->strike_through_color_defaulted_p has no place
in w32term.c, it should be in the terminal-independent part, and
w32_fill_area should be a w32-specific implementation of a GUI drawing
primitive.  Likewise with stuff like XGetGCValues on X and DC on w32.

IOW, if we are refactoring this stuff, let's do it right, not just
ad-hoc because the current code by some chance lends itself to a
particular division into parts, not just because it's easy.  If we go
the easy way, chances are that tomorrow someone who'd like to add a
new GUI feature will again have to duplicate code because of lack of
necessary primitive building blocks, which were left lumped together
with terminal-independent code instead of being separated.

The result of this refactoring should be more low-level and more
primitive interfaces, and they should each one make sense, not be
ad-hoc.  It means the job becomes more complex, and you will probably
need to ask questions regarding the GUI systems you are less familiar
with.  But the result will IMO much better and future-proof.

And once again, please put all the new gui_* functions extracted from
*term.c on a new file gui_term.c, not in xdisp.c.

Thanks.




Information forwarded to bug-gnu-emacs@HIDDEN:
bug#35468; Package emacs. Full text available.

Message received at submit <at> debbugs.gnu.org:


Received: (at submit) by debbugs.gnu.org; 28 Apr 2019 01:47:46 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Sat Apr 27 21:47:46 2019
Received: from localhost ([127.0.0.1]:36426 helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.84_2)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1hKYuu-0004jB-RT
	for submit <at> debbugs.gnu.org; Sat, 27 Apr 2019 21:47:46 -0400
Received: from eggs.gnu.org ([209.51.188.92]:32812)
 by debbugs.gnu.org with esmtp (Exim 4.84_2)
 (envelope-from <agrambot@HIDDEN>) id 1hKYus-0004ix-4c
 for submit <at> debbugs.gnu.org; Sat, 27 Apr 2019 21:47:44 -0400
Received: from lists.gnu.org ([209.51.188.17]:36208)
 by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32)
 (Exim 4.71) (envelope-from <agrambot@HIDDEN>) id 1hKYum-0001S8-N8
 for submit <at> debbugs.gnu.org; Sat, 27 Apr 2019 21:47:36 -0400
Received: from eggs.gnu.org ([209.51.188.92]:47064)
 by lists.gnu.org with esmtp (Exim 4.71)
 (envelope-from <agrambot@HIDDEN>) id 1hKYui-00017P-9j
 for bug-gnu-emacs@HIDDEN; Sat, 27 Apr 2019 21:47:36 -0400
X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on eggs.gnu.org
X-Spam-Level: 
X-Spam-Status: No, score=0.8 required=5.0 tests=BAYES_50,FREEMAIL_FROM
 autolearn=disabled version=3.3.2
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
 (envelope-from <agrambot@HIDDEN>) id 1hKYiM-0004D5-HY
 for bug-gnu-emacs@HIDDEN; Sat, 27 Apr 2019 21:34:50 -0400
Received: from mail-pg1-x536.google.com ([2607:f8b0:4864:20::536]:45063)
 by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16)
 (Exim 4.71) (envelope-from <agrambot@HIDDEN>) id 1hKYiL-0004CQ-Ta
 for bug-gnu-emacs@HIDDEN; Sat, 27 Apr 2019 21:34:46 -0400
Received: by mail-pg1-x536.google.com with SMTP id y27so2684028pgl.12
 for <bug-gnu-emacs@HIDDEN>; Sat, 27 Apr 2019 18:34:45 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;
 h=from:to:subject:date:message-id:user-agent:mime-version;
 bh=GFaklXzamYDn62MpIP+cI13t1zpLJLxXReIMdGfWyG8=;
 b=Wcnmc9IoIdsJRlg+vmiDzJXwUT/OvQKaL54hrMqKih+9kyboZ/5Jyjl51a+aPWR+i3
 e9k976WfUpZyGkTGZR/YD+DwO4NlGLkbf0PImt/j+5ReB0tLI7uKE517wysc1SNvylvp
 gGDKmpe4rBSMfE3+La+7D8lpJSwcO8a3qpX0lIczb5IzOSvP8Zfk+MBUJob4ock5V95h
 aFahDUn092KLUDEv9ZEpAhB8zRmti6qLder5gz9BW1zJ/c4V6w1oxgmSexU5CKvJ6h9N
 MvTKu5UvyjXD1FeHdL3ZaXNCISj2R3arW3jN/wCpbS9ReZmzSbvp3wOPv/DpOt0p5faa
 Ugog==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=1e100.net; s=20161025;
 h=x-gm-message-state:from:to:subject:date:message-id:user-agent
 :mime-version;
 bh=GFaklXzamYDn62MpIP+cI13t1zpLJLxXReIMdGfWyG8=;
 b=RLdZPW0C3jmLg3axy5fQg60yG2L3rm+f/7UQMsaP1I2Tw0KOReqLTMxhoxqnd7Z99B
 o9nPruFJFm7AAYHn+1dy74y9krtcv7762PNZ+FcT7A0tq3yCYjJp7XzI52t5rqc0oaWq
 tNSfh6AvJttHUhK7hWjtcyop5Jb8iuIV9Gz4H24Br6ufVcYWucX4RHHl5GotgZAjTzyR
 UuKGA2ez/msKgkCNx+U8TiKqRTgFcw7W4QVKxt0psyWdFyCK8Whkb0Uw5j5JLHvujl11
 6SI05FEPMUhrVy9QXUOK5BYBLMGb3ND9R29fUKfklkF18iIy/0xbVzXK/RHEtLlxNZcf
 EWAQ==
X-Gm-Message-State: APjAAAX/XBVUWV7qA89Ap8J2qqWH8Oe2qb3azFrcRhj8bQXyYdC7U3lD
 bSD4hbZSjnEAFhJilFBuFrk8Cvpq
X-Google-Smtp-Source: APXvYqyaNaIvzWivsNeeS4ZW8NhgoTCVjAXFduScBySjTNnUjRaofFtq4fYmst+BjZbM4HZ3GyxQKA==
X-Received: by 2002:a63:1359:: with SMTP id 25mr50741691pgt.92.1556415283900; 
 Sat, 27 Apr 2019 18:34:43 -0700 (PDT)
Received: from lylat ([2604:3d09:e37f:1500:1a72:4878:e793:7302])
 by smtp.gmail.com with ESMTPSA id 19sm40142426pga.88.2019.04.27.18.34.42
 for <bug-gnu-emacs@HIDDEN>
 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);
 Sat, 27 Apr 2019 18:34:42 -0700 (PDT)
From: Alex Gramiak <agrambot@HIDDEN>
To: bug-gnu-emacs@HIDDEN
Subject: [PATCH] Refactor draw_glyph_string on X and w32
Date: Sat, 27 Apr 2019 19:29:30 -0600
Message-ID: <877ebeor2d.fsf@HIDDEN>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.2 (gnu/linux)
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="=-=-="
X-detected-operating-system: by eggs.gnu.org: Genre and OS details not
 recognized.
X-Received-From: 2607:f8b0:4864:20::536
X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6.x
X-Spam-Score: -1.3 (-)
X-Debbugs-Envelope-To: submit
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.18
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <https://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <https://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit/>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <https://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>, 
 <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
Sender: "Debbugs-submit" <debbugs-submit-bounces <at> debbugs.gnu.org>
X-Spam-Score: -2.3 (--)

--=-=-=
Content-Type: text/plain

This merges x_draw_glyph_string and w32_draw_glyph_string together to
remove duplicated code.

I wanted to do it for NS as well, but it seems just a bit too different
to make it easily work.


--=-=-=
Content-Type: text/x-patch
Content-Disposition: attachment;
 filename=0001-Refactor-draw_glyph_string-on-X-and-w32.patch

From 1cd69c3bcfb79f8df61d1cacf75effb1947d869f Mon Sep 17 00:00:00 2001
From: Alexander Gramiak <agrambot@HIDDEN>
Date: Sat, 27 Apr 2019 18:45:59 -0600
Subject: [PATCH] Refactor draw_glyph_string on X and w32

* src/dispextern.h (draw_glyph_string_interface): New interface for
system-dependent functions for draw_glyph_string.
(redisplay_interface): Add draw_glyph_string_interface.

* src/xdisp.c (gui_draw_glyph_string): New function implementing the
common behaviour of x_draw_glyph_string and w32_draw_glyph_string.

* src/w32term.c (w32_draw_glyph_string): Remove in favour of
gui_draw_glyph_string.
(w32_draw_underwave): Rename to w32_draw_underwave_1.
(w32_draw_underwave, w32_draw_underline, w32_draw_overline)
(w32_draw_strike_through, w32_reset_glyph_string_clipping): Define
extracted functions out of w32_draw_glyph_string.

* src/xterm.c (x_draw_glyph_string): Remove in favour of
gui_draw_glyph_string.
(x_draw_underwave): Rename to x_draw_underwave_1.
(x_draw_underwave, x_draw_underline, x_draw_overline)
(x_draw_strike_through, x_reset_glyph_string_clipping): Define
extracted functions out of x_draw_glyph_string.
---
 src/dispextern.h |  98 +++++++++++
 src/nsterm.m     |  14 +-
 src/w32term.c    | 414 +++++++++++------------------------------------
 src/xdisp.c      | 292 +++++++++++++++++++++++++++++++++
 src/xterm.c      | 405 ++++++++++------------------------------------
 5 files changed, 568 insertions(+), 655 deletions(-)

diff --git a/src/dispextern.h b/src/dispextern.h
index 827eed86f1..20124be817 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -1389,6 +1389,98 @@ struct glyph_string
   struct glyph_string *next, *prev;
 };
 
+/* Structure holding system-dependent interface functions needed
+   for drawing glyph strings as used by `gui_draw_glyph_string'.
+
+   * If gui_draw_glyph_string is used as the RIF draw_glyph_string,
+   then all of the following must be defined with the possible
+   exception of draw_xwidget.  */
+
+struct draw_glyph_string_interface
+{
+  /* Set S->gc of glyph string S for drawing that glyph string.  */
+
+  void (*set_gc) (struct glyph_string *s);
+
+  /* Set clipping for output of glyph string S.  */
+
+  void (*set_clipping) (struct glyph_string *s);
+
+  /* Set SRC's clipping for output of glyph string DST.  This is
+     called when we are drawing DST's left_overhang or right_overhang
+     only in the area of SRC.  */
+
+  void (*set_clipping_exactly) (struct glyph_string *src,
+                                struct glyph_string *dst);
+
+  /* Reset clipping for output of glyph string S.  */
+
+  void (*reset_clipping) (struct glyph_string *s);
+
+  /* Draw the background of glyph_string S.  If S->background_filled_p
+   is non-zero don't draw it.  FORCE_P non-zero means draw the
+   background even if it wouldn't be drawn normally.  This is used
+   when a string preceding S draws into the background of S, or S
+   contains the first component of a composition.  */
+
+  void (*draw_background) (struct glyph_string *s, bool force_p);
+
+  /* Draw the foreground of glyph string S.  */
+
+  void (*draw_foreground) (struct glyph_string *s);
+
+  /* Draw a box around glyph string S.  */
+
+  void (*draw_box) (struct glyph_string *s);
+
+  /* Draw image glyph string S.
+
+            s->y
+   s->x      +-------------------------
+	     |   s->face->box
+	     |
+	     |     +-------------------------
+	     |     |  s->img->margin
+	     |     |
+	     |     |       +-------------------
+	     |     |       |  the image
+
+  */
+
+  void (*draw_image) (struct glyph_string *s);
+
+  /* Draw stretch glyph string S.  */
+
+  void (*draw_stretch) (struct glyph_string *s);
+
+  /* Draw xwidget stretch glyph string S.  */
+
+  void (*draw_xwidget) (struct glyph_string *s);
+
+  /* Draw the foreground of composite glyph string S.  */
+
+  void (*draw_composite_foreground) (struct glyph_string *s);
+
+  /* Draw the foreground of glyph string S for glyphless characters.  */
+
+  void (*draw_glyphless_foreground) (struct glyph_string *s);
+
+  /* Draw underwave glyph string S.  */
+
+  void (*draw_underwave) (struct glyph_string *s);
+
+  /* Draw underline glyph string S.  */
+
+  void (*draw_underline) (struct glyph_string *s);
+
+  /* Draw overline glyph string S.  */
+
+  void (*draw_overline) (struct glyph_string *s);
+
+  /* Draw strike-through glyph string S.  */
+
+  void (*draw_strike_through) (struct glyph_string *s, int y, int height);
+};
 #endif /* HAVE_WINDOW_SYSTEM */
 
 
@@ -2890,6 +2982,10 @@ struct redisplay_interface
   /* Draw a glyph string S.  */
   void (*draw_glyph_string) (struct glyph_string *s);
 
+  /* Interface for drawing glyph strings used by
+     gui_draw_glyph_string.  */
+  struct draw_glyph_string_interface *gsif;
+
   /* Define cursor CURSOR on frame F.  */
   void (*define_frame_cursor) (struct frame *f, Cursor cursor);
 
@@ -3288,6 +3384,8 @@ extern void gui_write_glyphs (struct window *, struct glyph_row *,
                               struct glyph *, enum glyph_row_area, int);
 extern void gui_insert_glyphs (struct window *, struct glyph_row *,
                                struct glyph *, enum glyph_row_area, int);
+extern void gui_draw_glyph_string (struct glyph_string *);
+
 extern void gui_clear_end_of_line (struct window *, struct glyph_row *,
                                    enum glyph_row_area, int);
 extern void gui_fix_overlapping_area (struct window *, struct glyph_row *,
diff --git a/src/nsterm.m b/src/nsterm.m
index 2a2b8cbaba..e7b3ab4ebf 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -5107,6 +5107,7 @@ static Lisp_Object ns_string_to_lispmod (const char *s)
   0, /* destroy_fringe_bitmap */
   ns_compute_glyph_string_overhangs,
   ns_draw_glyph_string,
+  NULL, /* draw_glyph_string_interface */
   ns_define_frame_cursor,
   ns_clear_frame_area,
   0, /* clear_under_internal_border */
@@ -9507,19 +9508,6 @@ Nil means use fullscreen the old (< 10.7) way.  The old way works better with
 	       doc: /* SKIP: real doc in xterm.c.  */);
   Vx_toolkit_scroll_bars = Qt;
 
-  DEFVAR_BOOL ("x-use-underline-position-properties",
-	       x_use_underline_position_properties,
-     doc: /* SKIP: real doc in xterm.c.  */);
-  x_use_underline_position_properties = 0;
-  DEFSYM (Qx_use_underline_position_properties,
-	  "x-use-underline-position-properties");
-
-  DEFVAR_BOOL ("x-underline-at-descent-line",
-	       x_underline_at_descent_line,
-     doc: /* SKIP: real doc in xterm.c.  */);
-  x_underline_at_descent_line = 0;
-  DEFSYM (Qx_underline_at_descent_line, "x-underline-at-descent-line");
-
   /* Tell Emacs about this window system.  */
   Fprovide (Qns, Qnil);
 
diff --git a/src/w32term.c b/src/w32term.c
index 8d5f57836c..947c67abb3 100644
--- a/src/w32term.c
+++ b/src/w32term.c
@@ -328,7 +328,7 @@ w32_get_scale_factor(struct w32_display_info *dpyinfo, int *scale_x, int *scale_
 */
 
 static void
-w32_draw_underwave (struct glyph_string *s, COLORREF color)
+w32_draw_underwave_1 (struct glyph_string *s, COLORREF color)
 {
   struct w32_display_info *dpyinfo = FRAME_DISPLAY_INFO (s->f);
 
@@ -394,6 +394,71 @@ w32_draw_underwave (struct glyph_string *s, COLORREF color)
   DeleteObject (hp);
 }
 
+static void
+w32_draw_underwave (struct glyph_string *s)
+{
+  COLORREF color;
+
+  if (s->face->underline_defaulted_p)
+    color = s->gc->foreground;
+  else
+    color = s->face->underline_color;
+
+  w32_draw_underwave_1 (s, color);
+}
+
+static void
+w32_draw_underline (struct glyph_string *s)
+{
+  const int y = s->ybase + s->underline_position;
+
+  if (s->face->underline_defaulted_p)
+    {
+      w32_fill_area (s->f, s->hdc, s->gc->foreground, s->x,
+                     y, s->width, 1);
+    }
+  else
+    {
+      w32_fill_area (s->f, s->hdc, s->face->underline_color, s->x,
+                     y, s->width, 1);
+    }
+}
+
+static void
+w32_draw_overline (struct glyph_string *s)
+{
+  unsigned long dy = 0, h = 1;
+
+  if (s->face->overline_color_defaulted_p)
+    {
+      w32_fill_area (s->f, s->hdc, s->gc->foreground, s->x,
+                     s->y + dy, s->width, h);
+    }
+  else
+    {
+      w32_fill_area (s->f, s->hdc, s->face->overline_color, s->x,
+                     s->y + dy, s->width, h);
+    }
+}
+
+static void
+w32_draw_strike_through (struct glyph_string *s, int y, int height)
+{
+  if (!FONT_TEXTMETRIC (s->font).tmStruckOut)
+    {
+      if (s->face->strike_through_color_defaulted_p)
+        {
+          w32_fill_area (s->f, s->hdc, s->gc->foreground, s->x,
+                         y, s->width, height);
+        }
+      else
+        {
+          w32_fill_area (s->f, s->hdc, s->face->strike_through_color, s->x,
+                         y, s->width, height);
+        }
+    }
+}
+
 /* Draw a hollow rectangle at the specified position.  */
 static void
 w32_draw_rectangle (HDC hdc, XGCValues *gc, int x, int y,
@@ -1132,6 +1197,13 @@ w32_set_glyph_string_clipping_exactly (struct glyph_string *src,
   w32_set_clip_rectangle (dst->hdc, &r);
 }
 
+static void
+w32_reset_glyph_string_clipping (struct glyph_string *s)
+{
+  w32_set_clip_rectangle (s->hdc, NULL);
+  s->num_clips = 0;
+}
+
 /* RIF:
    Compute left and right overhang of glyph string S.  */
 
@@ -2384,308 +2456,6 @@ w32_draw_stretch_glyph_string (struct glyph_string *s)
 }
 
 
-/* Draw glyph string S.  */
-
-static void
-w32_draw_glyph_string (struct glyph_string *s)
-{
-  bool relief_drawn_p = 0;
-
-  /* If S draws into the background of its successor, draw the
-     background of the successor first so that S can draw into it.
-     This makes S->next use XDrawString instead of XDrawImageString.  */
-  if (s->next && s->right_overhang && !s->for_overlaps)
-    {
-      int width;
-      struct glyph_string *next;
-      for (width = 0, next = s->next;
-	   next && width < s->right_overhang;
-           width += next->width, next = next->next)
-        if (next->first_glyph->type != IMAGE_GLYPH)
-          {
-            w32_set_glyph_string_gc (next);
-            w32_set_glyph_string_clipping (next);
-	    if (next->first_glyph->type == STRETCH_GLYPH)
-	      w32_draw_stretch_glyph_string (next);
-	    else
-	      w32_draw_glyph_string_background (next, true);
-            next->num_clips = 0;
-          }
-    }
-
-  /* Set up S->gc, set clipping and draw S.  */
-  w32_set_glyph_string_gc (s);
-
-  /* Draw relief (if any) in advance for char/composition so that the
-     glyph string can be drawn over it.  */
-  if (!s->for_overlaps
-      && s->face->box != FACE_NO_BOX
-      && (s->first_glyph->type == CHAR_GLYPH
-	  || s->first_glyph->type == COMPOSITE_GLYPH))
-
-    {
-      w32_set_glyph_string_clipping (s);
-      w32_draw_glyph_string_background (s, true);
-      w32_draw_glyph_string_box (s);
-      w32_set_glyph_string_clipping (s);
-      relief_drawn_p = 1;
-    }
-  else if (!s->clip_head /* draw_glyphs didn't specify a clip mask.  */
-           && !s->clip_tail
-           && ((s->prev && s->prev->hl != s->hl && s->left_overhang)
-               || (s->next && s->next->hl != s->hl && s->right_overhang)))
-    /* We must clip just this glyph.  left_overhang part has already
-       drawn when s->prev was drawn, and right_overhang part will be
-       drawn later when s->next is drawn. */
-    w32_set_glyph_string_clipping_exactly (s, s);
-  else
-    w32_set_glyph_string_clipping (s);
-
-  switch (s->first_glyph->type)
-    {
-    case IMAGE_GLYPH:
-      w32_draw_image_glyph_string (s);
-      break;
-
-    case STRETCH_GLYPH:
-      w32_draw_stretch_glyph_string (s);
-      break;
-
-    case CHAR_GLYPH:
-      if (s->for_overlaps)
-	s->background_filled_p = true;
-      else
-        w32_draw_glyph_string_background (s, false);
-      w32_draw_glyph_string_foreground (s);
-      break;
-
-    case COMPOSITE_GLYPH:
-      if (s->for_overlaps || (s->cmp_from > 0
-			      && ! s->first_glyph->u.cmp.automatic))
-	s->background_filled_p = true;
-      else
-	w32_draw_glyph_string_background (s, true);
-      w32_draw_composite_glyph_string_foreground (s);
-      break;
-
-    case GLYPHLESS_GLYPH:
-      if (s->for_overlaps)
-	s->background_filled_p = true;
-      else
-	w32_draw_glyph_string_background (s, false);
-      w32_draw_glyphless_glyph_string_foreground (s);
-      break;
-
-    default:
-      emacs_abort ();
-    }
-
-  if (!s->for_overlaps)
-    {
-      /* Draw underline.  */
-      if (s->face->underline_p)
-        {
-          if (s->face->underline_type == FACE_UNDER_WAVE)
-            {
-              COLORREF color;
-
-              if (s->face->underline_defaulted_p)
-                color = s->gc->foreground;
-              else
-                color = s->face->underline_color;
-
-              w32_draw_underwave (s, color);
-            }
-          else if (s->face->underline_type == FACE_UNDER_LINE)
-            {
-              unsigned long thickness, position;
-              int y;
-
-              if (s->prev && s->prev->face->underline_p
-		  && s->prev->face->underline_type == FACE_UNDER_LINE)
-                {
-                  /* We use the same underline style as the previous one.  */
-                  thickness = s->prev->underline_thickness;
-                  position = s->prev->underline_position;
-                }
-              else
-                {
-		  struct font *font = font_for_underline_metrics (s);
-		  unsigned long minimum_offset;
-		  BOOL underline_at_descent_line;
-		  BOOL use_underline_position_properties;
-		  Lisp_Object val
-		    = buffer_local_value (Qunderline_minimum_offset,
-					s->w->contents);
-		  if (FIXNUMP (val))
-		    minimum_offset = XFIXNAT (val);
-		  else
-		    minimum_offset = 1;
-		  val = buffer_local_value (Qx_underline_at_descent_line,
-					    s->w->contents);
-		  underline_at_descent_line
-		    = !(NILP (val) || EQ (val, Qunbound));
-		  val
-		    = buffer_local_value (Qx_use_underline_position_properties,
-					  s->w->contents);
-		  use_underline_position_properties
-		    = !(NILP (val) || EQ (val, Qunbound));
-
-                  /* Get the underline thickness.  Default is 1 pixel.  */
-                  if (font && font->underline_thickness > 0)
-                    thickness = font->underline_thickness;
-                  else
-                    thickness = 1;
-                  if (underline_at_descent_line
-                      || !font)
-                    position = (s->height - thickness) - (s->ybase - s->y);
-                  else
-                    {
-                      /* Get the underline position.  This is the
-                         recommended vertical offset in pixels from
-                         the baseline to the top of the underline.
-                         This is a signed value according to the
-                         specs, and its default is
-
-                         ROUND ((maximum_descent) / 2), with
-                         ROUND (x) = floor (x + 0.5)  */
-
-                      if (use_underline_position_properties
-                          && font->underline_position >= 0)
-                        position = font->underline_position;
-                      else
-                        position = (font->descent + 1) / 2;
-                    }
-                  position = max (position, minimum_offset);
-                }
-              /* Check the sanity of thickness and position.  We should
-                 avoid drawing underline out of the current line area.  */
-              if (s->y + s->height <= s->ybase + position)
-                position = (s->height - 1) - (s->ybase - s->y);
-              if (s->y + s->height < s->ybase + position + thickness)
-                thickness = (s->y + s->height) - (s->ybase + position);
-              s->underline_thickness = thickness;
-              s->underline_position =  position;
-              y = s->ybase + position;
-              if (s->face->underline_defaulted_p)
-                {
-                  w32_fill_area (s->f, s->hdc, s->gc->foreground, s->x,
-                                 y, s->width, 1);
-                }
-              else
-                {
-                  w32_fill_area (s->f, s->hdc, s->face->underline_color, s->x,
-                                 y, s->width, 1);
-                }
-            }
-        }
-      /* Draw overline.  */
-      if (s->face->overline_p)
-        {
-          unsigned long dy = 0, h = 1;
-
-          if (s->face->overline_color_defaulted_p)
-            {
-              w32_fill_area (s->f, s->hdc, s->gc->foreground, s->x,
-                             s->y + dy, s->width, h);
-            }
-          else
-            {
-              w32_fill_area (s->f, s->hdc, s->face->overline_color, s->x,
-                             s->y + dy, s->width, h);
-            }
-        }
-
-      /* Draw strike-through.  */
-      if (s->face->strike_through_p
-          && !FONT_TEXTMETRIC (s->font).tmStruckOut)
-        {
-	  /* Y-coordinate and height of the glyph string's first
-	     glyph.  We cannot use s->y and s->height because those
-	     could be larger if there are taller display elements
-	     (e.g., characters displayed with a larger font) in the
-	     same glyph row.  */
-	  int glyph_y = s->ybase - s->first_glyph->ascent;
-	  int glyph_height = s->first_glyph->ascent + s->first_glyph->descent;
-	  /* Strike-through width and offset from the glyph string's
-	     top edge.  */
-          unsigned long h = 1;
-          unsigned long dy = (glyph_height - h) / 2;
-
-          if (s->face->strike_through_color_defaulted_p)
-            {
-              w32_fill_area (s->f, s->hdc, s->gc->foreground, s->x,
-			     glyph_y + dy, s->width, h);
-            }
-          else
-            {
-              w32_fill_area (s->f, s->hdc, s->face->strike_through_color, s->x,
-                             glyph_y + dy, s->width, h);
-            }
-        }
-
-      /* Draw relief if not yet drawn.  */
-      if (!relief_drawn_p && s->face->box != FACE_NO_BOX)
-        w32_draw_glyph_string_box (s);
-
-      if (s->prev)
-        {
-          struct glyph_string *prev;
-
-          for (prev = s->prev; prev; prev = prev->prev)
-            if (prev->hl != s->hl
-                && prev->x + prev->width + prev->right_overhang > s->x)
-              {
-                /* As prev was drawn while clipped to its own area, we
-                   must draw the right_overhang part using s->hl now.  */
-		enum draw_glyphs_face save = prev->hl;
-
-		prev->hl = s->hl;
-		w32_set_glyph_string_gc (prev);
-		w32_set_glyph_string_clipping_exactly (s, prev);
-		if (prev->first_glyph->type == CHAR_GLYPH)
-		  w32_draw_glyph_string_foreground (prev);
-		else
-		  w32_draw_composite_glyph_string_foreground (prev);
-                w32_set_clip_rectangle (prev->hdc, NULL);
-		prev->hl = save;
-		prev->num_clips = 0;
-	      }
-	}
-
-      if (s->next)
-	{
-	  struct glyph_string *next;
-
-	  for (next = s->next; next; next = next->next)
-	    if (next->hl != s->hl
-		&& next->x - next->left_overhang < s->x + s->width)
-	      {
-		/* As next will be drawn while clipped to its own area,
-		   we must draw the left_overhang part using s->hl now.  */
-		enum draw_glyphs_face save = next->hl;
-
-		next->hl = s->hl;
-		w32_set_glyph_string_gc (next);
-		w32_set_glyph_string_clipping_exactly (s, next);
-		if (next->first_glyph->type == CHAR_GLYPH)
-		  w32_draw_glyph_string_foreground (next);
-		else
-		  w32_draw_composite_glyph_string_foreground (next);
-                w32_set_clip_rectangle (next->hdc, NULL);
-		next->hl = save;
-		next->num_clips = 0;
-		next->clip_head = s->next;
-	      }
-	}
-    }
-
-  /* Reset clipping.  */
-  w32_set_clip_rectangle (s->hdc, NULL);
-  s->num_clips = 0;
-}
-
-
 /* Shift display to make room for inserted glyphs.   */
 
 static void
@@ -7077,6 +6847,26 @@ w32_make_rdb (char *xrm_option)
 
 extern frame_parm_handler w32_frame_parm_handlers[];
 
+static struct draw_glyph_string_interface w32_draw_glyph_string_interface =
+{
+  w32_set_glyph_string_gc,
+  w32_set_glyph_string_clipping,
+  w32_set_glyph_string_clipping_exactly,
+  w32_reset_glyph_string_clipping,
+  w32_draw_glyph_string_background,
+  w32_draw_glyph_string_foreground,
+  w32_draw_glyph_string_box,
+  w32_draw_image_glyph_string,
+  w32_draw_stretch_glyph_string,
+  NULL, /* draw_xwidget_glyph_string  */
+  w32_draw_composite_glyph_string_foreground,
+  w32_draw_glyphless_glyph_string_foreground,
+  w32_draw_underwave,
+  w32_draw_underline,
+  w32_draw_overline,
+  w32_draw_strike_through
+};
+
 static struct redisplay_interface w32_redisplay_interface =
 {
   w32_frame_parm_handlers,
@@ -7096,7 +6886,8 @@ static struct redisplay_interface w32_redisplay_interface =
   w32_define_fringe_bitmap,
   w32_destroy_fringe_bitmap,
   w32_compute_glyph_string_overhangs,
-  w32_draw_glyph_string,
+  gui_draw_glyph_string,
+  &w32_draw_glyph_string_interface,
   w32_define_frame_cursor,
   w32_clear_frame_area,
   w32_clear_under_internal_border,
@@ -7471,21 +7262,6 @@ the cursor have no effect.  */);
 
   w32_use_visible_system_caret = 0;
 
-  /* We don't yet support this, but defining this here avoids whining
-     from cus-start.el and other places, like "M-x set-variable".  */
-  DEFVAR_BOOL ("x-use-underline-position-properties",
-	       x_use_underline_position_properties,
-     doc: /* SKIP: real doc in xterm.c.  */);
-  x_use_underline_position_properties = 0;
-  DEFSYM (Qx_use_underline_position_properties,
-	  "x-use-underline-position-properties");
-
-  DEFVAR_BOOL ("x-underline-at-descent-line",
-	       x_underline_at_descent_line,
-     doc: /* SKIP: real doc in xterm.c.  */);
-  x_underline_at_descent_line = 0;
-  DEFSYM (Qx_underline_at_descent_line, "x-underline-at-descent-line");
-
   DEFVAR_LISP ("x-toolkit-scroll-bars", Vx_toolkit_scroll_bars,
 	       doc: /* SKIP: real doc in xterm.c.  */);
   Vx_toolkit_scroll_bars = Qt;
diff --git a/src/xdisp.c b/src/xdisp.c
index a3dddfa8a5..2f5aaf6aff 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -29099,6 +29099,269 @@ gui_insert_glyphs (struct window *w, struct glyph_row *updated_row,
   unblock_input ();
 }
 
+/* EXPORT for RIF:
+   Draw glyph string S.  */
+
+void
+gui_draw_glyph_string (struct glyph_string *s)
+{
+  bool relief_drawn_p = false;
+  struct draw_glyph_string_interface *gsif = FRAME_RIF (s->f)->gsif;
+
+  if (!gsif)
+    emacs_abort ();
+
+  /* If S draws into the background of its successors, draw the
+     background of the successors first so that S can draw into it.  */
+  if (s->next && s->right_overhang && !s->for_overlaps)
+    {
+      int width;
+      struct glyph_string *next;
+
+      for (width = 0, next = s->next;
+	   next && width < s->right_overhang;
+	   width += next->width, next = next->next)
+	if (next->first_glyph->type != IMAGE_GLYPH)
+	  {
+	    gsif->set_gc (next);
+	    gsif->set_clipping (next);
+	    if (next->first_glyph->type == STRETCH_GLYPH)
+	      gsif->draw_stretch (next);
+	    else
+	      gsif->draw_background (next, true);
+	    next->num_clips = 0;
+	  }
+    }
+
+  /* Set up S->gc, set clipping and draw S.  */
+  gsif->set_gc (s);
+
+  /* Draw relief (if any) in advance for char/composition so that the
+     glyph string can be drawn over it.  */
+  if (!s->for_overlaps
+      && s->face->box != FACE_NO_BOX
+      && (s->first_glyph->type == CHAR_GLYPH
+	  || s->first_glyph->type == COMPOSITE_GLYPH))
+
+    {
+      gsif->set_clipping (s);
+      gsif->draw_background (s, true);
+      gsif->draw_box (s);
+      gsif->set_clipping (s);
+      relief_drawn_p = true;
+    }
+  else if (!s->clip_head /* draw_glyphs didn't specify a clip mask. */
+	   && !s->clip_tail
+	   && ((s->prev && s->prev->hl != s->hl && s->left_overhang)
+	       || (s->next && s->next->hl != s->hl && s->right_overhang)))
+    /* We must clip just this glyph.  left_overhang part has already
+       drawn when s->prev was drawn, and right_overhang part will be
+       drawn later when s->next is drawn. */
+    gsif->set_clipping_exactly (s, s);
+  else
+    gsif->set_clipping (s);
+
+  switch (s->first_glyph->type)
+    {
+    case IMAGE_GLYPH:
+      gsif->draw_image (s);
+      break;
+
+    case STRETCH_GLYPH:
+      gsif->draw_stretch (s);
+      break;
+
+    case CHAR_GLYPH:
+      if (s->for_overlaps)
+	s->background_filled_p = true;
+      else
+	gsif->draw_background (s, false);
+      gsif->draw_foreground (s);
+      break;
+
+    case COMPOSITE_GLYPH:
+      if (s->for_overlaps || (s->cmp_from > 0
+			      && ! s->first_glyph->u.cmp.automatic))
+	s->background_filled_p = true;
+      else
+	gsif->draw_background (s, true);
+      gsif->draw_composite_foreground (s);
+      break;
+
+    case XWIDGET_GLYPH:
+      if (gsif->draw_xwidget)
+        gsif->draw_xwidget (s);
+      break;
+
+    case GLYPHLESS_GLYPH:
+      if (s->for_overlaps)
+	s->background_filled_p = true;
+      else
+	gsif->draw_background (s, true);
+      gsif->draw_glyphless_foreground (s);
+      break;
+
+    default:
+      emacs_abort ();
+    }
+
+  if (!s->for_overlaps)
+    {
+      /* Draw underline.  */
+      if (s->face->underline_p)
+        {
+          if (s->face->underline_type == FACE_UNDER_WAVE)
+            gsif->draw_underwave (s);
+          else if (s->face->underline_type == FACE_UNDER_LINE)
+            {
+              unsigned long thickness, position;
+
+              if (s->prev && s->prev->face->underline_p
+		  && s->prev->face->underline_type == FACE_UNDER_LINE)
+                {
+                  /* We use the same underline style as the previous one.  */
+                  thickness = s->prev->underline_thickness;
+                  position = s->prev->underline_position;
+                }
+              else
+                {
+		  struct font *font = font_for_underline_metrics (s);
+		  unsigned long minimum_offset;
+		  bool underline_at_descent_line;
+		  bool use_underline_position_properties;
+		  Lisp_Object val
+		    = buffer_local_value (Qunderline_minimum_offset,
+					  s->w->contents);
+		  if (FIXNUMP (val))
+		    minimum_offset = XFIXNAT (val);
+		  else
+		    minimum_offset = 1;
+		  val = buffer_local_value (Qx_underline_at_descent_line,
+					    s->w->contents);
+		  underline_at_descent_line
+		    = !(NILP (val) || EQ (val, Qunbound));
+		  val
+		    = buffer_local_value (Qx_use_underline_position_properties,
+					  s->w->contents);
+		  use_underline_position_properties
+		    = !(NILP (val) || EQ (val, Qunbound));
+
+                  /* Get the underline thickness.  Default is 1 pixel.  */
+                  if (font && font->underline_thickness > 0)
+                    thickness = font->underline_thickness;
+                  else
+                    thickness = 1;
+                  if (underline_at_descent_line)
+                    position = (s->height - thickness) - (s->ybase - s->y);
+                  else
+                    {
+                      /* Get the underline position.  This is the
+                         recommended vertical offset in pixels from
+                         the baseline to the top of the underline.
+                         This is a signed value according to the
+                         specs, and its default is
+
+                         ROUND ((maximum descent) / 2), with
+                         ROUND(x) = floor (x + 0.5)  */
+
+                      if (use_underline_position_properties
+                          && font && font->underline_position >= 0)
+                        position = font->underline_position;
+                      else if (font)
+                        position = (font->descent + 1) / 2;
+                      else
+                        position = minimum_offset;
+                    }
+                  position = max (position, minimum_offset);
+                }
+              /* Check the sanity of thickness and position.  We should
+                 avoid drawing underline out of the current line area.  */
+              if (s->y + s->height <= s->ybase + position)
+                position = (s->height - 1) - (s->ybase - s->y);
+              if (s->y + s->height < s->ybase + position + thickness)
+                thickness = (s->y + s->height) - (s->ybase + position);
+              s->underline_thickness = thickness;
+              s->underline_position = position;
+              gsif->draw_underline (s);
+            }
+        }
+      /* Draw overline.  */
+      if (s->face->overline_p)
+        gsif->draw_overline (s);
+
+      /* Draw strike-through.  */
+      if (s->face->strike_through_p)
+        {
+          /* Y-coordinate and height of the glyph string's first
+	     glyph.  We cannot use s->y and s->height because those
+	     could be larger if there are taller display elements
+	     (e.g., characters displayed with a larger font) in the
+	     same glyph row.  */
+	  int glyph_y = s->ybase - s->first_glyph->ascent;
+	  int glyph_height = s->first_glyph->ascent + s->first_glyph->descent;
+	  /* Strike-through width and offset from the glyph string's
+	     top edge.  */
+          unsigned long dy = (glyph_height - 1) / 2;
+          gsif->draw_strike_through (s, glyph_y + dy, 1);
+        }
+
+      /* Draw relief if not yet drawn.  */
+      if (!relief_drawn_p && s->face->box != FACE_NO_BOX)
+	gsif->draw_box (s);
+
+      if (s->prev)
+	{
+	  struct glyph_string *prev;
+
+	  for (prev = s->prev; prev; prev = prev->prev)
+	    if (prev->hl != s->hl
+		&& prev->x + prev->width + prev->right_overhang > s->x)
+	      {
+		/* As prev was drawn while clipped to its own area, we
+		   must draw the right_overhang part using s->hl now.  */
+		enum draw_glyphs_face save = prev->hl;
+
+		prev->hl = s->hl;
+		gsif->set_gc (prev);
+		gsif->set_clipping_exactly (s, prev);
+		if (prev->first_glyph->type == CHAR_GLYPH)
+		  gsif->draw_foreground (prev);
+		else
+		  gsif->draw_composite_foreground (prev);
+		gsif->reset_clipping (prev);
+		prev->hl = save;
+	      }
+	}
+
+      if (s->next)
+	{
+	  struct glyph_string *next;
+
+	  for (next = s->next; next; next = next->next)
+	    if (next->hl != s->hl
+		&& next->x - next->left_overhang < s->x + s->width)
+	      {
+		/* As next will be drawn while clipped to its own area,
+		   we must draw the left_overhang part using s->hl now.  */
+		enum draw_glyphs_face save = next->hl;
+
+		next->hl = s->hl;
+		gsif->set_gc (next);
+		gsif->set_clipping_exactly (s, next);
+		if (next->first_glyph->type == CHAR_GLYPH)
+		  gsif->draw_foreground (next);
+		else
+		  gsif->draw_composite_foreground (next);
+		gsif->reset_clipping (next);
+		next->hl = save;
+		next->clip_head = s->next;
+	      }
+	}
+    }
+
+  /* Reset clipping.  */
+  gsif->reset_clipping (s);
+}
 
 /* EXPORT for RIF:
    Erase the current text line from the nominal cursor position
@@ -33344,6 +33607,35 @@ baseline.  The default value is 1.  */);
   underline_minimum_offset = 1;
   DEFSYM (Qunderline_minimum_offset, "underline-minimum-offset");
 
+#ifdef HAVE_WINDOW_SYSTEM
+  DEFVAR_BOOL ("x-use-underline-position-properties",
+	       x_use_underline_position_properties,
+     doc: /* Non-nil means make use of UNDERLINE_POSITION font properties.
+A value of nil means ignore them.  If you encounter fonts with bogus
+UNDERLINE_POSITION font properties, set this to nil.  You can also use
+`underline-minimum-offset' to override the font's UNDERLINE_POSITION for
+small font display sizes.  */);
+# ifdef HAVE_X_WINDOWS
+  x_use_underline_position_properties = true;
+# else
+  /* Other terms don't support this (yet?).  */
+  x_use_underline_position_properties = false;
+# endif
+  DEFSYM (Qx_use_underline_position_properties,
+	  "x-use-underline-position-properties");
+
+  DEFVAR_BOOL ("x-underline-at-descent-line",
+	       x_underline_at_descent_line,
+     doc: /* Non-nil means to draw the underline at the same place as the descent line.
+(If `line-spacing' is in effect, that moves the underline lower by
+that many pixels.)
+A value of nil means to draw the underline according to the value of the
+variable `x-use-underline-position-properties', which is usually at the
+baseline level.  The default value is nil.  */);
+  x_underline_at_descent_line = false;
+  DEFSYM (Qx_underline_at_descent_line, "x-underline-at-descent-line");
+#endif
+
   DEFVAR_BOOL ("display-hourglass", display_hourglass_p,
 	       doc: /* Non-nil means show an hourglass pointer, when Emacs is busy.
 This feature only works when on a window system that can change
diff --git a/src/xterm.c b/src/xterm.c
index b08f5f6a62..5fe4491f7e 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -1626,6 +1626,12 @@ x_set_glyph_string_clipping_exactly (struct glyph_string *src, struct glyph_stri
   x_set_clip_rectangles (dst->f, dst->gc, &r, 1);
 }
 
+static void
+x_reset_glyph_string_clipping (struct glyph_string *s)
+{
+  x_reset_clip_rectangles (s->f, s->gc);
+  s->num_clips = 0;
+}
 
 /* RIF:
    Compute left and right overhang of glyph string S.  */
@@ -3468,7 +3474,7 @@ x_get_scale_factor(Display *disp, int *scale_x, int *scale_y)
 
 */
 static void
-x_draw_underwave (struct glyph_string *s)
+x_draw_underwave_1 (struct glyph_string *s)
 {
   /* Adjust for scale/HiDPI.  */
   int scale_x, scale_y;
@@ -3535,319 +3541,73 @@ x_draw_underwave (struct glyph_string *s)
 #endif	/* not USE_CAIRO */
 }
 
-
-/* Draw glyph string S.  */
-
 static void
-x_draw_glyph_string (struct glyph_string *s)
+x_draw_underwave (struct glyph_string *s)
 {
-  bool relief_drawn_p = false;
-
-  /* If S draws into the background of its successors, draw the
-     background of the successors first so that S can draw into it.
-     This makes S->next use XDrawString instead of XDrawImageString.  */
-  if (s->next && s->right_overhang && !s->for_overlaps)
+  if (s->face->underline_defaulted_p)
+    x_draw_underwave_1 (s);
+  else
     {
-      int width;
-      struct glyph_string *next;
-
-      for (width = 0, next = s->next;
-	   next && width < s->right_overhang;
-	   width += next->width, next = next->next)
-	if (next->first_glyph->type != IMAGE_GLYPH)
-	  {
-	    x_set_glyph_string_gc (next);
-	    x_set_glyph_string_clipping (next);
-	    if (next->first_glyph->type == STRETCH_GLYPH)
-	      x_draw_stretch_glyph_string (next);
-	    else
-	      x_draw_glyph_string_background (next, true);
-	    next->num_clips = 0;
-	  }
+      XGCValues xgcv;
+      XGetGCValues (s->display, s->gc, GCForeground, &xgcv);
+      XSetForeground (s->display, s->gc, s->face->underline_color);
+      x_draw_underwave_1 (s);
+      XSetForeground (s->display, s->gc, xgcv.foreground);
     }
+}
 
-  /* Set up S->gc, set clipping and draw S.  */
-  x_set_glyph_string_gc (s);
-
-  /* Draw relief (if any) in advance for char/composition so that the
-     glyph string can be drawn over it.  */
-  if (!s->for_overlaps
-      && s->face->box != FACE_NO_BOX
-      && (s->first_glyph->type == CHAR_GLYPH
-	  || s->first_glyph->type == COMPOSITE_GLYPH))
+static void
+x_draw_underline (struct glyph_string *s)
+{
+  const int y = s->ybase + s->underline_position;
+  const int thickness = s->underline_thickness;
 
-    {
-      x_set_glyph_string_clipping (s);
-      x_draw_glyph_string_background (s, true);
-      x_draw_glyph_string_box (s);
-      x_set_glyph_string_clipping (s);
-      relief_drawn_p = true;
-    }
-  else if (!s->clip_head /* draw_glyphs didn't specify a clip mask. */
-	   && !s->clip_tail
-	   && ((s->prev && s->prev->hl != s->hl && s->left_overhang)
-	       || (s->next && s->next->hl != s->hl && s->right_overhang)))
-    /* We must clip just this glyph.  left_overhang part has already
-       drawn when s->prev was drawn, and right_overhang part will be
-       drawn later when s->next is drawn. */
-    x_set_glyph_string_clipping_exactly (s, s);
+  if (s->face->underline_defaulted_p)
+    x_fill_rectangle (s->f, s->gc,
+                      s->x, y, s->width, thickness);
   else
-    x_set_glyph_string_clipping (s);
-
-  switch (s->first_glyph->type)
     {
-    case IMAGE_GLYPH:
-      x_draw_image_glyph_string (s);
-      break;
-
-    case XWIDGET_GLYPH:
-      x_draw_xwidget_glyph_string (s);
-      break;
-
-    case STRETCH_GLYPH:
-      x_draw_stretch_glyph_string (s);
-      break;
-
-    case CHAR_GLYPH:
-      if (s->for_overlaps)
-	s->background_filled_p = true;
-      else
-	x_draw_glyph_string_background (s, false);
-      x_draw_glyph_string_foreground (s);
-      break;
-
-    case COMPOSITE_GLYPH:
-      if (s->for_overlaps || (s->cmp_from > 0
-			      && ! s->first_glyph->u.cmp.automatic))
-	s->background_filled_p = true;
-      else
-	x_draw_glyph_string_background (s, true);
-      x_draw_composite_glyph_string_foreground (s);
-      break;
+      XGCValues xgcv;
+      XGetGCValues (s->display, s->gc, GCForeground, &xgcv);
+      XSetForeground (s->display, s->gc, s->face->underline_color);
+      x_fill_rectangle (s->f, s->gc,
+                        s->x, y, s->width, thickness);
+      XSetForeground (s->display, s->gc, xgcv.foreground);
+    }
+}
 
-    case GLYPHLESS_GLYPH:
-      if (s->for_overlaps)
-	s->background_filled_p = true;
-      else
-	x_draw_glyph_string_background (s, true);
-      x_draw_glyphless_glyph_string_foreground (s);
-      break;
+static void
+x_draw_overline (struct glyph_string *s)
+{
+  unsigned long dy = 0, h = 1;
 
-    default:
-      emacs_abort ();
+  if (s->face->overline_color_defaulted_p)
+    x_fill_rectangle (s->f, s->gc, s->x, s->y + dy,
+                      s->width, h);
+  else
+    {
+      XGCValues xgcv;
+      XGetGCValues (s->display, s->gc, GCForeground, &xgcv);
+      XSetForeground (s->display, s->gc, s->face->overline_color);
+      x_fill_rectangle (s->f, s->gc, s->x, s->y + dy,
+                        s->width, h);
+      XSetForeground (s->display, s->gc, xgcv.foreground);
     }
+}
 
-  if (!s->for_overlaps)
+static void
+x_draw_strike_through (struct glyph_string *s, int y, int height)
+{
+  if (s->face->strike_through_color_defaulted_p)
+    x_fill_rectangle (s->f, s->gc, s->x, y, s->width, height);
+  else
     {
-      /* Draw underline.  */
-      if (s->face->underline_p)
-        {
-          if (s->face->underline_type == FACE_UNDER_WAVE)
-            {
-              if (s->face->underline_defaulted_p)
-                x_draw_underwave (s);
-              else
-                {
-                  XGCValues xgcv;
-                  XGetGCValues (s->display, s->gc, GCForeground, &xgcv);
-                  XSetForeground (s->display, s->gc, s->face->underline_color);
-                  x_draw_underwave (s);
-                  XSetForeground (s->display, s->gc, xgcv.foreground);
-                }
-            }
-          else if (s->face->underline_type == FACE_UNDER_LINE)
-            {
-              unsigned long thickness, position;
-              int y;
-
-              if (s->prev && s->prev->face->underline_p
-		  && s->prev->face->underline_type == FACE_UNDER_LINE)
-                {
-                  /* We use the same underline style as the previous one.  */
-                  thickness = s->prev->underline_thickness;
-                  position = s->prev->underline_position;
-                }
-              else
-                {
-		  struct font *font = font_for_underline_metrics (s);
-		  unsigned long minimum_offset;
-		  bool underline_at_descent_line;
-		  bool use_underline_position_properties;
-		  Lisp_Object val
-		    = buffer_local_value (Qunderline_minimum_offset,
-					  s->w->contents);
-		  if (FIXNUMP (val))
-		    minimum_offset = XFIXNAT (val);
-		  else
-		    minimum_offset = 1;
-		  val = buffer_local_value (Qx_underline_at_descent_line,
-					    s->w->contents);
-		  underline_at_descent_line
-		    = !(NILP (val) || EQ (val, Qunbound));
-		  val
-		    = buffer_local_value (Qx_use_underline_position_properties,
-					  s->w->contents);
-		  use_underline_position_properties
-		    = !(NILP (val) || EQ (val, Qunbound));
-
-                  /* Get the underline thickness.  Default is 1 pixel.  */
-                  if (font && font->underline_thickness > 0)
-                    thickness = font->underline_thickness;
-                  else
-                    thickness = 1;
-                  if (underline_at_descent_line)
-                    position = (s->height - thickness) - (s->ybase - s->y);
-                  else
-                    {
-                      /* Get the underline position.  This is the
-                         recommended vertical offset in pixels from
-                         the baseline to the top of the underline.
-                         This is a signed value according to the
-                         specs, and its default is
-
-                         ROUND ((maximum descent) / 2), with
-                         ROUND(x) = floor (x + 0.5)  */
-
-                      if (use_underline_position_properties
-                          && font && font->underline_position >= 0)
-                        position = font->underline_position;
-                      else if (font)
-                        position = (font->descent + 1) / 2;
-                      else
-                        position = minimum_offset;
-                    }
-                  position = max (position, minimum_offset);
-                }
-              /* Check the sanity of thickness and position.  We should
-                 avoid drawing underline out of the current line area.  */
-              if (s->y + s->height <= s->ybase + position)
-                position = (s->height - 1) - (s->ybase - s->y);
-              if (s->y + s->height < s->ybase + position + thickness)
-                thickness = (s->y + s->height) - (s->ybase + position);
-              s->underline_thickness = thickness;
-              s->underline_position = position;
-              y = s->ybase + position;
-              if (s->face->underline_defaulted_p)
-                x_fill_rectangle (s->f, s->gc,
-                                s->x, y, s->width, thickness);
-              else
-                {
-                  XGCValues xgcv;
-                  XGetGCValues (s->display, s->gc, GCForeground, &xgcv);
-                  XSetForeground (s->display, s->gc, s->face->underline_color);
-                  x_fill_rectangle (s->f, s->gc,
-                                  s->x, y, s->width, thickness);
-                  XSetForeground (s->display, s->gc, xgcv.foreground);
-                }
-            }
-        }
-      /* Draw overline.  */
-      if (s->face->overline_p)
-	{
-	  unsigned long dy = 0, h = 1;
-
-	  if (s->face->overline_color_defaulted_p)
-	    x_fill_rectangle (s->f, s->gc, s->x, s->y + dy,
-			    s->width, h);
-	  else
-	    {
-	      XGCValues xgcv;
-	      XGetGCValues (s->display, s->gc, GCForeground, &xgcv);
-	      XSetForeground (s->display, s->gc, s->face->overline_color);
-	      x_fill_rectangle (s->f, s->gc, s->x, s->y + dy,
-			      s->width, h);
-	      XSetForeground (s->display, s->gc, xgcv.foreground);
-	    }
-	}
-
-      /* Draw strike-through.  */
-      if (s->face->strike_through_p)
-	{
-	  /* Y-coordinate and height of the glyph string's first
-	     glyph.  We cannot use s->y and s->height because those
-	     could be larger if there are taller display elements
-	     (e.g., characters displayed with a larger font) in the
-	     same glyph row.  */
-	  int glyph_y = s->ybase - s->first_glyph->ascent;
-	  int glyph_height = s->first_glyph->ascent + s->first_glyph->descent;
-	  /* Strike-through width and offset from the glyph string's
-	     top edge.  */
-          unsigned long h = 1;
-          unsigned long dy = (glyph_height - h) / 2;
-
-	  if (s->face->strike_through_color_defaulted_p)
-	    x_fill_rectangle (s->f, s->gc, s->x, glyph_y + dy,
-			    s->width, h);
-	  else
-	    {
-	      XGCValues xgcv;
-	      XGetGCValues (s->display, s->gc, GCForeground, &xgcv);
-	      XSetForeground (s->display, s->gc, s->face->strike_through_color);
-	      x_fill_rectangle (s->f, s->gc, s->x, glyph_y + dy,
-			      s->width, h);
-	      XSetForeground (s->display, s->gc, xgcv.foreground);
-	    }
-	}
-
-      /* Draw relief if not yet drawn.  */
-      if (!relief_drawn_p && s->face->box != FACE_NO_BOX)
-	x_draw_glyph_string_box (s);
-
-      if (s->prev)
-	{
-	  struct glyph_string *prev;
-
-	  for (prev = s->prev; prev; prev = prev->prev)
-	    if (prev->hl != s->hl
-		&& prev->x + prev->width + prev->right_overhang > s->x)
-	      {
-		/* As prev was drawn while clipped to its own area, we
-		   must draw the right_overhang part using s->hl now.  */
-		enum draw_glyphs_face save = prev->hl;
-
-		prev->hl = s->hl;
-		x_set_glyph_string_gc (prev);
-		x_set_glyph_string_clipping_exactly (s, prev);
-		if (prev->first_glyph->type == CHAR_GLYPH)
-		  x_draw_glyph_string_foreground (prev);
-		else
-		  x_draw_composite_glyph_string_foreground (prev);
-		x_reset_clip_rectangles (prev->f, prev->gc);
-		prev->hl = save;
-		prev->num_clips = 0;
-	      }
-	}
-
-      if (s->next)
-	{
-	  struct glyph_string *next;
-
-	  for (next = s->next; next; next = next->next)
-	    if (next->hl != s->hl
-		&& next->x - next->left_overhang < s->x + s->width)
-	      {
-		/* As next will be drawn while clipped to its own area,
-		   we must draw the left_overhang part using s->hl now.  */
-		enum draw_glyphs_face save = next->hl;
-
-		next->hl = s->hl;
-		x_set_glyph_string_gc (next);
-		x_set_glyph_string_clipping_exactly (s, next);
-		if (next->first_glyph->type == CHAR_GLYPH)
-		  x_draw_glyph_string_foreground (next);
-		else
-		  x_draw_composite_glyph_string_foreground (next);
-		x_reset_clip_rectangles (next->f, next->gc);
-		next->hl = save;
-		next->num_clips = 0;
-		next->clip_head = s->next;
-	      }
-	}
+      XGCValues xgcv;
+      XGetGCValues (s->display, s->gc, GCForeground, &xgcv);
+      XSetForeground (s->display, s->gc, s->face->strike_through_color);
+      x_fill_rectangle (s->f, s->gc, s->x, y, s->width, height);
+      XSetForeground (s->display, s->gc, xgcv.foreground);
     }
-
-  /* Reset clipping.  */
-  x_reset_clip_rectangles (s->f, s->gc);
-  s->num_clips = 0;
 }
 
 /* Shift display to make room for inserted glyphs.   */
@@ -13060,6 +12820,26 @@ x_activate_timeout_atimer (void)
 
 extern frame_parm_handler x_frame_parm_handlers[];
 
+static struct draw_glyph_string_interface x_draw_glyph_string_interface =
+  {
+   x_set_glyph_string_gc,
+   x_set_glyph_string_clipping,
+   x_set_glyph_string_clipping_exactly,
+   x_reset_glyph_string_clipping,
+   x_draw_glyph_string_background,
+   x_draw_glyph_string_foreground,
+   x_draw_glyph_string_box,
+   x_draw_image_glyph_string,
+   x_draw_stretch_glyph_string,
+   x_draw_xwidget_glyph_string,
+   x_draw_composite_glyph_string_foreground,
+   x_draw_glyphless_glyph_string_foreground,
+   x_draw_underwave,
+   x_draw_underline,
+   x_draw_overline,
+   x_draw_strike_through
+  };
+
 static struct redisplay_interface x_redisplay_interface =
   {
     x_frame_parm_handlers,
@@ -13084,7 +12864,8 @@ static struct redisplay_interface x_redisplay_interface =
     0, /* destroy_fringe_bitmap */
 #endif
     x_compute_glyph_string_overhangs,
-    x_draw_glyph_string,
+    gui_draw_glyph_string,
+    &x_draw_glyph_string_interface,
     x_define_frame_cursor,
     x_clear_frame_area,
     x_clear_under_internal_border,
@@ -13327,28 +13108,6 @@ syms_of_xterm (void)
   DEFSYM (Qx_gtk_map_stock, "x-gtk-map-stock");
 #endif
 
-  DEFVAR_BOOL ("x-use-underline-position-properties",
-	       x_use_underline_position_properties,
-     doc: /* Non-nil means make use of UNDERLINE_POSITION font properties.
-A value of nil means ignore them.  If you encounter fonts with bogus
-UNDERLINE_POSITION font properties, set this to nil.  You can also use
-`underline-minimum-offset' to override the font's UNDERLINE_POSITION for
-small font display sizes.  */);
-  x_use_underline_position_properties = true;
-  DEFSYM (Qx_use_underline_position_properties,
-	  "x-use-underline-position-properties");
-
-  DEFVAR_BOOL ("x-underline-at-descent-line",
-	       x_underline_at_descent_line,
-     doc: /* Non-nil means to draw the underline at the same place as the descent line.
-(If `line-spacing' is in effect, that moves the underline lower by
-that many pixels.)
-A value of nil means to draw the underline according to the value of the
-variable `x-use-underline-position-properties', which is usually at the
-baseline level.  The default value is nil.  */);
-  x_underline_at_descent_line = false;
-  DEFSYM (Qx_underline_at_descent_line, "x-underline-at-descent-line");
-
   DEFVAR_BOOL ("x-mouse-click-focus-ignore-position",
 	       x_mouse_click_focus_ignore_position,
     doc: /* Non-nil means that a mouse click to focus a frame does not move point.
-- 
2.21.0


--=-=-=--




Acknowledgement sent to Alex Gramiak <agrambot@HIDDEN>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs@HIDDEN. Full text available.
Report forwarded to bug-gnu-emacs@HIDDEN:
bug#35468; Package emacs. Full text available.
Please note: This is a static page, with minimal formatting, updated once a day.
Click here to see this page with the latest information and nicer formatting.
Last modified: Sat, 4 May 2019 08:30:02 UTC

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997 nCipher Corporation Ltd, 1994-97 Ian Jackson.