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

From: Eli Zaretskii <[email-masked]>
Date: Mon, 16 Jan 2017 19:08:37 +0200

> From: [email-masked]
> Cc: [email-masked],  [email-masked],  [email-masked]
> Date: Sun, 15 Jan 2017 16:43:34 -0500
> 
> Yeah, I'm still a bit lost in all the different structures.  It seems
> (it.last_visible_y % frame_line_height) might work.

That would only be correct if all the screen lines use the default
font.  Otherwise, the last line could be only partially visible even
if the above is zero.

> The matrix row stuff in xdisp seems a more direct way of getting it,
> though I guess that's not really usable from window.c?

If by "matrix row" you meant the glyph matrix prepared by the previous
redisplay, then using that is unreliable, because that matrix might
not be up to date.

You can see that window_scroll_pixel_based already uses the move_it_*
family of functions, which is the right way of doing this stuff --
these functions simulate redisplay without actually displaying
anything, so you can compute the metrics of anything on display using
them.

> What would be the idiomatic way of doing this?
> 
> #define MR_PARTIALLY_VISIBLE(ROW)	\
>   ((ROW)->height != (ROW)->visible_height)

There's already such a calculation in window_scroll_pixel_based:

      /* 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.

> By the way, in window_scroll_pixel_based (lines 5139 to 5158) the
> position of "it" is saved twice, and it looks like the first one of
> these can never be used (because the second overwrites it), correct?

Yep, good catch.

Thanks.