acme: when selecting a longer text passage, scrolling downwards is not always working

[knieriem]

If I try to select a longer passage in an acme window that extends to the lower edge of the editor window, I'm over the years observing that sometimes scrolling down -- to select further text -- is not possible. When reaching the lower edge of the window with the mouse button, nothing happens. I can see the effect in full-screen mode, but also when acme only covers part of the screen.

It appears that it depends whether there is a small extra vertical space present at the bottom of a window, which varies with the arrangement of acme windows. The exact size of that small extra space is the remainder of the division of the window body's height by the font height, in pixel rows. If the remainder is zero, as in the image below, there will be no extra space, and scrolling down while selecting won't work:

If, instead, the remainder is non-zero, there will be an extra space of one up to font height minus one pixel rows. In the following image that space at the bottom of the editor window is displayed in black color (instead of window background) for better visibility. In this situation, scrolling down while selecting text works:

This effect sometimes makes copying larger chunks of text a bit awkward for me, as I have to rearrange acme windows first, hoping that the small extra space at the bottom of the window I'm interested in changes from zero to non-zero.

A naive approach could be that, if the window body's height is reduced by one pixel row in case the window is located at the bottom of the editor window AND the remainder described above is zero, scrolling down when selecting text should be possible in all cases:

--- a/src/cmd/acme/wind.c
+++ b/src/cmd/acme/wind.c
@@ -179,7 +179,7 @@ wintaglines(Window *w, Rectangle r)
 int
 winresize(Window *w, Rectangle r, int safe, int keepextra)
 {
-	int oy, y, mouseintag, mouseinbody;
+	int oy, y, mouseintag, mouseinbody, dybody, dybotxtra;
 	Point p;
 	Rectangle r1;
 
@@ -227,13 +227,34 @@ winresize(Window *w, Rectangle r, int safe, int keepextra)
 	r1.min.y = y;
 	if(!safe || !eqrect(w->body.all, r1)){
 		oy = y;
-		if(y+1+w->body.fr.font->height <= r.max.y){	/* room for one line */
+		dybody = r.max.y - (y+1);
+
+		/* If the window's rectangle's bottom line coincides with the
+		 * bottom of screen->r, and if an even number of text lines
+		 * fit into the body's height without leaving a remainder,
+		 * setting dybotxtra to 1 will reduce the body by one line,
+		 * which results in a thin bottom border line. This way
+		 * scrolling down while selecting remains possible.
+		 */
+		dybotxtra = 0;
+		if(screen->r.max.y == r.max.y){
+			if(dybody > 0)
+			if(dybody % w->body.fr.font->height == 0){
+				dybotxtra = 1;
+
+				/* draw the extra line in background color */
+				r1.min.y = r.max.y - dybotxtra;
+				r1.max.y = r.max.y;
+				draw(screen, r1, textcols[BACK], nil, ZP);
+			}
+		}
+		if(dybotxtra+w->body.fr.font->height <= dybody){	/* room for one line */
 			r1.min.y = y;
 			r1.max.y = y+1;
 			draw(screen, r1, tagcols[BORD], nil, ZP);
 			y++;
-			r1.min.y = min(y, r.max.y);
-			r1.max.y = r.max.y;
+			r1.max.y = r.max.y - dybotxtra;
+			r1.min.y = min(y, r1.max.y);
 		}else{
 			r1.min.y = y;
 			r1.max.y = y;

This change fixes the problem for me. Is there an easier way, or more suitable place than winresize() to change the described behaviour?

The patch adds a variable dybotxtra (extra vertical space at the bottom) which is zero in most cases, but one, in case an extra space of one pixel row has to be reserved from the window's body. The if condition in the line marked by the comment room for one line is only slighly adjusted -- by introducing dybody, which is used at multiple places, and by taking account of the value of dybotxtra.

---

It may be helpful, during debugging, to change the color of the extra vertical space -- not the single line introduced by me, but the normal remainder-sized area that is drawn in textresize() --, by setting it to a color different from the normal acme window background, like this:

--- a/src/cmd/acme/text.c
+++ b/src/cmd/acme/text.c
@@ -92,7 +92,11 @@ textresize(Text *t, Rectangle r, int keepextra)
 		r.min.x -= Scrollgap;
 		r.min.y = t->fr.r.max.y;
 		r.max.y = t->all.max.y;
-		draw(screen, r, t->fr.cols[BACK], nil, ZP);
+		/* Using but3col instead of t->fr.cols[BACK] for debug
+		 * purposes only, so that the extra free lines can be
+		 * better distiguished from normal background.
+		 */
+		draw(screen, r, but3col, nil, ZP);
 	}
 	return t->all.max.y;
 }

[knieriem]

A disadvantage of the approach above is that in case lines of text would have fitted into a window's body evenly before, i.e. without a remaining small space at the bottom, after applying the patch this would not be possible anymore: By reserving an extra pixel row (dybotxtra = 1) to enable scrolling, the last text line, that previously would have fitted into the body, is left out now because the vertical space was reduced by one - the reserved - pixel row. Instead, now there would be a quite large space of font-height minus one rows, which is visually less appealing.

Perhaps a more cleaner solution is not to reserve/consume a pixel row from the window's body, but instead to make sure scrolling down is activated one pixel row earlier (not at r.max.y+1, but at r.max.y).
Text selection, and scrolling-while-selecting, is done in text.c:textselect(), and libframe/frselect.c:frselect(). The following change, as an alternative to the one above, appears to fix the problem as well, by starting scroll-down one pixel row above:

--- a/src/libframe/frselect.c
+++ b/src/libframe/frselect.c
@@ -42,7 +42,7 @@ frselect(Frame *f, Mousectl *mc)	/* when called, button 1 is down */
 				p0 = f->p1;
 				p1 = f->p0;
 				scrled = 1;
-			}else if(mp.y > f->r.max.y){
+			}else if(mp.y >= f->r.max.y){
 				(*f->scroll)(f, (mp.y-f->r.max.y)/(int)f->font->height+1);
 				p0 = f->p0;
 				p1 = f->p1;

Since, by convention, the right and bottom edges (r.max.x and .y) are excluded from the represented rectangle, I wonder if applying >= instead of > wouldn't also be more correct, since mp.y == f->r.max.y means mouse pointer one pixel row below the rectangle, while mp.y > f->r.max.y would mean mouse ptr two pixel rows below the rectangle, if I'm not mistaken. Also, a comparison of the mouse pointer's y value >= r.max.y appears to match the comparison < r.min.y better than > r.max.y would, given that r.max.y refers to the row below the rectangle.

Edit: As it turns out, the patch doesn't fix the issue when acme is run in fullscreen, or in maximized window mode: In these cases there is no pixel row below an acme window at the bottom, the window body directly touches the bottom of the monitor screen. Therefore the mouse pointer doesn't get far enough below to fulfil the mp.y >= r.max.y condition. The if condition would need to be changed to mp.y >= r.max.y-1 to make scrolling-down-while-selecting work in maximized editor window mode too.