Subject: Re: bug#5718: scroll-margin in buffer with small line count.

From: Eli Zaretskii <[email-masked]>
Date: Sat, 21 Jan 2017 21:17:05 +0200

> From: [email-masked]
> Cc: [email-masked],  [email-masked],  [email-masked]
> Date: Sat, 21 Jan 2017 13:46:37 -0500
> 
> >       /* See if point is on a partially visible line at the end.  */
> >       if (it.what == IT_EOB)
> > 	partial_p = it.current_y + it.ascent + it.descent > it.last_visible_y;
> >       else
> > 	{
> > 	  move_it_by_lines (&it, 1);
> > 	  partial_p = it.current_y > it.last_visible_y;
> > 	}
> >
> > (This is preceded by moving the iterator to the point's screen line or
> > to EOB, whichever comes first.)  The value it.current_y is the Y
> > coordinate of the top edge of a glyph row (the value is zero for the
> > first screen line), so if it.last_visible_y is farther away from that
> > than the height of the glyph row, that glyph row is fully visible;
> > otherwise it isn't.
> 
> I'm not entirely clear why there is a branch in that code.

Because of line-spacing, perhaps?  Did you test your code when
line-spacing is at non-default value?  In general, it is safer to go
to the next screen line than reason about where it will start.

> +int
> +partial_line_height (const struct it *it_origin)
> +{
> +  struct it it = *it_origin;

When you copy the iterator structure and modify the copy, you need to
save and restore the bidi cache, using SAVE_IT and RESTORE_IT macros.
Otherwise, the code which calls this function will work incorrectly if
it uses the original iterator after the call, because the bidi cache
is not restored to its state before the call.