Hi!Before playing the patchwork game again in fixing the bugs in the subject and, without noticing, introducing a hand full new bugs, here are some patches which clean up the bidi mess:
The Bidi class mainly offers the isBoundary method whose use is spread over the LyX code. Unfortunately it needs a valid cache of the bidi boundaries. To produce that it needs a valid Row object, which in turn needs valid paragraph metrics which are not available after many cursor operations. Ignoring this fact leads to #3790, #3801 and #3809.
* So the only clean approach is to rip out all bidi references from the code and finally use the Font information of each position which also holds the RTL/LTR flag. This is done by nobidi.patch (see below).
* Then we can implement Text::setCurrentFont without any reference to Text::bidi and by that fix all the mentioned bugs, implemented in setcurrentfontnobidi.patch.
* Finally we should fix the spaces-around-RTL-segments-in-LTR- paragraphs-problem (and the symmetric case). While playing around with the cursor left/right on RTL/LTR boundaries, trying to write mixed bidi text and continuing after a boundary, I am very much convinced now that the already mentioned approach of rtlspaces.patch and rtlepgnobidi.patch is right thing to do. I will produce a small demonstration in another mail.
****************** setcurrentfontnobidi.patch ****************
Index: src/Text2.cpp
===================================================================
--- src/Text2.cpp (Revision 18674)
+++ src/Text2.cpp (Arbeitskopie)
@@ -11,6 +11,7 @@
* \author John Levon
* \author André Pönitz
* \author Allan Rae
+ * \author Stefan Schimanski
* \author Dekel Tsur
* \author Jürgen Vigna
*
@@ -760,42 +761,33 @@
pos_type pos = cur.pos();
Paragraph & par = cur.paragraph();
- // ignore empty paragraph
- if (par.empty())
- return;
-
- // if on boundary or at paragraph end, set font of previous char
- if ((pos > 0 && cur.boundary()) || pos == cur.lastpos())
+ // are we behind previous char in fact? -> go to that char
+ if (pos > 0 && cur.boundary())
--pos;
-
- // we changed the line and the bidi tables are outdated?
- if (!bidi.inRange(pos))
- bidi.computeTables(par, cur.buffer(), cur.textRow());
-
- // now in range?
- if (!bidi.inRange(pos))
- return;
-
- if (pos > 0) {
+
+ // find position to take the font from
+ if (pos != 0) {
+ // paragraph end? -> font of last char
if (pos == cur.lastpos())
--pos;
- else // potentional bug... BUG (Lgb)
- if (par.isSeparator(pos)) {
- if (pos > cur.textRow().pos() &&
- bidi.level(pos) % 2 ==
- bidi.level(pos - 1) % 2)
- --pos;
- else if (pos + 1 < cur.lastpos())
- ++pos;
- }
+ // on space? -> look at the words in front of space
+ else if (pos > 0 && par.isSeparator(pos)) {
+ // abc| def -> font of c
+ // abc |[WERBEH], i.e. boundary==true -> font of c
+ // abc [WERBEH]| def, font of the space
+ if (!isRTLBoundary(cur.buffer(), par, pos))
+ --pos;
+ }
}
-
+
+ // get font
BufferParams const & bufparams = cur.buffer().params();
current_font = par.getFontSettings(bufparams, pos);
real_current_font = getFont(cur.buffer(), par, pos);
+ // special case for paragraph end
if (cur.pos() == cur.lastpos()
- && bidi.isBoundary(cur.buffer(), par, cur.pos())
+ && isRTLBoundary(cur.buffer(), par, cur.pos())
&& !cur.boundary()) {
Language const * lang = par.getParLanguage(bufparams);
current_font.setLanguage(lang);
Index: src/Cursor.cpp
===================================================================
--- src/Cursor.cpp (Revision 18674)
+++ src/Cursor.cpp (Arbeitskopie)
@@ -1438,18 +1438,33 @@
Font Cursor::getFont() const
{
+ // The logic here should more or less match to the
Text::setCurrentFont
+ // logic, i.e. the cursor height should give a hint what will happen
+ // if a character is entered.
+
// HACK. far from being perfect...
- int s = 0;
// go up until first non-0 text is hit
// (innermost text is 0 in mathed)
+ int s = 0;
for (s = depth() - 1; s >= 0; --s)
if (operator[](s).text())
break;
CursorSlice const & sl = operator[](s);
Text const & text = *sl.text();
- Font font = text.getPar(sl.pit()).getFont(
- bv().buffer()->params(),
- sl.pos(),
+ Paragraph const & par = text.getPar(sl.pit());
+
+ // on boundary, so we are really at the character before
+ pos_type pos = sl.pos();
+ if (pos > 0 && boundary())
+ --pos;
+
+ // on space? Take the font before (only for RTL boundary stay)
+ if (pos > 0 && par.isSeparator(pos) &&
+ !text.isRTLBoundary(buffer(), par, pos))
+ --pos;
+
+ // get font at the position
+ Font font = par.getFont(bv().buffer()->params(), pos,
outerFont(sl.pit(), text.paragraphs()));
return font;
************************* nobidi.patch ***********************
Index: src/Text2.cpp
===================================================================
--- src/Text2.cpp (Revision 18674)
+++ src/Text2.cpp (Arbeitskopie)
@@ -1037,7 +1029,7 @@
// if left of boundary -> just jump to right side
// but for RTL boundaries don't, because: abc|DDEEFFghi ->
abcDDEEF|Fghi
if (cur.boundary() &&
- !bidi.isBoundary(cur.buffer(), cur.paragraph(),
cur.pos()))
+ !isRTLBoundary(cur.buffer(), cur.paragraph(),
cur.pos()))
return setCursor(cur, cur.pit(), cur.pos(), true,
false);
// next position is left of boundary,
@@ -1066,7 +1058,7 @@
// in front of RTL boundary? Stay on this side of the boundary
because:
// ab|cDDEEFFghi -> abc|DDEEFFghi
- if (bidi.isBoundary(cur.buffer(), cur.paragraph(), cur.pos() +
1))
+ if (isRTLBoundary(cur.buffer(), cur.paragraph(), cur.pos() + 1))
return setCursor(cur, cur.pit(), cur.pos() + 1, true,
true);
// move right
@@ -1161,7 +1153,8 @@
&& old.pos() < oldpar.size()
&& oldpar.isLineSeparator(old.pos())
&& oldpar.isLineSeparator(old.pos() - 1)
- && !oldpar.isDeleted(old.pos() - 1)) {
+ && !oldpar.isDeleted(old.pos() - 1)
+ && !old.text()->isRTLBoundary(old.buffer(),
oldpar, old.pos())) {
oldpar.eraseChar(old.pos() - 1,
cur.buffer().params().trackChanges);
#ifdef WITH_WARNINGS
#warning This will not work anymore when we have multiple views of
the same buffer
Index: src/TextMetrics.cpp
===================================================================
--- src/TextMetrics.cpp (Revision 18674)
+++ src/TextMetrics.cpp (Arbeitskopie)
@@ -893,7 +893,7 @@
bool const rtl = (text_->bidi.level(c) % 2 == 1);
if (left_side == rtl) {
++c;
- boundary = text_->bidi.isBoundary(buffer, par, c);
+ boundary = text_->isRTLBoundary(buffer, par, c);
}
}
Index: src/Text.cpp
===================================================================
--- src/Text.cpp (Revision 18674)
+++ src/Text.cpp (Arbeitskopie)
@@ -1524,7 +1529,7 @@
// but for RTL boundaries don't, because: abc|DDEEFFghi -> abcDDEEF|
Fghi
if (cur.boundary()) {
cur.boundary(false);
- } else if (bidi.isBoundary(buffer, cur.paragraph(),
cur.pos() + 1)) {
+ } else if (isRTLBoundary(buffer, cur.paragraph(),
cur.pos() + 1)) {
// in front of RTL boundary -> Stay on this side of the boundary
because:
// ab|cDDEEFFghi -> abc|DDEEFFghi
++cur.pos();
Index: src/Bidi.cpp
Index: src/Text3.cpp
===================================================================
--- src/Text3.cpp (Revision 18674)
+++ src/Text3.cpp (Arbeitskopie)
@@ -99,7 +99,8 @@
Font freefont(Font::ALL_IGNORE);
bool toggleall = false;
-
+ // FIXME: what is this little monster doing? Does it really need
+ // access to bidi?
void toggleAndShow(Cursor & cur, Text * text,
Font const & font, bool toggleall = true)
{
@@ -313,6 +314,19 @@
}
+bool Text::isRTLBoundary(Buffer const & buffer, Paragraph const & par,
+
pos_type pos) const
+{
+ // no RTL boundary at line start
+ if (pos == 0)
+ return false;
+
+ bool left = getFont(buffer, par, pos - 1).isVisibleRightToLeft();
+ bool right = getFont(buffer, par, pos).isVisibleRightToLeft();
+ return left != right;
+}
+
+
void Text::dispatch(Cursor & cur, FuncRequest & cmd)
{
LYXERR(Debug::ACTION) << "Text::dispatch: cmd: " << cmd << endl;
Index: src/Text.h
===================================================================
--- src/Text.h (Revision 18674)
+++ src/Text.h (Arbeitskopie)
@@ -335,6 +335,10 @@
bool isRTL(Buffer const &, Paragraph const & par) const;
/// is this position in the paragraph right-to-left?
bool isRTL(Buffer const & buffer, CursorSlice const & sl, bool
boundary) const;
+ /// is between pos-1 and pos an RTL<->LTR boundary?
+ bool isRTLBoundary(Buffer const & buffer, Paragraph const & par,
+ pos_type pos) const;
+
///
bool checkAndActivateInset(Cursor & cur, bool front);
nobidi.patch
Description: Binary data
setcurrentfontwithoutbidi.patch
Description: Binary data
rtlepmnobidi.patch
Description: Binary data
rtlspaces.patch
Description: Binary data
PGP.sig
Description: Signierter Teil der Nachricht
