[scribus-dev] Scribusboxes: comment about issue #13895
Jean Ghali
jghali at libertysurf.fr
Sun Apr 3 14:47:19 UTC 2016
Hi,
This message is about what I discovered while trying to fix issue #13895
(https://bugs.scribus.net/view.php?id=13895) and is mainly intended at Oman team. CTL
branch behavior should be checked so as to see if this issue also exists in that branch.
So here is what I discovered in respect to issue #13895.
In some situtations, particularly when layouting the first line, Scribus has to restart
layouting a line from scratch. This implies that glyphs layout data (character advance,
glyph scaling etc..) of currently line has to be reinitialized. Unfortunately the current
code fails to do so. As a consequence the indent setting for paragraph effects is applied
multiple times on the first line. Which triggers the issue...
My current solution is to introduce a basic TextShaper class, similarly to what has
already been done in CTL branch. Current patch attached for reference. This shaper class
allows to retrieve glyph runs on demand and cache initial run data so that it can get
retrieved in a clean state if the layout of a specific line has to be restarted from
scratch. This shaper class allows to compute only needed glyph runs when layouting a text
frame and not all runs from frame first char to story end as in trunk.
Hope it helps,
Jean
-------------- next part --------------
Index: scribus/pageitem.cpp
===================================================================
--- scribus/pageitem.cpp (revision 21154)
+++ scribus/pageitem.cpp (working copy)
@@ -2644,8 +2644,8 @@
}
else */
{
- gl.xadvance = font.glyphWidth(gl.glyph, style.fontSize() / 10);
- gl.yadvance = font.glyphBBox(gl.glyph, style.fontSize() / 10).ascent;
+ gl.xadvance = font.glyphWidth(gl.glyph, style.fontSize() / 10) * gl.scaleH;
+ gl.yadvance = font.glyphBBox(gl.glyph, style.fontSize() / 10).ascent * gl.scaleV;
}
if (gl.xadvance > 0)
gl.xadvance += tracking;
Index: scribus/pageitem_textframe.cpp
===================================================================
--- scribus/pageitem_textframe.cpp (revision 21154)
+++ scribus/pageitem_textframe.cpp (working copy)
@@ -57,6 +57,7 @@
#include "selection.h"
#include "text/boxes.h"
#include "text/screenpainter.h"
+#include "text/textshaper.h"
#include "ui/guidemanager.h"
#include "ui/marksmanager.h"
#include "undomanager.h"
@@ -1242,154 +1243,6 @@
}
}
-QList<GlyphRun> PageItem_TextFrame::shapeText()
-{
- // maps expanded characters to itemText tokens.
- QMap<int, int> textMap;
- QString text;
- for (int i = firstInFrame(); i < itemText.length(); ++i)
- {
- Mark* mark = itemText.mark(i);
- if (itemText.hasMark(i))
- {
- mark->OwnPage = OwnPage;
- //itemPtr and itemName set to this frame only if mark type is different than MARK2ItemType
- if (!mark->isType(MARK2ItemType))
- {
- mark->setItemPtr(this);
- mark->setItemName(itemName());
- }
-
- //anchors and indexes has no visible inserts in text
- if (mark->isType(MARKAnchorType) || mark->isType(MARKIndexType))
- continue;
-
- //set note marker charstyle
- if (mark->isNoteType())
- {
- TextNote* note = mark->getNotePtr();
- if (note == NULL)
- continue;
- mark->setItemPtr(this);
- NotesStyle* nStyle = note->notesStyle();
- Q_ASSERT(nStyle != NULL);
- QString chsName = nStyle->marksChStyle();
- CharStyle currStyle(itemText.charStyle(i));
- if (!chsName.isEmpty())
- {
- CharStyle marksStyle(m_Doc->charStyle(chsName));
- if (!currStyle.equiv(marksStyle))
- {
- currStyle.setParent(chsName);
- itemText.applyCharStyle(i, 1, currStyle);
- }
- }
-
- StyleFlag s(itemText.charStyle(i).effects());
- if (mark->isType(MARKNoteMasterType))
- {
- if (nStyle->isSuperscriptInMaster())
- s |= ScStyle_Superscript;
- else
- s &= ~ScStyle_Superscript;
- }
- else
- {
- if (nStyle->isSuperscriptInNote())
- s |= ScStyle_Superscript;
- else
- s &= ~ScStyle_Superscript;
- }
- if (s != itemText.charStyle(i).effects())
- {
- CharStyle haveSuperscript;
- haveSuperscript.setFeatures(s.featureList());
- itemText.applyCharStyle(i, 1, haveSuperscript);
- }
- }
- }
-
- bool bullet = false;
- if (i == 0 || itemText.text(i - 1) == SpecialChars::PARSEP)
- {
- ParagraphStyle style = itemText.paragraphStyle(i);
- if (style.hasBullet() || style.hasNum())
- {
- bullet = true;
- if (mark == NULL || !mark->isType(MARKBullNumType))
- {
- itemText.insertMark(new BulNumMark(), i);
- i--;
- continue;
- }
- if (style.hasBullet())
- mark->setString(style.bulletStr());
- else if (style.hasNum() && mark->getString().isEmpty())
- {
- mark->setString("?");
- m_Doc->flag_Renumber = true;
- }
- }
- }
-
- if (!bullet && mark && mark->isType(MARKBullNumType))
- {
- itemText.removeChars(i, 1);
- i--;
- continue;
- }
-
- QString str = ExpandToken(i);
- if (str.isEmpty())
- str = SpecialChars::ZWNBSPACE;
-
- for (int j = 0; j < str.length(); j++)
- textMap.insert(text.length() + j, i);
-
- text.append(str);
- }
-
- QList<GlyphRun> glyphRuns;
- glyphRuns.reserve(text.length());
- for (int i = 0; i < text.length(); i++)
- {
- const QChar ch(text.at(i));
- int a = textMap.value(i);
-
- GlyphRun run(&itemText.charStyle(a), itemText.flags(a), a, a, itemText.object(a));
- if (SpecialChars::isExpandingSpace(ch))
- run.setFlag(ScLayout_ExpandingSpace);
-
- GlyphLayout gl = layoutGlyphs(run.style(), QString(ch), itemText.flags(a));
-
- if (!glyphRuns.isEmpty())
- {
- GlyphLayout& last = glyphRuns.last().glyphs().last();
- last.xadvance += run.style().font().glyphKerning(last.glyph, gl.glyph, run.style().fontSize() / 10) * last.scaleH;
- }
-
- //show control characters for marks
- if (itemText.hasMark(a))
- {
- GlyphLayout control;
- control.glyph = SpecialChars::OBJECT.unicode() + ScFace::CONTROL_GLYPHS;
- run.glyphs().append(control);
- }
-
- if (SpecialChars::isExpandingSpace(ch))
- gl.xadvance *= run.style().wordTracking();
-
- if (itemText.hasObject(a))
- gl.xadvance = itemText.object(a)->width() + itemText.object(a)->lineWidth();
-
- run.glyphs().append(gl);
-
- glyphRuns.append(run);
- }
-
- return glyphRuns;
-}
-
void PageItem_TextFrame::layout()
{
// qDebug()<<"==Layout==" << itemName() ;
@@ -1526,7 +1379,8 @@
m_availableRegion = matrix.map(m_availableRegion);
}
- QList<GlyphRun> glyphRuns = shapeText();
+ QList<GlyphRun> glyphRuns;
+ TextShaper shaper(this, firstInFrame());
LineControl current(m_width, m_height, m_textDistanceMargins, lineCorr, m_Doc, glyphRuns, columnWidth(), ColGap);
current.nextColumn(textLayout);
@@ -1585,11 +1439,18 @@
setMaxY(-1);
double maxYAsc = 0.0, maxYDesc = 0.0;
int regionMinY = 0, regionMaxY= 0;
-
double autoLeftIndent = 0.0;
- for (int i = 0; i < glyphRuns.length(); ++i)
+
+ for (int i = 0; shaper.hasRun(i); ++i)
{
- if (glyphRuns[i].glyphs().isEmpty())
+ GlyphRun newRun = shaper.runAt(i);
+ if (i >= glyphRuns.count())
+ glyphRuns.append(newRun);
+ else
+ glyphRuns[i] = newRun;
+
+ GlyphRun& currentRun = glyphRuns[i];
+ if (currentRun.glyphs().isEmpty())
continue;
int a = glyphRuns[i].firstChar();
@@ -1811,11 +1672,11 @@
GlyphLayout& firstGlyph = glyphRuns[i].glyphs().first();
GlyphLayout& lastGlyph = glyphRuns[i].glyphs().last();
- // apply cjk kerning
- //TODO: cjk spacing and kerning should be done in layoutGlyphs!
- if (i + 1 < glyphRuns.length())
+ // Apply cjk kerning
+ // TODO: cjk spacing and kerning should be done in layoutGlyphs!
+ if (shaper.hasRun(i + 1))
{
- GlyphRun nextRun = glyphRuns[i + 1];
+ GlyphRun nextRun = shaper.runAt(i + 1);
// change xadvance, xoffset according to JIS X4051
int nextStat = SpecialChars::getCJKAttr(itemText.text(nextRun.firstChar()));
if (curStat != 0)
@@ -1909,7 +1770,7 @@
realAsce = qMax(realAsce, gm.ascent + gm.descent);
wide += gm.width;
}
- wide = (wide* scaleH) + (1 - scaleH);
+ wide = (wide * scaleH) + (1 - scaleH);
realAsce = realAsce * scaleV + offset;
if (realCharHeight == 0)
realCharHeight = font.height(style.charStyle().fontSize() / 10.0);
@@ -2636,7 +2497,8 @@
if ((DropCmode || BulNumMode) && !outs)
{
current.xPos += style.parEffectOffset();
- lastGlyph.xadvance += style.parEffectOffset();
+ lastGlyph.xadvance += DropCmode ? (style.parEffectOffset() / lastGlyph.scaleH)
+ : style.parEffectOffset();
if (DropCmode)
{
DropCmode = false;
@@ -2949,7 +2811,7 @@
}
}
}
- if (i == glyphRuns.length() - 1)
+ if (glyphRuns[i].lastChar() == itemText.length() - 1)
{
if (!current.afterOverflow || current.addLine)
{
Index: scribus/pageitem_textframe.h
===================================================================
--- scribus/pageitem_textframe.h (revision 21154)
+++ scribus/pageitem_textframe.h (working copy)
@@ -159,9 +159,6 @@
public:
void setTextFrameHeight();
-
-private:
- QList<GlyphRun> shapeText();
};
#endif
Index: scribus/sctextstruct.h
===================================================================
--- scribus/sctextstruct.h (revision 21154)
+++ scribus/sctextstruct.h (working copy)
@@ -75,6 +75,7 @@
const CharStyle* m_style;
LayoutFlags m_flags;
QList<GlyphLayout> m_glyphs;
+
int m_firstChar;
int m_lastChar;
PageItem* m_object;
@@ -92,9 +93,11 @@
bool hasFlag(LayoutFlags f) const { return (m_flags & f) == f; }
void setFlag(LayoutFlags f) { m_flags = static_cast<LayoutFlags>(m_flags | f); }
void clearFlag(LayoutFlags f) { m_flags = static_cast<LayoutFlags>(m_flags & ~f); }
+ void clearGlyphs() { m_glyphs.clear(); }
QList<GlyphLayout>& glyphs() { return m_glyphs; }
const QList<GlyphLayout>& glyphs() const { return m_glyphs; }
+ int glyphCount() const { return m_glyphs.count(); }
int firstChar() const { return m_firstChar; }
int lastChar() const { return m_lastChar; }
qreal width() const;
Index: scribus/text/CMakeLists.txt
===================================================================
--- scribus/text/CMakeLists.txt (revision 21154)
+++ scribus/text/CMakeLists.txt (working copy)
@@ -13,15 +13,16 @@
)
SET(SCRIBUS_TEXT_LIB_SOURCES
+ boxes.cpp
+ frect.cpp
+ fsize.cpp
+ screenpainter.cpp
+ sctext_shared.cpp
specialchars.cpp
storytext.cpp
textlayout.cpp
textlayoutpainter.cpp
- screenpainter.cpp
- sctext_shared.cpp
- fsize.cpp
- frect.cpp
- boxes.cpp
+ textshaper.cpp
)
QT5_WRAP_CPP(SCRIBUS_TEXT_MOC_SOURCES ${SCRIBUS_TEXT_MOC_CLASSES})
Index: scribus/text/textshaper.cpp
===================================================================
--- scribus/text/textshaper.cpp (revision 0)
+++ scribus/text/textshaper.cpp (revision 0)
@@ -0,0 +1,207 @@
+/*
+ For general Scribus (>=1.3.2) copyright and licensing information please refer
+ to the COPYING file provided with the program. Following this notice may exist
+ a copyright and/or license notice that predates the release of Scribus 1.3.2
+ for which a new license (GPL+exception) is in place.
+ */
+
+#include "pageitem.h"
+#include "pageitem_textframe.h"
+#include "scribusdoc.h"
+#include "sctextstruct.h"
+#include "style.h"
+#include "styles/charstyle.h"
+#include "styles/paragraphstyle.h"
+#include "textshaper.h"
+#include "text/specialchars.h"
+
+TextShaper::TextShaper(PageItem_TextFrame* textItem, int startIndex)
+ : m_startIndex(startIndex),
+ m_index(startIndex),
+ m_lastKernedIndex(-1),
+ m_item(textItem)
+{
+ if (m_item->lastInFrame() >= m_item->firstInFrame())
+ {
+ int charsCount = m_item->lastInFrame() - m_item->firstInFrame() + 1;
+ m_runs.reserve(charsCount);
+ }
+}
+
+bool TextShaper::hasRun(int i)
+{
+ if (i <= m_lastKernedIndex)
+ return true;
+
+ needChars(i);
+ return (i <= m_lastKernedIndex);
+}
+
+GlyphRun TextShaper::runAt(int i)
+{
+ // Ensure we get a kerned run
+ if (i <= m_lastKernedIndex)
+ return m_runs.at(i);
+
+ needChars(i);
+ return m_runs.at(i);
+}
+
+void TextShaper::needChars(int runIndex)
+{
+ int oldRunCount = m_runs.count();
+ StoryText& itemText = m_item->itemText;
+
+ for ( ; m_index < itemText.length(); ++m_index)
+ {
+ Mark* mark = itemText.mark(m_index);
+ if (itemText.hasMark(m_index))
+ {
+ mark->OwnPage = m_item->OwnPage;
+ //itemPtr and itemName set to this frame only if mark type is different than MARK2ItemType
+ if (!mark->isType(MARK2ItemType))
+ {
+ mark->setItemPtr(m_item);
+ mark->setItemName(m_item->itemName());
+ }
+
+ //anchors and indexes has no visible inserts in text
+ if (mark->isType(MARKAnchorType) || mark->isType(MARKIndexType))
+ continue;
+
+ //set note marker charstyle
+ if (mark->isNoteType())
+ {
+ TextNote* note = mark->getNotePtr();
+ if (note == NULL)
+ continue;
+ mark->setItemPtr(m_item);
+ NotesStyle* nStyle = note->notesStyle();
+ Q_ASSERT(nStyle != NULL);
+ QString chsName = nStyle->marksChStyle();
+ CharStyle currStyle(itemText.charStyle(m_index));
+ if (!chsName.isEmpty())
+ {
+ CharStyle marksStyle(m_item->doc()->charStyle(chsName));
+ if (!currStyle.equiv(marksStyle))
+ {
+ currStyle.setParent(chsName);
+ itemText.applyCharStyle(m_index, 1, currStyle);
+ }
+ }
+
+ StyleFlag s(itemText.charStyle(m_index).effects());
+ if (mark->isType(MARKNoteMasterType))
+ {
+ if (nStyle->isSuperscriptInMaster())
+ s |= ScStyle_Superscript;
+ else
+ s &= ~ScStyle_Superscript;
+ }
+ else
+ {
+ if (nStyle->isSuperscriptInNote())
+ s |= ScStyle_Superscript;
+ else
+ s &= ~ScStyle_Superscript;
+ }
+ if (s != itemText.charStyle(m_index).effects())
+ {
+ CharStyle haveSuperscript;
+ haveSuperscript.setFeatures(s.featureList());
+ itemText.applyCharStyle(m_index, 1, haveSuperscript);
+ }
+ }
+ }
+
+ bool bullet = false;
+ if (m_index == 0 || itemText.text(m_index - 1) == SpecialChars::PARSEP)
+ {
+ ParagraphStyle style = itemText.paragraphStyle(m_index);
+ if (style.hasBullet() || style.hasNum())
+ {
+ bullet = true;
+ if (mark == NULL || !mark->isType(MARKBullNumType))
+ {
+ itemText.insertMark(new BulNumMark(), m_index);
+ m_index--;
+ continue;
+ }
+ if (style.hasBullet())
+ mark->setString(style.bulletStr());
+ else if (style.hasNum() && mark->getString().isEmpty())
+ {
+ mark->setString("?");
+ m_item->doc()->flag_Renumber = true;
+ }
+ }
+ }
+
+ if (!bullet && mark && mark->isType(MARKBullNumType))
+ {
+ itemText.removeChars(m_index, 1);
+ m_index--;
+ continue;
+ }
+
+ QString str = m_item->ExpandToken(m_index);
+ if (str.isEmpty())
+ str = SpecialChars::ZWNBSPACE;
+
+ for (int j = 0; j < str.length(); j++)
+ {
+ const QChar ch(str.at(j));
+ GlyphRun run(&itemText.charStyle(m_index), itemText.flags(m_index), m_index, m_index, itemText.object(m_index));
+ initGlyphLayout(run, QString(ch), m_runs.count());
+ m_runs.append(run);
+ }
+
+ // This ensure we have a sufficient number of runs
+ // to kern the request run
+ if (m_runs.count() >= runIndex + 2)
+ {
+ ++m_index;
+ break;
+ }
+ }
+
+ // Last run cannot be kerned, increment m_lastKernedIndex to its max value
+ if (m_index >= itemText.length())
+ m_lastKernedIndex = m_runs.count() - 1;
+}
+
+void TextShaper::initGlyphLayout(GlyphRun& run, const QString& chars, int runIndex)
+{
+ int a = run.firstChar();
+ const QChar ch = chars.at(0);
+ const CharStyle& runStyle = run.style();
+ StoryText& itemText = m_item->itemText;
+
+ if (SpecialChars::isExpandingSpace(ch))
+ run.setFlag(ScLayout_ExpandingSpace);
+
+ GlyphLayout gl = m_item->layoutGlyphs(runStyle, chars, itemText.flags(a));
+
+ if (runIndex > 0)
+ {
+ GlyphLayout& last = m_runs[runIndex - 1].glyphs().last();
+ last.xadvance += runStyle.font().glyphKerning(last.glyph, gl.glyph, runStyle.fontSize() / 10) * last.scaleH;
+ m_lastKernedIndex = qMax(m_lastKernedIndex, runIndex - 1);
+ }
+
+ //show control characters for marks
+ if (itemText.hasMark(a))
+ {
+ GlyphLayout control;
+ control.glyph = SpecialChars::OBJECT.unicode() + ScFace::CONTROL_GLYPHS;
+ run.glyphs().append(control);
+ }
+
+ if (SpecialChars::isExpandingSpace(ch))
+ gl.xadvance *= runStyle.wordTracking();
+
+ if (itemText.hasObject(a))
+ gl.xadvance = itemText.object(a)->width() + itemText.object(a)->lineWidth();
+
+ run.glyphs().append(gl);
+}
Index: scribus/text/textshaper.h
===================================================================
--- scribus/text/textshaper.h (revision 0)
+++ scribus/text/textshaper.h (revision 0)
@@ -0,0 +1,58 @@
+/*
+ For general Scribus (>=1.3.2) copyright and licensing information please refer
+ to the COPYING file provided with the program. Following this notice may exist
+ a copyright and/or license notice that predates the release of Scribus 1.3.2
+ for which a new license (GPL+exception) is in place.
+ */
+
+#ifndef TEXTSHAPER_H
+#define TEXTSHAPER_H
+
+#include <QList>
+
+#include "scribusapi.h"
+#include "sctextstruct.h"
+#include "text/storytext.h"
+
+class PageItem_TextFrame;
+
+class SCRIBUS_API TextShaper
+{
+public:
+ TextShaper(PageItem_TextFrame* textItem, int startIndex = 0);
+
+ /**
+ * Test if run at specifid index can be retrieved
+ */
+ bool hasRun(int i);
+
+ /**
+ * Retrieve glyph run at specified index
+ */
+ GlyphRun runAt(int i);
+
+protected:
+
+ // Character index from where shaping start
+ int m_startIndex;
+
+ // Next character index to be shaped
+ int m_index;
+
+ // Last run which has been kerned
+ int m_lastKernedIndex;
+
+ // The item whose text is being shaped
+ PageItem_TextFrame* m_item;
+
+ // Glyph runs waiting to be retrieved
+ QList<GlyphRun> m_runs;
+
+ // Get next chars and put them in char queue
+ void needChars(int runIndex);
+
+ // Initialize layout data and glyph list of a text run
+ void initGlyphLayout(GlyphRun& run, const QString& chars, int runIndex);
+};
+
+#endif
More information about the scribus-dev
mailing list