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

[Eli Zaretskii <eliz@gnu.org>]

> Date: Tue, 22 Nov 2011 16:26:41 +0900
> From: YAMAMOTO Mitsuharu <mituharu@math.s.chiba-u.ac.jp>
> Cc: David Reitter <david.reitter@gmail.com>,
> 	emacs-devel@gnu.org
> 
> Actually, I told what was wrong with the current code in my first
> post:

I was too stupid to glean the details from that, sorry.  The details
you provided now were the missing link, thanks.

> 	/* Assign matrix rows.  */
> 	for (j = 0; j < r->nrows; ++j)
> 	  {
> 	    struct glyph_row *from, *to;
> 	    int to_overlapped_p;
> 
> 	    to = MATRIX_ROW (current_matrix, r->desired_vpos + j);
> 	    from = MATRIX_ROW (desired_matrix, r->desired_vpos + j);
> 	    to_overlapped_p = to->overlapped_p;
> 	    from->redraw_fringe_bitmaps_p = from->fringe_bitmap_periodic_p;
> 	    assign_row (to, from);
> 	    to->enabled_p = 1, from->enabled_p = 0;
> 	    to->overlapped_p = to_overlapped_p;
> 	  }
> 
> If there's an overlap (I don't mean the variable to_overlapped_p) in
> the copy destinations of two runs, then `from' row in the desired
> matrix becomes a bogus one that had been in the current matrix after
> processing the first run (because assign_row actually does swap), and
> that row is disabled by `from->enabled_p = 0'.  So, when processing
> the second run, the `from' row is now the previously disabled bogus
> row and it is assigned to a row in the current matrix.

Are you saying that this loop was implemented under the assumption
that there's no overlap in the destinations?

Anyway, if the problem is that assign_row leaves the `from' row with
bogus glyph information (and I know it does, as I recently fixed an
assertion violation caused by that), then isn't the problem in
assign_row, to be fixed there?  Assignment as a concept does not imply
a change to the source in any way, so having a function called
assign_row that actually destroys the source means people will (and
do) introduce bugs when they use their mental model of assignment.

Alternatively, maybe we should assign only those rows that have their
enabled_p flag set?  Why would we even want to copy disabled glyph
rows?

Your changes are not here, but elsewhere, which makes me bother if we
are not dancing around the bug and sweeping the root cause under a
thick carpet.

Also, what about the unconditional setting of to->enabled_p to 1 in
the above loop, regardless of what was that flag in `from'?  Does it
look right to you?