From ec42b2e4c9daa458df1c5c0454d61fbe4987701c Mon Sep 17 00:00:00 2001 From: Jean-Marc Lasgouttes Date: Fri, 23 May 2014 18:59:53 +0200 Subject: [PATCH] Do not split words at selection boundary The display of partially-selected word is now done in a new Painter::text method which displays the string twice with different clip settings. This allows to catter for the case where Color_selectiontext is not black. Morover, the code that uses unicode override characters to force the direction of a string is moved to lstrings.h. Fixes: #9116 --- 00README_STR_METRICS_BRANCH | 6 ++- src/frontends/Painter.h | 24 +++++++-- src/frontends/qt4/GuiFontMetrics.cpp | 22 ++------- src/frontends/qt4/GuiPainter.cpp | 55 ++++++++++++++++++--- src/frontends/qt4/GuiPainter.h | 22 +++++++-- src/rowpainter.cpp | 73 +++++++++++++++++----------- src/support/lstrings.cpp | 18 +++++++ src/support/lstrings.h | 3 ++ 8 files changed, 161 insertions(+), 62 deletions(-) diff --git a/00README_STR_METRICS_BRANCH b/00README_STR_METRICS_BRANCH index 568ac8f3c4..2dc3cadd11 100644 --- a/00README_STR_METRICS_BRANCH +++ b/00README_STR_METRICS_BRANCH @@ -47,6 +47,10 @@ What is done: justified. This speeds-up painting by reducing the number of strings to draw. +* Do not cut strings at selection boundary in RowPainter. This avoids + ligature/kerning breaking in latin text, and bad rendering problems + in Arabic. + Next steps: @@ -72,5 +76,3 @@ Steps for later (aka out of the scope of this branch): happens. This depends on the update machinery. This would allow to get rid of the Bidi.cpp code. - - diff --git a/src/frontends/Painter.h b/src/frontends/Painter.h index 7267839621..3302d16072 100644 --- a/src/frontends/Painter.h +++ b/src/frontends/Painter.h @@ -14,9 +14,11 @@ #define PAINTER_H #include "support/strfwd.h" +#include "support/types.h" namespace lyx { +class Font; class FontInfo; namespace graphics { class Image; } @@ -106,12 +108,26 @@ public: virtual void image(int x, int y, int w, int h, graphics::Image const & image) = 0; - /// draw a string at position x, y (y is the baseline) - /** - * \return the width of the drawn text. - */ + /** draw a string at position x, y (y is the baseline). The + * text direction is deduced from \c str. + * \return the width of the drawn text. + */ virtual int text(int x, int y, docstring const & str, FontInfo const & f) = 0; + /** draw a string at position x, y (y is the baseline). The + * text direction is enforced by the \c Font. + * \return the width of the drawn text. + */ + virtual int text(int x, int y, docstring const & str, Font const & f) = 0; + + /** draw a string at position x, y (y is the baseline), but + * make sure that the part between \c from and \c to is in + * \c other color. The text direction is enforced by the \c Font. + * \return the width of the drawn text. + */ + virtual int text(int x, int y, docstring const & str, Font const & f, + Color other, size_type from, size_type to) = 0; + void setDrawingEnabled(bool drawing_enabled) { drawing_enabled_ = drawing_enabled; } diff --git a/src/frontends/qt4/GuiFontMetrics.cpp b/src/frontends/qt4/GuiFontMetrics.cpp index 2ac588ceba..492a0a9324 100644 --- a/src/frontends/qt4/GuiFontMetrics.cpp +++ b/src/frontends/qt4/GuiFontMetrics.cpp @@ -22,10 +22,12 @@ #include "insets/Inset.h" #include "support/lassert.h" +#include "support/lstrings.h" #include using namespace std; +using namespace lyx::support; namespace lyx { namespace frontend { @@ -42,7 +44,7 @@ namespace { * why this works well for symbol fonts used in mathed too, even though * these are not real ucs4 characters. These are codepoints in the * computer modern fonts used, nothing unicode related. - * See comment in QLPainter::text() for more explanation. + * See comment in GuiPainter::text() for more explanation. **/ inline QChar const ucs4_to_qchar(char_type const ucs4) { @@ -146,22 +148,7 @@ namespace { void setTextLayout(QTextLayout & tl, docstring const & s, QFont const & font, bool const rtl) { - QString qs; - /* In LyX, the character direction is forced by the language. - * Therefore, we have to signal that fact to Qt. - * Source: http://www.iamcal.com/understanding-bidirectional-text/ - */ - // Left-to-right override: forces to draw text left-to-right - char_type const LRO = 0x202D; - // Right-to-left override: forces to draw text right-to-left - char_type const RLO = 0x202E; - // Pop directional formatting: return to previous state - char_type const PDF = 0x202C; - if (rtl) - qs = toqstr(RLO + s + PDF); - else - qs = toqstr(LRO + s + PDF); - + QString const qs = toqstr(directedString(s, rtl)); tl.setText(qs); tl.setFont(font); tl.beginLayout(); @@ -175,6 +162,7 @@ int GuiFontMetrics::pos2x(docstring const & s, int const pos, bool const rtl) co { QTextLayout tl; setTextLayout(tl, s, font_, rtl); + // we take into account the unicode formatting characters return tl.lineForTextPosition(pos + 1).cursorToX(pos + 1); } diff --git a/src/frontends/qt4/GuiPainter.cpp b/src/frontends/qt4/GuiPainter.cpp index 1b81d62eff..a33aebf0e0 100644 --- a/src/frontends/qt4/GuiPainter.cpp +++ b/src/frontends/qt4/GuiPainter.cpp @@ -20,13 +20,14 @@ #include "GuiImage.h" #include "qt_helpers.h" -#include "FontInfo.h" +#include "Font.h" #include "Language.h" #include "LyXRC.h" #include "insets/Inset.h" #include "support/lassert.h" +#include "support/lstrings.h" #include "support/debug.h" #include @@ -41,6 +42,7 @@ #endif using namespace std; +using namespace lyx::support; namespace lyx { namespace frontend { @@ -305,17 +307,16 @@ int GuiPainter::text(int x, int y, docstring const & s, QFont const & ff = getFont(f); GuiFontMetrics const & fm = getFontMetrics(f); - int textwidth; - // Here we use the font width cache instead of // textwidth = fontMetrics().width(str); // because the above is awfully expensive on MacOSX - textwidth = fm.width(s); - textDecoration(f, x, y, textwidth); + int const textwidth = fm.width(s); if (!isDrawingEnabled()) return textwidth; + textDecoration(f, x, y, textwidth); + // Qt4 does not display a glyph whose codepoint is the // same as that of a soft-hyphen (0x00ad), unless it // occurs at a line-break. As a kludge, we force Qt to @@ -389,8 +390,6 @@ int GuiPainter::text(int x, int y, docstring const & s, setQPainterPen(computeColor(f.realColor())); if (font() != ff) setFont(ff); - // We need to draw the text as LTR as we use our own bidi code. - QPainter::setLayoutDirection(Qt::LeftToRight); drawText(x, y, str); //LYXERR(Debug::PAINTING, "draw " << string(str.toUtf8()) // << " at " << x << "," << y); @@ -398,6 +397,48 @@ int GuiPainter::text(int x, int y, docstring const & s, } +int GuiPainter::text(int x, int y, docstring const & str, Font const & f) +{ + docstring const dstr = directedString(str, f.isVisibleRightToLeft()); + return text(x, y, dstr, f.fontInfo()); +} + + +int GuiPainter::text(int x, int y, docstring const & str, Font const & f, + Color other, size_type from, size_type to) +{ + GuiFontMetrics const & fm = getFontMetrics(f.fontInfo()); + FontInfo fi = f.fontInfo(); + bool const rtl = f.isVisibleRightToLeft(); + docstring const dstr = directedString(str, rtl); + + // dimensions + int const ascent = fm.maxAscent(); + int const height = fm.maxAscent() + fm.maxDescent(); + int xmin = fm.pos2x(str, from, rtl); + int xmax = fm.pos2x(str, to, rtl); + if (xmin > xmax) + swap(xmin, xmax); + + // First the part in other color + Color const orig = fi.realColor(); + fi.setPaintColor(other); + setClipRect(QRect(x + xmin, y - ascent, xmax - xmin, height)); + int const textwidth = text(x, y, dstr, fi); + + // Then the part in normal color + // Note that in Qt5, it is not possible to use Qt::UniteClip + fi.setPaintColor(orig); + setClipRect(QRect(x, y - ascent, xmin, height)); + text(x, y, dstr, fi); + setClipRect(QRect(x + xmax, y - ascent, textwidth - xmax, height)); + text(x, y, dstr, fi); + setClipping(false); + + return textwidth; +} + + void GuiPainter::textDecoration(FontInfo const & f, int x, int y, int width) { if (f.underbar() == FONT_ON) diff --git a/src/frontends/qt4/GuiPainter.h b/src/frontends/qt4/GuiPainter.h index a4768c37c7..a3ac38c301 100644 --- a/src/frontends/qt4/GuiPainter.h +++ b/src/frontends/qt4/GuiPainter.h @@ -86,9 +86,25 @@ public: virtual void image(int x, int y, int w, int h, lyx::graphics::Image const & image); - /// draw a string at position x, y (y is the baseline) - virtual int text(int x, int y, - docstring const & str, FontInfo const & f); + /** draw a string at position x, y (y is the baseline). The + * text direction is deduced from \c str. + * \return the width of the drawn text. + */ + virtual int text(int x, int y, docstring const & str, FontInfo const & f); + + /** draw a string at position x, y (y is the baseline). The + * text direction is enforced by the \c Font. + * \return the width of the drawn text. + */ + virtual int text(int x, int y, docstring const & str, Font const & f); + + /** draw a string at position x, y (y is the baseline), but + * make sure that the part between \c from and \c to is in + * \c other color. The text direction is enforced by the \c Font. + * \return the width of the drawn text. + */ + virtual int text(int x, int y, docstring const & str, Font const & f, + Color other, size_type from, size_type to); /// draw a char at position x, y (y is the baseline) virtual int text(int x, int y, char_type c, FontInfo const & f); diff --git a/src/rowpainter.cpp b/src/rowpainter.cpp index 6dc975f8a9..b6e7f24f95 100644 --- a/src/rowpainter.cpp +++ b/src/rowpainter.cpp @@ -164,6 +164,7 @@ void RowPainter::paintChars(pos_type & vpos, Font const & font) { // This method takes up 70% of time when typing pos_type pos = bidi_.vis2log(vpos); + pos_type start_pos = pos; // first character char_type c = par_.getChar(pos); docstring str; @@ -190,10 +191,6 @@ void RowPainter::paintChars(pos_type & vpos, Font const & font) // Track-change status. Change const & change_running = par_.lookupChange(pos); - // selected text? - bool const selection = (pos >= row_.sel_beg && pos < row_.sel_end) - || pi_.selected; - // spelling correct? bool const spell_state = lyxrc.spellcheck_continuously && par_.isMisspelled(pos); @@ -201,12 +198,8 @@ void RowPainter::paintChars(pos_type & vpos, Font const & font) // collect as much similar chars as we can for (++vpos ; vpos < end ; ++vpos) { pos = bidi_.vis2log(vpos); - if (pos < font_span.first || pos > font_span.last) - break; - bool const new_selection = pos >= row_.sel_beg && pos < row_.sel_end; - if (new_selection != selection) - // Selection ends or starts here. + if (!font_span.inside(pos)) break; bool const new_spell_state = @@ -225,6 +218,9 @@ void RowPainter::paintChars(pos_type & vpos, Font const & font) if (c == '\t') break; + // When row_.separator == 0, it is possible to print a + // string longer than a word in one fell swoop. + // Therefore there is no need to break at spaces. if (!isPrintableNonspace(c) && (c != ' ' || row_.separator > 0)) break; @@ -241,6 +237,19 @@ void RowPainter::paintChars(pos_type & vpos, Font const & font) str.push_back(c); } + // Make pos point to the last character in the string. + // Using "pos = bidi_.vis2log(vpos)" does not work for some reason. + if (vpos < end) + pos = bidi_.vis2log(vpos - 1); + + // Now make pos point to the position _after_ the string. + // Using vis2log for that is not a good idea in general, we + // want logical ordering. + if (font.isVisibleRightToLeft()) + --pos; + else + ++pos; + if (str[0] == '\t') str.replace(0,1,from_ascii(" ")); @@ -253,30 +262,36 @@ void RowPainter::paintChars(pos_type & vpos, Font const & font) * See also http://thread.gmane.org/gmane.editors.lyx.devel/79740 * for an earlier thread on the subject */ - // Left-to-right override: forces to draw text left-to-right - char_type const LRO = 0x202D; - // Right-to-left override: forces to draw text right-to-left - char_type const RLO = 0x202E; - // Pop directional formatting: return to previous state - char_type const PDF = 0x202C; if (font.isVisibleRightToLeft()) { reverse(str.begin(), str.end()); - str = RLO + str + PDF; - } else - str = LRO + str + PDF; + // If the string is reversed, the positions need to be adjusted + ++pos; + ++start_pos; + swap(start_pos, pos); + } - if (!selection && !change_running.changed()) { - x_ += pi_.pain.text(int(x_), yo_, str, font.fontInfo()); - return; + // at least part of text selected? + bool const some_sel = (pos >= row_.sel_beg && start_pos < row_.sel_end) + || pi_.selected; + // all the text selected? + bool const all_sel = (start_pos >= row_.sel_beg && pos < row_.sel_end) + || pi_.selected; + + if (all_sel) { + Font copy = font; + copy.fontInfo().setPaintColor(Color_selectiontext); + x_ += pi_.pain.text(int(x_), yo_, str, copy); + } else if (change_running.changed()) { + Font copy = font; + copy.fontInfo().setPaintColor(change_running.color()); + x_ += pi_.pain.text(int(x_), yo_, str, copy); + } else if (!some_sel) { + x_ += pi_.pain.text(int(x_), yo_, str, font); + } else { + x_ += pi_.pain.text(int(x_), yo_, str, font, Color_selectiontext, + max(row_.sel_beg, start_pos) - start_pos, + min(row_.sel_end, pos) - start_pos); } - - FontInfo copy = font.fontInfo(); - if (change_running.changed()) - copy.setPaintColor(change_running.color()); - else if (selection) - copy.setPaintColor(Color_selectiontext); - - x_ += pi_.pain.text(int(x_), yo_, str, copy); } diff --git a/src/support/lstrings.cpp b/src/support/lstrings.cpp index 8508e4ef13..cec1812897 100644 --- a/src/support/lstrings.cpp +++ b/src/support/lstrings.cpp @@ -1502,5 +1502,23 @@ docstring bformat(docstring const & fmt, return subst(str, from_ascii("%%"), from_ascii("%")); } + +docstring directedString(docstring const & s, bool const rtl) +{ + /* In LyX, the character direction is forced by the language. + * Therefore, we have to signal that fact to Qt. + * Source: http://www.iamcal.com/understanding-bidirectional-text/ + */ + // Left-to-right override: forces to draw text left-to-right + char_type const LRO = 0x202D; + // Right-to-left override: forces to draw text right-to-left + char_type const RLO = 0x202E; + // Pop directional formatting: return to previous state + char_type const PDF = 0x202C; + return (rtl ? RLO : LRO) + s + PDF; +} + + + } // namespace support } // namespace lyx diff --git a/src/support/lstrings.h b/src/support/lstrings.h index 0d21e954a4..6fb1daf156 100644 --- a/src/support/lstrings.h +++ b/src/support/lstrings.h @@ -327,6 +327,9 @@ template<> docstring bformat(docstring const & fmt, int arg1, int arg2); template<> docstring bformat(docstring const & fmt, docstring arg1, docstring arg2, docstring arg3); template<> docstring bformat(docstring const & fmt, docstring arg1, docstring arg2, docstring arg3, docstring arg4); +/// Return a string with Unicode overrides to enforce the writing direction +docstring directedString(docstring const & s, bool rtl); + } // namespace support } // namespace lyx