From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.devel Subject: Re: Truncating scroll runs that copy to where we copied to Date: Tue, 22 Nov 2011 03:52:30 -0500 Message-ID: References: <8339dibqlc.fsf@gnu.org> <6D358885-0CD8-4FC0-9598-549AB7E3D4AC@gmail.com> Reply-To: Eli Zaretskii NNTP-Posting-Host: lo.gmane.org X-Trace: dough.gmane.org 1321951964 14374 80.91.229.12 (22 Nov 2011 08:52:44 GMT) X-Complaints-To: usenet@dough.gmane.org NNTP-Posting-Date: Tue, 22 Nov 2011 08:52:44 +0000 (UTC) Cc: david.reitter@gmail.com, emacs-devel@gnu.org To: YAMAMOTO Mitsuharu Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Tue Nov 22 09:52:40 2011 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([140.186.70.17]) by lo.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1RSm5n-0006ph-1g for ged-emacs-devel@m.gmane.org; Tue, 22 Nov 2011 09:52:39 +0100 Original-Received: from localhost ([::1]:42326 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RSm5m-0003DF-K1 for ged-emacs-devel@m.gmane.org; Tue, 22 Nov 2011 03:52:38 -0500 Original-Received: from eggs.gnu.org ([140.186.70.92]:47889) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RSm5g-0002s5-Dm for emacs-devel@gnu.org; Tue, 22 Nov 2011 03:52:36 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RSm5f-00018z-6t for emacs-devel@gnu.org; Tue, 22 Nov 2011 03:52:32 -0500 Original-Received: from fencepost.gnu.org ([140.186.70.10]:45814) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RSm5e-00018v-P2 for emacs-devel@gnu.org; Tue, 22 Nov 2011 03:52:31 -0500 Original-Received: from eliz by fencepost.gnu.org with local (Exim 4.71) (envelope-from ) id 1RSm5e-0000hI-Ap; Tue, 22 Nov 2011 03:52:30 -0500 In-reply-to: (message from YAMAMOTO Mitsuharu on Tue, 22 Nov 2011 16:26:41 +0900) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3) X-Received-From: 140.186.70.10 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:146139 Archived-At: > Date: Tue, 22 Nov 2011 16:26:41 +0900 > From: YAMAMOTO Mitsuharu > Cc: David Reitter , > 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?