Stefan Schimanski wrote:
Another possibility of course (which I proposed somewhere in the other
thread about the backspace case of Dov) is so redo the metrics of the
current paragraph before setting the current font again. But then I
guess they are computed twice, also during the paragraph update later.
Stefan
Am 02.06.2007 um 18:13 schrieb Stefan Schimanski:
Hi!
In http://bugzilla.lyx.org/show_bug.cgi?id=3790 a bug is reported that
LyX crashes when deleting a display math at the end of a paragraph. In
fact the same bug can also be triggered by backspace, enter to rebreak
the paragraph and probably many more ways.
The reason is that Text::setCursor (which is often called after some
user action on the text to update the cursor to the new position)
tries to set the current font by Text::setCurrentFont. For this the
correct paragraph metrics are needed and the RTL/LTR status of the
char behind or in front of the cursor.
The latter triggers a Bidi::computeTables call which goes through the
current line of the paragraph and sets the RTL value correctly. But
for that is depends on the Row object which is only valid if the
paragraph metrics are valid.
The crashes which can easily be triggered with RC1, come from the
Bidi::computeTables which uses that Row::endpos() information to know
the end of the row. But after actions like backspace this value is
wrong, hence invalid accesses are the consequence.
The easy fix for all that is this one:
Index: src/Bidi.cpp
===================================================================
--- src/Bidi.cpp (Revision 18631)
+++ src/Bidi.cpp (Arbeitskopie)
@@ -66,8 +66,11 @@
}
start_ = row.pos();
- end_ = row.endpos() - 1;
-
+
+ // possibly the row metrics are outdated and the endpos() was
computed
+ // before the paragraph was changed (e.g. by backspace)
+ end_ = std::min(par.size(), row.endpos()) - 1;
+
if (start_ > end_) {
start_ = -1;
return;
The paragraph knows the right size all the time because it is what the
action handlers (e.g. for backspace) modify.
In fact a clean solution would look differently IMO. Text::setCursor
should not update the font at all, but trigger some kind of update
(via the the Cursor's UpdateFlags) which then, after the new paragraph
metrics are calculated, sets the font at the cursors position. Not
sure though if this is something for intra-RC1-RC2 patches. Any opinions?
The crashes I encountered up to now can all be fixed the upper patch.
So I suggest to put that in for now.
Stefan
Hi!
I think your second solution (deferring the setting of the font to until
after the metrics are calculated) sounds best.
If that's too risky / too hard until after the 1.5.0 release, then I
prefer your previous solution, of forcing a recalculation of the metrics
before calling computeTables --- as you pointed out to me, even though
we'll be computing the metrics twice that way, it's probably negligible
next to the redrawing...
The suggestion of modifying computeTables itself seems a little hackish:
so why are you trusting the row for the start_ position? Maybe that's
not correct, either? Also, there are situations which I'm not sure that
the hack would solve: when we move from one row to the previous one ---
then obviously the paragraph size is not an issue, because we're not on
the last row of the paragraph --- but the row metrics are still
incorrect. (When we first noticed this problem a few weeks ago, I solved
it by adding the call to computeTables from within setCurrentFont, which
is now causing the problem; this could probably be removed if you redo
the metrics?).
Dov
