bug#36633: Scrolling by pixels: visible stretch glyphs overwrite header-line.

bug#36633: Scrolling by pixels: visible stretch glyphs overwrite header-line.

[Keith David Bershatsky]

Step 1:  Launch Emacs 26.

Step 2:  Evaluate the following code.

(progn
  (find-library "simple")
  (pixel-scroll-mode 1)
  (re-search-forward "\t\t\t\t\t" nil t)
  (set-window-start (selected-window) (match-beginning 0))
  (setq header-line-format "hello-world")
  (setq buffer-display-table (make-display-table))
  (aset buffer-display-table
        ?\t
        (vector (make-glyph-code ?\u00BB 'font-lock-warning-face)
                (make-glyph-code ?\t 'highlight)))
  (setq tab-width 8))

Step 3:  Move the mousewheel downward one time; i.e., one ratchet.

Step 4:  Observe the visible stretch glyphs overwriting the header-line.

BEHAVIOR EXPECTED:  The header-line should not be overwritten by the visible stretch glyphs that are being scrolled upwards from the row beneath the header-line.

I am on OSX 10.6.8 and performed this minimal working example with:

GNU Emacs 26.1 (build 1, x86_64-apple-darwin10.8.0, NS appkit-1038.36 Version 10.6.8 (Build 10K549)) of 2018-05-30

bug#36633: Scrolling by pixels: visible stretch glyphs overwrite header-line.

[Eli Zaretskii]

> Date: Fri, 12 Jul 2019 19:10:44 -0700
> From: Keith David Bershatsky <esq@lawlist.com>
> 
> Step 1:  Launch Emacs 26.
> 
> Step 2:  Evaluate the following code.
> 
> (progn
>   (find-library "simple")
>   (pixel-scroll-mode 1)
>   (re-search-forward "\t\t\t\t\t" nil t)
>   (set-window-start (selected-window) (match-beginning 0))
>   (setq header-line-format "hello-world")
>   (setq buffer-display-table (make-display-table))
>   (aset buffer-display-table
>         ?\t
>         (vector (make-glyph-code ?\u00BB 'font-lock-warning-face)
>                 (make-glyph-code ?\t 'highlight)))
>   (setq tab-width 8))
> 
> Step 3:  Move the mousewheel downward one time; i.e., one ratchet.
> 
> Step 4:  Observe the visible stretch glyphs overwriting the header-line.

This doesn't happen to me on MS-Windows, so I think this is
Darwin-specific.

Can someone try reproducing this on GNU/Linux and report?

bug#36633: Scrolling by pixels: visible stretch glyphs overwrite header-line.

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

> > Date: Fri, 12 Jul 2019 19:10:44 -0700
> > From: Keith David Bershatsky <esq@lawlist.com>
> >
> > Step 1:  Launch Emacs 26.
> >
> > Step 2:  Evaluate the following code.
> >
> > (progn
> >   (find-library "simple")
> >   (pixel-scroll-mode 1)
> >   (re-search-forward "\t\t\t\t\t" nil t)
> >   (set-window-start (selected-window) (match-beginning 0))
> >   (setq header-line-format "hello-world")
> >   (setq buffer-display-table (make-display-table))
> >   (aset buffer-display-table
> >         ?\t
> >         (vector (make-glyph-code ?\u00BB 'font-lock-warning-face)
> >                 (make-glyph-code ?\t 'highlight)))
> >   (setq tab-width 8))
> >
> > Step 3:  Move the mousewheel downward one time; i.e., one ratchet.
> >
> > Step 4:  Observe the visible stretch glyphs overwriting the header-line.
>
> This doesn't happen to me on MS-Windows, so I think this is
> Darwin-specific.
>
> Can someone try reproducing this on GNU/Linux and report?

I can't reproduce this here.

In GNU Emacs 26.2 (build 2, x86_64-pc-linux-gnu, GTK+ Version 3.22.11)
 of 2019-04-13 built on joffe
Windowing system distributor 'The X.Org Foundation', version 11.0.11902000
System Description:    Debian GNU/Linux 9.9 (stretch)

Thanks,
Stefan Kangas

bug#36633: Scrolling by pixels: visible stretch glyphs overwrite header-line.

[Eli Zaretskii]

> From: Stefan Kangas <stefan@marxist.se>
> Date: Sat, 13 Jul 2019 10:32:16 +0200
> Cc: Keith David Bershatsky <esq@lawlist.com>, 36633@debbugs.gnu.org
> 
> > > (progn
> > >   (find-library "simple")
> > >   (pixel-scroll-mode 1)
> > >   (re-search-forward "\t\t\t\t\t" nil t)
> > >   (set-window-start (selected-window) (match-beginning 0))
> > >   (setq header-line-format "hello-world")
> > >   (setq buffer-display-table (make-display-table))
> > >   (aset buffer-display-table
> > >         ?\t
> > >         (vector (make-glyph-code ?\u00BB 'font-lock-warning-face)
> > >                 (make-glyph-code ?\t 'highlight)))
> > >   (setq tab-width 8))
> > >
> > > Step 3:  Move the mousewheel downward one time; i.e., one ratchet.
> > >
> > > Step 4:  Observe the visible stretch glyphs overwriting the header-line.
> >
> > This doesn't happen to me on MS-Windows, so I think this is
> > Darwin-specific.
> >
> > Can someone try reproducing this on GNU/Linux and report?
> 
> I can't reproduce this here.

Thanks.  I guess this confirms that the problem is specific to macOS.

bug#36633: Scrolling by pixels: visible stretch glyphs overwrite header-line.

[Alan Third]

On Fri, Jul 12, 2019 at 07:10:44PM -0700, Keith David Bershatsky wrote:
> Step 1:  Launch Emacs 26.
> 
> Step 2:  Evaluate the following code.
> 
> (progn
>   (find-library "simple")
>   (pixel-scroll-mode 1)
>   (re-search-forward "\t\t\t\t\t" nil t)
>   (set-window-start (selected-window) (match-beginning 0))
>   (setq header-line-format "hello-world")
>   (setq buffer-display-table (make-display-table))
>   (aset buffer-display-table
>         ?\t
>         (vector (make-glyph-code ?\u00BB 'font-lock-warning-face)
>                 (make-glyph-code ?\t 'highlight)))
>   (setq tab-width 8))
> 
> Step 3:  Move the mousewheel downward one time; i.e., one ratchet.
> 
> Step 4:  Observe the visible stretch glyphs overwriting the header-line.

Hi Keith, I can’t replicate this behaviour. The worst I see is some
flickering of the header line, which is unfortunately to be expected
from the current implementation.

When you do it is the header line being overwritten and not
immediately updated?
-- 
Alan Third

bug#36633: Scrolling by pixels: visible stretch glyphs overwrite header-line.

[Keith David Bershatsky]

Here are links to two (2) youtube videos showing the issue on Emacs 26.1 (built by https://emacsformacosx.com/ a few months ago) and also the master branch that I built using a commit from 07/10/2019 (40cd6278ba965f3a9fcdddc568eb9ee7c69cf899).  I don't have a 26.2 build at the moment.  The issue is less noticeable on the master branch, but can still be seen nevertheless.  I have not studied the code as to what makes this feature work, but offhand would think that the header-line should not be overwritten by the row moving upwards (rather than updated after it has been overwritten).

Emacs 26.1:  https://youtu.be/Y8cZfURSwZc

Emacs Master Branch:  https://youtu.be/BO3r9_6OvYo

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

> Date: [07-14-2019 03:39:35] <14 Jul 2019 11:39:35 +0100>
> From: Alan Third <alan@idiocy.org>
> 
> * * *
> 
> Hi Keith, I can't replicate this behaviour. The worst I see is some
> flickering of the header line, which is unfortunately to be expected
> from the current implementation.
> 
> When you do it is the header line being overwritten and not immediately updated?
> --
> Alan Third

bug#36633: Scrolling by pixels: visible stretch glyphs overwrite header-line.

[Keith David Bershatsky]

I forgot to cc Stefan K. and Eli Z. with the attached post ....

;;;;;;;;;;;;;;;;;;;;;; FORWARDED MESSAGE ;;;;;;;;;;;;;;;;;;;;;;

Date:  Sun, 14 Jul 2019 09:47:25 -0700
From:  Keith David Bershatsky <esq@lawlist.com>
To:  Alan Third <alan@idiocy.org>
Cc:  36633@debbugs.gnu.org
Subject:  Re: bug#36633: Scrolling by pixels: visible stretch glyphs overwrite header-line.

Here are links to two (2) youtube videos showing the issue on Emacs 26.1 (built by https://emacsformacosx.com/ a few months ago) and also the master branch that I built using a commit from 07/10/2019 (40cd6278ba965f3a9fcdddc568eb9ee7c69cf899).  I don't have a 26.2 build at the moment.  The issue is less noticeable on the master branch, but can still be seen nevertheless.  I have not studied the code as to what makes this feature work, but offhand would think that the header-line should not be overwritten by the row moving upwards (rather than updated after it has been overwritten).

Emacs 26.1:  https://youtu.be/Y8cZfURSwZc

Emacs Master Branch:  https://youtu.be/BO3r9_6OvYo

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

> Date: [07-14-2019 03:39:35] <14 Jul 2019 11:39:35 +0100>
> From: Alan Third <alan@idiocy.org>
> 
> * * *
> 
> Hi Keith, I can't replicate this behaviour. The worst I see is some
> flickering of the header line, which is unfortunately to be expected
> from the current implementation.
> 
> When you do it is the header line being overwritten and not immediately updated?
> --
> Alan Third

bug#36633: Scrolling by pixels: visible stretch glyphs overwrite header-line.

[Tak Kunihiro]

I confirmed the existence of ghost (of the first line with green blocks)
shown on the headline significantly with 26.2 on macOS 10.6.8 and less
significantly with 26.2 on macOS 10.14.5.

The essence of related code is `set-window-vscroll'.

 (let ((n (frame-char-height)))
  (when (> n 0)
    (let ((vs0 (window-vscroll nil t)))
      (dolist (vs (number-sequence (1+ vs0) (+ vs0 n)))
        (set-window-vscroll nil vs t) (sit-for pixel-wait)))))

I do not know how to fix it.


Keith David Bershatsky <esq@lawlist.com> writes:

> Here are links to two (2) youtube videos showing the issue on Emacs
> 26.1 (built by https://emacsformacosx.com/ a few months ago) and
> also
> the master branch that I built using a commit from 07/10/2019
> (40cd6278ba965f3a9fcdddc568eb9ee7c69cf899).  I don't have a 26.2
> build
> at the moment.  The issue is less noticeable on the master branch,
> but
> can still be seen nevertheless.  I have not studied the code as to
> what makes this feature work, but offhand would think that the
> header-line should not be overwritten by the row moving upwards
> (rather than updated after it has been overwritten).
>
> Emacs 26.1:  https://youtu.be/Y8cZfURSwZc
>
> Emacs Master Branch:  https://youtu.be/BO3r9_6OvYo

bug#36633: Scrolling by pixels: visible stretch glyphs overwrite header-line.

[Alan Third]

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

On Sun, Jul 14, 2019 at 09:47:25AM -0700, Keith David Bershatsky wrote:
> Here are links to two (2) youtube videos showing the issue on Emacs
> 26.1 (built by https://emacsformacosx.com/ a few months ago) and
> also the master branch that I built using a commit from 07/10/2019
> (40cd6278ba965f3a9fcdddc568eb9ee7c69cf899). I don't have a 26.2
> build at the moment. The issue is less noticeable on the master
> branch, but can still be seen nevertheless. I have not studied the
> code as to what makes this feature work, but offhand would think
> that the header-line should not be overwritten by the row moving
> upwards (rather than updated after it has been overwritten).
> 
> Emacs 26.1:  https://youtu.be/Y8cZfURSwZc
> 
> Emacs Master Branch:  https://youtu.be/BO3r9_6OvYo

OK, that’s really not what I was expecting to see. 

It looks to me like the stretch glyph code in nsterm.m is just
nonsensical... I don’t really know what to make of it. I guess I’m
missing something?

Anyway, patch attached. I don’t see anything particularly wrong with
it applied, but it’s possible I’ve reintroduced some bug where the
modeline gets partially overwritten.

The patch is against master, but it’s probably possible to apply it to
26 manually.
-- 
Alan Third

[-- Attachment #2: 0001-Fix-stretch-glyphs-overlap-with-line-above-bug-36633.patch --]
[-- Type: text/plain, Size: 1904 bytes --]

From 637813be1f6cd6119bddd7864e1f7778e46f2b71 Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Tue, 16 Jul 2019 21:32:23 +0100
Subject: [PATCH] Fix stretch glyphs overlap with line above (bug#36633)

* src/nsterm.m (ns_dumpglyphs_stretch): Move overwriting of the
clipping rectangle to after performing clipping.
---
 src/nsterm.m | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/src/nsterm.m b/src/nsterm.m
index 02331826d9..b8754278f0 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -3913,10 +3913,13 @@ Function modeled after x_draw_glyph_string_box ().
   if (!s->background_filled_p)
     {
       n = ns_get_glyph_string_clip_rect (s, r);
-      *r = NSMakeRect (s->x, s->y, s->background_width, s->height);
 
       if (ns_clip_to_rect (s->f, r, n))
         {
+          /* FIXME: Why are we reusing the clipping rectangles? The
+             other terms don't appear to do anything like this.  */
+          *r = NSMakeRect (s->x, s->y, s->background_width, s->height);
+
           if (s->hl == DRAW_MOUSE_FACE)
             {
               face = FACE_FROM_ID_OR_NULL (s->f,
@@ -3951,13 +3954,6 @@ Function modeled after x_draw_glyph_string_box ().
                         r[i].origin.x += leftoverrun;
                         r[i].size.width -= leftoverrun;
                       }
-
-                    /* XXX: Try to work between problem where a stretch glyph on
-                       a partially-visible bottom row will clear part of the
-                       modeline, and another where list-buffers headers and similar
-                       rows erroneously have visible_height set to 0.  Not sure
-                       where this is coming from as other terms seem not to show.  */
-                    r[i].size.height = min (s->height, s->row->visible_height);
                 }
 
               [bgCol set];
-- 
2.21.0

bug#36633: Scrolling by pixels: visible stretch glyphs overwrite header-line.

[Keith David Bershatsky]

Thank you, Alan, for your work on issue 36633.

Today, I built an Emacs master branch on OSX 10.6.8 using:

Last Commit : Tue Jul 16 16:04:54 2019 -0700
            : Glenn Morris <rgm@gnu.org>
            : 7e70c78867ab2b8e280f6a087b32a947b13776a7
            : An electric test is now passing

with the following configuration:

CFLAGS='-Wall -O0 -g3' ./configure \
--with-ns \
--enable-checking='yes,glyphs' \
--enable-check-lisp-object-type \
--without-compress-install \
--with-gnutls=no \
--with-mailutils \
--without-makeinfo

I did not see any visible change using the test outlined in the original post.  Here is a new screen-recording using the above-mentioned build:

https://youtu.be/IS3Dgy-326U

bug#36633: Scrolling by pixels: visible stretch glyphs overwrite header-line.

[Alan Third]

On Tue, Jul 16, 2019 at 11:34:57PM -0700, Keith David Bershatsky wrote:
> Thank you, Alan, for your work on issue 36633.
> 
> Today, I built an Emacs master branch on OSX 10.6.8 using:
> 
> Last Commit : Tue Jul 16 16:04:54 2019 -0700
>             : Glenn Morris <rgm@gnu.org>
>             : 7e70c78867ab2b8e280f6a087b32a947b13776a7
>             : An electric test is now passing

Did you apply the patch in the email? It clears up all the flickering
I could see here, so I’m a bit surprised to hear it did nothing at
all.
-- 
Alan Third

bug#36633: Scrolling by pixels: visible stretch glyphs overwrite header-line.

[Keith David Bershatsky]

I misunderstood about the need to apply the patch and assumed the master branch already contained the fix.  I will be at a computer later on today and will apply the patch and report back with the results.

On Jul 17, 2019, at 11:31 AM, Alan Third <alan@idiocy.org> wrote:

> On Tue, Jul 16, 2019 at 11:34:57PM -0700, Keith David Bershatsky wrote:
>> Thank you, Alan, for your work on issue 36633.
>> 
>> Today, I built an Emacs master branch on OSX 10.6.8 using:
>> 
>> Last Commit : Tue Jul 16 16:04:54 2019 -0700
>>            : Glenn Morris <rgm@gnu.org>
>>            : 7e70c78867ab2b8e280f6a087b32a947b13776a7
>>            : An electric test is now passing
> 
> Did you apply the patch in the email? It clears up all the flickering
> I could see here, so I’m a bit surprised to hear it did nothing at
> all.
> -- 
> Alan Third

bug#36633: Scrolling by pixels: visible stretch glyphs overwrite header-line.

[Tak Kunihiro]

I applied the patch to the master and confirmed the ghost disappeared on
Emacs 27.0.50 on macOS 10.6.8.

bug#36633: Scrolling by pixels: visible stretch glyphs overwrite header-line.

[Keith David Bershatsky]

I applied the following patch:

0001-Fix-stretch-glyphs-overlap-with-line-above-bug-36633.patch

and can confirm that the header-line is no longer being overwritten by the visible stretch glyphs as described in the initial post as to issue 36633.  Thank you for your work on this issue!

The following is a screenshot of the results with the above-mentioned patch applied:

https://youtu.be/g8DkCW-vaKA

bug#36633: Scrolling by pixels: visible stretch glyphs overwrite header-line.

[Alan Third]

On Wed, Jul 17, 2019 at 08:56:50PM -0700, Keith David Bershatsky wrote:
> I applied the following patch:
> 
> 0001-Fix-stretch-glyphs-overlap-with-line-above-bug-36633.patch
> 
> and can confirm that the header-line is no longer being overwritten by the visible stretch glyphs as described in the initial post as to issue 36633.  Thank you for your work on this issue!
> 
> The following is a screenshot of the results with the above-mentioned patch applied:
> 
> https://youtu.be/g8DkCW-vaKA

Fix pushed to master.

That flickering on the modeline is annoying. I suspect it’s happening
because the scrolling happens at a different time from the redrawing.
I really need to get back on the offscreen drawing work.
-- 
Alan Third