bug#26445: 26.0.50; Scroll margin and cursor movement working incorrectly when scrolling over different height lines

bug#26445: 26.0.50; Scroll margin and cursor movement working incorrectly when scrolling over different height lines

[Alexander Miller]

This is probably a continuation of bug#25792.

The issue is that scrolling downwards with a scroll-margin
has the cursor jumping again - this time the issue is caused by 
different height
lines. This does not simply mean being inside a buffer where some lines
are fontified to use a different height attribute than others. The
stutter already occurs due to the miniscule height differences
introduced by box or over- and underline face attributes.

The following bits of code can reproduce the issue in emacs -q:

* (custom-set-faces `(button ((t (:box "#ffffff" :underline nil 
:overline nil
:background nil :foreground "#ffffff")))))
* (setq scroll-margin 10 scroll-conservatively 101)
* (defun maketext ()
(interactive)
(dotimes (i 1000)
(insert (format "\ntext %s\n" i))
(insert-text-button (format "text %s" i))))
* (maketext)

Scrolling down in this text induces the same stutter as in the previous
bug. Strangely enough the issue only occurs when the cursor is in a
column other than the very first. Only scrolling downwards is affected,
at least in a way I was able to reproduce it in emacs -q.
I also do have the same issue with upwards scrolling in org-agenda buffers
when running a heavily modified spacemacs, with a theme that makes heavy
use of boxed and different height text, so it's difficult to say where
things start exactly.

The second, maybe wholly unrelated, part is the cursor's column
movement. This time scrolling in either direction is affected. To
reproduce put the cursor at the very end of a line containing
unboxed text and scroll down or up. The cursor will move to the previous
column on every boxed line and move back again for unboxed lines.


In GNU Emacs 26.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.22.10)
of 2017-04-10 built on a-laptop
Repository revision: 3ccd0ff1064a2836c379b13c2d5f4b11c5da1f88
Windowing system distributor 'The X.Org Foundation

bug#26445: 26.0.50; Scroll margin and cursor movement working incorrectly when scrolling over different height lines

[Noam Postavsky]

found 26445 24.5
tag 26445 confirmed
severity 26445 minor
quit

On Tue, Apr 11, 2017 at 12:50 PM, Alexander Miller <alexanderm@web.de> wrote:
> This is probably a continuation of bug#25792.

The symptoms are similar, but this one seems to be a long-standing
bug, I get the same behaviour in 24.5 (haven't tested earlier
versions, but I wouldn't expect anything different).

This code in `try_cursor_movement' is what's different for scrolling
vs non-scrolling lines.

          /* If within the scroll margin, scroll.  Note that
         MATRIX_ROW_BOTTOM_Y gives the pixel position at which
         the next line would be drawn, and that
         this_scroll_margin can be zero.  */
          if (MATRIX_ROW_BOTTOM_Y (row) > last_y
          || PT > MATRIX_ROW_END_CHARPOS (row)
          /* Line is completely visible last line in window
             and PT is to be set in the next line.  */
          || (MATRIX_ROW_BOTTOM_Y (row) == last_y
              && PT == MATRIX_ROW_END_CHARPOS (row)
              && !row->ends_at_zv_p
              && !MATRIX_ROW_ENDS_IN_MIDDLE_OF_CHAR_P (row)))
        scroll_p = true;

I think the root issue might be that scroll-margin is given in lines,
and then it's translated to pixels under the assumption that lines are
all using the default height. Although my initial attempt to make
window_scroll_margin take different line heights into account doesn't
seem to have any effect. AFAICT, the next test, PT >
MATRIX_ROW_END_CHARPOS (row), just triggers instead.

modified   src/window.c
@@ -4820,10 +4820,17 @@ window_scroll_margin (struct window *window,
enum margin_unit unit)
         }
       int max_margin = min ((window_lines - 1)/2,
                             (int) (window_lines * ratio));
-      int margin = clip_to_bounds (0, scroll_margin, max_margin);
-      return (unit == MARGIN_IN_PIXELS)
-        ? margin * frame_line_height
-        : margin;
+      int margin_lines = clip_to_bounds (0, scroll_margin, max_margin);
+      if (unit == MARGIN_IN_LINES)
+          return margin_lines;
+      else
+        {
+          struct it it;
+          init_iterator (&it, window, BEGV, BEGV_BYTE, NULL, DEFAULT_FACE_ID);
+          move_it_to (&it, -1, -1, it.last_visible_y, -1, MOVE_TO_Y);
+          move_it_by_lines (&it, -margin_lines);
+          return it.last_visible_y - it.current_y;
+        }
     }
   else
     return 0;

bug#26445: 26.0.50; Scroll margin and cursor movement working incorrectly when scrolling over different height lines

[Eli Zaretskii]

> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Date: Thu, 13 Apr 2017 15:09:14 -0400
> Cc: 26445@debbugs.gnu.org
> 
> This code in `try_cursor_movement' is what's different for scrolling
> vs non-scrolling lines.
> 
>           /* If within the scroll margin, scroll.  Note that
>          MATRIX_ROW_BOTTOM_Y gives the pixel position at which
>          the next line would be drawn, and that
>          this_scroll_margin can be zero.  */
>           if (MATRIX_ROW_BOTTOM_Y (row) > last_y
>           || PT > MATRIX_ROW_END_CHARPOS (row)
>           /* Line is completely visible last line in window
>              and PT is to be set in the next line.  */
>           || (MATRIX_ROW_BOTTOM_Y (row) == last_y
>               && PT == MATRIX_ROW_END_CHARPOS (row)
>               && !row->ends_at_zv_p
>               && !MATRIX_ROW_ENDS_IN_MIDDLE_OF_CHAR_P (row)))
>         scroll_p = true;

Not sure what you are saying: do you see some problem in the above
code?  If so, where do you see a potential problem.

> I think the root issue might be that scroll-margin is given in lines,
> and then it's translated to pixels under the assumption that lines are
> all using the default height.

If that's the problem, I see no good solutions here, because
scroll-conservatively > 100 explicitly requests to position point as
close to the window edge as possible, and the alternating lines of
different height in the recipe of this bug report force us to stop one
line earlier every other scroll.  Am I missing something?

> Although my initial attempt to make
> window_scroll_margin take different line heights into account doesn't
> seem to have any effect. AFAICT, the next test, PT >
> MATRIX_ROW_END_CHARPOS (row), just triggers instead.

Not sure how to interpret what you say here.  If point is beyond
MATRIX_ROW_END_CHARPOS of a row, it means point is not in this row,
because MATRIX_ROW_END_CHARPOS gives the buffer position of the first
character in the next row.  There are no coordinates, pixel or
otherwise, involved here.

> modified   src/window.c
> @@ -4820,10 +4820,17 @@ window_scroll_margin (struct window *window,
> enum margin_unit unit)
>          }
>        int max_margin = min ((window_lines - 1)/2,
>                              (int) (window_lines * ratio));
> -      int margin = clip_to_bounds (0, scroll_margin, max_margin);
> -      return (unit == MARGIN_IN_PIXELS)
> -        ? margin * frame_line_height
> -        : margin;
> +      int margin_lines = clip_to_bounds (0, scroll_margin, max_margin);
> +      if (unit == MARGIN_IN_LINES)
> +          return margin_lines;
> +      else
> +        {
> +          struct it it;
> +          init_iterator (&it, window, BEGV, BEGV_BYTE, NULL, DEFAULT_FACE_ID);
> +          move_it_to (&it, -1, -1, it.last_visible_y, -1, MOVE_TO_Y);
> +          move_it_by_lines (&it, -margin_lines);
> +          return it.last_visible_y - it.current_y;
> +        }

Is this a suggested patch?  If so, how can it fix the issue?  When a
line is taller than the canonical height, positioning point on it
might enter the scroll-margin, and we still need to back up, don't we?
IOW, the above code still move "by lines", and will "stutter" if lines
are intermittently of different height.  What possible solution could
we come up with in such situations?  It seems to me that the user gets
what they asked for.

bug#26445: 26.0.50; Scroll margin and cursor movement working incorrectly when scrolling over different height lines

[Noam Postavsky]

On Thu, Apr 13, 2017 at 3:39 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Noam Postavsky <npostavs@users.sourceforge.net>
>> Date: Thu, 13 Apr 2017 15:09:14 -0400
>> Cc: 26445@debbugs.gnu.org
>>
>> This code in `try_cursor_movement' is what's different for scrolling
>> vs non-scrolling lines.
>>
>>           /* If within the scroll margin, scroll.  Note that
>>          MATRIX_ROW_BOTTOM_Y gives the pixel position at which
>>          the next line would be drawn, and that
>>          this_scroll_margin can be zero.  */
>>           if (MATRIX_ROW_BOTTOM_Y (row) > last_y
>>           || PT > MATRIX_ROW_END_CHARPOS (row)
>>           /* Line is completely visible last line in window
>>              and PT is to be set in the next line.  */
>>           || (MATRIX_ROW_BOTTOM_Y (row) == last_y
>>               && PT == MATRIX_ROW_END_CHARPOS (row)
>>               && !row->ends_at_zv_p
>>               && !MATRIX_ROW_ENDS_IN_MIDDLE_OF_CHAR_P (row)))
>>         scroll_p = true;
>
> Not sure what you are saying: do you see some problem in the above
> code?  If so, where do you see a potential problem.

Just noting that this appears to be where the difference between
scrolling and not scrolling on the alternating lines in the recipe is
decided. Specifically the first test: MATRIX_ROW_BOTTOM_Y (row) >
last_y.

>
>> I think the root issue might be that scroll-margin is given in lines,
>> and then it's translated to pixels under the assumption that lines are
>> all using the default height.
>
> If that's the problem, I see no good solutions here, because
> scroll-conservatively > 100 explicitly requests to position point as
> close to the window edge as possible, and the alternating lines of
> different height in the recipe of this bug report force us to stop one
> line earlier every other scroll.  Am I missing something?

I would have to sketch this out on paper to be sure, but you're probably right.

>
>> Although my initial attempt to make
>> window_scroll_margin take different line heights into account doesn't
>> seem to have any effect. AFAICT, the next test, PT >
>> MATRIX_ROW_END_CHARPOS (row), just triggers instead.
>
> Not sure how to interpret what you say here.

Just that the debugger seemed to be showing me that the divergence
between the scrolling and non-scrolling is at the 2nd test after
applying the patch below.

>  If point is beyond
> MATRIX_ROW_END_CHARPOS of a row, it means point is not in this row,
> because MATRIX_ROW_END_CHARPOS gives the buffer position of the first
> character in the next row.  There are no coordinates, pixel or
> otherwise, involved here.

>> modified   src/window.c
>> @@ -4820,10 +4820,17 @@ window_scroll_margin (struct window *window,
>> enum margin_unit unit)
>>          }
>>        int max_margin = min ((window_lines - 1)/2,
>>                              (int) (window_lines * ratio));
>> -      int margin = clip_to_bounds (0, scroll_margin, max_margin);
>> -      return (unit == MARGIN_IN_PIXELS)
>> -        ? margin * frame_line_height
>> -        : margin;
>> +      int margin_lines = clip_to_bounds (0, scroll_margin, max_margin);
>> +      if (unit == MARGIN_IN_LINES)
>> +          return margin_lines;
>> +      else
>> +        {
>> +          struct it it;
>> +          init_iterator (&it, window, BEGV, BEGV_BYTE, NULL, DEFAULT_FACE_ID);
>> +          move_it_to (&it, -1, -1, it.last_visible_y, -1, MOVE_TO_Y);
>> +          move_it_by_lines (&it, -margin_lines);
>> +          return it.last_visible_y - it.current_y;
>> +        }
>
> Is this a suggested patch?  If so, how can it fix the issue?

It's not, because it doesn't :) I'm just noting what fails to fix
this, for future reference.

>  When a
> line is taller than the canonical height, positioning point on it
> might enter the scroll-margin, and we still need to back up, don't we?
> IOW, the above code still move "by lines", and will "stutter" if lines
> are intermittently of different height.  What possible solution could
> we come up with in such situations?  It seems to me that the user gets
> what they asked for.

Yes, maybe the answer to this bug is just not to set
scroll-conservatively so high then.

bug#26445: 26.0.50; Scroll margin and cursor movement working incorrectly when scrolling over different height lines

[Alexander Miller]

On 13/04/17 22:05, Noam Postavsky wrote:
> Yes, maybe the answer to this bug is just not to set
> scroll-conservatively so high then.
That does help with the height difference introduced by thinly boxed 
text. Unfortunately there are
still other cases where where a scroll stutter still remains, caused by 
unicode symbols.
For example using prettify-symbols-mode to replace -> with ⭢ or configs 
for programs like
polybar which naturally contain a host of characters like    🔇 ⏭.
I guess if you guys say it cannot be reasonably done I'll consider 
getting rid of my margin altogether,
though I'm still curious why there's no problem when the cursor is at 
the very start of the line.

Then there's also the second part the bug report - the cursor switching 
columns when scrolling.

bug#26445: 26.0.50; Scroll margin and cursor movement working incorrectly when scrolling over different height lines

[Eli Zaretskii]

> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Date: Thu, 13 Apr 2017 16:05:31 -0400
> Cc: Alexander Miller <alexanderm@web.de>, 26445@debbugs.gnu.org
> 
> >> I think the root issue might be that scroll-margin is given in lines,
> >> and then it's translated to pixels under the assumption that lines are
> >> all using the default height.
> >
> > If that's the problem, I see no good solutions here, because
> > scroll-conservatively > 100 explicitly requests to position point as
> > close to the window edge as possible, and the alternating lines of
> > different height in the recipe of this bug report force us to stop one
> > line earlier every other scroll.  Am I missing something?
> 
> I would have to sketch this out on paper to be sure, but you're probably right.

The only solution I see is to use window-vscroll to position point
more accurately one line before scroll-margin.  That sounds gross to
me, though, and will cause a similar "stutter" on the first screen
line of the window, so someone else in some other use case will
probably complain.

If we do want to make such changes, the place to make them is in
line-move and line-move-partial, not in xdisp.c.

> >  When a
> > line is taller than the canonical height, positioning point on it
> > might enter the scroll-margin, and we still need to back up, don't we?
> > IOW, the above code still move "by lines", and will "stutter" if lines
> > are intermittently of different height.  What possible solution could
> > we come up with in such situations?  It seems to me that the user gets
> > what they asked for.
> 
> Yes, maybe the answer to this bug is just not to set
> scroll-conservatively so high then.

Or accept the consequences of it being high.

bug#26445: 26.0.50; Scroll margin and cursor movement working incorrectly when scrolling over different height lines

[Eli Zaretskii]

> Cc: 26445@debbugs.gnu.org
> From: Alexander Miller <alexanderm@web.de>
> Date: Thu, 13 Apr 2017 23:07:48 +0200
> 
> On 13/04/17 22:05, Noam Postavsky wrote:
> > Yes, maybe the answer to this bug is just not to set
> > scroll-conservatively so high then.
> That does help with the height difference introduced by thinly boxed 
> text. Unfortunately there are
> still other cases where where a scroll stutter still remains, caused by 
> unicode symbols.
> For example using prettify-symbols-mode to replace -> with ⭢ or configs 
> for programs like
> polybar which naturally contain a host of characters like    🔇 ⏭.

That depends on what font you have installed that's used to display
those characters.  It is best to have fonts of approximately the same
height.

> I guess if you guys say it cannot be reasonably done I'll consider 
> getting rid of my margin altogether,

There's no need to get rid of margins, as I think the problem you
describe is purely aesthetic, and quite expected when you have lines
of different height.  Why does it bother you so much?

> though I'm still curious why there's no problem when the cursor is at 
> the very start of the line.

Because the window layout begins at the first screen line, and the
y-coordinate of a screen line is its top edge.  So the display engine
can always position the first screen line exactly, whereas where the
last screen line ends depends on what is in-between, and the scroll
margin at the end of the window forces the last pixel of the point's
line to be above the margin.

I also think that if you produce text where lines have more than just
2 different heights, you will see a similar problem while scrolling
up as well.

> Then there's also the second part the bug report - the cursor switching 
> columns when scrolling.

That's not a bug: the button-face line is slightly longer than the
lines in the default face, and scrolling attempts to preserve the
x-coordinate of point, not the column (because the same column could
be at very different x-coordinate due to fonts, faces, inline images,
etc.).  You can see what's going on if you put the cursor on a
boxed-text line, then scroll: you will see a small jitter of the
cursor in the horizontal direction.

bug#26445: 26.0.50; Scroll margin and cursor movement working incorrectly when scrolling over different height lines

[Eli Zaretskii]

> Date: Fri, 14 Apr 2017 10:06:41 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 26445@debbugs.gnu.org, npostavs@users.sourceforge.net
> 
> > For example using prettify-symbols-mode to replace -> with ⭢ or configs 
> > for programs like
> > polybar which naturally contain a host of characters like    🔇 ⏭.
> 
> That depends on what font you have installed that's used to display
> those characters.  It is best to have fonts of approximately the same
> height.

Btw, , , and  are private-use area (PUA) characters, so I'm not
sure what you expected from Emacs to do with them.

bug#26445: 26.0.50; Scroll margin and cursor movement working incorrectly when scrolling over different height lines

[Alexander Miller]

On 14/04/17 09:06, Eli Zaretskii wrote:
> That depends on what font you have installed that's used to display
> those characters.  It is best to have fonts of approximately the same
> height.
How would I go about finding out which fonts have which height and how 
to make
emacs use them? Looking up font size and height just turns up a bunch of 
pages
about css. For emacs I found I could do something like
(set-fontset-font "fontset-default" nil (font-spec :size 1 :name "Symbola"))
but that doesn't seem to do anything.
> There's no need to get rid of margins, as I think the problem you
> describe is purely aesthetic, and quite expected when you have lines
> of different height.  Why does it bother you so much?
I just don't like the inconsistency. The scroll stutter happens when I'm 
quickly
scrolling through a buffer and suddenly see my cursor change position for
seemingly no reason. To me it just plain looks like something that is 
not supposed
to happen. I certainly wouldn't have bothered with the bug report if 
there was a
clear and visibly text height difference, but the issue already appears 
when the height
difference cannot be seen with the naked eye.

> Btw, , , and  are private-use area (PUA) characters, so I'm not
> sure what you expected from Emacs to do with them.
I've picked those up from Font Awesome, which brings us back to my 
previous question.
Is it possible to tell emacs that Font Awesome Icons should have the 
same size as
my default text font, and if yes how do I do it?

bug#26445: 26.0.50; Scroll margin and cursor movement working incorrectly when scrolling over different height lines

[Eli Zaretskii]

> Cc: npostavs@users.sourceforge.net, 26445@debbugs.gnu.org
> From: Alexander Miller <alexanderm@web.de>
> Date: Fri, 14 Apr 2017 12:56:30 +0200
> 
> On 14/04/17 09:06, Eli Zaretskii wrote:
> > That depends on what font you have installed that's used to display
> > those characters.  It is best to have fonts of approximately the same
> > height.
> How would I go about finding out which fonts have which height

I don't know any way except by trying, sorry.  Perhaps some font
expert could offer a more useful method.

> and how to make emacs use them?

You do that by customizing the default fontset.  There are examples in
the Emacs manual, and you can see more examples in fontset.el which
comes with Emacs.

> For emacs I found I could do something like
> (set-fontset-font "fontset-default" nil (font-spec :size 1 :name "Symbola"))
> but that doesn't seem to do anything.

If you have Symbola installed, Emacs will already use it by default.
You can see which font is used for a character by invoking "C-u C-x ="
at that character's position.  If Symbola is not used for some
characters, you could customize your default fontset so that it is.

> > There's no need to get rid of margins, as I think the problem you
> > describe is purely aesthetic, and quite expected when you have lines
> > of different height.  Why does it bother you so much?
> I just don't like the inconsistency. The scroll stutter happens when I'm 
> quickly scrolling through a buffer and suddenly see my cursor change
> position for seemingly no reason.

There's no way around that when screen lines have different height.
The most you can do is make sure (most of) your fonts are of the same
height.  FWIW, the characters you mentioned in your email are
displayed on my system using Symbola, and the height of the line is
the same as for the surrounding text.  So perhaps you should just make
sure your Emacs uses Symbola for them?

> > Btw, , , and  are private-use area (PUA) characters, so I'm not
> > sure what you expected from Emacs to do with them.
> I've picked those up from Font Awesome, which brings us back to my 
> previous question.

I tried to answer it above.

> Is it possible to tell emacs that Font Awesome Icons should have the
> same size as my default text font, and if yes how do I do it?

Once you tell Emacs to use that font for some range of characters, it
should choose the best size automatically.

bug#26445: 26.0.50; Scroll margin and cursor movement working incorrectly when scrolling over different height lines

[Eli Zaretskii]

> From: Alexander Miller <alexanderm@web.de>
> Date: Fri, 14 Apr 2017 17:03:16 +0200
> 
> On 14/04/17 14:16, Eli Zaretskii wrote:
> > Once you tell Emacs to use that font for some range of characters, it
> > should choose the best size automatically.
> Not always. I did not experiment too much, but one example is ⇛ displayed
> by DejaVu Sans being slightly larger than the surrounding Fantasque Sans 
> Mono text.

I said "the best size".  That doesn't always mean "perfect", because
not every font supports all the required sizes.

> At any rate I now have all the tools I need to fix my height issue: For 
> entire character
> ranges being too large (like Font Awesome) the entire range's height can be
> changed with
> (set-fontset-font "fontset-default" '(#xf039 . #xf26e)
>                    (font-spec :size 12 :name "Font Awesome"))
> The exact range is probably not correct, I still need to look up where 
> it starts and ends.
> 
> The same solution can be used to fix the size of a single character:
> (set-fontset-font "fontset-default" '(?\⇛ . ?\⇛)
>                    (font-spec :size 12 :name "Symbola"))

This will work as long as you don't use face scaling.

> The bug can be closed now. Thanks for the help and happy easter.

Thanks, closing.