* 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-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 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: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-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 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 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 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 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: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 public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).