From 059de2d04c40c63a013896e99a444b65529eb82d Mon Sep 17 00:00:00 2001 From: Jean-Marc Lasgouttes Date: Fri, 11 Oct 2013 16:12:20 +0200 Subject: [PATCH] Fix getColumnNearX for nested insets The horizontal position of the inset was not taken in account. The rounding is not always the same as with the old code, but this is not really important. Additional changes: * improve debug output of rows * remove Bidi& argument of the RowPainter constructor, since it is always an empty Bifi that is passed. This means that the Bidi class is not used at all any more in TextMetrics.cpp. The only remaining user is RowPainter. --- 00README_STR_METRICS_BRANCH | 18 +++++++++++++----- src/Row.cpp | 4 +++- src/TextMetrics.cpp | 27 +++++++++++++++++---------- src/rowpainter.cpp | 6 ++---- src/rowpainter.h | 10 ++++------ 5 files changed, 39 insertions(+), 26 deletions(-) diff --git a/00README_STR_METRICS_BRANCH b/00README_STR_METRICS_BRANCH index 31cc489162..18c68ca2fb 100644 --- a/00README_STR_METRICS_BRANCH +++ b/00README_STR_METRICS_BRANCH @@ -17,18 +17,24 @@ What is done: metrics are computed. The list of elements is stored in the row object in visual ordering, not logical. -* Re-implement cursorX and getColumnNearX using row elements +* Re-implement cursorX and getColumnNearX using row elements. * Implement proper string metrics computation (with cache), when lyxrc.force_paint_single_char is false. In this case, remove also useless workarounds which disable kerning and ligatures. +Next steps needed: + +* 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 of cursorX and getColumnNearX (which have been +* 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). + order to check computations. * Re-implement row painting using row elements. This is not difficult in principle, but the code is intricate and needs some careful @@ -43,9 +49,11 @@ Difference in behavior (aka bug fixes) actual text, not default font. * When cursor is after a LTR separator just before a RTL chunk, the - cursor posiiton is computed better with the new code. + cursor position is computed better with the new code. Other differences (aka real bugs) -You tell me. +* there are still difference in what breaks words. In particular, + RowPainter breaks strings at: selection end, spellchecking + extremity. diff --git a/src/Row.cpp b/src/Row.cpp index 1a3bb247f0..d784979d34 100644 --- a/src/Row.cpp +++ b/src/Row.cpp @@ -218,9 +218,11 @@ ostream & operator<<(ostream & os, Row const & row) << " descent: " << row.dim_.des << " separator: " << row.separator << " label_hfill : " << row.label_hfill << "\n"; + double x = row.x; Row::Elements::const_iterator it = row.elements_.begin(); for ( ; it != row.elements_.end() ; ++it) { - os << "** " << *it << endl; + os << "x=" << x << " => " << *it << endl; + x += it->width(); } return os; } diff --git a/src/TextMetrics.cpp b/src/TextMetrics.cpp index f4e03c38e9..d949e3a75a 100644 --- a/src/TextMetrics.cpp +++ b/src/TextMetrics.cpp @@ -21,7 +21,9 @@ #include "TextMetrics.h" +#ifdef KEEP_OLD_METRICS_CODE #include "Bidi.h" +#endif #include "Buffer.h" #include "buffer_funcs.h" #include "BufferParams.h" @@ -1115,9 +1117,18 @@ void TextMetrics::setRowHeight(Row & row, pit_type const pit, pos_type TextMetrics::getColumnNearX(pit_type const pit, Row const & row, int & x, bool & boundary) const { - boundary = false; + + /// For the main Text, it is possible that this pit is not + /// yet in the CoordCache when moving cursor up. + /// x Paragraph coordinate is always 0 for main text anyway. + int const xo = origin_.x_; + x -= xo; +#ifdef KEEP_OLD_METRICS_CODE + int const x_orig = x; +#endif pos_type pos = row.pos(); + boundary = false; if (row.x >= x || row.empty()) x = row.x; else if (x >= row.width() - row.right_margin) { @@ -1158,16 +1169,13 @@ pos_type TextMetrics::getColumnNearX(pit_type const pit, && row.back().endpos == row.endpos()) boundary = row.right_boundary(); + x += xo; #if !defined(KEEP_OLD_METRICS_CODE) return pos - row.pos(); #else Buffer const & buffer = bv_->buffer(); - /// For the main Text, it is possible that this pit is not - /// yet in the CoordCache when moving cursor up. - /// x Paragraph coordinate is always 0 for main text anyway. - int const xo = origin_.x_; - int x2 = x - xo; + int x2 = x_orig; Paragraph const & par = text_->getPar(pit); Bidi bidi; bidi.computeTables(par, buffer, row); @@ -1280,8 +1288,8 @@ pos_type TextMetrics::getColumnNearX(pit_type const pit, if (abs(x2 - x) > 0.1 || boundary != boundary || c != pos) { - lyxerr << "new=(x=" << x << ", b=" << boundary << ", p=" << pos << "), " - << "old=(x=" << x2 << ", b=" << boundary2 << ", p=" << c << "), " << row; + lyxerr << "getColumnNearX: new=(x=" << x - xo << ", b=" << boundary << ", p=" << pos << "), " + << "old=(x=" << x2 - xo << ", b=" << boundary2 << ", p=" << c << "), " << row; } if (!c || end == par.size()) @@ -2100,7 +2108,6 @@ void TextMetrics::drawParagraph(PainterInfo & pi, pit_type pit, int x, int y) co if (pm.rows().empty()) return; - Bidi bidi; bool const original_drawing_state = pi.pain.isDrawingEnabled(); int const ww = bv_->workHeight(); size_t const nrows = pm.rows().size(); @@ -2141,7 +2148,7 @@ void TextMetrics::drawParagraph(PainterInfo & pi, pit_type pit, int x, int y) co && y - row.ascent() < ww); // It is not needed to draw on screen if we are not inside. pi.pain.setDrawingEnabled(inside && original_drawing_state); - RowPainter rp(pi, *text_, pit, row, bidi, x, y); + RowPainter rp(pi, *text_, pit, row, x, y); if (selection) row.setSelectionAndMargins(sel_beg_par, sel_end_par); diff --git a/src/rowpainter.cpp b/src/rowpainter.cpp index 63b21c7ba9..5b69701bf8 100644 --- a/src/rowpainter.cpp +++ b/src/rowpainter.cpp @@ -14,7 +14,6 @@ #include "rowpainter.h" -#include "Bidi.h" #include "Buffer.h" #include "CoordCache.h" #include "Cursor.h" @@ -56,13 +55,12 @@ using frontend::FontMetrics; RowPainter::RowPainter(PainterInfo & pi, - Text const & text, pit_type pit, Row const & row, Bidi & bidi, int x, int y) + Text const & text, pit_type pit, Row const & row, int x, int y) : pi_(pi), text_(text), text_metrics_(pi_.base.bv->textMetrics(&text)), pars_(text.paragraphs()), row_(row), pit_(pit), par_(text.paragraphs()[pit]), - pm_(text_metrics_.parMetrics(pit)), - bidi_(bidi), change_(pi_.change_), + pm_(text_metrics_.parMetrics(pit)), change_(pi_.change_), xo_(x), yo_(y), width_(text_metrics_.width()), solid_line_thickness_(1.0), solid_line_offset_(1), dotted_line_thickness_(1.0), dotted_line_offset_(2) diff --git a/src/rowpainter.h b/src/rowpainter.h index 480a2dd56c..644cb3773d 100644 --- a/src/rowpainter.h +++ b/src/rowpainter.h @@ -14,13 +14,13 @@ #ifndef ROWPAINTER_H #define ROWPAINTER_H +#include "Bidi.h" #include "Changes.h" #include "support/types.h" namespace lyx { -class Bidi; class BufferView; class Font; class FontInfo; @@ -44,7 +44,7 @@ class RowPainter { public: /// initialise and run painter RowPainter(PainterInfo & pi, Text const & text, - pit_type pit, Row const & row, Bidi & bidi, int x, int y); + pit_type pit, Row const & row, int x, int y); /// paint various parts /// FIXME: transfer to TextMetrics @@ -98,10 +98,8 @@ private: Paragraph const & par_; ParagraphMetrics const & pm_; - /// bidi cache, comes from outside the rowpainter because - /// rowpainters are normally created in a for loop and there only - /// one of them is active at a time. - Bidi & bidi_; + /// bidi cache + Bidi bidi_; /// row changed? (change tracking) Change const change_; -- 2.39.2