* move_it_vertically_backward question
[not found] <87lf0pw78r.fsf.ref@yahoo.com>
@ 2021-12-13 2:47 ` Po Lu
2021-12-13 14:50 ` Eli Zaretskii
0 siblings, 1 reply; 53+ messages in thread
From: Po Lu @ 2021-12-13 2:47 UTC (permalink / raw)
To: emacs-devel
I've been trying to introduce a new redisplay primitive to speed up
precision pixel scrolling, but I don't understand the behaviour of
move_it_vertically_backward. According to the comment, it will move IT
backwards by at least as many pixels as DY, but that's not how it seems
to behave, at least in the primitive I'm trying to implement.
DEFUN ("point-and-pixel-height-of-unseen-line-above",
Fpoint_and_pixel_height_of_unseen_line_above,
Spoint_and_pixel_height_of_unseen_line_above, 1, 1, 0,
doc: /* Find a visual line at least PIXELS above window start.
Return the dimensions of the line as a cons of the buffer position of
the start of the line, and the vertical distance in pixels between that
line and the start of the window. */)
(Lisp_Object pixels)
{
int pix;
struct it it;
struct text_pos pt;
struct window *w;
struct buffer *old_buffer = NULL;
Lisp_Object result;
CHECK_FIXNAT (pixels);
pix = XFIXNAT (pixels);
w = XWINDOW (selected_window);
if (XBUFFER (w->contents) != current_buffer)
{
old_buffer = current_buffer;
set_buffer_internal_1 (XBUFFER (w->contents));
}
SET_TEXT_POS_FROM_MARKER (pt, w->start);
void *itdata = bidi_shelve_cache ();
start_display (&it, w, pt);
it.vpos = it.current_y = 0;
last_height = 0;
move_it_by_lines (&it, 0);
move_it_vertically_backward (&it, pix);
result = Fcons (make_fixnum (IT_CHARPOS (it)),
make_fixnum (-it.current_y));
if (old_buffer)
set_buffer_internal_1 (old_buffer);
bidi_unshelve_cache (itdata, false);
return result;
}
If there are 3 lines above window start, all of which are 17 pixels
tall, calling this new primitive with PIXELS anything between 18 and 33
will result in the start and height of the first line being returned,
while I would have expected it to move onto the second line, as anything
between 18 and 33 pixels above window start should be inside the second
line.
Ideas? Thanks in advance.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-13 2:47 ` move_it_vertically_backward question Po Lu
@ 2021-12-13 14:50 ` Eli Zaretskii
2021-12-14 0:53 ` Po Lu
0 siblings, 1 reply; 53+ messages in thread
From: Eli Zaretskii @ 2021-12-13 14:50 UTC (permalink / raw)
To: Po Lu; +Cc: emacs-devel
> From: Po Lu <luangruo@yahoo.com>
> Date: Mon, 13 Dec 2021 10:47:48 +0800
>
> I've been trying to introduce a new redisplay primitive to speed up
> precision pixel scrolling
I'm not sure this is the way to go, judging by what you told on
help-gnu-emacs. The main reason is that window-text-pixel-size, which
you said doesn't do what you want, uses exactly the same technique as
you are trying to do in this new primitive, and so whatever problems
you have with window-text-pixel-size, you will bump into them (or
similar problems) with your new primitive as well. This stuff is
always extremely tricky when advanced display features are used, like
display and overlay strings with embedded newlines, line-prefix, line
numbers, etc. There's no easy way around this complexity.
Instead, we should understand better why window-text-pixel-size
doesn't fit your bill, and then extend it so that it does what you
want in your use cases.
That said, to answer your question:
> If there are 3 lines above window start, all of which are 17 pixels
> tall, calling this new primitive with PIXELS anything between 18 and 33
> will result in the start and height of the first line being returned,
> while I would have expected it to move onto the second line, as anything
> between 18 and 33 pixels above window start should be inside the second
> line.
The commentary to the function doesn't tell the whole story. In fact,
it always undershoots first, because of this:
/* Estimate how many newlines we must move back. */
nlines = max (1, dy / default_line_pixel_height (it->w));
This is integer division, so it truncates the number of lines. E.g.,
in your case, with DY between 17 and 33 you get 1 line, not more.
Then it moves back by that number of physical lines, which is why you
get the line before window-start.
And then it can move farther back, but only if the undershoot is
"large enough":
/* If we did not reach target_y, try to move further backward if
we can. If we moved too far backward, try to move forward. */
if (target_y < it->current_y
/* This is heuristic. In a window that's 3 lines high, with
a line height of 13 pixels each, recentering with point
on the bottom line will try to move -39/2 = 19 pixels
backward. Try to avoid moving into the first line. */
&& (it->current_y - target_y
> min (window_box_height (it->w), line_height * 2 / 3))
&& IT_CHARPOS (*it) > BEGV)
{
move_trace (" not far enough -> move_vert %d\n",
target_y - it->current_y);
dy = it->current_y - target_y;
goto move_further_back;
}
That "2/3rd of line height" threshold heuristic is the reason why it
not always moves one more line back. On my system, the default pixel
height of a line is 16, so it moves to the second line before
window-start for DY >= 27, since 2/3rd of 16 is 10.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-13 14:50 ` Eli Zaretskii
@ 2021-12-14 0:53 ` Po Lu
2021-12-14 12:52 ` Eli Zaretskii
0 siblings, 1 reply; 53+ messages in thread
From: Po Lu @ 2021-12-14 0:53 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: emacs-devel
Eli Zaretskii <eliz@gnu.org> writes:
> I'm not sure this is the way to go, judging by what you told on
> help-gnu-emacs. The main reason is that window-text-pixel-size, which
> you said doesn't do what you want, uses exactly the same technique as
> you are trying to do in this new primitive, and so whatever problems
> you have with window-text-pixel-size, you will bump into them (or
> similar problems) with your new primitive as well. This stuff is
> always extremely tricky when advanced display features are used, like
> display and overlay strings with embedded newlines, line-prefix, line
> numbers, etc. There's no easy way around this complexity.
Yes, I understand that much. This primitive is supposed to solve a
performance problem though: right now, we go through quite a few hoops
to find such a position and return its height, and I've been getting
complaints that the speed of that is unacceptable.
Since there seems to be no faster way to do what I'm trying to,
introducing a new primitive seems to be the way to go.
> Instead, we should understand better why window-text-pixel-size
> doesn't fit your bill, and then extend it so that it does what you
> want in your use cases.
While the performance of `window-text-pixel-size' itself is ample,
finding the target window start is not: we have to find the beginning of
the visual line, then (vertical-motion -1) in a loop calculating the
height with `window-text-pixel-size' until it reaches an appropriate
value.
> The commentary to the function doesn't tell the whole story. In fact,
> it always undershoots first, because of this:
>
> /* Estimate how many newlines we must move back. */
> nlines = max (1, dy / default_line_pixel_height (it->w));
>
> This is integer division, so it truncates the number of lines. E.g.,
> in your case, with DY between 17 and 33 you get 1 line, not more.
> Then it moves back by that number of physical lines, which is why you
> get the line before window-start.
>
> And then it can move farther back, but only if the undershoot is
> "large enough":
>
> /* If we did not reach target_y, try to move further backward if
> we can. If we moved too far backward, try to move forward. */
> if (target_y < it->current_y
> /* This is heuristic. In a window that's 3 lines high, with
> a line height of 13 pixels each, recentering with point
> on the bottom line will try to move -39/2 = 19 pixels
> backward. Try to avoid moving into the first line. */
> && (it->current_y - target_y
> > min (window_box_height (it->w), line_height * 2 / 3))
> && IT_CHARPOS (*it) > BEGV)
> {
> move_trace (" not far enough -> move_vert %d\n",
> target_y - it->current_y);
> dy = it->current_y - target_y;
> goto move_further_back;
> }
> That "2/3rd of line height" threshold heuristic is the reason why it
> not always moves one more line back. On my system, the default pixel
> height of a line is 16, so it moves to the second line before
> window-start for DY >= 27, since 2/3rd of 16 is 10.
Hmm. Is it legal to pass a negative Y argument to `move_it_to'?
Perhaps that can be used instead.
Thanks.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-14 0:53 ` Po Lu
@ 2021-12-14 12:52 ` Eli Zaretskii
2021-12-14 13:28 ` Po Lu
0 siblings, 1 reply; 53+ messages in thread
From: Eli Zaretskii @ 2021-12-14 12:52 UTC (permalink / raw)
To: Po Lu; +Cc: emacs-devel
> From: Po Lu <luangruo@yahoo.com>
> Cc: emacs-devel@gnu.org
> Date: Tue, 14 Dec 2021 08:53:35 +0800
>
> Yes, I understand that much. This primitive is supposed to solve a
> performance problem though: right now, we go through quite a few hoops
> to find such a position and return its height, and I've been getting
> complaints that the speed of that is unacceptable.
Extending an existing primitive will give you the same performance as
a new primitive. But it will also save you from reinventing the wheel
and from having to deal with all the subtleties of primitives that
simulate display to find some place or measure of the text as it will
be on display.
> Since there seems to be no faster way to do what I'm trying to,
> introducing a new primitive seems to be the way to go.
I'm saying that extending an existing primitive to cover your use case
might be a better way forward.
> > Instead, we should understand better why window-text-pixel-size
> > doesn't fit your bill, and then extend it so that it does what you
> > want in your use cases.
>
> While the performance of `window-text-pixel-size' itself is ample,
> finding the target window start is not: we have to find the beginning of
> the visual line, then (vertical-motion -1) in a loop calculating the
> height with `window-text-pixel-size' until it reaches an appropriate
> value.
Which might mean that window-text-pixel-size should support a
specification of FROM and TO that is not just buffer position, but
something like "beginning of previous line".
> Hmm. Is it legal to pass a negative Y argument to `move_it_to'?
"Legal", as in "no against the law"? Yes. "Valid"? no. The
move_to_* functions can generally only move forward, because they use
the normal "iteration through buffer text" infrastructure, and that
examines characters in the order of increasing buffer positions. We
don't have algorithms that can perform layout calculations while going
back in the buffer.
So to move back, you need first go far enough back (e.g., with
move_it_vertically_backward), then move forward to find the place
where you wanted to find yourself wrt the starting point. Se an
example in move_it_by_lines.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-14 12:52 ` Eli Zaretskii
@ 2021-12-14 13:28 ` Po Lu
2021-12-14 13:45 ` Eli Zaretskii
0 siblings, 1 reply; 53+ messages in thread
From: Po Lu @ 2021-12-14 13:28 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: emacs-devel
Eli Zaretskii <eliz@gnu.org> writes:
> Extending an existing primitive will give you the same performance as
> a new primitive. But it will also save you from reinventing the wheel
> and from having to deal with all the subtleties of primitives that
> simulate display to find some place or measure of the text as it will
> be on display.
> I'm saying that extending an existing primitive to cover your use case
> might be a better way forward.
I don't know of any existing primitive that may make sense to extend in
this manner though.
> Which might mean that window-text-pixel-size should support a
> specification of FROM and TO that is not just buffer position, but
> something like "beginning of previous line".
Yes, but that would still entail having to repetitively loop in Lisp
until the desired position is found, assuming the delta by which we are
scrolling is larger than the height of the previous screen line, while
this primitive looks for a line that is at least a specified amount of
pixels above the window start, which lets us avoid the extra looping in
Lisp if the line is not tall enough.
Also, for the result to be useful, `window-text-pixel-size' would also
have to return the buffer position of the measured line's start, which
doesn't make sense for a function that is only intended to measure size.
> So to move back, you need first go far enough back (e.g., with
> move_it_vertically_backward), then move forward to find the place
> where you wanted to find yourself wrt the starting point. Se an
> example in move_it_by_lines.
Thanks. Something like this seems to work:
while (-it.current_y < pix)
{
last_y = it.current_y;
move_it_by_lines (&it, -1);
}
But I have a few questions: what is TRT if `move_it_by_lines' can't move
further back? And also, does `move_it_by_lines' (or for that matter,
the move_it_* functions in general) only move within the accessible
portion of the buffer, or does narrowing have to be handled in some
specific manner?
Thanks.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-14 13:28 ` Po Lu
@ 2021-12-14 13:45 ` Eli Zaretskii
2021-12-15 1:18 ` Po Lu
2021-12-15 2:13 ` Po Lu
0 siblings, 2 replies; 53+ messages in thread
From: Eli Zaretskii @ 2021-12-14 13:45 UTC (permalink / raw)
To: Po Lu; +Cc: emacs-devel
> From: Po Lu <luangruo@yahoo.com>
> Cc: emacs-devel@gnu.org
> Date: Tue, 14 Dec 2021 21:28:44 +0800
>
> > I'm saying that extending an existing primitive to cover your use case
> > might be a better way forward.
>
> I don't know of any existing primitive that may make sense to extend in
> this manner though.
>
> > Which might mean that window-text-pixel-size should support a
> > specification of FROM and TO that is not just buffer position, but
> > something like "beginning of previous line".
>
> Yes, but that would still entail having to repetitively loop in Lisp
> until the desired position is found
No, I meant to ask window-text-pixel-size to do that, by passing it a
specially-formatted value of FROM (and possibly also TO).
> assuming the delta by which we are
> scrolling is larger than the height of the previous screen line, while
> this primitive looks for a line that is at least a specified amount of
> pixels above the window start, which lets us avoid the extra looping in
> Lisp if the line is not tall enough.
So let's teach window-text-pixel-size to be able to accept the FROM
argument which says "N pixels above position POS".
> Also, for the result to be useful, `window-text-pixel-size' would also
> have to return the buffer position of the measured line's start, which
> doesn't make sense for a function that is only intended to measure size.
What makes sense is what we decide that will make sense. We have a
lot of functions that normally return a simple value, but if asked
nicely return additional information. A random example:
pos-visible-in-window-p.
> > So to move back, you need first go far enough back (e.g., with
> > move_it_vertically_backward), then move forward to find the place
> > where you wanted to find yourself wrt the starting point. Se an
> > example in move_it_by_lines.
>
> Thanks. Something like this seems to work:
>
> while (-it.current_y < pix)
> {
> last_y = it.current_y;
> move_it_by_lines (&it, -1);
> }
That's not very efficient if PIX can be a large value.
> But I have a few questions: what is TRT if `move_it_by_lines' can't move
> further back?
Why cannot it?
> And also, does `move_it_by_lines' (or for that matter,
> the move_it_* functions in general) only move within the accessible
> portion of the buffer, or does narrowing have to be handled in some
> specific manner?
The former. In general, very little code in Emacs accesses buffer
text outside of the current restriction; the display code in its
entirety certainly doesn't. You need to manually widen the buffer to
do so. One of the few exceptions is the display-line-numbers feature,
in one of its optional formats of displaying line numbers, where the
user _wants_ it to ignore the narrowing.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-14 13:45 ` Eli Zaretskii
@ 2021-12-15 1:18 ` Po Lu
2021-12-15 3:27 ` Eli Zaretskii
2021-12-15 2:13 ` Po Lu
1 sibling, 1 reply; 53+ messages in thread
From: Po Lu @ 2021-12-15 1:18 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: emacs-devel
Eli Zaretskii <eliz@gnu.org> writes:
> Why cannot it?
For example, if it has reached the beginning of the buffer, it doesn't
make any sense to move further back.
> The former. In general, very little code in Emacs accesses buffer
> text outside of the current restriction; the display code in its
> entirety certainly doesn't. You need to manually widen the buffer to
> do so. One of the few exceptions is the display-line-numbers feature,
> in one of its optional formats of displaying line numbers, where the
> user _wants_ it to ignore the narrowing.
Thanks.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-14 13:45 ` Eli Zaretskii
2021-12-15 1:18 ` Po Lu
@ 2021-12-15 2:13 ` Po Lu
2021-12-15 10:28 ` Po Lu
2021-12-15 13:25 ` Eli Zaretskii
1 sibling, 2 replies; 53+ messages in thread
From: Po Lu @ 2021-12-15 2:13 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: emacs-devel
Eli Zaretskii <eliz@gnu.org> writes:
> No, I meant to ask window-text-pixel-size to do that, by passing it a
> specially-formatted value of FROM (and possibly also TO).
>
> So let's teach window-text-pixel-size to be able to accept the FROM
> argument which says "N pixels above position POS".
I still don't believe that would be better than a separate primitive
(since it doesn't make sense to supply the width information to the
pixel scrolling code), and unfortunately I don't quite understand how
`window_text_pixel_size' works either: for instance, it seems to disable
the bidirectional reordering of text? I also don't see why `it' is
saved into `it2' only to be restored before anything interesting is done
with `it'.
Correctly pixel-scrolling over bidirectional text will probably be a
very important feature for people working with RTL languages, but I'm
not sure how many of them are likely to use such a feature.
> That's not very efficient if PIX can be a large value.
Yes. Maybe calling `move_it_vertically_backward' itself until a
suitable position is reached will be better, as I really don't want to
reinvent "move_it_vertically_backwards but slightly different".
Something like this:
move_it_vertically_backward (&it, pix);
while (-it.current_y < pix)
{
last_y = it.current_y;
move_it_vertically_backward (&it, pix + it.current_y);
if (it.current_y == last_y)
break;
}
Thanks.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-15 1:18 ` Po Lu
@ 2021-12-15 3:27 ` Eli Zaretskii
2021-12-15 3:30 ` Po Lu
0 siblings, 1 reply; 53+ messages in thread
From: Eli Zaretskii @ 2021-12-15 3:27 UTC (permalink / raw)
To: Po Lu; +Cc: emacs-devel
> From: Po Lu <luangruo@yahoo.com>
> Cc: emacs-devel@gnu.org
> Date: Wed, 15 Dec 2021 09:18:43 +0800
>
> Eli Zaretskii <eliz@gnu.org> writes:
>
> > Why cannot it?
>
> For example, if it has reached the beginning of the buffer, it doesn't
> make any sense to move further back.
If you want to detect that, you need to test IT_CHARPOS (it).
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-15 3:27 ` Eli Zaretskii
@ 2021-12-15 3:30 ` Po Lu
2021-12-15 13:27 ` Eli Zaretskii
0 siblings, 1 reply; 53+ messages in thread
From: Po Lu @ 2021-12-15 3:30 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: emacs-devel
Eli Zaretskii <eliz@gnu.org> writes:
>> For example, if it has reached the beginning of the buffer, it doesn't
>> make any sense to move further back.
> If you want to detect that, you need to test IT_CHARPOS (it).
Something like what `IT_POS_VALID_AFTER_MOVE_P' does?
Makes sense now, thanks!
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-15 2:13 ` Po Lu
@ 2021-12-15 10:28 ` Po Lu
2021-12-15 13:56 ` Eli Zaretskii
2021-12-15 13:25 ` Eli Zaretskii
1 sibling, 1 reply; 53+ messages in thread
From: Po Lu @ 2021-12-15 10:28 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: emacs-devel
Po Lu <luangruo@yahoo.com> writes:
> Something like this:
>
> move_it_vertically_backward (&it, pix);
>
> while (-it.current_y < pix)
> {
> last_y = it.current_y;
> move_it_vertically_backward (&it, pix + it.current_y);
>
> if (it.current_y == last_y)
> break;
> }
BTW, if this will work, I'd like to install this primitive along with
the modification to the precision scrolling code that makes use of it
as-is, at least as a temporary solution to the performance issues until
we can decide which primitive to move its functionality into.
Thanks.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-15 2:13 ` Po Lu
2021-12-15 10:28 ` Po Lu
@ 2021-12-15 13:25 ` Eli Zaretskii
2021-12-15 13:38 ` Po Lu
1 sibling, 1 reply; 53+ messages in thread
From: Eli Zaretskii @ 2021-12-15 13:25 UTC (permalink / raw)
To: Po Lu; +Cc: emacs-devel
> From: Po Lu <luangruo@yahoo.com>
> Cc: emacs-devel@gnu.org
> Date: Wed, 15 Dec 2021 10:13:39 +0800
>
> Eli Zaretskii <eliz@gnu.org> writes:
>
> > No, I meant to ask window-text-pixel-size to do that, by passing it a
> > specially-formatted value of FROM (and possibly also TO).
> >
> > So let's teach window-text-pixel-size to be able to accept the FROM
> > argument which says "N pixels above position POS".
>
> I still don't believe that would be better than a separate primitive
> (since it doesn't make sense to supply the width information to the
> pixel scrolling code), and unfortunately I don't quite understand how
> `window_text_pixel_size' works either:
So you don't have a sufficient understanding of how
window_text_pixel_size works, but you are already sure it isn't a good
candidate for the job you'd like to do? Maybe it's better to wait
until your understanding is sufficient, before making up your mind
about that?
> for instance, it seems to disable the bidirectional reordering of
> text?
Yes, because text metrics between two (reasonably chosen) buffer
positions are unaffected by reordering. There's a comment in the code
explaining further why is that.
> I also don't see why `it' is saved into `it2' only to be restored
> before anything interesting is done with `it'.
Do you understand why we use SAVE_IT and RESTORE_IT in the display
engine? (There's a comment near their definition which explains the
basic reasons.) If you do, I don't really see how such a question
could pop up. Maybe the comment that explains the need for this gork
should be improved or clarified?
> Correctly pixel-scrolling over bidirectional text will probably be a
> very important feature for people working with RTL languages, but I'm
> not sure how many of them are likely to use such a feature.
See above: we don't lose anything in such situations. I would not
have allowed code into the display engine that would break bidi
support.
> > That's not very efficient if PIX can be a large value.
>
> Yes. Maybe calling `move_it_vertically_backward' itself until a
> suitable position is reached will be better, as I really don't want to
> reinvent "move_it_vertically_backwards but slightly different".
>
> Something like this:
>
> move_it_vertically_backward (&it, pix);
>
> while (-it.current_y < pix)
> {
> last_y = it.current_y;
> move_it_vertically_backward (&it, pix + it.current_y);
>
> if (it.current_y == last_y)
> break;
> }
I cannot say anything intelligent because I don't think I understand
the requirements well enough. AFAICT, you never described them in
enough detail, neither on help-gnu-emacs nor here.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-15 3:30 ` Po Lu
@ 2021-12-15 13:27 ` Eli Zaretskii
2021-12-15 13:39 ` Po Lu
0 siblings, 1 reply; 53+ messages in thread
From: Eli Zaretskii @ 2021-12-15 13:27 UTC (permalink / raw)
To: Po Lu; +Cc: emacs-devel
> From: Po Lu <luangruo@yahoo.com>
> Cc: emacs-devel@gnu.org
> Date: Wed, 15 Dec 2021 11:30:10 +0800
>
> Eli Zaretskii <eliz@gnu.org> writes:
>
> >> For example, if it has reached the beginning of the buffer, it doesn't
> >> make any sense to move further back.
>
> > If you want to detect that, you need to test IT_CHARPOS (it).
>
> Something like what `IT_POS_VALID_AFTER_MOVE_P' does?
No, just
if (IT_CHARPOS (it) <= BEGV)
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-15 13:25 ` Eli Zaretskii
@ 2021-12-15 13:38 ` Po Lu
2021-12-15 14:50 ` Eli Zaretskii
0 siblings, 1 reply; 53+ messages in thread
From: Po Lu @ 2021-12-15 13:38 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: emacs-devel
Eli Zaretskii <eliz@gnu.org> writes:
>> I still don't believe that would be better than a separate primitive
>> (since it doesn't make sense to supply the width information to the
>> pixel scrolling code), and unfortunately I don't quite understand how
>> `window_text_pixel_size' works either:
> So you don't have a sufficient understanding of how
> window_text_pixel_size works, but you are already sure it isn't a good
> candidate for the job you'd like to do? Maybe it's better to wait
> until your understanding is sufficient, before making up your mind
> about that?
No, I meant "don't quite understand" as in the two questions I asked
you, which you've answered below:
> Do you understand why we use SAVE_IT and RESTORE_IT in the display
> engine? (There's a comment near their definition which explains the
> basic reasons.) If you do, I don't really see how such a question
> could pop up. Maybe the comment that explains the need for this gork
> should be improved or clarified?
I didn't understand the comment when I first saw it, but now I do, so I
retract my question. Thanks.
> See above: we don't lose anything in such situations. I would not
> have allowed code into the display engine that would break bidi
> support.
>> Yes. Maybe calling `move_it_vertically_backward' itself until a
>> suitable position is reached will be better, as I really don't want to
>> reinvent "move_it_vertically_backwards but slightly different".
>>
>> Something like this:
>>
>> move_it_vertically_backward (&it, pix);
>>
>> while (-it.current_y < pix)
>> {
>> last_y = it.current_y;
>> move_it_vertically_backward (&it, pix + it.current_y);
>>
>> if (it.current_y == last_y)
>> break;
>> }
> I cannot say anything intelligent because I don't think I understand
> the requirements well enough. AFAICT, you never described them in
> enough detail, neither on help-gnu-emacs nor here.
The requirement is basically to find the start of a line that is at
least above window start by a given number of pixels, and to also find
the vertical distance between the top of that line and the start of the
window.
For instance, if we're scrolling upwards (upwards in this context is
opposite to the terminology we use for line-based scrolling: it means
moving the display downwards, like scroll-down) by 17 pixels, and there
are two lines each 14 pixels in height above window start, the new
window start will be set to the start position of the second line above
window start (which this function should return), and vscroll will be
set to 7 (which is 24 - 17, where the 24 is the aformentioned vertical
distance that this function should also return).
Thanks.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-15 13:27 ` Eli Zaretskii
@ 2021-12-15 13:39 ` Po Lu
0 siblings, 0 replies; 53+ messages in thread
From: Po Lu @ 2021-12-15 13:39 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: emacs-devel
Eli Zaretskii <eliz@gnu.org> writes:
> No, just
>
> if (IT_CHARPOS (it) <= BEGV)
Thanks.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-15 10:28 ` Po Lu
@ 2021-12-15 13:56 ` Eli Zaretskii
0 siblings, 0 replies; 53+ messages in thread
From: Eli Zaretskii @ 2021-12-15 13:56 UTC (permalink / raw)
To: Po Lu; +Cc: emacs-devel
> From: Po Lu <luangruo@yahoo.com>
> Cc: emacs-devel@gnu.org
> Date: Wed, 15 Dec 2021 18:28:50 +0800
>
> Po Lu <luangruo@yahoo.com> writes:
>
> > Something like this:
> >
> > move_it_vertically_backward (&it, pix);
> >
> > while (-it.current_y < pix)
> > {
> > last_y = it.current_y;
> > move_it_vertically_backward (&it, pix + it.current_y);
> >
> > if (it.current_y == last_y)
> > break;
> > }
>
> BTW, if this will work, I'd like to install this primitive along with
> the modification to the precision scrolling code that makes use of it
> as-is, at least as a temporary solution to the performance issues until
> we can decide which primitive to move its functionality into.
I would like our primitives to be sufficiently general and generic to
justify their existence. I'm not sure I understand what exactly would
you like to install, but the original code you posted is definitely
unfit for being a primitive (too specific to
pixel-scroll-precision-mode), and given the unfinished discussion of
whether window-text-pixel-size could do the job, I think it's very
probable that we are not yet ready to seriously talk about what should
be installed.
So I suggest to continue discussing window-text-pixel-size until we
understand completely your needs on the one hand, and what can be done
by extending window-text-pixel-size on the other hand. Then we will
be able to make the decision.
Thanks.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-15 13:38 ` Po Lu
@ 2021-12-15 14:50 ` Eli Zaretskii
2021-12-16 0:41 ` Po Lu
0 siblings, 1 reply; 53+ messages in thread
From: Eli Zaretskii @ 2021-12-15 14:50 UTC (permalink / raw)
To: Po Lu; +Cc: emacs-devel
> From: Po Lu <luangruo@yahoo.com>
> Cc: emacs-devel@gnu.org
> Date: Wed, 15 Dec 2021 21:38:14 +0800
>
> The requirement is basically to find the start of a line that is at
> least above window start by a given number of pixels, and to also find
> the vertical distance between the top of that line and the start of the
> window.
>
> For instance, if we're scrolling upwards (upwards in this context is
> opposite to the terminology we use for line-based scrolling: it means
> moving the display downwards, like scroll-down) by 17 pixels, and there
> are two lines each 14 pixels in height above window start, the new
> window start will be set to the start position of the second line above
> window start (which this function should return), and vscroll will be
> set to 7 (which is 24 - 17, where the 24 is the aformentioned vertical
> distance that this function should also return).
We have code in the display engine that does this in several places.
For example, look at what we do when we recenter the window in
redisplay_window. We could factor that code out of there, put it in a
separate function, and teach window-text-pixel-size to find the FROM
position using that new function. Then the value of the TO argument
for window-text-pixel-size is of course well known, and you could run
the rest of the function to provide you with the pixel dimensions of
the text in-between.
Would that fit the bill?
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-15 14:50 ` Eli Zaretskii
@ 2021-12-16 0:41 ` Po Lu
2021-12-16 8:29 ` Eli Zaretskii
0 siblings, 1 reply; 53+ messages in thread
From: Po Lu @ 2021-12-16 0:41 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: emacs-devel
Eli Zaretskii <eliz@gnu.org> writes:
> We have code in the display engine that does this in several places.
> For example, look at what we do when we recenter the window in
> redisplay_window. We could factor that code out of there, put it in a
> separate function, and teach window-text-pixel-size to find the FROM
> position using that new function. Then the value of the TO argument
> for window-text-pixel-size is of course well known, and you could run
> the rest of the function to provide you with the pixel dimensions of
> the text in-between.
>
> Would that fit the bill?
I think it would, with the addition of a way to specify "the end of the
visual line preceeding window start" as the TO argument. That is
because window start could be inside a screen line, causing its height
to be taken into account when `window-text-pixel-size' calculates
height.
Thanks.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-16 0:41 ` Po Lu
@ 2021-12-16 8:29 ` Eli Zaretskii
2021-12-16 9:25 ` Po Lu
0 siblings, 1 reply; 53+ messages in thread
From: Eli Zaretskii @ 2021-12-16 8:29 UTC (permalink / raw)
To: Po Lu; +Cc: emacs-devel
> From: Po Lu <luangruo@yahoo.com>
> Cc: emacs-devel@gnu.org
> Date: Thu, 16 Dec 2021 08:41:46 +0800
>
> Eli Zaretskii <eliz@gnu.org> writes:
>
> > We have code in the display engine that does this in several places.
> > For example, look at what we do when we recenter the window in
> > redisplay_window. We could factor that code out of there, put it in a
> > separate function, and teach window-text-pixel-size to find the FROM
> > position using that new function. Then the value of the TO argument
> > for window-text-pixel-size is of course well known, and you could run
> > the rest of the function to provide you with the pixel dimensions of
> > the text in-between.
> >
> > Would that fit the bill?
>
> I think it would, with the addition of a way to specify "the end of the
> visual line preceeding window start" as the TO argument. That is
> because window start could be inside a screen line, causing its height
> to be taken into account when `window-text-pixel-size' calculates
> height.
I don't object to supporting such specifications for TO, but why do
you need that in this case? why not use window-start as TO?
Or maybe I misunderstand what you mean by "window start could be
inside a screen line". Do you mean the window start is not at the
leftmost character/display element shown on that screen line? When
does that happen?
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-16 8:29 ` Eli Zaretskii
@ 2021-12-16 9:25 ` Po Lu
2021-12-16 10:04 ` Eli Zaretskii
0 siblings, 1 reply; 53+ messages in thread
From: Po Lu @ 2021-12-16 9:25 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: emacs-devel
Eli Zaretskii <eliz@gnu.org> writes:
> Or maybe I misunderstand what you mean by "window start could be
> inside a screen line". Do you mean the window start is not at the
> leftmost character/display element shown on that screen line?
Yes.
> When does that happen?
It happens in Info buffers, at the very least. I don't know why that
is, but it happens.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-16 9:25 ` Po Lu
@ 2021-12-16 10:04 ` Eli Zaretskii
2021-12-16 10:27 ` Po Lu
0 siblings, 1 reply; 53+ messages in thread
From: Eli Zaretskii @ 2021-12-16 10:04 UTC (permalink / raw)
To: Po Lu; +Cc: emacs-devel
> From: Po Lu <luangruo@yahoo.com>
> Cc: emacs-devel@gnu.org
> Date: Thu, 16 Dec 2021 17:25:10 +0800
>
> Eli Zaretskii <eliz@gnu.org> writes:
>
> > Or maybe I misunderstand what you mean by "window start could be
> > inside a screen line". Do you mean the window start is not at the
> > leftmost character/display element shown on that screen line?
>
> Yes.
>
> > When does that happen?
>
> It happens in Info buffers, at the very least. I don't know why that
> is, but it happens.
I guess that's because of display and/or overlay strings at the
beginning of the node or something.
We should provide a way for window-text-pixel-size to stop at the
visual beginning of a line, but I'm not sure the right way of saying
that is specifying the buffer position of the previous line: there
could be a display/overlay string there as well, or somesuch.
So are you going to work on extending window-text-pixel-size like
this, and if so, do you need my help?
Thanks.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-16 10:04 ` Eli Zaretskii
@ 2021-12-16 10:27 ` Po Lu
2021-12-16 12:17 ` Po Lu
0 siblings, 1 reply; 53+ messages in thread
From: Po Lu @ 2021-12-16 10:27 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: emacs-devel
Eli Zaretskii <eliz@gnu.org> writes:
> I guess that's because of display and/or overlay strings at the
> beginning of the node or something.
> We should provide a way for window-text-pixel-size to stop at the
> visual beginning of a line, but I'm not sure the right way of saying
> that is specifying the buffer position of the previous line: there
> could be a display/overlay string there as well, or somesuch.
I can't think of anything better.
> So are you going to work on extending window-text-pixel-size like
> this, and if so, do you need my help?
I will work on this, and I'll make some noise if I need help. Thanks.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-16 10:27 ` Po Lu
@ 2021-12-16 12:17 ` Po Lu
2021-12-16 13:27 ` Eli Zaretskii
0 siblings, 1 reply; 53+ messages in thread
From: Po Lu @ 2021-12-16 12:17 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: emacs-devel
Po Lu <luangruo@yahoo.com> writes:
> I can't think of anything better.
Now that I think of it, how about this: we introduce a primitive
`line-above-point-by', which returns the line N pixels above a given
point in the current window, and a new `no-ascent-descent' argument to
window-text-pixel-size, that avoids adding the ascent and descent of the
iterator to the height before returning it?
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-16 12:17 ` Po Lu
@ 2021-12-16 13:27 ` Eli Zaretskii
2021-12-16 13:34 ` Po Lu
0 siblings, 1 reply; 53+ messages in thread
From: Eli Zaretskii @ 2021-12-16 13:27 UTC (permalink / raw)
To: Po Lu; +Cc: emacs-devel
> From: Po Lu <luangruo@yahoo.com>
> Cc: emacs-devel@gnu.org
> Date: Thu, 16 Dec 2021 20:17:45 +0800
>
> Po Lu <luangruo@yahoo.com> writes:
>
> > I can't think of anything better.
>
> Now that I think of it, how about this: we introduce a primitive
> `line-above-point-by', which returns the line N pixels above a given
> point in the current window
What would such a primitive be used for, except for your specific use
case?
And even in that use case, it's better to have a single primitive that
does both this job and the text-dimension measuring job than having 2
separate primitives that you'd need to call from Lisp. If nothing
else, you lose the information collected by the iterator in the first
call, and cannot use it in the second. Moreover, position found by
the iterator cannot be always easily communicated to another
primitive, because that position could be inside a display or overlay
string, not at a buffer position.
> and a new `no-ascent-descent' argument to window-text-pixel-size,
> that avoids adding the ascent and descent of the iterator to the
> height before returning it?
Since ascent+descent are the line height, I don't understand why this
would be useful.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-16 13:27 ` Eli Zaretskii
@ 2021-12-16 13:34 ` Po Lu
2021-12-16 13:59 ` Eli Zaretskii
0 siblings, 1 reply; 53+ messages in thread
From: Po Lu @ 2021-12-16 13:34 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: emacs-devel
Eli Zaretskii <eliz@gnu.org> writes:
> What would such a primitive be used for, except for your specific use
> case?
I can't think of any offhand, but I can't think of any other use case
for the modification to the `from' parameter of `window-text-pixel-size'
either.
> Since ascent+descent are the line height, I don't understand why this
> would be useful.
In this case, we don't want the line height of the line up to TO
(because window start might be inside a line, such as in Info buffers
and when there is a line-prefix property).
Thanks.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-16 13:34 ` Po Lu
@ 2021-12-16 13:59 ` Eli Zaretskii
2021-12-17 1:45 ` Po Lu
0 siblings, 1 reply; 53+ messages in thread
From: Eli Zaretskii @ 2021-12-16 13:59 UTC (permalink / raw)
To: Po Lu; +Cc: emacs-devel
> From: Po Lu <luangruo@yahoo.com>
> Cc: emacs-devel@gnu.org
> Date: Thu, 16 Dec 2021 21:34:40 +0800
>
> Eli Zaretskii <eliz@gnu.org> writes:
>
> > What would such a primitive be used for, except for your specific use
> > case?
>
> I can't think of any offhand, but I can't think of any other use case
> for the modification to the `from' parameter of `window-text-pixel-size'
> either.
The difference is that window-text-pixel-size already exists, we just
extend it for another use case.
> > Since ascent+descent are the line height, I don't understand why this
> > would be useful.
>
> In this case, we don't want the line height of the line up to TO
> (because window start might be inside a line, such as in Info buffers
> and when there is a line-prefix property).
Well, then this is equivalent to telling window-text-pixel-size to
stop at the visual left edge of a screen line, right? That is,
interpret TO not as a literal buffer position, but as the visual
beginning of the line which shows position TO.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-16 13:59 ` Eli Zaretskii
@ 2021-12-17 1:45 ` Po Lu
2021-12-18 10:28 ` Eli Zaretskii
0 siblings, 1 reply; 53+ messages in thread
From: Po Lu @ 2021-12-17 1:45 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: emacs-devel
[-- Attachment #1: Type: text/plain, Size: 876 bytes --]
Eli Zaretskii <eliz@gnu.org> writes:
> The difference is that window-text-pixel-size already exists, we just
> extend it for another use case.
> Well, then this is equivalent to telling window-text-pixel-size to
> stop at the visual left edge of a screen line, right? That is,
> interpret TO not as a literal buffer position, but as the visual
> beginning of the line which shows position TO.
Yes, I think the two meanings are equivalent. I documented it as the
former, but it wouldn't hurt to document it like this instead.
Anyway, here are two patches that implement the extensions to
`window-text-pixel-size', along with the code in
pixel-scroll-precision-mode that makes use of them. I tried to update
the documentation, but to be frank I'm not good at writing documentation
and couldn't word it very well, so someone else should take a look at
it.
Thanks a lot!
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-new-argument-no-descents-ascents-to-window-text-.patch --]
[-- Type: text/x-patch, Size: 8681 bytes --]
From 0ebd752969a2ace5e8f897bb140b8daeb21d978e Mon Sep 17 00:00:00 2001
From: Po Lu <luangruo@yahoo.com>
Date: Thu, 16 Dec 2021 20:24:16 +0800
Subject: [PATCH 1/2] Add new argument `no-descents-ascents' to
`window-text-pixel-size'
* doc/lispref/display.texi (Size of Displayed Text): Update
documentation.
* etc/NEWS: Announce new argument.
* src/xdisp.c (window_text_pixel_size): Allow controlling if
the iterator's ascent and descent will be appended to the
pixel height returned. All callers changed.
(Fwindow_text_pixel_size): New argument `no-ascents-descents'.
All callers changed.
---
doc/lispref/display.texi | 6 +++++-
etc/NEWS | 5 +++++
src/haikufns.c | 3 ++-
src/w32fns.c | 3 ++-
src/xdisp.c | 42 ++++++++++++++++++++++++++++++----------
src/xfns.c | 3 ++-
6 files changed, 48 insertions(+), 14 deletions(-)
diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
index b82473f9c2..bf0d1c05e1 100644
--- a/doc/lispref/display.texi
+++ b/doc/lispref/display.texi
@@ -2086,7 +2086,7 @@ Size of Displayed Text
(@pxref{Resizing Windows}) to make a window exactly as large as the text
it contains.
-@defun window-text-pixel-size &optional window from to x-limit y-limit mode-lines
+@defun window-text-pixel-size &optional window from to x-limit y-limit mode-lines no-ascents-descents
This function returns the size of the text of @var{window}'s buffer in
pixels. @var{window} must be a live window and defaults to the
selected one. The return value is a cons of the maximum pixel-width
@@ -2136,6 +2136,10 @@ Size of Displayed Text
height of all of these lines, if present, in the return value.
@end defun
+The optional argument @var{no-ascents-descents} controls whether or
+not to count the height of text in @var{to}'s screen line as part of
+the returned pixel-height.
+
@code{window-text-pixel-size} treats the text displayed in a window as a
whole and does not care about the size of individual lines. The
following function does.
diff --git a/etc/NEWS b/etc/NEWS
index 8d9c645694..b3e335f00a 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -984,6 +984,11 @@ when they have changed.
This can be used to check whether a specific font has a glyph for a
character.
++++
+** 'window-text-pixel-size' now accepts a new argument `no-ascents-descents'.
+This controls whether or not the screen line at the end of the
+measured area will be counted during the height calculation.
+
** XDG support
*** New function 'xdg-state-home' returns 'XDG_STATE_HOME' environment variable.
diff --git a/src/haikufns.c b/src/haikufns.c
index 868fc71f97..737b033899 100644
--- a/src/haikufns.c
+++ b/src/haikufns.c
@@ -1970,7 +1970,8 @@ DEFUN ("x-show-tip", Fx_show_tip, Sx_show_tip, 1, 6, 0,
try_window (window, pos, TRY_WINDOW_IGNORE_FONTS_CHANGE);
/* Calculate size of tooltip window. */
size = Fwindow_text_pixel_size (window, Qnil, Qnil, Qnil,
- make_fixnum (w->pixel_height), Qnil);
+ make_fixnum (w->pixel_height), Qnil,
+ Qnil);
/* Add the frame's internal border to calculated size. */
width = XFIXNUM (Fcar (size)) + 2 * FRAME_INTERNAL_BORDER_WIDTH (tip_f);
height = XFIXNUM (Fcdr (size)) + 2 * FRAME_INTERNAL_BORDER_WIDTH (tip_f);
diff --git a/src/w32fns.c b/src/w32fns.c
index 65463b5261..02a6d78b51 100644
--- a/src/w32fns.c
+++ b/src/w32fns.c
@@ -7525,7 +7525,8 @@ DEFUN ("x-show-tip", Fx_show_tip, Sx_show_tip, 1, 6, 0,
try_window (window, pos, TRY_WINDOW_IGNORE_FONTS_CHANGE);
/* Calculate size of tooltip window. */
size = Fwindow_text_pixel_size (window, Qnil, Qnil, Qnil,
- make_fixnum (w->pixel_height), Qnil);
+ make_fixnum (w->pixel_height), Qnil,
+ Qnil);
/* Add the frame's internal border to calculated size. */
width = XFIXNUM (Fcar (size)) + 2 * FRAME_INTERNAL_BORDER_WIDTH (tip_f);
height = XFIXNUM (Fcdr (size)) + 2 * FRAME_INTERNAL_BORDER_WIDTH (tip_f);
diff --git a/src/xdisp.c b/src/xdisp.c
index 5e549c9c63..0772238f2d 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -10833,7 +10833,7 @@ in_display_vector_p (struct it *it)
argument. */
static Lisp_Object
window_text_pixel_size (Lisp_Object window, Lisp_Object from, Lisp_Object to, Lisp_Object x_limit,
- Lisp_Object y_limit, Lisp_Object mode_lines)
+ Lisp_Object y_limit, Lisp_Object mode_lines, Lisp_Object no_ascents_descents)
{
struct window *w = decode_live_window (window);
struct it it;
@@ -10969,8 +10969,19 @@ window_text_pixel_size (Lisp_Object window, Lisp_Object from, Lisp_Object to, Li
if (IT_CHARPOS (it) == end)
{
x += it.pixel_width;
- it.max_ascent = max (it.max_ascent, it.ascent);
- it.max_descent = max (it.max_descent, it.descent);
+
+ /* DTRT if no_ascents_descents is t. */
+ if (no_ascents_descents)
+ {
+ saw_display_prop_at_end_p = true;
+ doff = (max (it.max_ascent, it.ascent)
+ + max (it.max_descent, it.descent));
+ }
+ else
+ {
+ it.max_ascent = max (it.max_ascent, it.ascent);
+ it.max_descent = max (it.max_descent, it.descent);
+ }
}
}
else
@@ -10991,8 +11002,14 @@ window_text_pixel_size (Lisp_Object window, Lisp_Object from, Lisp_Object to, Li
/* Subtract height of header-line and tab-line which was counted
automatically by start_display. */
- y = it.current_y + it.max_ascent + it.max_descent
- - WINDOW_TAB_LINE_HEIGHT (w) - WINDOW_HEADER_LINE_HEIGHT (w);
+ y = (it.current_y + (NILP (no_ascents_descents)
+ ? it.max_ascent + it.max_descent
+ : 0)
+ - WINDOW_TAB_LINE_HEIGHT (w) - WINDOW_HEADER_LINE_HEIGHT (w));
+
+ if (saw_display_prop_at_end_p)
+ y += doff;
+
/* Don't return more than Y-LIMIT. */
if (y > max_y)
y = max_y;
@@ -11039,7 +11056,7 @@ window_text_pixel_size (Lisp_Object window, Lisp_Object from, Lisp_Object to, Li
return Fcons (make_fixnum (x - start_x), make_fixnum (y));
}
-DEFUN ("window-text-pixel-size", Fwindow_text_pixel_size, Swindow_text_pixel_size, 0, 6, 0,
+DEFUN ("window-text-pixel-size", Fwindow_text_pixel_size, Swindow_text_pixel_size, 0, 7, 0,
doc: /* Return the size of the text of WINDOW's buffer in pixels.
WINDOW must be a live window and defaults to the selected one. The
return value is a cons of the maximum pixel-width of any text line
@@ -11086,9 +11103,12 @@ DEFUN ("window-text-pixel-size", Fwindow_text_pixel_size, Swindow_text_pixel_siz
height of the mode-, tab- or header-line of WINDOW in the return value.
If it is the symbol `mode-line', 'tab-line' or `header-line', include
only the height of that line, if present, in the return value. If t,
-include the height of any of these, if present, in the return value. */)
+include the height of any of these, if present, in the return value.
+
+NO-ASCENTS-DESCENTS means to not add the height of the screen line at
+TO to the returned height. */)
(Lisp_Object window, Lisp_Object from, Lisp_Object to, Lisp_Object x_limit,
- Lisp_Object y_limit, Lisp_Object mode_lines)
+ Lisp_Object y_limit, Lisp_Object mode_lines, Lisp_Object no_ascents_descents)
{
struct window *w = decode_live_window (window);
struct buffer *b = XBUFFER (w->contents);
@@ -11101,7 +11121,8 @@ DEFUN ("window-text-pixel-size", Fwindow_text_pixel_size, Swindow_text_pixel_siz
set_buffer_internal_1 (b);
}
- value = window_text_pixel_size (window, from, to, x_limit, y_limit, mode_lines);
+ value = window_text_pixel_size (window, from, to, x_limit, y_limit, mode_lines,
+ no_ascents_descents);
if (old_b)
set_buffer_internal_1 (old_b);
@@ -11151,7 +11172,8 @@ DEFUN ("buffer-text-pixel-size", Fbuffer_text_pixel_size, Sbuffer_text_pixel_siz
set_marker_both (w->old_pointm, buffer, BEG, BEG_BYTE);
}
- value = window_text_pixel_size (window, Qnil, Qnil, x_limit, y_limit, Qnil);
+ value = window_text_pixel_size (window, Qnil, Qnil, x_limit, y_limit, Qnil,
+ Qnil);
unbind_to (count, Qnil);
diff --git a/src/xfns.c b/src/xfns.c
index dc25d7bfca..30ed358fb2 100644
--- a/src/xfns.c
+++ b/src/xfns.c
@@ -7169,7 +7169,8 @@ DEFUN ("x-show-tip", Fx_show_tip, Sx_show_tip, 1, 6, 0,
try_window (window, pos, TRY_WINDOW_IGNORE_FONTS_CHANGE);
/* Calculate size of tooltip window. */
size = Fwindow_text_pixel_size (window, Qnil, Qnil, Qnil,
- make_fixnum (w->pixel_height), Qnil);
+ make_fixnum (w->pixel_height), Qnil,
+ Qnil);
/* Add the frame's internal border to calculated size. */
width = XFIXNUM (Fcar (size)) + 2 * FRAME_INTERNAL_BORDER_WIDTH (tip_f);
height = XFIXNUM (Fcdr (size)) + 2 * FRAME_INTERNAL_BORDER_WIDTH (tip_f);
--
2.33.1
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Allow-window-text-pixel-size-to-measure-from-pixels-.patch --]
[-- Type: text/x-patch, Size: 9888 bytes --]
From 0910fb8e2dfb71c2dff44c6b99633ab61baa7f0e Mon Sep 17 00:00:00 2001
From: Po Lu <luangruo@yahoo.com>
Date: Fri, 17 Dec 2021 09:40:41 +0800
Subject: [PATCH 2/2] Allow window-text-pixel-size to measure from pixels above
a position
* doc/lispref/display.texi (Size of Displayed Text): Announce
new meaning of `from'.
* etc/NEWS: Announce changes.
* lisp/pixel-scroll.el (pixel-scroll-precision-scroll-up-page):
Use new feature.
* src/xdisp.c (window_text_pixel_size): Understand a special
format of `from' that specifies the amount of pixels above a
position.
(Fwindow_text_pixel_size): Update doc string.
---
doc/lispref/display.texi | 20 ++++++++-----
etc/NEWS | 3 ++
lisp/pixel-scroll.el | 34 +++++++++++----------
src/xdisp.c | 64 ++++++++++++++++++++++++++++++++--------
4 files changed, 84 insertions(+), 37 deletions(-)
diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
index bf0d1c05e1..766b2c5d64 100644
--- a/doc/lispref/display.texi
+++ b/doc/lispref/display.texi
@@ -2090,19 +2090,23 @@ Size of Displayed Text
This function returns the size of the text of @var{window}'s buffer in
pixels. @var{window} must be a live window and defaults to the
selected one. The return value is a cons of the maximum pixel-width
-of any text line and the maximum pixel-height of all text lines. This
-function exists to allow Lisp programs to adjust the dimensions of
-@var{window} to the buffer text it needs to display.
+of any text line and the maximum pixel-height of all text lines, or if
+@var{from} is a cons, a list of the pixel-width, pixel-height, and the
+buffer position of the first line that was measured. This function
+exists to allow Lisp programs to adjust the dimensions of @var{window}
+to the buffer text it needs to display.
The optional argument @var{from}, if non-@code{nil}, specifies the
first text position to consider, and defaults to the minimum
accessible position of the buffer. If @var{from} is @code{t}, it
stands for the minimum accessible position that is not a newline
-character. The optional argument @var{to}, if non-@code{nil},
-specifies the last text position to consider, and defaults to the
-maximum accessible position of the buffer. If @var{to} is @code{t},
-it stands for the maximum accessible position that is not a newline
-character.
+character. If @var{from} is a cons, the cdr specifies a position, and
+the car specifies the minimum amount of pixels above that position to
+start measuring from. The optional argument @var{to}, if
+non-@code{nil}, specifies the last text position to consider, and
+defaults to the maximum accessible position of the buffer. If
+@var{to} is @code{t}, it stands for the maximum accessible position
+that is not a newline character.
The optional argument @var{x-limit}, if non-@code{nil}, specifies the
maximum X coordinate beyond which text should be ignored; it is
diff --git a/etc/NEWS b/etc/NEWS
index b3e335f00a..47145adbc0 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -989,6 +989,9 @@ character.
This controls whether or not the screen line at the end of the
measured area will be counted during the height calculation.
++++
+** 'window-text-pixel-size' can now measure from a set amount of pixels above a position.
+
** XDG support
*** New function 'xdg-state-home' returns 'XDG_STATE_HOME' environment variable.
diff --git a/lisp/pixel-scroll.el b/lisp/pixel-scroll.el
index fa0185b16e..efcabae108 100644
--- a/lisp/pixel-scroll.el
+++ b/lisp/pixel-scroll.el
@@ -516,22 +516,24 @@ pixel-scroll-precision-scroll-up-page
usable-height))))
(goto-char up-point)))
(let ((current-vscroll (window-vscroll nil t)))
- (if (<= delta current-vscroll)
- (set-window-vscroll nil (- current-vscroll delta) t)
- (setq delta (- delta current-vscroll))
- (set-window-vscroll nil 0 t)
- (while (> delta 0)
- (let ((position (pixel-point-and-height-at-unseen-line)))
- (unless (cdr position)
- (signal 'beginning-of-buffer nil))
- (set-window-start nil (car position) t)
- ;; If the line above is taller than the window height (i.e. there's
- ;; a very tall image), keep point on it.
- (when (> (cdr position) usable-height)
- (goto-char (car position)))
- (setq delta (- delta (cdr position)))))
- (when (< delta 0)
- (set-window-vscroll nil (- delta) t))))))
+ (setq delta (- delta current-vscroll))
+ (set-window-vscroll nil 0 t)
+ (when (> delta 0)
+ (let* ((start (window-start))
+ (dims (window-text-pixel-size nil (cons start delta)
+ start nil nil nil t))
+ (height (nth 1 dims))
+ (position (nth 2 dims)))
+ (set-window-start nil position t)
+ ;; If the line above is taller than the window height (i.e. there's
+ ;; a very tall image), keep point on it.
+ (when (> height usable-height)
+ (goto-char position))
+ (when (or (not position) (eq position start))
+ (signal 'beginning-of-buffer nil))
+ (setq delta (- delta height))))
+ (when (< delta 0)
+ (set-window-vscroll nil (- delta) t)))))
(defun pixel-scroll-precision-interpolate (delta)
"Interpolate a scroll of DELTA pixels.
diff --git a/src/xdisp.c b/src/xdisp.c
index 0772238f2d..e84f99499b 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -10840,7 +10840,8 @@ window_text_pixel_size (Lisp_Object window, Lisp_Object from, Lisp_Object to, Li
ptrdiff_t start, end, bpos;
struct text_pos startp;
void *itdata = NULL;
- int c, max_x = 0, max_y = 0, x = 0, y = 0;
+ int c, max_x = 0, max_y = 0, x = 0, y = 0, movement = 0, doff;
+ bool saw_display_prop_at_end_p = false;
if (NILP (from))
{
@@ -10866,6 +10867,13 @@ window_text_pixel_size (Lisp_Object window, Lisp_Object from, Lisp_Object to, Li
break;
}
}
+ else if (CONSP (from))
+ {
+ start = clip_to_bounds (BEGV, fix_position (XCAR (from)), ZV);
+ bpos = CHAR_TO_BYTE (start);
+ CHECK_FIXNAT (XCDR (from));
+ movement = XFIXNAT (XCDR (from));
+ }
else
{
start = clip_to_bounds (BEGV, fix_position (from), ZV);
@@ -10912,6 +10920,27 @@ window_text_pixel_size (Lisp_Object window, Lisp_Object from, Lisp_Object to, Li
itdata = bidi_shelve_cache ();
start_display (&it, w, startp);
+
+ if (movement > 0)
+ {
+ int last_y;
+ it.current_y = 0;
+
+ move_it_by_lines (&it, 0);
+
+ while (-it.current_y < movement)
+ {
+ last_y = it.current_y;
+ move_it_vertically_backward (&it, movement + it.current_y);
+
+ if (it.current_y == last_y)
+ break;
+ }
+
+ it.current_y = 0;
+ start = clip_to_bounds (BEGV, IT_CHARPOS (it), ZV);
+ }
+
int start_y = it.current_y;
/* It makes no sense to measure dimensions of region of text that
crosses the point where bidi reordering changes scan direction.
@@ -11005,7 +11034,10 @@ window_text_pixel_size (Lisp_Object window, Lisp_Object from, Lisp_Object to, Li
y = (it.current_y + (NILP (no_ascents_descents)
? it.max_ascent + it.max_descent
: 0)
- - WINDOW_TAB_LINE_HEIGHT (w) - WINDOW_HEADER_LINE_HEIGHT (w));
+ - (movement < 0
+ ? (WINDOW_TAB_LINE_HEIGHT (w)
+ - WINDOW_HEADER_LINE_HEIGHT (w))
+ : 0));
if (saw_display_prop_at_end_p)
y += doff;
@@ -11053,26 +11085,32 @@ window_text_pixel_size (Lisp_Object window, Lisp_Object from, Lisp_Object to, Li
bidi_unshelve_cache (itdata, false);
- return Fcons (make_fixnum (x - start_x), make_fixnum (y));
+ return (!movement
+ ? Fcons (make_fixnum (x - start_x), make_fixnum (y))
+ : list3i (x - start_x, y, start));
}
DEFUN ("window-text-pixel-size", Fwindow_text_pixel_size, Swindow_text_pixel_size, 0, 7, 0,
- doc: /* Return the size of the text of WINDOW's buffer in pixels.
-WINDOW must be a live window and defaults to the selected one. The
-return value is a cons of the maximum pixel-width of any text line
-and the pixel-height of all the text lines in the accessible portion
-of buffer text.
+ doc: /* Return the size of the text of WINDOW's buffer in
+pixels. WINDOW must be a live window and defaults to the selected
+one. The return value is a cons of the maximum pixel-width of any
+text line and the pixel-height of all the text lines in the accessible
+portion of buffer text or a list of the maximum pixel-width,
+pixel-height, and the buffer position of the line at FROM.
This function exists to allow Lisp programs to adjust the dimensions
of WINDOW to the buffer text it needs to display.
The optional argument FROM, if non-nil, specifies the first text
position to consider, and defaults to the minimum accessible position
-of the buffer. If FROM is t, it stands for the minimum accessible
-position that starts a non-empty line. TO, if non-nil, specifies the
-last text position and defaults to the maximum accessible position of
-the buffer. If TO is t, it stands for the maximum accessible position
-that ends a non-empty line.
+of the buffer. If FROM is a cons, the cdr specifies the amount of
+pixels above the buffer position to begin measuring text, and the car
+specifies the buffer position. In that case, a list is returned. If
+FROM is t, it stands for the minimum accessible position that starts a
+non-empty line. TO, if non-nil, specifies the last text position and
+defaults to the maximum accessible position of the buffer. If TO is
+t, it stands for the maximum accessible position that ends a non-empty
+line.
The optional argument X-LIMIT, if non-nil, specifies the maximum X
coordinate beyond which the text should be ignored. It is therefore
--
2.33.1
^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-17 1:45 ` Po Lu
@ 2021-12-18 10:28 ` Eli Zaretskii
2021-12-18 10:49 ` Po Lu
2021-12-19 0:54 ` Po Lu
0 siblings, 2 replies; 53+ messages in thread
From: Eli Zaretskii @ 2021-12-18 10:28 UTC (permalink / raw)
To: Po Lu; +Cc: emacs-devel
> From: Po Lu <luangruo@yahoo.com>
> Cc: emacs-devel@gnu.org
> Date: Fri, 17 Dec 2021 09:45:53 +0800
>
> > The difference is that window-text-pixel-size already exists, we just
> > extend it for another use case.
>
> > Well, then this is equivalent to telling window-text-pixel-size to
> > stop at the visual left edge of a screen line, right? That is,
> > interpret TO not as a literal buffer position, but as the visual
> > beginning of the line which shows position TO.
>
> Yes, I think the two meanings are equivalent. I documented it as the
> former, but it wouldn't hurt to document it like this instead.
I think it would be a lot easier to understand if we use the
description I proposed. Lisp programmers do not necessarily know what
are ascent and descent, and what they have to do with this function's
operation.
However, I don't think I see where that additional argument causes the
function to stop at the visual left edge of a screen line containing
TO. What did I miss?
> Anyway, here are two patches that implement the extensions to
> `window-text-pixel-size', along with the code in
> pixel-scroll-precision-mode that makes use of them.
Please for the next iteration show them as a single patch. One of
them seems to depend on the other, and it's hard to review the changes
piecemeal.
> I tried to update the documentation, but to be frank I'm not good at
> writing documentation and couldn't word it very well
Don't give up trying! No one is born with these abilities, they are
acquired by writing documentation and learning from that.
For now, in my comments below I focused on the code, and left the
documentation for later, when we arrive at the final version of the
code.
> @@ -10969,8 +10969,19 @@ window_text_pixel_size (Lisp_Object window, Lisp_Object from, Lisp_Object to, Li
> if (IT_CHARPOS (it) == end)
> {
> x += it.pixel_width;
> - it.max_ascent = max (it.max_ascent, it.ascent);
> - it.max_descent = max (it.max_descent, it.descent);
> +
> + /* DTRT if no_ascents_descents is t. */
> + if (no_ascents_descents)
> + {
> + saw_display_prop_at_end_p = true;
This bool value seems to be a misnomer: we don't really have evidence
that we found a display property at end of text.
Also, you should use NILP in the if-clause, as no_ascents_descents is
a Lisp object.
> @@ -10991,8 +11002,14 @@ window_text_pixel_size (Lisp_Object window, Lisp_Object from, Lisp_Object to, Li
>
> /* Subtract height of header-line and tab-line which was counted
> automatically by start_display. */
> - y = it.current_y + it.max_ascent + it.max_descent
> - - WINDOW_TAB_LINE_HEIGHT (w) - WINDOW_HEADER_LINE_HEIGHT (w);
> + y = (it.current_y + (NILP (no_ascents_descents)
> + ? it.max_ascent + it.max_descent
> + : 0)
> + - WINDOW_TAB_LINE_HEIGHT (w) - WINDOW_HEADER_LINE_HEIGHT (w));
> +
> + if (saw_display_prop_at_end_p)
> + y += doff;
This is quite confusing: you actually add the same value, but in two
different ways and with 2 different (but subtly equivalent)
conditions. It would be good to make this less confusing.
> @@ -10866,6 +10867,13 @@ window_text_pixel_size (Lisp_Object window, Lisp_Object from, Lisp_Object to, Li
> break;
> }
> }
> + else if (CONSP (from))
> + {
> + start = clip_to_bounds (BEGV, fix_position (XCAR (from)), ZV);
> + bpos = CHAR_TO_BYTE (start);
> + CHECK_FIXNAT (XCDR (from));
> + movement = XFIXNAT (XCDR (from));
> + }
> else
> {
> start = clip_to_bounds (BEGV, fix_position (from), ZV);
There's code duplication here, which it would be good to avoid: the
calculation of bpos and the call to clip_to_bounds in calculation of
start.
The name 'movement' is not the best name. I would suggest 'offset' or
maybe even 'y_offset' instead.
Also, the assumption that this offset is always positive, and that it
means the position _before_ FROM, is also something that makes the
function less general than it could be. It would be better to have
positive values mean _after_ FROM and negative values to mean before
it. Then you could use move_it_vertically instead of
move_it_vertically_backward.
> + if (movement > 0)
> + {
> + int last_y;
> + it.current_y = 0;
> +
> + move_it_by_lines (&it, 0);
^^^^^^^^^^^^^^^^
Why is this needed?
> + while (-it.current_y < movement)
> + {
> + last_y = it.current_y;
> + move_it_vertically_backward (&it, movement + it.current_y);
> +
> + if (it.current_y == last_y)
> + break;
> + }
> +
> + it.current_y = 0;
> + start = clip_to_bounds (BEGV, IT_CHARPOS (it), ZV);
> + }
> +
> int start_y = it.current_y;
> /* It makes no sense to measure dimensions of region of text that
> crosses the point where bidi reordering changes scan direction.
The code that follows this, which you leave intact, will reseat the
iterator to the beginning of the visual line where you ended up after
the loop that goes backward. Is that really TRT? what if there's a
multi-line display or overlay string at that place? Did you test the
code in that case? AFAIU, you have already assured that you are at
the beginning of a visual line, so the reseat, and the following
return to the START position, might not be needed. When you return to
START, you may end up very far from where the loop going backward
ended, if START is "covered" by a display property or overlay string.
> @@ -11053,26 +11085,32 @@ window_text_pixel_size (Lisp_Object window, Lisp_Object from, Lisp_Object to, Li
>
> bidi_unshelve_cache (itdata, false);
>
> - return Fcons (make_fixnum (x - start_x), make_fixnum (y));
> + return (!movement
> + ? Fcons (make_fixnum (x - start_x), make_fixnum (y))
> + : list3i (x - start_x, y, start));
Why not use list2i instead of Fcons there?
Do these changes solve the performance problem in
pixel-scroll-precision-mode?
Thanks for working.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-18 10:28 ` Eli Zaretskii
@ 2021-12-18 10:49 ` Po Lu
2021-12-18 11:03 ` Eli Zaretskii
2021-12-19 0:54 ` Po Lu
1 sibling, 1 reply; 53+ messages in thread
From: Po Lu @ 2021-12-18 10:49 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: emacs-devel
[-- Attachment #1: Type: text/plain, Size: 2597 bytes --]
Eli Zaretskii <eliz@gnu.org> writes:
> I think it would be a lot easier to understand if we use the
> description I proposed. Lisp programmers do not necessarily know what
> are ascent and descent, and what they have to do with this function's
> operation.
Thanks, I renamed it to `ignore-line-at-end'.
> Please for the next iteration show them as a single patch. One of
> them seems to depend on the other, and it's hard to review the changes
> piecemeal.
Agreed. To make things easier, let's work on the `no-ascents-descents'
argument first, and only afterwards on the extension to TO.
> However, I don't think I see where that additional argument causes the
> function to stop at the visual left edge of a screen line containing
> TO. What did I miss?
My understanding is that if we don't add the current ascent and descent
of the iterator to the returned height, the effect is the same.
> Don't give up trying! No one is born with these abilities, they are
> acquired by writing documentation and learning from that.
Thanks.
>> @@ -10969,8 +10969,19 @@ window_text_pixel_size (Lisp_Object window, Lisp_Object from, Lisp_Object to, Li
>> if (IT_CHARPOS (it) == end)
>> {
>> x += it.pixel_width;
>> - it.max_ascent = max (it.max_ascent, it.ascent);
>> - it.max_descent = max (it.max_descent, it.descent);
>> +
>> + /* DTRT if no_ascents_descents is t. */
>> + if (no_ascents_descents)
>> + {
>> + saw_display_prop_at_end_p = true;
> This bool value seems to be a misnomer: we don't really have evidence
> that we found a display property at end of text.
How about `calculating_overshoot'?
> Also, you should use NILP in the if-clause, as no_ascents_descents is
> a Lisp object.
Thanks, good catch.
> This is quite confusing: you actually add the same value, but in two
> different ways and with 2 different (but subtly equivalent)
> conditions. It would be good to make this less confusing.
Thanks, I tried to do that. How's the attached patch?
>> @@ -11053,26 +11085,32 @@ window_text_pixel_size (Lisp_Object window, Lisp_Object from, Lisp_Object to, Li
>>
>> bidi_unshelve_cache (itdata, false);
>>
>> - return Fcons (make_fixnum (x - start_x), make_fixnum (y));
>> + return (!movement
>> + ? Fcons (make_fixnum (x - start_x), make_fixnum (y))
>> + : list3i (x - start_x, y, start));
>
> Why not use list2i instead of Fcons there?
`window-text-pixel-size' returns by default the measurements as a pair,
not a list.
> Do these changes solve the performance problem in
> pixel-scroll-precision-mode?
Yes, they do.
Thanks.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-new-argument-ignore-line-at-end-to-window-text-p.patch --]
[-- Type: text/x-patch, Size: 9027 bytes --]
From aee17a8fa18ad098962aab72ecdb7d8abde4ddb3 Mon Sep 17 00:00:00 2001
From: Po Lu <luangruo@yahoo.com>
Date: Sat, 18 Dec 2021 18:48:11 +0800
Subject: [PATCH] Add new argument `ignore-line-at-end' to
`window-text-pixel-size'
* doc/lispref/display.texi (Size of Displayed Text): Update
documentation.
* etc/NEWS: Announce new argument.
* src/xdisp.c (window_text_pixel_size): Allow controlling if
the iterator's ascent and descent will be appended to the
pixel height returned. All callers changed.
(Fwindow_text_pixel_size): New argument `ignore-line-at-end'.
All callers changed.
---
doc/lispref/display.texi | 6 +++++-
etc/NEWS | 5 +++++
src/haikufns.c | 3 ++-
src/w32fns.c | 3 ++-
src/xdisp.c | 46 +++++++++++++++++++++++++++++++---------
src/xfns.c | 3 ++-
6 files changed, 52 insertions(+), 14 deletions(-)
diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
index b82473f9c2..92bfd2fea1 100644
--- a/doc/lispref/display.texi
+++ b/doc/lispref/display.texi
@@ -2086,7 +2086,7 @@ Size of Displayed Text
(@pxref{Resizing Windows}) to make a window exactly as large as the text
it contains.
-@defun window-text-pixel-size &optional window from to x-limit y-limit mode-lines
+@defun window-text-pixel-size &optional window from to x-limit y-limit mode-lines ignore-line-at-end
This function returns the size of the text of @var{window}'s buffer in
pixels. @var{window} must be a live window and defaults to the
selected one. The return value is a cons of the maximum pixel-width
@@ -2136,6 +2136,10 @@ Size of Displayed Text
height of all of these lines, if present, in the return value.
@end defun
+The optional argument @var{ignore-line-at-end} controls whether or
+not to count the height of text in @var{to}'s screen line as part of
+the returned pixel-height.
+
@code{window-text-pixel-size} treats the text displayed in a window as a
whole and does not care about the size of individual lines. The
following function does.
diff --git a/etc/NEWS b/etc/NEWS
index bd1ed4da00..20056b30c5 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -984,6 +984,11 @@ when they have changed.
This can be used to check whether a specific font has a glyph for a
character.
++++
+** 'window-text-pixel-size' now accepts a new argument `ignore-line-at-end'.
+This controls whether or not the screen line at the end of the
+measured area will be counted during the height calculation.
+
** XDG support
*** New function 'xdg-state-home' returns 'XDG_STATE_HOME' environment variable.
diff --git a/src/haikufns.c b/src/haikufns.c
index 868fc71f97..737b033899 100644
--- a/src/haikufns.c
+++ b/src/haikufns.c
@@ -1970,7 +1970,8 @@ DEFUN ("x-show-tip", Fx_show_tip, Sx_show_tip, 1, 6, 0,
try_window (window, pos, TRY_WINDOW_IGNORE_FONTS_CHANGE);
/* Calculate size of tooltip window. */
size = Fwindow_text_pixel_size (window, Qnil, Qnil, Qnil,
- make_fixnum (w->pixel_height), Qnil);
+ make_fixnum (w->pixel_height), Qnil,
+ Qnil);
/* Add the frame's internal border to calculated size. */
width = XFIXNUM (Fcar (size)) + 2 * FRAME_INTERNAL_BORDER_WIDTH (tip_f);
height = XFIXNUM (Fcdr (size)) + 2 * FRAME_INTERNAL_BORDER_WIDTH (tip_f);
diff --git a/src/w32fns.c b/src/w32fns.c
index 65463b5261..02a6d78b51 100644
--- a/src/w32fns.c
+++ b/src/w32fns.c
@@ -7525,7 +7525,8 @@ DEFUN ("x-show-tip", Fx_show_tip, Sx_show_tip, 1, 6, 0,
try_window (window, pos, TRY_WINDOW_IGNORE_FONTS_CHANGE);
/* Calculate size of tooltip window. */
size = Fwindow_text_pixel_size (window, Qnil, Qnil, Qnil,
- make_fixnum (w->pixel_height), Qnil);
+ make_fixnum (w->pixel_height), Qnil,
+ Qnil);
/* Add the frame's internal border to calculated size. */
width = XFIXNUM (Fcar (size)) + 2 * FRAME_INTERNAL_BORDER_WIDTH (tip_f);
height = XFIXNUM (Fcdr (size)) + 2 * FRAME_INTERNAL_BORDER_WIDTH (tip_f);
diff --git a/src/xdisp.c b/src/xdisp.c
index e74411c817..943c0a366f 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -10833,7 +10833,7 @@ in_display_vector_p (struct it *it)
argument. */
static Lisp_Object
window_text_pixel_size (Lisp_Object window, Lisp_Object from, Lisp_Object to, Lisp_Object x_limit,
- Lisp_Object y_limit, Lisp_Object mode_lines)
+ Lisp_Object y_limit, Lisp_Object mode_lines, Lisp_Object ignore_line_at_end)
{
struct window *w = decode_live_window (window);
struct it it;
@@ -10841,6 +10841,8 @@ window_text_pixel_size (Lisp_Object window, Lisp_Object from, Lisp_Object to, Li
struct text_pos startp;
void *itdata = NULL;
int c, max_x = 0, max_y = 0, x = 0, y = 0;
+ bool calculating_overshoot = false;
+ int doff = 0;
if (NILP (from))
{
@@ -10969,8 +10971,19 @@ window_text_pixel_size (Lisp_Object window, Lisp_Object from, Lisp_Object to, Li
if (IT_CHARPOS (it) == end)
{
x += it.pixel_width;
- it.max_ascent = max (it.max_ascent, it.ascent);
- it.max_descent = max (it.max_descent, it.descent);
+
+ /* DTRT if ignore_line_at_end is t. */
+ if (!NILP (ignore_line_at_end))
+ {
+ calculating_overshoot = true;
+ doff = (max (it.max_ascent, it.ascent)
+ + max (it.max_descent, it.descent));
+ }
+ else
+ {
+ it.max_ascent = max (it.max_ascent, it.ascent);
+ it.max_descent = max (it.max_descent, it.descent);
+ }
}
}
else
@@ -10991,8 +11004,16 @@ window_text_pixel_size (Lisp_Object window, Lisp_Object from, Lisp_Object to, Li
/* Subtract height of header-line and tab-line which was counted
automatically by start_display. */
- y = it.current_y + it.max_ascent + it.max_descent
- - WINDOW_TAB_LINE_HEIGHT (w) - WINDOW_HEADER_LINE_HEIGHT (w);
+ if (!NILP (ignore_line_at_end))
+ y = (it.current_y - WINDOW_TAB_LINE_HEIGHT (w)
+ - WINDOW_HEADER_LINE_HEIGHT (w));
+ else
+ y = (it.current_y + it.max_ascent + it.max_descent
+ - WINDOW_TAB_LINE_HEIGHT (w) - WINDOW_HEADER_LINE_HEIGHT (w));
+
+ if (calculating_overshoot)
+ y += doff;
+
/* Don't return more than Y-LIMIT. */
if (y > max_y)
y = max_y;
@@ -11039,7 +11060,7 @@ window_text_pixel_size (Lisp_Object window, Lisp_Object from, Lisp_Object to, Li
return Fcons (make_fixnum (x - start_x), make_fixnum (y));
}
-DEFUN ("window-text-pixel-size", Fwindow_text_pixel_size, Swindow_text_pixel_size, 0, 6, 0,
+DEFUN ("window-text-pixel-size", Fwindow_text_pixel_size, Swindow_text_pixel_size, 0, 7, 0,
doc: /* Return the size of the text of WINDOW's buffer in pixels.
WINDOW must be a live window and defaults to the selected one. The
return value is a cons of the maximum pixel-width of any text line
@@ -11086,9 +11107,12 @@ DEFUN ("window-text-pixel-size", Fwindow_text_pixel_size, Swindow_text_pixel_siz
height of the mode-, tab- or header-line of WINDOW in the return value.
If it is the symbol `mode-line', 'tab-line' or `header-line', include
only the height of that line, if present, in the return value. If t,
-include the height of any of these, if present, in the return value. */)
+include the height of any of these, if present, in the return value.
+
+IGNORE-LINE-AT-END means to not add the height of the screen line at
+TO to the returned height. */)
(Lisp_Object window, Lisp_Object from, Lisp_Object to, Lisp_Object x_limit,
- Lisp_Object y_limit, Lisp_Object mode_lines)
+ Lisp_Object y_limit, Lisp_Object mode_lines, Lisp_Object ignore_line_at_end)
{
struct window *w = decode_live_window (window);
struct buffer *b = XBUFFER (w->contents);
@@ -11101,7 +11125,8 @@ DEFUN ("window-text-pixel-size", Fwindow_text_pixel_size, Swindow_text_pixel_siz
set_buffer_internal_1 (b);
}
- value = window_text_pixel_size (window, from, to, x_limit, y_limit, mode_lines);
+ value = window_text_pixel_size (window, from, to, x_limit, y_limit, mode_lines,
+ ignore_line_at_end);
if (old_b)
set_buffer_internal_1 (old_b);
@@ -11151,7 +11176,8 @@ DEFUN ("buffer-text-pixel-size", Fbuffer_text_pixel_size, Sbuffer_text_pixel_siz
set_marker_both (w->old_pointm, buffer, BEG, BEG_BYTE);
}
- value = window_text_pixel_size (window, Qnil, Qnil, x_limit, y_limit, Qnil);
+ value = window_text_pixel_size (window, Qnil, Qnil, x_limit, y_limit, Qnil,
+ Qnil);
unbind_to (count, Qnil);
diff --git a/src/xfns.c b/src/xfns.c
index dc25d7bfca..30ed358fb2 100644
--- a/src/xfns.c
+++ b/src/xfns.c
@@ -7169,7 +7169,8 @@ DEFUN ("x-show-tip", Fx_show_tip, Sx_show_tip, 1, 6, 0,
try_window (window, pos, TRY_WINDOW_IGNORE_FONTS_CHANGE);
/* Calculate size of tooltip window. */
size = Fwindow_text_pixel_size (window, Qnil, Qnil, Qnil,
- make_fixnum (w->pixel_height), Qnil);
+ make_fixnum (w->pixel_height), Qnil,
+ Qnil);
/* Add the frame's internal border to calculated size. */
width = XFIXNUM (Fcar (size)) + 2 * FRAME_INTERNAL_BORDER_WIDTH (tip_f);
height = XFIXNUM (Fcdr (size)) + 2 * FRAME_INTERNAL_BORDER_WIDTH (tip_f);
--
2.33.1
^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-18 10:49 ` Po Lu
@ 2021-12-18 11:03 ` Eli Zaretskii
2021-12-18 11:18 ` Po Lu
0 siblings, 1 reply; 53+ messages in thread
From: Eli Zaretskii @ 2021-12-18 11:03 UTC (permalink / raw)
To: Po Lu; +Cc: emacs-devel
> From: Po Lu <luangruo@yahoo.com>
> Cc: emacs-devel@gnu.org
> Date: Sat, 18 Dec 2021 18:49:43 +0800
>
> >> + /* DTRT if no_ascents_descents is t. */
> >> + if (no_ascents_descents)
> >> + {
> >> + saw_display_prop_at_end_p = true;
>
> > This bool value seems to be a misnomer: we don't really have evidence
> > that we found a display property at end of text.
>
> How about `calculating_overshoot'?
Why do you need a flag variable at all? Cannot 'doff' serve as that
flag? Or, if not, why not test the argument -- it's just one NILP
test, after all?
> /* Subtract height of header-line and tab-line which was counted
> automatically by start_display. */
> - y = it.current_y + it.max_ascent + it.max_descent
> - - WINDOW_TAB_LINE_HEIGHT (w) - WINDOW_HEADER_LINE_HEIGHT (w);
> + if (!NILP (ignore_line_at_end))
> + y = (it.current_y - WINDOW_TAB_LINE_HEIGHT (w)
> + - WINDOW_HEADER_LINE_HEIGHT (w));
> + else
> + y = (it.current_y + it.max_ascent + it.max_descent
> + - WINDOW_TAB_LINE_HEIGHT (w) - WINDOW_HEADER_LINE_HEIGHT (w));
> +
> + if (calculating_overshoot)
> + y += doff;
This is almost the same as the original code.
The problem that I wanted to point out is that ignore_line_at_end and
calculating_overshoot are related: one determines the value of the
other. And the value of 'doff' is very similar to it.max_ascent +
it.max_descent. So this code left me thinking for too long what
exactly is going on here, and why? At first glance it even looked
like you are adding 'doff' in both cases, just indirectly.
It is this confusing aspect that I wanted you to fix.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-18 11:03 ` Eli Zaretskii
@ 2021-12-18 11:18 ` Po Lu
2021-12-18 11:29 ` Eli Zaretskii
0 siblings, 1 reply; 53+ messages in thread
From: Po Lu @ 2021-12-18 11:18 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: emacs-devel
Eli Zaretskii <eliz@gnu.org> writes:
> Why do you need a flag variable at all? Cannot 'doff' serve as that
> flag? Or, if not, why not test the argument -- it's just one NILP
> test, after all?
I get your point now. Is there any other problem with this patch, or
can I install it now?
Thanks.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-18 11:18 ` Po Lu
@ 2021-12-18 11:29 ` Eli Zaretskii
2021-12-18 11:31 ` Po Lu
0 siblings, 1 reply; 53+ messages in thread
From: Eli Zaretskii @ 2021-12-18 11:29 UTC (permalink / raw)
To: Po Lu; +Cc: emacs-devel
> From: Po Lu <luangruo@yahoo.com>
> Cc: emacs-devel@gnu.org
> Date: Sat, 18 Dec 2021 19:18:00 +0800
>
> Eli Zaretskii <eliz@gnu.org> writes:
>
> > Why do you need a flag variable at all? Cannot 'doff' serve as that
> > flag? Or, if not, why not test the argument -- it's just one NILP
> > test, after all?
>
> I get your point now. Is there any other problem with this patch, or
> can I install it now?
??? I thought we still need to get the move_it_vertically part right,
and then review the documentation?
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-18 11:29 ` Eli Zaretskii
@ 2021-12-18 11:31 ` Po Lu
2021-12-18 11:35 ` Eli Zaretskii
0 siblings, 1 reply; 53+ messages in thread
From: Po Lu @ 2021-12-18 11:31 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: emacs-devel
Eli Zaretskii <eliz@gnu.org> writes:
>> From: Po Lu <luangruo@yahoo.com>
>> Cc: emacs-devel@gnu.org
>> Date: Sat, 18 Dec 2021 19:18:00 +0800
>>
>> Eli Zaretskii <eliz@gnu.org> writes:
>>
>> > Why do you need a flag variable at all? Cannot 'doff' serve as that
>> > flag? Or, if not, why not test the argument -- it's just one NILP
>> > test, after all?
>>
>> I get your point now. Is there any other problem with this patch, or
>> can I install it now?
> ??? I thought we still need to get the move_it_vertically part right,
> and then review the documentation?
Yes, but they're separate patches that add different (and independent)
features. I'm talking about the patch that adds `ignore-line-at-end',
whose documentation I don't really see a problem with.
Thanks.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-18 11:31 ` Po Lu
@ 2021-12-18 11:35 ` Eli Zaretskii
2021-12-18 11:39 ` Po Lu
0 siblings, 1 reply; 53+ messages in thread
From: Eli Zaretskii @ 2021-12-18 11:35 UTC (permalink / raw)
To: Po Lu; +Cc: emacs-devel
> From: Po Lu <luangruo@yahoo.com>
> Cc: emacs-devel@gnu.org
> Date: Sat, 18 Dec 2021 19:31:40 +0800
>
> Eli Zaretskii <eliz@gnu.org> writes:
>
> >> From: Po Lu <luangruo@yahoo.com>
> >> Cc: emacs-devel@gnu.org
> >> Date: Sat, 18 Dec 2021 19:18:00 +0800
> >>
> >> Eli Zaretskii <eliz@gnu.org> writes:
> >>
> >> > Why do you need a flag variable at all? Cannot 'doff' serve as that
> >> > flag? Or, if not, why not test the argument -- it's just one NILP
> >> > test, after all?
> >>
> >> I get your point now. Is there any other problem with this patch, or
> >> can I install it now?
>
> > ??? I thought we still need to get the move_it_vertically part right,
> > and then review the documentation?
>
> Yes, but they're separate patches that add different (and independent)
> features. I'm talking about the patch that adds `ignore-line-at-end',
> whose documentation I don't really see a problem with.
OK.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-18 11:35 ` Eli Zaretskii
@ 2021-12-18 11:39 ` Po Lu
0 siblings, 0 replies; 53+ messages in thread
From: Po Lu @ 2021-12-18 11:39 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: emacs-devel
Eli Zaretskii <eliz@gnu.org> writes:
>> From: Po Lu <luangruo@yahoo.com>
>> Cc: emacs-devel@gnu.org
>> Date: Sat, 18 Dec 2021 19:31:40 +0800
>>
>> Eli Zaretskii <eliz@gnu.org> writes:
>>
>> >> From: Po Lu <luangruo@yahoo.com>
>> >> Cc: emacs-devel@gnu.org
>> >> Date: Sat, 18 Dec 2021 19:18:00 +0800
>> >>
>> >> Eli Zaretskii <eliz@gnu.org> writes:
>> >>
>> >> > Why do you need a flag variable at all? Cannot 'doff' serve as that
>> >> > flag? Or, if not, why not test the argument -- it's just one NILP
>> >> > test, after all?
>> >>
>> >> I get your point now. Is there any other problem with this patch, or
>> >> can I install it now?
>>
>> > ??? I thought we still need to get the move_it_vertically part right,
>> > and then review the documentation?
>>
>> Yes, but they're separate patches that add different (and independent)
>> features. I'm talking about the patch that adds `ignore-line-at-end',
>> whose documentation I don't really see a problem with.
>
> OK.
Thanks, I installed it now. (And I also found a separate use for it in
another package I am developing.)
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-18 10:28 ` Eli Zaretskii
2021-12-18 10:49 ` Po Lu
@ 2021-12-19 0:54 ` Po Lu
2021-12-19 8:29 ` Eli Zaretskii
1 sibling, 1 reply; 53+ messages in thread
From: Po Lu @ 2021-12-19 0:54 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: emacs-devel
Eli Zaretskii <eliz@gnu.org> writes:
>> @@ -10866,6 +10867,13 @@ window_text_pixel_size (Lisp_Object window, Lisp_Object from, Lisp_Object to, Li
>> break;
>> }
>> }
>> + else if (CONSP (from))
>> + {
>> + start = clip_to_bounds (BEGV, fix_position (XCAR (from)), ZV);
>> + bpos = CHAR_TO_BYTE (start);
>> + CHECK_FIXNAT (XCDR (from));
>> + movement = XFIXNAT (XCDR (from));
>> + }
>> else
>> {
>> start = clip_to_bounds (BEGV, fix_position (from), ZV);
> There's code duplication here, which it would be good to avoid: the
> calculation of bpos and the call to clip_to_bounds in calculation of
> start.
I don't think the duplication here is a particularly big problem. The
code takes the car of FROM, and then checks that the cdr of FROM is a
positive fixnum, which is a subtle difference from the other code.
> The name 'movement' is not the best name. I would suggest 'offset' or
> maybe even 'y_offset' instead.
Thanks.
> Also, the assumption that this offset is always positive, and that it
> means the position _before_ FROM, is also something that makes the
> function less general than it could be. It would be better to have
> positive values mean _after_ FROM and negative values to mean before
> it. Then you could use move_it_vertically instead of
> move_it_vertically_backward.
Does `move_it_vertically' have the same limitation of
`move_it_vertically_backwards' where the iterator might end up either
before or after the target position?
>> + if (movement > 0)
>> + {
>> + int last_y;
>> + it.current_y = 0;
>> +
>> + move_it_by_lines (&it, 0);
> Why is this needed?
That will move the iterator to the beginning of the visual line. I
don't think it makes any sense to move it elsewhere, as IIUC the
behaviour of moving an iterator upwards from a non-beginning position is
not very well defined.
>> + while (-it.current_y < movement)
>> + {
>> + last_y = it.current_y;
>> + move_it_vertically_backward (&it, movement + it.current_y);
>> +
>> + if (it.current_y == last_y)
>> + break;
>> + }
>> +
>> + it.current_y = 0;
>> + start = clip_to_bounds (BEGV, IT_CHARPOS (it), ZV);
>> + }
>> +
>> int start_y = it.current_y;
>> /* It makes no sense to measure dimensions of region of text that
>> crosses the point where bidi reordering changes scan direction.
> The code that follows this, which you leave intact, will reseat the
> iterator to the beginning of the visual line where you ended up after
> the loop that goes backward. Is that really TRT? what if there's a
> multi-line display or overlay string at that place? Did you test the
> code in that case? AFAIU, you have already assured that you are at
> the beginning of a visual line, so the reseat, and the following
> return to the START position, might not be needed. When you return to
> START, you may end up very far from where the loop going backward
> ended, if START is "covered" by a display property or overlay string.
Yes, that's the correct behaviour here, at least for the pixel scrolling
code (which I tested with both overlays and multi-line display
properties).
Thanks.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-19 0:54 ` Po Lu
@ 2021-12-19 8:29 ` Eli Zaretskii
2021-12-19 9:16 ` Po Lu
0 siblings, 1 reply; 53+ messages in thread
From: Eli Zaretskii @ 2021-12-19 8:29 UTC (permalink / raw)
To: Po Lu; +Cc: emacs-devel
> From: Po Lu <luangruo@yahoo.com>
> Cc: emacs-devel@gnu.org
> Date: Sun, 19 Dec 2021 08:54:29 +0800
>
> >> + else if (CONSP (from))
> >> + {
> >> + start = clip_to_bounds (BEGV, fix_position (XCAR (from)), ZV);
> >> + bpos = CHAR_TO_BYTE (start);
> >> + CHECK_FIXNAT (XCDR (from));
> >> + movement = XFIXNAT (XCDR (from));
> >> + }
> >> else
> >> {
> >> start = clip_to_bounds (BEGV, fix_position (from), ZV);
>
> > There's code duplication here, which it would be good to avoid: the
> > calculation of bpos and the call to clip_to_bounds in calculation of
> > start.
>
> I don't think the duplication here is a particularly big problem.
Not a big problem, it just makes the code a tad harder to read and
comprehend.
> > Also, the assumption that this offset is always positive, and that it
> > means the position _before_ FROM, is also something that makes the
> > function less general than it could be. It would be better to have
> > positive values mean _after_ FROM and negative values to mean before
> > it. Then you could use move_it_vertically instead of
> > move_it_vertically_backward.
>
> Does `move_it_vertically' have the same limitation of
> `move_it_vertically_backwards' where the iterator might end up either
> before or after the target position?
Look at the code: when DY is non-positive, move_it_vertically
delegates to move_it_vertically_backwards.
> >> + if (movement > 0)
> >> + {
> >> + int last_y;
> >> + it.current_y = 0;
> >> +
> >> + move_it_by_lines (&it, 0);
>
> > Why is this needed?
>
> That will move the iterator to the beginning of the visual line. I
> don't think it makes any sense to move it elsewhere, as IIUC the
> behaviour of moving an iterator upwards from a non-beginning position is
> not very well defined.
I think you assume that the new argument to window-text-pixel-size is
non-nil; then indeed we should move to the beginning of the visual
line. But in general, I don't think that's true.
Anyway, I don't necessarily object that we should do this always, but
then we should document that when FROM is a cons cell, that specifies
the beginning of the screen line at that pixel offset.
> > The code that follows this, which you leave intact, will reseat the
> > iterator to the beginning of the visual line where you ended up after
> > the loop that goes backward. Is that really TRT? what if there's a
> > multi-line display or overlay string at that place? Did you test the
> > code in that case? AFAIU, you have already assured that you are at
> > the beginning of a visual line, so the reseat, and the following
> > return to the START position, might not be needed. When you return to
> > START, you may end up very far from where the loop going backward
> > ended, if START is "covered" by a display property or overlay string.
>
> Yes, that's the correct behaviour here, at least for the pixel scrolling
> code (which I tested with both overlays and multi-line display
> properties).
When window-start is inside a display property or overlay (i.e., the
first thing shown in the window is the result of that display
property/overlay), starting from the underlying buffer position will
almost definitely affect the results, because that buffer position
could be at a very different position on the screen. For example,
what happens if window-start position has a before-string, and that
string has a 'display' property specifying an image? This should
display the image as the first display element at the window
beginning, and the buffer position of window-start will then be to the
right and possibly also at a different Y coordinate.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-19 8:29 ` Eli Zaretskii
@ 2021-12-19 9:16 ` Po Lu
2021-12-19 9:27 ` Eli Zaretskii
0 siblings, 1 reply; 53+ messages in thread
From: Po Lu @ 2021-12-19 9:16 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: emacs-devel
Eli Zaretskii <eliz@gnu.org> writes:
> Look at the code: when DY is non-positive, move_it_vertically
> delegates to move_it_vertically_backwards.
Thanks.
> Anyway, I don't necessarily object that we should do this always, but
> then we should document that when FROM is a cons cell, that specifies
> the beginning of the screen line at that pixel offset.
I will document that.
> When window-start is inside a display property or overlay (i.e., the
> first thing shown in the window is the result of that display
> property/overlay), starting from the underlying buffer position will
> almost definitely affect the results, because that buffer position
> could be at a very different position on the screen. For example,
> what happens if window-start position has a before-string, and that
> string has a 'display' property specifying an image? This should
> display the image as the first display element at the window
> beginning, and the buffer position of window-start will then be to the
> right and possibly also at a different Y coordinate.
Hmm, perhaps that condition should be removed in this case then.
Thanks.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-19 9:16 ` Po Lu
@ 2021-12-19 9:27 ` Eli Zaretskii
2021-12-19 10:25 ` Po Lu
0 siblings, 1 reply; 53+ messages in thread
From: Eli Zaretskii @ 2021-12-19 9:27 UTC (permalink / raw)
To: Po Lu; +Cc: emacs-devel
> From: Po Lu <luangruo@yahoo.com>
> Cc: emacs-devel@gnu.org
> Date: Sun, 19 Dec 2021 17:16:31 +0800
>
> > When window-start is inside a display property or overlay (i.e., the
> > first thing shown in the window is the result of that display
> > property/overlay), starting from the underlying buffer position will
> > almost definitely affect the results, because that buffer position
> > could be at a very different position on the screen. For example,
> > what happens if window-start position has a before-string, and that
> > string has a 'display' property specifying an image? This should
> > display the image as the first display element at the window
> > beginning, and the buffer position of window-start will then be to the
> > right and possibly also at a different Y coordinate.
>
> Hmm, perhaps that condition should be removed in this case then.
Not sure I understand what condition did you allude to here, and how
did you propose to remove it.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-19 9:27 ` Eli Zaretskii
@ 2021-12-19 10:25 ` Po Lu
2021-12-19 18:07 ` Eli Zaretskii
0 siblings, 1 reply; 53+ messages in thread
From: Po Lu @ 2021-12-19 10:25 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: emacs-devel
[-- Attachment #1: Type: text/plain, Size: 1337 bytes --]
Eli Zaretskii <eliz@gnu.org> writes:
>> From: Po Lu <luangruo@yahoo.com>
>> Cc: emacs-devel@gnu.org
>> Date: Sun, 19 Dec 2021 17:16:31 +0800
>>
>> > When window-start is inside a display property or overlay (i.e., the
>> > first thing shown in the window is the result of that display
>> > property/overlay), starting from the underlying buffer position will
>> > almost definitely affect the results, because that buffer position
>> > could be at a very different position on the screen. For example,
>> > what happens if window-start position has a before-string, and that
>> > string has a 'display' property specifying an image? This should
>> > display the image as the first display element at the window
>> > beginning, and the buffer position of window-start will then be to the
>> > right and possibly also at a different Y coordinate.
>>
>> Hmm, perhaps that condition should be removed in this case then.
>
> Not sure I understand what condition did you allude to here, and how
> did you propose to remove it.
I wanted to say that I removed the call reseat in that case as we're
certainly already at the beginning of a visual line.
Sorry for being vague. Anyway, here's a patch that extends the argument
to also be able to measure from a certain amount of pixels after a given
position, and not just before.
Thanks.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Allow-window-text-pixel-size-to-measure-pixels-aroun.patch --]
[-- Type: text/x-patch, Size: 10528 bytes --]
From 60b26f3f81f83987baf03cb7ebc03a4aac513fc4 Mon Sep 17 00:00:00 2001
From: Po Lu <luangruo@yahoo.com>
Date: Sun, 19 Dec 2021 18:19:34 +0800
Subject: [PATCH] Allow window-text-pixel-size to measure pixels around a
position
* doc/lispref/display.texi (Size of Displayed Text): Announce
new meaning of `from'.
* etc/NEWS: Announce changes.
* lisp/pixel-scroll.el (pixel-scroll-precision-scroll-up-page):
Use new feature.
* src/xdisp.c (window_text_pixel_size): Understand a special
format of `from' that specifies the amount of pixels above a
position.
(Fwindow_text_pixel_size): Update doc string.
---
doc/lispref/display.texi | 20 ++++++----
etc/NEWS | 3 ++
lisp/pixel-scroll.el | 34 ++++++++--------
src/xdisp.c | 84 +++++++++++++++++++++++++++++++---------
4 files changed, 98 insertions(+), 43 deletions(-)
diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
index 98a15404f9..c3485fa281 100644
--- a/doc/lispref/display.texi
+++ b/doc/lispref/display.texi
@@ -2090,19 +2090,23 @@ Size of Displayed Text
This function returns the size of the text of @var{window}'s buffer in
pixels. @var{window} must be a live window and defaults to the
selected one. The return value is a cons of the maximum pixel-width
-of any text line and the maximum pixel-height of all text lines. This
-function exists to allow Lisp programs to adjust the dimensions of
-@var{window} to the buffer text it needs to display.
+of any text line and the maximum pixel-height of all text lines, or if
+@var{from} is a cons, a list of the pixel-width, pixel-height, and the
+buffer position of the first line that was measured. This function
+exists to allow Lisp programs to adjust the dimensions of @var{window}
+to the buffer text it needs to display.
The optional argument @var{from}, if non-@code{nil}, specifies the
first text position to consider, and defaults to the minimum
accessible position of the buffer. If @var{from} is @code{t}, it
stands for the minimum accessible position that is not a newline
-character. The optional argument @var{to}, if non-@code{nil},
-specifies the last text position to consider, and defaults to the
-maximum accessible position of the buffer. If @var{to} is @code{t},
-it stands for the maximum accessible position that is not a newline
-character.
+character. If @var{from} is a cons, the cdr specifies a position, and
+the car specifies the minimum amount of pixels above that position to
+start measuring from. The optional argument @var{to}, if
+non-@code{nil}, specifies the last text position to consider, and
+defaults to the maximum accessible position of the buffer. If
+@var{to} is @code{t}, it stands for the maximum accessible position
+that is not a newline character.
The optional argument @var{x-limit}, if non-@code{nil}, specifies the
maximum X coordinate beyond which text should be ignored; it is
diff --git a/etc/NEWS b/etc/NEWS
index b50e7e5db0..056184019f 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -997,6 +997,9 @@ This controls whether or not the last screen line of the text being
measured will be counted for the purpose of calculating the text
dimensions.
++++
+** 'window-text-pixel-size' can now measure from a set amount of pixels above a position.
+
** XDG support
*** New function 'xdg-state-home' returns 'XDG_STATE_HOME' environment variable.
diff --git a/lisp/pixel-scroll.el b/lisp/pixel-scroll.el
index fa0185b16e..4aae166f2e 100644
--- a/lisp/pixel-scroll.el
+++ b/lisp/pixel-scroll.el
@@ -516,22 +516,24 @@ pixel-scroll-precision-scroll-up-page
usable-height))))
(goto-char up-point)))
(let ((current-vscroll (window-vscroll nil t)))
- (if (<= delta current-vscroll)
- (set-window-vscroll nil (- current-vscroll delta) t)
- (setq delta (- delta current-vscroll))
- (set-window-vscroll nil 0 t)
- (while (> delta 0)
- (let ((position (pixel-point-and-height-at-unseen-line)))
- (unless (cdr position)
- (signal 'beginning-of-buffer nil))
- (set-window-start nil (car position) t)
- ;; If the line above is taller than the window height (i.e. there's
- ;; a very tall image), keep point on it.
- (when (> (cdr position) usable-height)
- (goto-char (car position)))
- (setq delta (- delta (cdr position)))))
- (when (< delta 0)
- (set-window-vscroll nil (- delta) t))))))
+ (setq delta (- delta current-vscroll))
+ (set-window-vscroll nil 0 t)
+ (when (> delta 0)
+ (let* ((start (window-start))
+ (dims (window-text-pixel-size nil (cons start (- delta))
+ start nil nil nil t))
+ (height (nth 1 dims))
+ (position (nth 2 dims)))
+ (set-window-start nil position t)
+ ;; If the line above is taller than the window height (i.e. there's
+ ;; a very tall image), keep point on it.
+ (when (> height usable-height)
+ (goto-char position))
+ (when (or (not position) (eq position start))
+ (signal 'beginning-of-buffer nil))
+ (setq delta (- delta height))))
+ (when (< delta 0)
+ (set-window-vscroll nil (- delta) t)))))
(defun pixel-scroll-precision-interpolate (delta)
"Interpolate a scroll of DELTA pixels.
diff --git a/src/xdisp.c b/src/xdisp.c
index 3a1bc1613f..b7fb281473 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -10841,8 +10841,7 @@ window_text_pixel_size (Lisp_Object window, Lisp_Object from, Lisp_Object to,
ptrdiff_t start, end, bpos;
struct text_pos startp;
void *itdata = NULL;
- int c, max_x = 0, max_y = 0, x = 0, y = 0;
- int doff = 0;
+ int c, max_x = 0, max_y = 0, x = 0, y = 0, movement = 0, doff = 0;
if (NILP (from))
{
@@ -10868,6 +10867,13 @@ window_text_pixel_size (Lisp_Object window, Lisp_Object from, Lisp_Object to,
break;
}
}
+ else if (CONSP (from))
+ {
+ start = clip_to_bounds (BEGV, fix_position (XCAR (from)), ZV);
+ bpos = CHAR_TO_BYTE (start);
+ CHECK_FIXNUM (XCDR (from));
+ movement = XFIXNUM (XCDR (from));
+ }
else
{
start = clip_to_bounds (BEGV, fix_position (from), ZV);
@@ -10914,7 +10920,9 @@ window_text_pixel_size (Lisp_Object window, Lisp_Object from, Lisp_Object to,
itdata = bidi_shelve_cache ();
start_display (&it, w, startp);
+
int start_y = it.current_y;
+
/* It makes no sense to measure dimensions of region of text that
crosses the point where bidi reordering changes scan direction.
By using unidirectional movement here we at least support the use
@@ -10923,13 +10931,45 @@ window_text_pixel_size (Lisp_Object window, Lisp_Object from, Lisp_Object to,
same directionality. */
it.bidi_p = false;
- /* Start at the beginning of the line containing FROM. Otherwise
- IT.current_x will be incorrectly set to zero at some arbitrary
- non-zero X coordinate. */
- reseat_at_previous_visible_line_start (&it);
- it.current_x = it.hpos = 0;
- if (IT_CHARPOS (it) != start)
- move_it_to (&it, start, -1, -1, -1, MOVE_TO_POS);
+ if (movement != 0)
+ {
+ int last_y;
+ it.current_y = 0;
+
+ move_it_by_lines (&it, 0);
+
+ if (movement < 0)
+ {
+ while (it.current_y > movement)
+ {
+ last_y = it.current_y;
+ move_it_vertically_backward (&it,
+ abs (movement) + it.current_y);
+
+ if (it.current_y == last_y)
+ break;
+ }
+ }
+ else
+ {
+ move_it_vertically (&it, movement);
+ }
+
+ it.current_y = (WINDOW_TAB_LINE_HEIGHT (w)
+ + WINDOW_HEADER_LINE_HEIGHT (w));
+ start = clip_to_bounds (BEGV, IT_CHARPOS (it), ZV);
+ start_y = it.current_y;
+ }
+ else
+ {
+ /* Start at the beginning of the line containing FROM. Otherwise
+ IT.current_x will be incorrectly set to zero at some arbitrary
+ non-zero X coordinate. */
+ reseat_at_previous_visible_line_start (&it);
+ it.current_x = it.hpos = 0;
+ if (IT_CHARPOS (it) != start)
+ move_it_to (&it, start, -1, -1, -1, MOVE_TO_POS);
+ }
/* Now move to TO. */
int start_x = it.current_x;
@@ -11052,25 +11092,31 @@ window_text_pixel_size (Lisp_Object window, Lisp_Object from, Lisp_Object to,
bidi_unshelve_cache (itdata, false);
- return Fcons (make_fixnum (x - start_x), make_fixnum (y));
+ return (!movement
+ ? Fcons (make_fixnum (x - start_x), make_fixnum (y))
+ : list3i (x - start_x, y, start));
}
DEFUN ("window-text-pixel-size", Fwindow_text_pixel_size, Swindow_text_pixel_size, 0, 7, 0,
- doc: /* Return the size of the text of WINDOW's buffer in pixels.
-WINDOW must be a live window and defaults to the selected one. The
-return value is a cons of the maximum pixel-width of any text line
-and the pixel-height of all the text lines in the accessible portion
-of buffer text.
+ doc: /* Return the size of the text of WINDOW's buffer in
+pixels. WINDOW must be a live window and defaults to the selected
+one. The return value is a cons of the maximum pixel-width of any
+text line and the pixel-height of all the text lines in the accessible
+portion of buffer text or a list of the maximum pixel-width,
+pixel-height, and the buffer position of the line at FROM.
This function exists to allow Lisp programs to adjust the dimensions
of WINDOW to the buffer text it needs to display.
The optional argument FROM, if non-nil, specifies the first text
position to consider, and defaults to the minimum accessible position
-of the buffer. If FROM is t, it stands for the minimum accessible
-position that starts a non-empty line. TO, if non-nil, specifies the
-last text position and defaults to the maximum accessible position of
-the buffer. If TO is t, it stands for the maximum accessible position
+of the buffer. If FROM is a cons, the cdr specifies the amount of
+pixels above or below a buffer position to begin measuring text, and
+the car specifies the buffer position. In that case, a list is
+returned. If FROM is t, it stands for the minimum accessible position
+that starts a non-empty line. TO, if non-nil, specifies the last text
+position and defaults to the maximum accessible position of the
+buffer. If TO is t, it stands for the maximum accessible position
that ends a non-empty line.
The optional argument X-LIMIT, if non-nil, specifies the maximum X
--
2.33.1
^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-19 10:25 ` Po Lu
@ 2021-12-19 18:07 ` Eli Zaretskii
2021-12-20 1:05 ` Po Lu
0 siblings, 1 reply; 53+ messages in thread
From: Eli Zaretskii @ 2021-12-19 18:07 UTC (permalink / raw)
To: Po Lu; +Cc: emacs-devel
> From: Po Lu <luangruo@yahoo.com>
> Cc: emacs-devel@gnu.org
> Date: Sun, 19 Dec 2021 18:25:07 +0800
>
> + if (movement != 0)
> + {
> + int last_y;
> + it.current_y = 0;
The last assignment should be unnecessary here: start-display always
initializes the coordinates to zero.
> + if (movement < 0)
> + {
> + while (it.current_y > movement)
> + {
> + last_y = it.current_y;
> + move_it_vertically_backward (&it,
> + abs (movement) + it.current_y);
> +
> + if (it.current_y == last_y)
> + break;
> + }
> + }
> + else
> + {
> + move_it_vertically (&it, movement);
> + }
I don't understand the different logic depending on the sign of
'movement' (and didn't we agree to use a better name for it?).
> + it.current_y = (WINDOW_TAB_LINE_HEIGHT (w)
> + + WINDOW_HEADER_LINE_HEIGHT (w));
> + start = clip_to_bounds (BEGV, IT_CHARPOS (it), ZV);
> + start_y = it.current_y;
> + }
> + else
> + {
> + /* Start at the beginning of the line containing FROM. Otherwise
> + IT.current_x will be incorrectly set to zero at some arbitrary
> + non-zero X coordinate. */
> + reseat_at_previous_visible_line_start (&it);
> + it.current_x = it.hpos = 0;
> + if (IT_CHARPOS (it) != start)
> + move_it_to (&it, start, -1, -1, -1, MOVE_TO_POS);
> + }
And here: why a different initial value for it.current_y, depending on
how FROM was specified?
(Again, I didn't yet pay attention to the documentation.)
Thanks.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-19 18:07 ` Eli Zaretskii
@ 2021-12-20 1:05 ` Po Lu
2021-12-21 12:58 ` Po Lu
0 siblings, 1 reply; 53+ messages in thread
From: Po Lu @ 2021-12-20 1:05 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: emacs-devel
Eli Zaretskii <eliz@gnu.org> writes:
>> From: Po Lu <luangruo@yahoo.com>
>> Cc: emacs-devel@gnu.org
>> Date: Sun, 19 Dec 2021 18:25:07 +0800
>>
>> + if (movement != 0)
>> + {
>> + int last_y;
>> + it.current_y = 0;
> The last assignment should be unnecessary here: start-display always
> initializes the coordinates to zero.
The comment (and code) below seems to make it seem as if start_display
initializes it to WINDOW_TAB_LINE_HEIGHT + WINDOW_HEADER_LINE_HEIGHT.
>> + if (movement < 0)
>> + {
>> + while (it.current_y > movement)
>> + {
>> + last_y = it.current_y;
>> + move_it_vertically_backward (&it,
>> + abs (movement) + it.current_y);
>> +
>> + if (it.current_y == last_y)
>> + break;
>> + }
>> + }
>> + else
>> + {
>> + move_it_vertically (&it, movement);
>> + }
> I don't understand the different logic depending on the sign of
> 'movement' (and didn't we agree to use a better name for it?).
Sorry for forgetting that. I'll fix it ASAP. As for the logic,
movement being negative means to move backwards from the specified
buffer position, while it being positive means to move forwards.
>> + it.current_y = (WINDOW_TAB_LINE_HEIGHT (w)
>> + + WINDOW_HEADER_LINE_HEIGHT (w));
>> + start = clip_to_bounds (BEGV, IT_CHARPOS (it), ZV);
>> + start_y = it.current_y;
>> + }
>> + else
>> + {
>> + /* Start at the beginning of the line containing FROM. Otherwise
>> + IT.current_x will be incorrectly set to zero at some arbitrary
>> + non-zero X coordinate. */
>> + reseat_at_previous_visible_line_start (&it);
>> + it.current_x = it.hpos = 0;
>> + if (IT_CHARPOS (it) != start)
>> + move_it_to (&it, start, -1, -1, -1, MOVE_TO_POS);
>> + }
> And here: why a different initial value for it.current_y, depending on
> how FROM was specified?
It should be that way, as we subtract the tab line height and header
line height from that value afterwards.
Thanks.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-20 1:05 ` Po Lu
@ 2021-12-21 12:58 ` Po Lu
2021-12-21 17:07 ` Eli Zaretskii
0 siblings, 1 reply; 53+ messages in thread
From: Po Lu @ 2021-12-21 12:58 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: emacs-devel
Yahoo said this reply wasn't delivered successfully (?), so I'm sending
it again. Thanks.
> The last assignment should be unnecessary here: start-display always
> initializes the coordinates to zero.
The comment (and code) below seems to make it seem as if start_display
initializes it to WINDOW_TAB_LINE_HEIGHT + WINDOW_HEADER_LINE_HEIGHT.
>> + if (movement < 0)
>> + {
>> + while (it.current_y > movement)
>> + {
>> + last_y = it.current_y;
>> + move_it_vertically_backward (&it,
>> + abs (movement) + it.current_y);
>> +
>> + if (it.current_y == last_y)
>> + break;
>> + }
>> + }
>> + else
>> + {
>> + move_it_vertically (&it, movement);
>> + }
> I don't understand the different logic depending on the sign of
> 'movement' (and didn't we agree to use a better name for it?).
Sorry for forgetting that. I'll fix it ASAP. As for the logic,
movement being negative means to move backwards from the specified
buffer position, while it being positive means to move forwards.
>> + it.current_y = (WINDOW_TAB_LINE_HEIGHT (w)
>> + + WINDOW_HEADER_LINE_HEIGHT (w));
>> + start = clip_to_bounds (BEGV, IT_CHARPOS (it), ZV);
>> + start_y = it.current_y;
>> + }
>> + else
>> + {
>> + /* Start at the beginning of the line containing FROM. Otherwise
>> + IT.current_x will be incorrectly set to zero at some arbitrary
>> + non-zero X coordinate. */
>> + reseat_at_previous_visible_line_start (&it);
>> + it.current_x = it.hpos = 0;
>> + if (IT_CHARPOS (it) != start)
>> + move_it_to (&it, start, -1, -1, -1, MOVE_TO_POS);
>> + }
> And here: why a different initial value for it.current_y, depending on
> how FROM was specified?
It should be that way, as we subtract the tab line height and header
line height from that value afterwards.
Thanks.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-21 12:58 ` Po Lu
@ 2021-12-21 17:07 ` Eli Zaretskii
2021-12-22 0:49 ` Po Lu
0 siblings, 1 reply; 53+ messages in thread
From: Eli Zaretskii @ 2021-12-21 17:07 UTC (permalink / raw)
To: Po Lu; +Cc: emacs-devel
> From: Po Lu <luangruo@yahoo.com>
> Cc: emacs-devel@gnu.org
> Date: Tue, 21 Dec 2021 20:58:55 +0800
>
> Yahoo said this reply wasn't delivered successfully (?), so I'm sending
> it again. Thanks.
No, it arrived OK and I've seen it. I'm okay with your responses.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-21 17:07 ` Eli Zaretskii
@ 2021-12-22 0:49 ` Po Lu
2021-12-22 14:59 ` Eli Zaretskii
2021-12-23 1:30 ` Po Lu
0 siblings, 2 replies; 53+ messages in thread
From: Po Lu @ 2021-12-22 0:49 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: emacs-devel
[-- Attachment #1: Type: text/plain, Size: 320 bytes --]
Eli Zaretskii <eliz@gnu.org> writes:
> No, it arrived OK and I've seen it. I'm okay with your responses.
Thanks, btw, does anyone know how much mail was lost during the outage?
Also, here's a patch with some of the changes made. I tried with the
documentation as well, but I'm pretty bad with that stuff.
Thanks.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Allow-window-text-pixel-size-to-measure-pixels-aroun.patch --]
[-- Type: text/x-patch, Size: 10598 bytes --]
From 73ee5056751a784355768e8c0dd855a1b095ba60 Mon Sep 17 00:00:00 2001
From: Po Lu <luangruo@yahoo.com>
Date: Sun, 19 Dec 2021 18:19:34 +0800
Subject: [PATCH] Allow window-text-pixel-size to measure pixels around a
position
* doc/lispref/display.texi (Size of Displayed Text): Announce
new meaning of `from'.
* etc/NEWS: Announce changes.
* lisp/pixel-scroll.el (pixel-scroll-precision-scroll-up-page):
Use new feature.
* src/xdisp.c (window_text_pixel_size): Understand a special
format of `from' that specifies the amount of pixels above a
position.
(Fwindow_text_pixel_size): Update doc string.
---
doc/lispref/display.texi | 20 ++++++----
etc/NEWS | 3 ++
lisp/pixel-scroll.el | 34 ++++++++--------
src/xdisp.c | 85 +++++++++++++++++++++++++++++++---------
4 files changed, 99 insertions(+), 43 deletions(-)
diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
index 98a15404f9..c3485fa281 100644
--- a/doc/lispref/display.texi
+++ b/doc/lispref/display.texi
@@ -2090,19 +2090,23 @@ Size of Displayed Text
This function returns the size of the text of @var{window}'s buffer in
pixels. @var{window} must be a live window and defaults to the
selected one. The return value is a cons of the maximum pixel-width
-of any text line and the maximum pixel-height of all text lines. This
-function exists to allow Lisp programs to adjust the dimensions of
-@var{window} to the buffer text it needs to display.
+of any text line and the maximum pixel-height of all text lines, or if
+@var{from} is a cons, a list of the pixel-width, pixel-height, and the
+buffer position of the first line that was measured. This function
+exists to allow Lisp programs to adjust the dimensions of @var{window}
+to the buffer text it needs to display.
The optional argument @var{from}, if non-@code{nil}, specifies the
first text position to consider, and defaults to the minimum
accessible position of the buffer. If @var{from} is @code{t}, it
stands for the minimum accessible position that is not a newline
-character. The optional argument @var{to}, if non-@code{nil},
-specifies the last text position to consider, and defaults to the
-maximum accessible position of the buffer. If @var{to} is @code{t},
-it stands for the maximum accessible position that is not a newline
-character.
+character. If @var{from} is a cons, the cdr specifies a position, and
+the car specifies the minimum amount of pixels above that position to
+start measuring from. The optional argument @var{to}, if
+non-@code{nil}, specifies the last text position to consider, and
+defaults to the maximum accessible position of the buffer. If
+@var{to} is @code{t}, it stands for the maximum accessible position
+that is not a newline character.
The optional argument @var{x-limit}, if non-@code{nil}, specifies the
maximum X coordinate beyond which text should be ignored; it is
diff --git a/etc/NEWS b/etc/NEWS
index 57fe40c488..9e901ff9ca 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1010,6 +1010,9 @@ This controls whether or not the last screen line of the text being
measured will be counted for the purpose of calculating the text
dimensions.
++++
+** 'window-text-pixel-size' can now measure from a set amount of pixels above a position.
+
** XDG support
*** New function 'xdg-state-home' returns 'XDG_STATE_HOME' environment variable.
diff --git a/lisp/pixel-scroll.el b/lisp/pixel-scroll.el
index fa0185b16e..4aae166f2e 100644
--- a/lisp/pixel-scroll.el
+++ b/lisp/pixel-scroll.el
@@ -516,22 +516,24 @@ pixel-scroll-precision-scroll-up-page
usable-height))))
(goto-char up-point)))
(let ((current-vscroll (window-vscroll nil t)))
- (if (<= delta current-vscroll)
- (set-window-vscroll nil (- current-vscroll delta) t)
- (setq delta (- delta current-vscroll))
- (set-window-vscroll nil 0 t)
- (while (> delta 0)
- (let ((position (pixel-point-and-height-at-unseen-line)))
- (unless (cdr position)
- (signal 'beginning-of-buffer nil))
- (set-window-start nil (car position) t)
- ;; If the line above is taller than the window height (i.e. there's
- ;; a very tall image), keep point on it.
- (when (> (cdr position) usable-height)
- (goto-char (car position)))
- (setq delta (- delta (cdr position)))))
- (when (< delta 0)
- (set-window-vscroll nil (- delta) t))))))
+ (setq delta (- delta current-vscroll))
+ (set-window-vscroll nil 0 t)
+ (when (> delta 0)
+ (let* ((start (window-start))
+ (dims (window-text-pixel-size nil (cons start (- delta))
+ start nil nil nil t))
+ (height (nth 1 dims))
+ (position (nth 2 dims)))
+ (set-window-start nil position t)
+ ;; If the line above is taller than the window height (i.e. there's
+ ;; a very tall image), keep point on it.
+ (when (> height usable-height)
+ (goto-char position))
+ (when (or (not position) (eq position start))
+ (signal 'beginning-of-buffer nil))
+ (setq delta (- delta height))))
+ (when (< delta 0)
+ (set-window-vscroll nil (- delta) t)))))
(defun pixel-scroll-precision-interpolate (delta)
"Interpolate a scroll of DELTA pixels.
diff --git a/src/xdisp.c b/src/xdisp.c
index 0c35d24c26..7e32a6c8d8 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -10841,8 +10841,7 @@ window_text_pixel_size (Lisp_Object window, Lisp_Object from, Lisp_Object to,
ptrdiff_t start, end, bpos;
struct text_pos startp;
void *itdata = NULL;
- int c, max_x = 0, max_y = 0, x = 0, y = 0;
- int doff = 0;
+ int c, max_x = 0, max_y = 0, x = 0, y = 0, vertical_offset = 0, doff = 0;
if (NILP (from))
{
@@ -10868,6 +10867,13 @@ window_text_pixel_size (Lisp_Object window, Lisp_Object from, Lisp_Object to,
break;
}
}
+ else if (CONSP (from))
+ {
+ start = clip_to_bounds (BEGV, fix_position (XCAR (from)), ZV);
+ bpos = CHAR_TO_BYTE (start);
+ CHECK_FIXNUM (XCDR (from));
+ vertical_offset = XFIXNUM (XCDR (from));
+ }
else
{
start = clip_to_bounds (BEGV, fix_position (from), ZV);
@@ -10914,7 +10920,9 @@ window_text_pixel_size (Lisp_Object window, Lisp_Object from, Lisp_Object to,
itdata = bidi_shelve_cache ();
start_display (&it, w, startp);
+
int start_y = it.current_y;
+
/* It makes no sense to measure dimensions of region of text that
crosses the point where bidi reordering changes scan direction.
By using unidirectional movement here we at least support the use
@@ -10923,13 +10931,46 @@ window_text_pixel_size (Lisp_Object window, Lisp_Object from, Lisp_Object to,
same directionality. */
it.bidi_p = false;
- /* Start at the beginning of the line containing FROM. Otherwise
- IT.current_x will be incorrectly set to zero at some arbitrary
- non-zero X coordinate. */
- reseat_at_previous_visible_line_start (&it);
- it.current_x = it.hpos = 0;
- if (IT_CHARPOS (it) != start)
- move_it_to (&it, start, -1, -1, -1, MOVE_TO_POS);
+ if (vertical_offset != 0)
+ {
+ int last_y;
+ it.current_y = 0;
+
+ move_it_by_lines (&it, 0);
+
+ if (vertical_offset < 0)
+ {
+ while (it.current_y > vertical_offset)
+ {
+ last_y = it.current_y;
+ move_it_vertically_backward (&it,
+ (abs (vertical_offset)
+ + it.current_y));
+
+ if (it.current_y == last_y)
+ break;
+ }
+ }
+ else
+ {
+ move_it_vertically (&it, vertical_offset);
+ }
+
+ it.current_y = (WINDOW_TAB_LINE_HEIGHT (w)
+ + WINDOW_HEADER_LINE_HEIGHT (w));
+ start = clip_to_bounds (BEGV, IT_CHARPOS (it), ZV);
+ start_y = it.current_y;
+ }
+ else
+ {
+ /* Start at the beginning of the line containing FROM. Otherwise
+ IT.current_x will be incorrectly set to zero at some arbitrary
+ non-zero X coordinate. */
+ reseat_at_previous_visible_line_start (&it);
+ it.current_x = it.hpos = 0;
+ if (IT_CHARPOS (it) != start)
+ move_it_to (&it, start, -1, -1, -1, MOVE_TO_POS);
+ }
/* Now move to TO. */
int start_x = it.current_x;
@@ -11052,25 +11093,31 @@ window_text_pixel_size (Lisp_Object window, Lisp_Object from, Lisp_Object to,
bidi_unshelve_cache (itdata, false);
- return Fcons (make_fixnum (x - start_x), make_fixnum (y));
+ return (!vertical_offset
+ ? Fcons (make_fixnum (x - start_x), make_fixnum (y))
+ : list3i (x - start_x, y, start));
}
DEFUN ("window-text-pixel-size", Fwindow_text_pixel_size, Swindow_text_pixel_size, 0, 7, 0,
- doc: /* Return the size of the text of WINDOW's buffer in pixels.
-WINDOW must be a live window and defaults to the selected one. The
-return value is a cons of the maximum pixel-width of any text line
-and the pixel-height of all the text lines in the accessible portion
-of buffer text.
+ doc: /* Return the size of the text of WINDOW's buffer in
+pixels. WINDOW must be a live window and defaults to the selected
+one. The return value is a cons of the maximum pixel-width of any
+text line and the pixel-height of all the text lines in the accessible
+portion of buffer text or a list of the maximum pixel-width,
+pixel-height, and the buffer position of the line at FROM.
This function exists to allow Lisp programs to adjust the dimensions
of WINDOW to the buffer text it needs to display.
The optional argument FROM, if non-nil, specifies the first text
position to consider, and defaults to the minimum accessible position
-of the buffer. If FROM is t, it stands for the minimum accessible
-position that starts a non-empty line. TO, if non-nil, specifies the
-last text position and defaults to the maximum accessible position of
-the buffer. If TO is t, it stands for the maximum accessible position
+of the buffer. If FROM is a cons, the cdr specifies the amount of
+pixels above or below a buffer position to begin measuring text, and
+the car specifies the buffer position. In that case, a list is
+returned. If FROM is t, it stands for the minimum accessible position
+that starts a non-empty line. TO, if non-nil, specifies the last text
+position and defaults to the maximum accessible position of the
+buffer. If TO is t, it stands for the maximum accessible position
that ends a non-empty line.
The optional argument X-LIMIT, if non-nil, specifies the maximum X
--
2.33.1
^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-22 0:49 ` Po Lu
@ 2021-12-22 14:59 ` Eli Zaretskii
2021-12-23 1:30 ` Po Lu
1 sibling, 0 replies; 53+ messages in thread
From: Eli Zaretskii @ 2021-12-22 14:59 UTC (permalink / raw)
To: Po Lu; +Cc: emacs-devel
> From: Po Lu <luangruo@yahoo.com>
> Cc: emacs-devel@gnu.org
> Date: Wed, 22 Dec 2021 08:49:45 +0800
>
> does anyone know how much mail was lost during the outage?
AFAICT, none of it was lost. It's just that when the outage ends, the
servers start delivering email not in the chronological order, so many
times you get the later messages before the earlier ones.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-22 0:49 ` Po Lu
2021-12-22 14:59 ` Eli Zaretskii
@ 2021-12-23 1:30 ` Po Lu
2021-12-23 9:49 ` Eli Zaretskii
1 sibling, 1 reply; 53+ messages in thread
From: Po Lu @ 2021-12-23 1:30 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: emacs-devel
[-- Attachment #1: Type: text/plain, Size: 407 bytes --]
Po Lu <luangruo@yahoo.com> writes:
> Also, here's a patch with some of the changes made. I tried with the
> documentation as well, but I'm pretty bad with that stuff.
I tried updating the documentation again to better fit what's actually
done (including documenting that measurements always begin at the
beginning of the visual line).
Please take a look and see if it's OK to install this now, thanks.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Allow-window-text-pixel-size-to-measure-pixels-aroun.patch --]
[-- Type: text/x-patch, Size: 10642 bytes --]
From 2e01b17fb6ccc3bf8b6552bfb5b59811d4b04422 Mon Sep 17 00:00:00 2001
From: Po Lu <luangruo@yahoo.com>
Date: Thu, 23 Dec 2021 09:12:02 +0800
Subject: [PATCH] Allow window-text-pixel-size to measure pixels around a
position
* doc/lispref/display.texi (Size of Displayed Text): Announce
new meaning of `from'.
* etc/NEWS: Announce changes.
* lisp/pixel-scroll.el (pixel-scroll-precision-scroll-up-page):
Use new feature.
* src/xdisp.c (window_text_pixel_size): Understand a special
format of `from' that specifies the amount of pixels above a
position.
(Fwindow_text_pixel_size): Update doc string.
---
doc/lispref/display.texi | 20 ++++++----
etc/NEWS | 3 ++
lisp/pixel-scroll.el | 34 ++++++++--------
src/xdisp.c | 85 +++++++++++++++++++++++++++++++---------
4 files changed, 99 insertions(+), 43 deletions(-)
diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
index 98a15404f9..7e6ee59824 100644
--- a/doc/lispref/display.texi
+++ b/doc/lispref/display.texi
@@ -2090,19 +2090,23 @@ Size of Displayed Text
This function returns the size of the text of @var{window}'s buffer in
pixels. @var{window} must be a live window and defaults to the
selected one. The return value is a cons of the maximum pixel-width
-of any text line and the maximum pixel-height of all text lines. This
-function exists to allow Lisp programs to adjust the dimensions of
-@var{window} to the buffer text it needs to display.
+of any text line and the maximum pixel-height of all text lines, or if
+@var{from} is a cons, a list of the pixel-width, pixel-height, and the
+buffer position of the first line that was measured. This function
+exists to allow Lisp programs to adjust the dimensions of @var{window}
+to the buffer text it needs to display.
The optional argument @var{from}, if non-@code{nil}, specifies the
first text position to consider, and defaults to the minimum
accessible position of the buffer. If @var{from} is @code{t}, it
stands for the minimum accessible position that is not a newline
-character. The optional argument @var{to}, if non-@code{nil},
-specifies the last text position to consider, and defaults to the
-maximum accessible position of the buffer. If @var{to} is @code{t},
-it stands for the maximum accessible position that is not a newline
-character.
+character. If @var{from} is a cons, the cdr specifies a position, and
+the car specifies the minimum amount of pixels above or below the
+beginning of the visual line at that position to start measuring from.
+The optional argument @var{to}, if non-@code{nil}, specifies the last
+text position to consider, and defaults to the maximum accessible
+position of the buffer. If @var{to} is @code{t}, it stands for the
+maximum accessible position that is not a newline character.
The optional argument @var{x-limit}, if non-@code{nil}, specifies the
maximum X coordinate beyond which text should be ignored; it is
diff --git a/etc/NEWS b/etc/NEWS
index 86f18078f6..08e19a0eb2 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1020,6 +1020,9 @@ This controls whether or not the last screen line of the text being
measured will be counted for the purpose of calculating the text
dimensions.
++++
+** 'window-text-pixel-size' can now measure from a set amount of pixels above a position.
+
** XDG support
*** New function 'xdg-state-home' returns 'XDG_STATE_HOME' environment variable.
diff --git a/lisp/pixel-scroll.el b/lisp/pixel-scroll.el
index fa0185b16e..4aae166f2e 100644
--- a/lisp/pixel-scroll.el
+++ b/lisp/pixel-scroll.el
@@ -516,22 +516,24 @@ pixel-scroll-precision-scroll-up-page
usable-height))))
(goto-char up-point)))
(let ((current-vscroll (window-vscroll nil t)))
- (if (<= delta current-vscroll)
- (set-window-vscroll nil (- current-vscroll delta) t)
- (setq delta (- delta current-vscroll))
- (set-window-vscroll nil 0 t)
- (while (> delta 0)
- (let ((position (pixel-point-and-height-at-unseen-line)))
- (unless (cdr position)
- (signal 'beginning-of-buffer nil))
- (set-window-start nil (car position) t)
- ;; If the line above is taller than the window height (i.e. there's
- ;; a very tall image), keep point on it.
- (when (> (cdr position) usable-height)
- (goto-char (car position)))
- (setq delta (- delta (cdr position)))))
- (when (< delta 0)
- (set-window-vscroll nil (- delta) t))))))
+ (setq delta (- delta current-vscroll))
+ (set-window-vscroll nil 0 t)
+ (when (> delta 0)
+ (let* ((start (window-start))
+ (dims (window-text-pixel-size nil (cons start (- delta))
+ start nil nil nil t))
+ (height (nth 1 dims))
+ (position (nth 2 dims)))
+ (set-window-start nil position t)
+ ;; If the line above is taller than the window height (i.e. there's
+ ;; a very tall image), keep point on it.
+ (when (> height usable-height)
+ (goto-char position))
+ (when (or (not position) (eq position start))
+ (signal 'beginning-of-buffer nil))
+ (setq delta (- delta height))))
+ (when (< delta 0)
+ (set-window-vscroll nil (- delta) t)))))
(defun pixel-scroll-precision-interpolate (delta)
"Interpolate a scroll of DELTA pixels.
diff --git a/src/xdisp.c b/src/xdisp.c
index 1f896c256e..d20fdc4016 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -10841,8 +10841,7 @@ window_text_pixel_size (Lisp_Object window, Lisp_Object from, Lisp_Object to,
ptrdiff_t start, end, bpos;
struct text_pos startp;
void *itdata = NULL;
- int c, max_x = 0, max_y = 0, x = 0, y = 0;
- int doff = 0;
+ int c, max_x = 0, max_y = 0, x = 0, y = 0, vertical_offset = 0, doff = 0;
if (NILP (from))
{
@@ -10868,6 +10867,13 @@ window_text_pixel_size (Lisp_Object window, Lisp_Object from, Lisp_Object to,
break;
}
}
+ else if (CONSP (from))
+ {
+ start = clip_to_bounds (BEGV, fix_position (XCAR (from)), ZV);
+ bpos = CHAR_TO_BYTE (start);
+ CHECK_FIXNUM (XCDR (from));
+ vertical_offset = XFIXNUM (XCDR (from));
+ }
else
{
start = clip_to_bounds (BEGV, fix_position (from), ZV);
@@ -10914,7 +10920,9 @@ window_text_pixel_size (Lisp_Object window, Lisp_Object from, Lisp_Object to,
itdata = bidi_shelve_cache ();
start_display (&it, w, startp);
+
int start_y = it.current_y;
+
/* It makes no sense to measure dimensions of region of text that
crosses the point where bidi reordering changes scan direction.
By using unidirectional movement here we at least support the use
@@ -10923,13 +10931,46 @@ window_text_pixel_size (Lisp_Object window, Lisp_Object from, Lisp_Object to,
same directionality. */
it.bidi_p = false;
- /* Start at the beginning of the line containing FROM. Otherwise
- IT.current_x will be incorrectly set to zero at some arbitrary
- non-zero X coordinate. */
- reseat_at_previous_visible_line_start (&it);
- it.current_x = it.hpos = 0;
- if (IT_CHARPOS (it) != start)
- move_it_to (&it, start, -1, -1, -1, MOVE_TO_POS);
+ if (vertical_offset != 0)
+ {
+ int last_y;
+ it.current_y = 0;
+
+ move_it_by_lines (&it, 0);
+
+ if (vertical_offset < 0)
+ {
+ while (it.current_y > vertical_offset)
+ {
+ last_y = it.current_y;
+ move_it_vertically_backward (&it,
+ (abs (vertical_offset)
+ + it.current_y));
+
+ if (it.current_y == last_y)
+ break;
+ }
+ }
+ else
+ {
+ move_it_vertically (&it, vertical_offset);
+ }
+
+ it.current_y = (WINDOW_TAB_LINE_HEIGHT (w)
+ + WINDOW_HEADER_LINE_HEIGHT (w));
+ start = clip_to_bounds (BEGV, IT_CHARPOS (it), ZV);
+ start_y = it.current_y;
+ }
+ else
+ {
+ /* Start at the beginning of the line containing FROM. Otherwise
+ IT.current_x will be incorrectly set to zero at some arbitrary
+ non-zero X coordinate. */
+ reseat_at_previous_visible_line_start (&it);
+ it.current_x = it.hpos = 0;
+ if (IT_CHARPOS (it) != start)
+ move_it_to (&it, start, -1, -1, -1, MOVE_TO_POS);
+ }
/* Now move to TO. */
int start_x = it.current_x;
@@ -11052,25 +11093,31 @@ window_text_pixel_size (Lisp_Object window, Lisp_Object from, Lisp_Object to,
bidi_unshelve_cache (itdata, false);
- return Fcons (make_fixnum (x - start_x), make_fixnum (y));
+ return (!vertical_offset
+ ? Fcons (make_fixnum (x - start_x), make_fixnum (y))
+ : list3i (x - start_x, y, start));
}
DEFUN ("window-text-pixel-size", Fwindow_text_pixel_size, Swindow_text_pixel_size, 0, 7, 0,
- doc: /* Return the size of the text of WINDOW's buffer in pixels.
-WINDOW must be a live window and defaults to the selected one. The
-return value is a cons of the maximum pixel-width of any text line
-and the pixel-height of all the text lines in the accessible portion
-of buffer text.
+ doc: /* Return the size of the text of WINDOW's buffer in
+pixels. WINDOW must be a live window and defaults to the selected
+one. The return value is a cons of the maximum pixel-width of any
+text line and the pixel-height of all the text lines in the accessible
+portion of buffer text or a list of the maximum pixel-width,
+pixel-height, and the buffer position of the line at FROM.
This function exists to allow Lisp programs to adjust the dimensions
of WINDOW to the buffer text it needs to display.
The optional argument FROM, if non-nil, specifies the first text
position to consider, and defaults to the minimum accessible position
-of the buffer. If FROM is t, it stands for the minimum accessible
-position that starts a non-empty line. TO, if non-nil, specifies the
-last text position and defaults to the maximum accessible position of
-the buffer. If TO is t, it stands for the maximum accessible position
+of the buffer. If FROM is a cons, the cdr specifies the amount of
+pixels above or below a buffer position to begin measuring text, and
+the car specifies the buffer position. In that case, a list is
+returned. If FROM is t, it stands for the minimum accessible position
+that starts a non-empty line. TO, if non-nil, specifies the last text
+position and defaults to the maximum accessible position of the
+buffer. If TO is t, it stands for the maximum accessible position
that ends a non-empty line.
The optional argument X-LIMIT, if non-nil, specifies the maximum X
--
2.33.1
^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-23 1:30 ` Po Lu
@ 2021-12-23 9:49 ` Eli Zaretskii
2021-12-23 10:29 ` Po Lu
0 siblings, 1 reply; 53+ messages in thread
From: Eli Zaretskii @ 2021-12-23 9:49 UTC (permalink / raw)
To: Po Lu; +Cc: emacs-devel
> From: Po Lu <luangruo@yahoo.com>
> Cc: emacs-devel@gnu.org
> Date: Thu, 23 Dec 2021 09:30:34 +0800
>
> > Also, here's a patch with some of the changes made. I tried with the
> > documentation as well, but I'm pretty bad with that stuff.
>
> I tried updating the documentation again to better fit what's actually
> done (including documenting that measurements always begin at the
> beginning of the visual line).
>
> Please take a look and see if it's OK to install this now, thanks.
Does the code DTRT in your use cases, and is it OK performance-wise to
consider it stable enough for master?
> * src/xdisp.c (window_text_pixel_size): Understand a special
> format of `from' that specifies the amount of pixels above a
> position. ^^^^^
"above or below". Or maybe just "at a given pixel offset from".
> diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
> index 98a15404f9..7e6ee59824 100644
> --- a/doc/lispref/display.texi
> +++ b/doc/lispref/display.texi
> @@ -2090,19 +2090,23 @@ Size of Displayed Text
> This function returns the size of the text of @var{window}'s buffer in
> pixels. @var{window} must be a live window and defaults to the
> selected one. The return value is a cons of the maximum pixel-width
> -of any text line and the maximum pixel-height of all text lines. This
> -function exists to allow Lisp programs to adjust the dimensions of
> -@var{window} to the buffer text it needs to display.
> +of any text line and the maximum pixel-height of all text lines, or if
> +@var{from} is a cons, a list of the pixel-width, pixel-height, and the
> +buffer position of the first line that was measured.
This sentence now becomes too long and thus confusing. I suggest to
leave the original sentence alone, and add a new sentence, something
like
The return value can also optionally (see below) include the buffer
position of the first line whose dimensions were measured.
> This function
> +exists to allow Lisp programs to adjust the dimensions of @var{window}
> +to the buffer text it needs to display.
I would add "and for other similar situations" to this sentence.
> +character. If @var{from} is a cons, the cdr specifies a position, and
> +the car specifies the minimum amount of pixels above or below the
> +beginning of the visual line at that position to start measuring from.
First, I think you swapped the car and cdr.
Second, I would suggest to simplify/clarify:
... its @code{car} specifies a buffer position, and its @code{car}
specifies the vertical offset in pixels from that position to the
first screen line whose text is to be measured. (The measurement
will start from the visual beginning of that screen line.)
And finally, please add here the detailed description of the return
value, both when FROM is a position and when it's a cons cell. Since
the forms of FROM are already described, such a description of the
return value here will be less confusing.
> ++++
> +** 'window-text-pixel-size' can now measure from a set amount of pixels above a position.
This is too long for a NEWS heading. Can you make it shorter, leaving
the details for the body? I would also add more explanations, because
this sounds very vague.
> + else if (CONSP (from))
> + {
> + start = clip_to_bounds (BEGV, fix_position (XCAR (from)), ZV);
> + bpos = CHAR_TO_BYTE (start);
> + CHECK_FIXNUM (XCDR (from));
> + vertical_offset = XFIXNUM (XCDR (from));
In principle, the offset could be a bignum or even a float, so the
restriction here seems to be too stringent, no? A float in particular
could be useful, I think, in case the caller calculated the pixels as
a float.
> + if (vertical_offset != 0)
> + {
> + int last_y;
> + it.current_y = 0;
> +
> + move_it_by_lines (&it, 0);
> +
> + if (vertical_offset < 0)
> + {
> + while (it.current_y > vertical_offset)
> + {
> + last_y = it.current_y;
> + move_it_vertically_backward (&it,
> + (abs (vertical_offset)
> + + it.current_y));
> +
> + if (it.current_y == last_y)
> + break;
> + }
> + }
> + else
> + {
> + move_it_vertically (&it, vertical_offset);
> + }
> +
Can you add a comment explaining why in the negative case you call
move_it_vertically_backward in a loop, while in the positive case you
call move_it_vertically just once?
> DEFUN ("window-text-pixel-size", Fwindow_text_pixel_size, Swindow_text_pixel_size, 0, 7, 0,
> - doc: /* Return the size of the text of WINDOW's buffer in pixels.
> -WINDOW must be a live window and defaults to the selected one. The
> -return value is a cons of the maximum pixel-width of any text line
> -and the pixel-height of all the text lines in the accessible portion
> -of buffer text.
> + doc: /* Return the size of the text of WINDOW's buffer in
> +pixels. WINDOW must be a live window and defaults to the selected
The first line must be a complete sentence, so your reformatting of it
is a step backward.
> +one. The return value is a cons of the maximum pixel-width of any
> +text line and the pixel-height of all the text lines in the accessible
> +portion of buffer text or a list of the maximum pixel-width,
> +pixel-height, and the buffer position of the line at FROM.
Here, too, I would suggest to first say what is the form of the return
value in the "usual" case, and then add a separate sentence like
If FROM is a cons cell, the return value includes, in addition to
the dimensions, also a third element that provides the buffer
position from which measuring of the text dimensions was actually
started.
> +of the buffer. If FROM is a cons, the cdr specifies the amount of
> +pixels above or below a buffer position to begin measuring text, and
> +the car specifies the buffer position.
Consider rewording like I suggested for the manual.
> In that case, a list is
> +returned.
This was already said above, so it's redundant.
Thanks.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-23 9:49 ` Eli Zaretskii
@ 2021-12-23 10:29 ` Po Lu
2021-12-23 10:39 ` Eli Zaretskii
0 siblings, 1 reply; 53+ messages in thread
From: Po Lu @ 2021-12-23 10:29 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: emacs-devel
Eli Zaretskii <eliz@gnu.org> writes:
> Does the code DTRT in your use cases, and is it OK performance-wise to
> consider it stable enough for master?
Thanks, I fixed the other issues you pointed out, and yes, it works very
well.
> In principle, the offset could be a bignum or even a float, so the
> restriction here seems to be too stringent, no? A float in particular
> could be useful, I think, in case the caller calculated the pixels as
> a float.
I don't think that's consistent with everywhere else we accept a pixel
value (for example, posn-at-x-y).
Thanks.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-23 10:29 ` Po Lu
@ 2021-12-23 10:39 ` Eli Zaretskii
2021-12-23 10:42 ` Po Lu
0 siblings, 1 reply; 53+ messages in thread
From: Eli Zaretskii @ 2021-12-23 10:39 UTC (permalink / raw)
To: Po Lu; +Cc: emacs-devel
> From: Po Lu <luangruo@yahoo.com>
> Cc: emacs-devel@gnu.org
> Date: Thu, 23 Dec 2021 18:29:24 +0800
>
> > In principle, the offset could be a bignum or even a float, so the
> > restriction here seems to be too stringent, no? A float in particular
> > could be useful, I think, in case the caller calculated the pixels as
> > a float.
>
> I don't think that's consistent with everywhere else we accept a pixel
> value (for example, posn-at-x-y).
So you'd rather have the caller convert the value? I won't insist,
but the price of supporting float values is very low.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-23 10:39 ` Eli Zaretskii
@ 2021-12-23 10:42 ` Po Lu
2021-12-23 10:50 ` Eli Zaretskii
0 siblings, 1 reply; 53+ messages in thread
From: Po Lu @ 2021-12-23 10:42 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: emacs-devel
[-- Attachment #1: Type: text/plain, Size: 350 bytes --]
Eli Zaretskii <eliz@gnu.org> writes:
> So you'd rather have the caller convert the value?
Yeah, I think that would be more consistent. Bignum deltas will
probably be rare, and it is IMHO always better to manually coerce
floating point values.
Anyway, I would like to install this change now. If that's fine by you,
please let me know. Thanks.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Allow-window-text-pixel-size-to-measure-pixels-aroun.patch --]
[-- Type: text/x-patch, Size: 11056 bytes --]
From c7f430262f1698cdabbb7f130b8cab9c3f066e45 Mon Sep 17 00:00:00 2001
From: Po Lu <luangruo@yahoo.com>
Date: Thu, 23 Dec 2021 09:12:02 +0800
Subject: [PATCH] Allow window-text-pixel-size to measure pixels around a
position
* doc/lispref/display.texi (Size of Displayed Text): Announce
new meaning of `from'.
* etc/NEWS: Announce changes.
* lisp/pixel-scroll.el (pixel-scroll-precision-scroll-up-page):
Use new feature.
* src/xdisp.c (window_text_pixel_size): Understand a special
format of `from' that specifies the amount of pixels above or
below a position.
(Fwindow_text_pixel_size): Update doc string.
---
doc/lispref/display.texi | 22 +++++++---
etc/NEWS | 5 +++
lisp/pixel-scroll.el | 34 +++++++--------
src/xdisp.c | 89 ++++++++++++++++++++++++++++++++--------
4 files changed, 110 insertions(+), 40 deletions(-)
diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
index 98a15404f9..449a58a3bb 100644
--- a/doc/lispref/display.texi
+++ b/doc/lispref/display.texi
@@ -2092,17 +2092,27 @@ Size of Displayed Text
selected one. The return value is a cons of the maximum pixel-width
of any text line and the maximum pixel-height of all text lines. This
function exists to allow Lisp programs to adjust the dimensions of
-@var{window} to the buffer text it needs to display.
+@var{window} to the buffer text it needs to display, and for other
+similar situations.
+
+The return value can also optionally (see below) include the buffer
+position of the first line whose dimensions were measured.
The optional argument @var{from}, if non-@code{nil}, specifies the
first text position to consider, and defaults to the minimum
accessible position of the buffer. If @var{from} is @code{t}, it
stands for the minimum accessible position that is not a newline
-character. The optional argument @var{to}, if non-@code{nil},
-specifies the last text position to consider, and defaults to the
-maximum accessible position of the buffer. If @var{to} is @code{t},
-it stands for the maximum accessible position that is not a newline
-character.
+character. If @var{from} is a cons, its @code{car} specifies a buffer
+position, and its @code{cdr} specifies the vertical offset in pixels
+from that position to the first screen line whose text is to be
+measured. (The measurement will start from the visual beginning of
+that screen line.) In that case, the return value will instead be a
+list of the pixel-width, pixel-height, and the buffer position of the
+first line that was measured. The optional argument @var{to}, if
+non-@code{nil}, specifies the last text position to consider, and
+defaults to the maximum accessible position of the buffer. If
+@var{to} is @code{t}, it stands for the maximum accessible position
+that is not a newline character.
The optional argument @var{x-limit}, if non-@code{nil}, specifies the
maximum X coordinate beyond which text should be ignored; it is
diff --git a/etc/NEWS b/etc/NEWS
index 948dbba261..583ed11b93 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1020,6 +1020,11 @@ This controls whether or not the last screen line of the text being
measured will be counted for the purpose of calculating the text
dimensions.
++++
+** 'window-text-pixel-size' understands a new meaning of 'from'.
+Specifying a cons as the from argument allows to start measuring text
+from a specified amount of pixels above a position.
+
** XDG support
*** New function 'xdg-state-home' returns 'XDG_STATE_HOME' environment variable.
diff --git a/lisp/pixel-scroll.el b/lisp/pixel-scroll.el
index fa0185b16e..4aae166f2e 100644
--- a/lisp/pixel-scroll.el
+++ b/lisp/pixel-scroll.el
@@ -516,22 +516,24 @@ pixel-scroll-precision-scroll-up-page
usable-height))))
(goto-char up-point)))
(let ((current-vscroll (window-vscroll nil t)))
- (if (<= delta current-vscroll)
- (set-window-vscroll nil (- current-vscroll delta) t)
- (setq delta (- delta current-vscroll))
- (set-window-vscroll nil 0 t)
- (while (> delta 0)
- (let ((position (pixel-point-and-height-at-unseen-line)))
- (unless (cdr position)
- (signal 'beginning-of-buffer nil))
- (set-window-start nil (car position) t)
- ;; If the line above is taller than the window height (i.e. there's
- ;; a very tall image), keep point on it.
- (when (> (cdr position) usable-height)
- (goto-char (car position)))
- (setq delta (- delta (cdr position)))))
- (when (< delta 0)
- (set-window-vscroll nil (- delta) t))))))
+ (setq delta (- delta current-vscroll))
+ (set-window-vscroll nil 0 t)
+ (when (> delta 0)
+ (let* ((start (window-start))
+ (dims (window-text-pixel-size nil (cons start (- delta))
+ start nil nil nil t))
+ (height (nth 1 dims))
+ (position (nth 2 dims)))
+ (set-window-start nil position t)
+ ;; If the line above is taller than the window height (i.e. there's
+ ;; a very tall image), keep point on it.
+ (when (> height usable-height)
+ (goto-char position))
+ (when (or (not position) (eq position start))
+ (signal 'beginning-of-buffer nil))
+ (setq delta (- delta height))))
+ (when (< delta 0)
+ (set-window-vscroll nil (- delta) t)))))
(defun pixel-scroll-precision-interpolate (delta)
"Interpolate a scroll of DELTA pixels.
diff --git a/src/xdisp.c b/src/xdisp.c
index 1f896c256e..a6c122aee8 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -10841,8 +10841,7 @@ window_text_pixel_size (Lisp_Object window, Lisp_Object from, Lisp_Object to,
ptrdiff_t start, end, bpos;
struct text_pos startp;
void *itdata = NULL;
- int c, max_x = 0, max_y = 0, x = 0, y = 0;
- int doff = 0;
+ int c, max_x = 0, max_y = 0, x = 0, y = 0, vertical_offset = 0, doff = 0;
if (NILP (from))
{
@@ -10868,6 +10867,13 @@ window_text_pixel_size (Lisp_Object window, Lisp_Object from, Lisp_Object to,
break;
}
}
+ else if (CONSP (from))
+ {
+ start = clip_to_bounds (BEGV, fix_position (XCAR (from)), ZV);
+ bpos = CHAR_TO_BYTE (start);
+ CHECK_FIXNUM (XCDR (from));
+ vertical_offset = XFIXNUM (XCDR (from));
+ }
else
{
start = clip_to_bounds (BEGV, fix_position (from), ZV);
@@ -10914,7 +10920,9 @@ window_text_pixel_size (Lisp_Object window, Lisp_Object from, Lisp_Object to,
itdata = bidi_shelve_cache ();
start_display (&it, w, startp);
+
int start_y = it.current_y;
+
/* It makes no sense to measure dimensions of region of text that
crosses the point where bidi reordering changes scan direction.
By using unidirectional movement here we at least support the use
@@ -10923,13 +10931,50 @@ window_text_pixel_size (Lisp_Object window, Lisp_Object from, Lisp_Object to,
same directionality. */
it.bidi_p = false;
- /* Start at the beginning of the line containing FROM. Otherwise
- IT.current_x will be incorrectly set to zero at some arbitrary
- non-zero X coordinate. */
- reseat_at_previous_visible_line_start (&it);
- it.current_x = it.hpos = 0;
- if (IT_CHARPOS (it) != start)
- move_it_to (&it, start, -1, -1, -1, MOVE_TO_POS);
+ if (vertical_offset != 0)
+ {
+ int last_y;
+ it.current_y = 0;
+
+ move_it_by_lines (&it, 0);
+
+ /* `move_it_vertically_backward' is called by move_it_vertically
+ to move by a negative value (upwards), but it is not always
+ guaranteed to leave the iterator at or above the position
+ given by the offset, which this loop ensures. */
+ if (vertical_offset < 0)
+ {
+ while (it.current_y > vertical_offset)
+ {
+ last_y = it.current_y;
+ move_it_vertically_backward (&it,
+ (abs (vertical_offset)
+ + it.current_y));
+
+ if (it.current_y == last_y)
+ break;
+ }
+ }
+ else
+ {
+ move_it_vertically (&it, vertical_offset);
+ }
+
+ it.current_y = (WINDOW_TAB_LINE_HEIGHT (w)
+ + WINDOW_HEADER_LINE_HEIGHT (w));
+ start = clip_to_bounds (BEGV, IT_CHARPOS (it), ZV);
+ start_y = it.current_y;
+ }
+ else
+ {
+ /* Start at the beginning of the line containing FROM. Otherwise
+ IT.current_x will be incorrectly set to zero at some arbitrary
+ non-zero X coordinate. */
+ reseat_at_previous_visible_line_start (&it);
+ it.current_x = it.hpos = 0;
+ if (IT_CHARPOS (it) != start)
+ move_it_to (&it, start, -1, -1, -1, MOVE_TO_POS);
+ }
/* Now move to TO. */
int start_x = it.current_x;
@@ -11052,26 +11097,34 @@ window_text_pixel_size (Lisp_Object window, Lisp_Object from, Lisp_Object to,
bidi_unshelve_cache (itdata, false);
- return Fcons (make_fixnum (x - start_x), make_fixnum (y));
+ return (!vertical_offset
+ ? Fcons (make_fixnum (x - start_x), make_fixnum (y))
+ : list3i (x - start_x, y, start));
}
DEFUN ("window-text-pixel-size", Fwindow_text_pixel_size, Swindow_text_pixel_size, 0, 7, 0,
doc: /* Return the size of the text of WINDOW's buffer in pixels.
WINDOW must be a live window and defaults to the selected one. The
-return value is a cons of the maximum pixel-width of any text line
-and the pixel-height of all the text lines in the accessible portion
-of buffer text.
+return value is a cons of the maximum pixel-width of any text line and
+the pixel-height of all the text lines in the accessible portion of
+buffer text.
+
+If FROM is a cons cell, the return value includes, in addition to the
+dimensions, also a third element that provides the buffer position
+from which measuring of the text dimensions was actually started.
This function exists to allow Lisp programs to adjust the dimensions
of WINDOW to the buffer text it needs to display.
The optional argument FROM, if non-nil, specifies the first text
position to consider, and defaults to the minimum accessible position
-of the buffer. If FROM is t, it stands for the minimum accessible
-position that starts a non-empty line. TO, if non-nil, specifies the
-last text position and defaults to the maximum accessible position of
-the buffer. If TO is t, it stands for the maximum accessible position
-that ends a non-empty line.
+of the buffer. If FROM is a cons, its car specifies a buffer
+position, and its cdr specifies the vertical offset in pixels from
+that position to the first screen line to be measured. If FROM is t,
+it stands for the minimum accessible position that starts a non-empty
+line. TO, if non-nil, specifies the last text position and defaults
+to the maximum accessible position of the buffer. If TO is t, it
+stands for the maximum accessible position that ends a non-empty line.
The optional argument X-LIMIT, if non-nil, specifies the maximum X
coordinate beyond which the text should be ignored. It is therefore
--
2.33.1
^ permalink raw reply related [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-23 10:42 ` Po Lu
@ 2021-12-23 10:50 ` Eli Zaretskii
2021-12-23 10:55 ` Po Lu
0 siblings, 1 reply; 53+ messages in thread
From: Eli Zaretskii @ 2021-12-23 10:50 UTC (permalink / raw)
To: Po Lu; +Cc: emacs-devel
> From: Po Lu <luangruo@yahoo.com>
> Cc: emacs-devel@gnu.org
> Date: Thu, 23 Dec 2021 18:42:32 +0800
>
> Anyway, I would like to install this change now. If that's fine by you,
> please let me know. Thanks.
Fine with me, after fixing the gotcha below.
> ++++
> +** 'window-text-pixel-size' understands a new meaning of 'from'.
> +Specifying a cons as the from argument allows to start measuring text
> +from a specified amount of pixels above a position.
^^^^^
"above or below"
Thanks.
^ permalink raw reply [flat|nested] 53+ messages in thread
* Re: move_it_vertically_backward question
2021-12-23 10:50 ` Eli Zaretskii
@ 2021-12-23 10:55 ` Po Lu
0 siblings, 0 replies; 53+ messages in thread
From: Po Lu @ 2021-12-23 10:55 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: emacs-devel
Eli Zaretskii <eliz@gnu.org> writes:
>> From: Po Lu <luangruo@yahoo.com>
>> Cc: emacs-devel@gnu.org
>> Date: Thu, 23 Dec 2021 18:42:32 +0800
>>
>> Anyway, I would like to install this change now. If that's fine by you,
>> please let me know. Thanks.
>
> Fine with me, after fixing the gotcha below.
>
>> ++++
>> +** 'window-text-pixel-size' understands a new meaning of 'from'.
>> +Specifying a cons as the from argument allows to start measuring text
>> +from a specified amount of pixels above a position.
> ^^^^^
> "above or below"
>
> Thanks.
Thanks, done.
^ permalink raw reply [flat|nested] 53+ messages in thread
end of thread, other threads:[~2021-12-23 10:55 UTC | newest]
Thread overview: 53+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <87lf0pw78r.fsf.ref@yahoo.com>
2021-12-13 2:47 ` move_it_vertically_backward question Po Lu
2021-12-13 14:50 ` Eli Zaretskii
2021-12-14 0:53 ` Po Lu
2021-12-14 12:52 ` Eli Zaretskii
2021-12-14 13:28 ` Po Lu
2021-12-14 13:45 ` Eli Zaretskii
2021-12-15 1:18 ` Po Lu
2021-12-15 3:27 ` Eli Zaretskii
2021-12-15 3:30 ` Po Lu
2021-12-15 13:27 ` Eli Zaretskii
2021-12-15 13:39 ` Po Lu
2021-12-15 2:13 ` Po Lu
2021-12-15 10:28 ` Po Lu
2021-12-15 13:56 ` Eli Zaretskii
2021-12-15 13:25 ` Eli Zaretskii
2021-12-15 13:38 ` Po Lu
2021-12-15 14:50 ` Eli Zaretskii
2021-12-16 0:41 ` Po Lu
2021-12-16 8:29 ` Eli Zaretskii
2021-12-16 9:25 ` Po Lu
2021-12-16 10:04 ` Eli Zaretskii
2021-12-16 10:27 ` Po Lu
2021-12-16 12:17 ` Po Lu
2021-12-16 13:27 ` Eli Zaretskii
2021-12-16 13:34 ` Po Lu
2021-12-16 13:59 ` Eli Zaretskii
2021-12-17 1:45 ` Po Lu
2021-12-18 10:28 ` Eli Zaretskii
2021-12-18 10:49 ` Po Lu
2021-12-18 11:03 ` Eli Zaretskii
2021-12-18 11:18 ` Po Lu
2021-12-18 11:29 ` Eli Zaretskii
2021-12-18 11:31 ` Po Lu
2021-12-18 11:35 ` Eli Zaretskii
2021-12-18 11:39 ` Po Lu
2021-12-19 0:54 ` Po Lu
2021-12-19 8:29 ` Eli Zaretskii
2021-12-19 9:16 ` Po Lu
2021-12-19 9:27 ` Eli Zaretskii
2021-12-19 10:25 ` Po Lu
2021-12-19 18:07 ` Eli Zaretskii
2021-12-20 1:05 ` Po Lu
2021-12-21 12:58 ` Po Lu
2021-12-21 17:07 ` Eli Zaretskii
2021-12-22 0:49 ` Po Lu
2021-12-22 14:59 ` Eli Zaretskii
2021-12-23 1:30 ` Po Lu
2021-12-23 9:49 ` Eli Zaretskii
2021-12-23 10:29 ` Po Lu
2021-12-23 10:39 ` Eli Zaretskii
2021-12-23 10:42 ` Po Lu
2021-12-23 10:50 ` Eli Zaretskii
2021-12-23 10:55 ` Po Lu
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.