commit 21c30a09e12a7ad94bde592d3fea3e66f6fb38f5
Author: Jean-Marc Lasgouttes <[email protected]>
Date: Mon Sep 14 22:13:39 2015 +0200
Rewrite Cursor::getSurroundingPos without Bidi class
New method TextMetrics::findRowElement, excerpted from CursorX.
Reimplement getSurroundingPos using Row information. This is easy when
the cursor is inside a row element. At row element edges, different
situations can occur; hopefully all these situations are taken into
account.
Rename the old getSurroundingPos to getSurroundingPosOrig and
transform getSurroundingPos into a wrapper that compares the two
methods. This will be removed when we are confident that the new
function is equivalent to the old one.
It will then be possible to remove also the Bidi class (at last!).
diff --git a/src/Cursor.cpp b/src/Cursor.cpp
index 743be4d..f4e9ce8 100644
--- a/src/Cursor.cpp
+++ b/src/Cursor.cpp
@@ -848,7 +848,104 @@ bool Cursor::posVisLeft(bool skip_inset)
}
-void Cursor::getSurroundingPos(pos_type & left_pos, pos_type & right_pos)
+namespace {
+
+// Return true on success
+bool findNonVirtual(Row const & row, Row::const_iterator & cit, bool onleft)
+{
+ if (onleft) {
+ while (cit != row.begin() && cit->isVirtual())
+ --cit;
+ } else {
+ while (cit != row.end() && cit->isVirtual())
+ ++cit;
+ }
+ return cit != row.end() && !cit->isVirtual();
+}
+
+}
+
+void Cursor::getSurroundingPosNew(pos_type & left_pos, pos_type & right_pos)
const
+{
+ // by default, we know nothing.
+ left_pos = -1;
+ right_pos = -1;
+
+ Row const & row = textRow();
+ TextMetrics const & tm = bv_->textMetrics(text());
+ double dummy = 0;
+ Row::const_iterator cit = tm.findRowElement(row, pos(), boundary(),
dummy);
+ // Handle the case of empty row
+ if (cit == row.end()) {
+ if (paragraph().isRTL(buffer()->params()))
+ right_pos = row.pos();
+ else
+ left_pos = row.pos() - 1;
+ return;
+ }
+
+ // skip virtual elements and exit if no non-virtual one exists
+ if (!findNonVirtual(row, cit, !cit->isRTL()))
+ return;
+
+ // if the position is at the left side of the element, we have to
+ // look at the previous element
+ if (pos() == cit->left_pos()) {
+ LYXERR(Debug::RTL, "getSurroundingPos(" << pos() << (boundary()
? "b" : "")
+ << "), AT LEFT of *cit=" << *cit);
+ // this one is easy (see common case below)
+ right_pos = pos() - (cit->isRTL() ? 1 : 0);
+ // at the left of the row
+ if (cit == row.begin())
+ return;
+ --cit;
+ if (!findNonVirtual(row, cit, true))
+ return;
+ // [...[ is the row element, | is cursor position (! with
boundary)
+ // [ 1 2 [ is a ltr row element with pos=1 and endpos=3
+ // ] 2 1] is an rtl row element with pos=1 and endpos=3
+ // [ 1 2 [ [|3 4 [ => (2, 3)
+ // or [ 1 2 [ ]!4 3 ] => (2, 4)
+ // or ] 2 1 ] [|3 4 [ => (1, 3)
+ // or ] 4 3 ] ]!2 1 ] => (3, 2)
+ left_pos = cit->right_pos() - (cit->isRTL() ? 0 : 1);
+ // happens with consecutive row of same direction
+ if (left_pos == right_pos) {
+ left_pos += cit->isRTL() ? 1 : -1;
+ }
+ }
+ // same code but with the element at the right
+ else if (pos() == cit->right_pos()) {
+ LYXERR(Debug::RTL, "getSurroundingPos(" << pos() << (boundary()
? "b" : "")
+ << "), AT RIGHT of *cit=" << *cit);
+ // this one is easy (see common case below)
+ left_pos = pos() - (cit->isRTL() ? 0 : 1);
+ // at the right of the row
+ if (cit + 1 == row.end())
+ return;
+ ++cit;
+ if (!findNonVirtual(row, cit, false))
+ return;
+ // [ 1 2![ [ 3 4 [ => (2, 3)
+ // or [ 1 2![ ] 4 3 ] => (2, 4)
+ // or ] 2 1|] [ 3 4 [ => (1, 3)
+ // or ] 4 3|] ] 2 1 ] => (3, 2)
+ right_pos = cit->left_pos() - (cit->isRTL() ? 1 : 0);
+ // happens with consecutive row of same direction
+ if (right_pos == left_pos)
+ right_pos += cit->isRTL() ? -1 : 1;
+ }
+ // common case: both positions are inside the row element
+ else {
+ // [ 1 2|3 [ => (2, 3)
+ // or ] 3|2 1 ] => (3, 2)
+ left_pos = pos() - (cit->isRTL() ? 0 : 1);
+ right_pos = pos() - (cit->isRTL() ? 1 : 0);
+ }
+}
+
+
+void Cursor::getSurroundingPosOrig(pos_type & left_pos, pos_type & right_pos)
const
{
// preparing bidi tables
Paragraph const & par = paragraph();
@@ -963,6 +1060,34 @@ void Cursor::getSurroundingPos(pos_type & left_pos,
pos_type & right_pos)
}
+void Cursor::getSurroundingPos(pos_type & left_pos, pos_type & right_pos) const
+{
+ // Check result wrt old implementation
+ // FIXME: remove after correct testing.
+ pos_type lp, rp;
+ getSurroundingPosNew(lp, rp);
+ getSurroundingPosOrig(left_pos, right_pos);
+ if (lp != left_pos || rp != right_pos) {
+ Row const & row = textRow();
+ TextMetrics const & tm = bv_->textMetrics(text());
+ double dummy = 0;
+ Row::const_iterator cit = tm.findRowElement(row, pos(),
boundary(), dummy);
+ if (cit != row.end())
+ LYXERR0("Wrong surroundingpos: old=(" << left_pos << ",
" << right_pos
+ << "), new=(" << lp << ", " << rp
+ << ") *cit= " << *cit
+ << "\ncur = " << *this << "\nrow =" <<
row);
+ else
+ LYXERR0("Wrong surroundingpos: old=(" << left_pos << ",
" << right_pos
+ << "), new=(" << lp << ", " << rp
+ << ") in empty row"
+ << "\ncur = " << *this << "\nrow =" <<
row);
+ }
+ LYXERR(Debug::RTL,"getSurroundingPos(" << pos() << (boundary() ? "b" :
"")
+ << ") => (" << left_pos << ", " << right_pos <<")");
+}
+
+
bool Cursor::posVisToNewRow(bool movingLeft)
{
Paragraph const & par = paragraph();
diff --git a/src/Cursor.h b/src/Cursor.h
index eab5367..94d8f1a 100644
--- a/src/Cursor.h
+++ b/src/Cursor.h
@@ -225,7 +225,9 @@ public:
* If the cursor is at the edge of a row, the position which is "over
the
* edge" will be returned as -1.
*/
- void getSurroundingPos(pos_type & left_pos, pos_type & right_pos);
+ void getSurroundingPos(pos_type & left_pos, pos_type & right_pos) const;
+ void getSurroundingPosNew(pos_type & left_pos, pos_type & right_pos)
const;
+ void getSurroundingPosOrig(pos_type & left_pos, pos_type & right_pos)
const;
/// the row in the paragraph we're in
Row const & textRow() const;
diff --git a/src/Row.h b/src/Row.h
index c0580c0..a51e3ce 100644
--- a/src/Row.h
+++ b/src/Row.h
@@ -89,6 +89,10 @@ public:
//
bool isRTL() const { return font.isVisibleRightToLeft(); }
+ // This is true for virtual elements.
+ // Note that we do not use the type here. The two definitions
+ // should be equivalent
+ bool isVirtual() const { return pos == endpos; }
// The kind of row element
Type type;
diff --git a/src/TextMetrics.cpp b/src/TextMetrics.cpp
index cd6e227..d62dae7 100644
--- a/src/TextMetrics.cpp
+++ b/src/TextMetrics.cpp
@@ -1410,17 +1410,10 @@ Inset * TextMetrics::checkInsetHit(int x, int y)
}
-int TextMetrics::cursorX(CursorSlice const & sl,
- bool boundary) const
+Row::const_iterator const
+TextMetrics::findRowElement(Row const & row, pos_type const pos,
+ bool const boundary, double & x) const
{
- LASSERT(sl.text() == text_, return 0);
-
- ParagraphMetrics const & pm = par_metrics_[sl.pit()];
- if (pm.rows().empty())
- return 0;
- Row const & row = pm.getRow(sl.pos(), boundary);
- pos_type const pos = sl.pos();
-
/**
* When boundary is true, position i is in the row element (pos, endpos)
* if
@@ -1431,20 +1424,21 @@ int TextMetrics::cursorX(CursorSlice const & sl,
*/
int const boundary_corr = (boundary && pos) ? -1 : 0;
+ x = row.left_margin;
+
/** Early return in trivial cases
* 1) the row is empty
* 2) the position is the left-most position of the row; there
- * is a quirck herehowever: if the first element is virtual
+ * is a quirk here however: if the first element is virtual
* (end-of-par marker for example), then we have to look
* closer
*/
if (row.empty()
- || (pos == row.begin()->left_pos()
- && pos != row.begin()->right_pos()))
- return row.left_margin;
+ || (pos == row.begin()->left_pos() && !boundary
+ && !row.begin()->isVirtual()))
+ return row.begin();
Row::const_iterator cit = row.begin();
- double x = row.left_margin;
for ( ; cit != row.end() ; ++cit) {
/** Look whether the cursor is inside the element's
* span. Note that it is necessary to take the
@@ -1452,15 +1446,35 @@ int TextMetrics::cursorX(CursorSlice const & sl,
* elements, which have pos == endpos.
*/
if (pos + boundary_corr >= cit->pos
- && (pos + boundary_corr < cit->endpos
- || cit->pos == cit->endpos)) {
+ && (pos + boundary_corr < cit->endpos || cit->isVirtual()))
{
x += cit->pos2x(pos);
break;
}
x += cit->full_width();
}
+ if (cit == row.end())
+ --cit;
+
+ return cit;
+}
+
+
+int TextMetrics::cursorX(CursorSlice const & sl,
+ bool boundary) const
+{
+ LASSERT(sl.text() == text_, return 0);
+
+ ParagraphMetrics const & pm = par_metrics_[sl.pit()];
+ if (pm.rows().empty())
+ return 0;
+ Row const & row = pm.getRow(sl.pos(), boundary);
+ pos_type const pos = sl.pos();
+
+ double x = 0;
+ findRowElement(row, pos, boundary, x);
return int(x);
+
}
diff --git a/src/TextMetrics.h b/src/TextMetrics.h
index eb48224..f100cb8 100644
--- a/src/TextMetrics.h
+++ b/src/TextMetrics.h
@@ -191,12 +191,16 @@ public:
/// x,y are screen coordinates
void setCursorFromCoordinates(Cursor & cur, int x, int y);
+ /// Helper function: find row element that contains pos, and
+ /// compute x offset.
+ Row::const_iterator const
+ findRowElement(Row const & row, pos_type const pos,
+ bool const boundary, double & x) const;
+
///
- int cursorX(CursorSlice const & cursor,
- bool boundary) const;
+ int cursorX(CursorSlice const & cursor, bool boundary) const;
///
- int cursorY(CursorSlice const & cursor,
- bool boundary) const;
+ int cursorY(CursorSlice const & cursor, bool boundary) const;
///
bool cursorHome(Cursor & cur);
