From 8649ac7fe66ac0efca20918fff2fea051cc88512 Mon Sep 17 00:00:00 2001 From: Jean-Marc Lasgouttes Date: Wed, 19 Mar 2014 14:44:53 +0100 Subject: [PATCH] Some fixes related to RTL text * fix handling of boundary situations in Row::Elements::x2pos; * fix handling of boundary situations in TextMetrics::getColumnNearX; * make sure to always use Font::isVisibleRightToLeft instead of Font::isRightToLeft; * Improve debug messages. --- 00README_STR_METRICS_BRANCH | 18 +++++++++---- src/Row.cpp | 16 ++++++------ src/Row.h | 4 +++ src/TextMetrics.cpp | 51 ++++++++++++++++++++----------------- 4 files changed, 52 insertions(+), 37 deletions(-) diff --git a/00README_STR_METRICS_BRANCH b/00README_STR_METRICS_BRANCH index 18c68ca2fb..e38d4b63c4 100644 --- a/00README_STR_METRICS_BRANCH +++ b/00README_STR_METRICS_BRANCH @@ -24,21 +24,26 @@ What is done: useless workarounds which disable kerning and ligatures. -Next steps needed: +Next steps: * check what happens with arabic and/or hebrew text. There may be some problems related to compose characters. I suspect that some code is needed in FontMetrics::width. -Next possible steps: - * Get rid of old code in cursorX and getColumnNearX; it has been kept for comparison purpose, guarded with KEEP_OLD_METRICS_CODE in order to check computations. +* rename getColumnNearX to getPosNearX or x2pos (and change code + accordingly). It does not make sense to return a position relative + to the start of row, since nobody needs this. + * Re-implement row painting using row elements. This is not difficult in principle, but the code is intricate and needs some careful - analysis. + analysis. First thing that needs to be done is to break row elements + with the same criterions. Currently TextMetrics::breakRow does not + consider on-the-fly spellchecking and selection changes, but it is + not clear to me that it is required. * Profile and see how performance can be improved. @@ -56,4 +61,7 @@ Other differences (aka real bugs) * there are still difference in what breaks words. In particular, RowPainter breaks strings at: selection end, spellchecking - extremity. + extremities. + +* when clicking in the right margin, GetColumnNearX does not return + the same value as before. diff --git a/src/Row.cpp b/src/Row.cpp index 9da174fa26..d074d73eb8 100644 --- a/src/Row.cpp +++ b/src/Row.cpp @@ -64,27 +64,26 @@ pos_type Row::Element::x2pos(double &x, bool const low) const FontMetrics const & fm = theFontMetrics(font); double last_w = 0; double w = 0; - size_t i = 1; + size_t i = 0; // non-STRING element only contain one position if (type != STRING) { - i = 0; w = width(); } else { // FIXME: implement dichotomy search? - for ( ; i <= str.size() ; ++i) { + for ( ; i < str.size() ; ++i) { last_w = w; - w = fm.width(str.substr(0,i)); - if (w > x2) { - --i; + w = fm.width(str.substr(0, i + 1)); + if (w > x2) break; - } } // if (i == str.size()) // lyxerr << " NOT FOUND "; } + if (i == str.size()) + x2 = w; // round to the closest side - if (!low && (x2 - last_w > w - x2)) { + else if (!low && (x2 - last_w > w - x2)) { x2 = w; ++i; } else @@ -214,6 +213,7 @@ ostream & operator<<(ostream & os, Row const & row) os << " pos: " << row.pos_ << " end: " << row.end_ << " x: " << row.x << " width: " << row.dim_.wid + << " right_margin: " << row.right_margin << " ascent: " << row.dim_.asc << " descent: " << row.dim_.des << " separator: " << row.separator diff --git a/src/Row.h b/src/Row.h index 2282c82d93..0a45aa0f1b 100644 --- a/src/Row.h +++ b/src/Row.h @@ -186,6 +186,10 @@ public: /// bool empty() const { return elements_.empty(); } /// + Element & front() { return elements_.front(); } + /// + Element const & front() const { return elements_.front(); } + /// Element & back() { return elements_.back(); } /// Element const & back() const { return elements_.back(); } diff --git a/src/TextMetrics.cpp b/src/TextMetrics.cpp index ca6408d2bc..8563975585 100644 --- a/src/TextMetrics.cpp +++ b/src/TextMetrics.cpp @@ -15,7 +15,7 @@ * Full author contact details are available in file CREDITS. */ -//#define KEEP_OLD_METRICS_CODE 1 +#define KEEP_OLD_METRICS_CODE 1 #include @@ -810,8 +810,8 @@ void TextMetrics::breakRow(Row & row, int const right_margin, pit_type const pit int const width = max_width_ - right_margin; pos_type const body_pos = par.beginOfBody(); row.clear(); - row.dimension().wid = leftMargin(max_width_, pit, pos); - row.x = row.width(); + row.x = leftMargin(max_width_, pit, pos); + row.dimension().wid = row.x; row.right_margin = right_margin; if (pos >= end || row.width() > width) { @@ -1129,11 +1129,16 @@ pos_type TextMetrics::getColumnNearX(pit_type const pit, pos_type pos = row.pos(); boundary = false; - if (row.x >= x || row.empty()) + if (row.empty()) x = row.x; - else if (x >= row.width() - row.right_margin) { + else if (x < row.x) { + pos = row.front().font.isVisibleRightToLeft() ? + row.front().endpos : row.front().pos; + x = row.x; + } else if (x > row.width() - row.right_margin) { + pos = row.back().font.isVisibleRightToLeft() ? + row.back().pos : row.back().endpos; x = row.width() - row.right_margin; - pos = row.back().endpos; } else { double w = row.x; Row::const_iterator cit = row.begin(); @@ -1170,9 +1175,7 @@ pos_type TextMetrics::getColumnNearX(pit_type const pit, boundary = row.right_boundary(); x += xo; -#if !defined(KEEP_OLD_METRICS_CODE) - return pos - row.pos(); -#else +#ifdef KEEP_OLD_METRICS_CODE Buffer const & buffer = bv_->buffer(); int x2 = x_orig; @@ -1284,14 +1287,15 @@ pos_type TextMetrics::getColumnNearX(pit_type const pit, #endif x2 = int(tmpx) + xo; - pos_type const col = c - row.pos(); + //pos_type const col = c - row.pos(); if (abs(x2 - x) > 0.1 || boundary != boundary || c != pos) { - lyxerr << "getColumnNearX: new=(x=" << x - xo << ", b=" << boundary << ", p=" << pos << "), " + lyxerr << "getColumnNearX(" << x_orig << "): new=(x=" << x - xo << ", b=" << boundary << ", p=" << pos << "), " << "old=(x=" << x2 - xo << ", b=" << boundary2 << ", p=" << c << "), " << row; } +#if 0 if (!c || end == par.size()) return col; @@ -1301,7 +1305,9 @@ pos_type TextMetrics::getColumnNearX(pit_type const pit, } return min(col, end - 1 - row.pos()); -#endif +#endif // 0 +#endif // KEEP_OLD_METRICS_CODE + return pos - row.pos(); } @@ -1617,7 +1623,7 @@ int TextMetrics::cursorX(CursorSlice const & sl, int const boundary_corr = (boundary && pos) ? -1 : 0; if (row.empty() - || (row.begin()->font.isRightToLeft() + || (row.begin()->font.isVisibleRightToLeft() && pos == row.begin()->endpos)) return int(x); @@ -1632,10 +1638,9 @@ int TextMetrics::cursorX(CursorSlice const & sl, } if (cit == row.end() - && (row.back().font.isRightToLeft() || pos != row.back().endpos)) - lyxerr << "NOT FOUND!" - << "pos=" << pos << "(" << boundary_corr << ")" << "\n" - << row; + && (row.back().font.isVisibleRightToLeft() || pos != row.back().endpos)) + LYXERR0("cursorX(" << pos - boundary_corr + << ", " << boundary_corr << "): NOT FOUND! " << row); #ifdef KEEP_OLD_METRICS_CODE Paragraph const & par = text_->paragraphs()[pit]; @@ -1774,18 +1779,16 @@ int TextMetrics::cursorX(CursorSlice const & sl, } if (abs(x2 - x) > 0.01) { - lyxerr << "cursorX: x2=" << x2 << ", x=" << x; + lyxerr << "cursorX(" << pos - boundary_corr << ", " << boundary_corr + << "): old=" << x2 << ", new=" << x; if (cit == row.end()) - lyxerr << "Element not found for " - << pos - boundary_corr << "(" << boundary_corr << ")"; + lyxerr << "Element not found\n"; else - lyxerr << " in [" << cit->pos << "/" - << pos - boundary_corr << "(" << boundary_corr << ")" - << "/" << cit->endpos << "] of " << *cit << "\n"; + lyxerr << " found in " << *cit << "\n"; lyxerr << row <