Re: Truncating scroll runs that copy to where we copied to

[YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>]

>>>>> On Sun, 20 Nov 2011 16:13:59 +0900, YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp> said:

> I think that `scrolling_window' needs to truncate scroll runs that
> copy to where we copied to; otherwise, `assign_row (to, from)'
> assigns a previously disabled bogus row in the desired matrix when
> we have an overlap in the copy destination.  Such truncation can
> also avoid unnecessary copy in the actual graphics operation.

> Could someone double-check the code below?

I found a few problems with the code myself:

* The truncation should also be done when r->current_y == r->desired_y,
  because `assign_row (to, from)' is executed for this case.

* The array `runs' should be kept sorted by copied pixel lines even
  after the truncation.

* The condition for the invalidation

		if ((p->current_y >= r->desired_y
		     && p->current_y < r->desired_y + r->height)
		    || (p->current_y + p->height >= r->desired_y
			&& (p->current_y + p->height
			    < r->desired_y + r->height)))

  in the current code is slightly wrong (off-by-one in 2 places), and
  the corrected one can be simplified under the condition that `runs'
  is sorted (see the comment in the code below).

Below is a revised patch.

				     YAMAMOTO Mitsuharu
				mituharu@math.s.chiba-u.ac.jp

=== modified file 'src/dispnew.c'
*** src/dispnew.c	2011-11-19 08:39:42 +0000
--- src/dispnew.c	2011-11-22 00:19:27 +0000
***************
*** 4551,4568 ****
  	  {
  	    rif->clear_window_mouse_face (w);
  	    rif->scroll_run_hook (w, r);
  
! 	    /* Invalidate runs that copy from where we copied to.  */
! 	    for (j = i + 1; j < nruns; ++j)
  	      {
! 		struct run *p = runs[j];
  
! 		if ((p->current_y >= r->desired_y
  		     && p->current_y < r->desired_y + r->height)
! 		    || (p->current_y + p->height >= r->desired_y
  			&& (p->current_y + p->height
! 			    < r->desired_y + r->height)))
! 		  p->nrows = 0;
  	      }
  	  }
  
--- 4551,4619 ----
  	  {
  	    rif->clear_window_mouse_face (w);
  	    rif->scroll_run_hook (w, r);
+ 	  }
+ 
+ 	/* Truncate runs that copy to where we copied to, and
+ 	   invalidate runs that copy from where we copied to.  */
+ 	for (j = nruns - 1; j > i; --j)
+ 	  {
+ 	    struct run *p = runs[j];
+ 	    int truncated_p = 0;
  
! 	    if (p->nrows > 0
! 		&& p->desired_y < r->desired_y + r->height
! 		&& p->desired_y + p->height > r->desired_y)
  	      {
! 		if (p->desired_y < r->desired_y)
! 		  {
! 		    p->nrows = r->desired_vpos - p->desired_vpos;
! 		    p->height = r->desired_y - p->desired_y;
! 		    truncated_p = 1;
! 		  }
! 		else
! 		  {
! 		    int nrows_copied = (r->desired_vpos + r->nrows
! 					- p->desired_vpos);
! 
! 		    if (p->nrows <= nrows_copied)
! 		      p->nrows = 0;
! 		    else
! 		      {
! 			int height_copied = (r->desired_y + r->height
! 					     - p->desired_y);
! 
! 			p->current_vpos += nrows_copied;
! 			p->desired_vpos += nrows_copied;
! 			p->nrows -= nrows_copied;
! 			p->current_y += height_copied;
! 			p->desired_y += height_copied;
! 			p->height -= height_copied;
! 			truncated_p = 1;
! 		      }
! 		  }
! 	      }
  
! 	    if (r->current_y != r->desired_y
! 		/* The condition below is equivalent to
! 		   ((p->current_y >= r->desired_y
  		     && p->current_y < r->desired_y + r->height)
! 		    || (p->current_y + p->height > r->desired_y
  			&& (p->current_y + p->height
! 			    <= r->desired_y + r->height)))
! 		   because we have 0 < p->height <= r->height.  */
! 		&& p->current_y < r->desired_y + r->height
! 		&& p->current_y + p->height > r->desired_y)
! 	      p->nrows = 0;
! 
! 	    /* Reorder runs by copied pixel lines if truncated.  */
! 	    if (truncated_p && p->nrows > 0)
! 	      {
! 		int k = nruns - 1;
! 
! 		while (runs[k]->nrows == 0 || runs[k]->height < p->height)
! 		  k--;
! 		memmove (runs + j, runs + j + 1, (k - j) * sizeof (*runs));
! 		runs[k] = p;
  	      }
  	  }