[SCM] WebKit Debian packaging branch, webkit-1.3, updated. upstream/1.3.7-4207-g178b198
evan at chromium.org
evan at chromium.org
Sun Feb 20 22:56:02 UTC 2011
The following commit has been merged in the webkit-1.3 branch:
commit e7c7be48f3d1cbe246591fb4659414bee2278b08
Author: evan at chromium.org <evan at chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Fri Jan 14 00:48:53 2011 +0000
2011-01-13 Evan Martin <evan at chromium.org>
Reviewed by Tony Chang.
[chromium] drop backwards iteration in Linux complex text code
https://bugs.webkit.org/show_bug.cgi?id=52403
ComplexTextController previously supported iterating through the text in
both directions, but this resulted in duplicate code for each path.
Instead, by being more careful about flipping signs where appropriate,
we can refactor the code into one code path.
No tests, just a refactoring; should be covered by existing tests.
* platform/graphics/chromium/ComplexTextControllerLinux.cpp:
(WebCore::ComplexTextController::ComplexTextController):
(WebCore::ComplexTextController::reset):
(WebCore::ComplexTextController::nextScriptRun):
* platform/graphics/chromium/ComplexTextControllerLinux.h:
* platform/graphics/chromium/FontLinux.cpp:
(WebCore::glyphIndexForXPositionInScriptRun):
(WebCore::Font::selectionRectForComplexText):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@75756 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 26881d7..482f94d 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,26 @@
+2011-01-13 Evan Martin <evan at chromium.org>
+
+ Reviewed by Tony Chang.
+
+ [chromium] drop backwards iteration in Linux complex text code
+ https://bugs.webkit.org/show_bug.cgi?id=52403
+
+ ComplexTextController previously supported iterating through the text in
+ both directions, but this resulted in duplicate code for each path.
+ Instead, by being more careful about flipping signs where appropriate,
+ we can refactor the code into one code path.
+
+ No tests, just a refactoring; should be covered by existing tests.
+
+ * platform/graphics/chromium/ComplexTextControllerLinux.cpp:
+ (WebCore::ComplexTextController::ComplexTextController):
+ (WebCore::ComplexTextController::reset):
+ (WebCore::ComplexTextController::nextScriptRun):
+ * platform/graphics/chromium/ComplexTextControllerLinux.h:
+ * platform/graphics/chromium/FontLinux.cpp:
+ (WebCore::glyphIndexForXPositionInScriptRun):
+ (WebCore::Font::selectionRectForComplexText):
+
2011-01-13 Dimitri Glazkov <dglazkov at chromium.org>
Reviewed by Darin Adler.
diff --git a/Source/WebCore/platform/graphics/chromium/ComplexTextControllerLinux.cpp b/Source/WebCore/platform/graphics/chromium/ComplexTextControllerLinux.cpp
index e9adcc3..99159e6 100644
--- a/Source/WebCore/platform/graphics/chromium/ComplexTextControllerLinux.cpp
+++ b/Source/WebCore/platform/graphics/chromium/ComplexTextControllerLinux.cpp
@@ -50,7 +50,6 @@ ComplexTextController::ComplexTextController(const TextRun& run, unsigned starti
, m_startingX(startingX)
, m_offsetX(m_startingX)
, m_run(getNormalizedTextRun(run, m_normalizedRun, m_normalizedBuffer))
- , m_iterateBackwards(m_run.rtl())
, m_wordSpacingAdjustment(0)
, m_padding(0)
, m_padPerWordBreak(0)
@@ -140,54 +139,33 @@ void ComplexTextController::setPadding(int padding)
void ComplexTextController::reset()
{
- if (m_iterateBackwards)
- m_indexOfNextScriptRun = m_run.length() - 1;
- else
- m_indexOfNextScriptRun = 0;
+ m_indexOfNextScriptRun = 0;
m_offsetX = m_startingX;
}
-void ComplexTextController::setBackwardsIteration(bool isBackwards)
-{
- m_iterateBackwards = isBackwards;
- reset();
-}
-
// Advance to the next script run, returning false when the end of the
// TextRun has been reached.
bool ComplexTextController::nextScriptRun()
{
- if (m_iterateBackwards) {
- // In right-to-left mode we need to render the shaped glyph backwards and
- // also render the script runs themselves backwards. So given a TextRun:
- // AAAAAAACTTTTTTT (A = Arabic, C = Common, T = Thai)
- // we render:
- // TTTTTTCAAAAAAA
- // (and the glyphs in each A, C and T section are backwards too)
- if (!hb_utf16_script_run_prev(&m_numCodePoints, &m_item.item, m_run.characters(), m_run.length(), &m_indexOfNextScriptRun))
- return false;
- m_currentFontData = m_font->glyphDataForCharacter(m_item.string[m_item.item.pos], false).fontData;
- } else {
- if (!hb_utf16_script_run_next(&m_numCodePoints, &m_item.item, m_run.characters(), m_run.length(), &m_indexOfNextScriptRun))
- return false;
-
- // It is actually wrong to consider script runs at all in this code.
- // Other WebKit code (e.g. Mac) segments complex text just by finding
- // the longest span of text covered by a single font.
- // But we currently need to call hb_utf16_script_run_next anyway to fill
- // in the harfbuzz data structures to e.g. pick the correct script's shaper.
- // So we allow that to run first, then do a second pass over the range it
- // found and take the largest subregion that stays within a single font.
- m_currentFontData = m_font->glyphDataForCharacter(m_item.string[m_item.item.pos], false).fontData;
- unsigned endOfRun;
- for (endOfRun = 1; endOfRun < m_item.item.length; ++endOfRun) {
- const SimpleFontData* nextFontData = m_font->glyphDataForCharacter(m_item.string[m_item.item.pos + endOfRun], false).fontData;
- if (nextFontData != m_currentFontData)
- break;
- }
- m_item.item.length = endOfRun;
- m_indexOfNextScriptRun = m_item.item.pos + endOfRun;
+ if (!hb_utf16_script_run_next(&m_numCodePoints, &m_item.item, m_run.characters(), m_run.length(), &m_indexOfNextScriptRun))
+ return false;
+
+ // It is actually wrong to consider script runs at all in this code.
+ // Other WebKit code (e.g. Mac) segments complex text just by finding
+ // the longest span of text covered by a single font.
+ // But we currently need to call hb_utf16_script_run_next anyway to fill
+ // in the harfbuzz data structures to e.g. pick the correct script's shaper.
+ // So we allow that to run first, then do a second pass over the range it
+ // found and take the largest subregion that stays within a single font.
+ m_currentFontData = m_font->glyphDataForCharacter(m_item.string[m_item.item.pos], false).fontData;
+ unsigned endOfRun;
+ for (endOfRun = 1; endOfRun < m_item.item.length; ++endOfRun) {
+ const SimpleFontData* nextFontData = m_font->glyphDataForCharacter(m_item.string[m_item.item.pos + endOfRun], false).fontData;
+ if (nextFontData != m_currentFontData)
+ break;
}
+ m_item.item.length = endOfRun;
+ m_indexOfNextScriptRun = m_item.item.pos + endOfRun;
setupFontForScriptRun();
shapeGlyphs();
diff --git a/Source/WebCore/platform/graphics/chromium/ComplexTextControllerLinux.h b/Source/WebCore/platform/graphics/chromium/ComplexTextControllerLinux.h
index 4ebbd89..e264b99 100644
--- a/Source/WebCore/platform/graphics/chromium/ComplexTextControllerLinux.h
+++ b/Source/WebCore/platform/graphics/chromium/ComplexTextControllerLinux.h
@@ -52,9 +52,8 @@ class SimpleFontData;
// only ever done with script runs since the shapers only know how to deal with
// a single script.
//
-// After creating it, the script runs are either iterated backwards or forwards.
-// It defaults to backwards for RTL and forwards otherwise (which matches the
-// presentation order), however you can set it with |setBackwardsIteration|.
+// Iteration is always in logical (aka reading) order. For RTL text that means
+// the rightmost part of the text will be first.
//
// Once you have setup the object, call |nextScriptRun| to get the first script
// run. This will return false when the iteration is complete. At any time you
@@ -70,7 +69,6 @@ public:
// WebKit uses this to justify text.
void setPadding(int);
void reset();
- void setBackwardsIteration(bool);
// Advance to the next script run, returning false when the end of the
// TextRun has been reached.
bool nextScriptRun();
@@ -148,7 +146,6 @@ private:
OwnPtr<TextRun> m_normalizedRun;
OwnArrayPtr<UChar> m_normalizedBuffer; // A buffer for normalized run.
const TextRun& m_run;
- bool m_iterateBackwards;
int m_wordSpacingAdjustment; // delta adjustment (pixels) for each word break.
float m_padding; // pixels to be distributed over the line at word breaks.
float m_padPerWordBreak; // pixels to be added to each word break.
diff --git a/Source/WebCore/platform/graphics/chromium/FontLinux.cpp b/Source/WebCore/platform/graphics/chromium/FontLinux.cpp
index b256e70..f1eadf2 100644
--- a/Source/WebCore/platform/graphics/chromium/FontLinux.cpp
+++ b/Source/WebCore/platform/graphics/chromium/FontLinux.cpp
@@ -234,28 +234,21 @@ float Font::floatWidthForComplexText(const TextRun& run, HashSet<const SimpleFon
return controller.widthOfFullRun();
}
-static int glyphIndexForXPositionInScriptRun(const ComplexTextController& controller, int x)
+static int glyphIndexForXPositionInScriptRun(const ComplexTextController& controller, int targetX)
{
- const HB_Fixed* advances = controller.advances();
- int letterSpacing = controller.letterSpacing();
- int glyphIndex;
- if (controller.rtl()) {
- for (glyphIndex = controller.length() - 1; glyphIndex >= 0; --glyphIndex) {
- // When iterating LTR over RTL text, we must include the whitespace
- // _before_ the glyph, so no + 1 here.
- if (x < (static_cast<int>(controller.length()) - glyphIndex) * letterSpacing + truncateFixedPointToInteger(advances[glyphIndex]))
- break;
- x -= truncateFixedPointToInteger(advances[glyphIndex]);
- }
- } else {
- for (glyphIndex = 0; static_cast<unsigned>(glyphIndex) < controller.length(); ++glyphIndex) {
- if (x < (glyphIndex * letterSpacing + truncateFixedPointToInteger(advances[glyphIndex])))
- break;
- x -= truncateFixedPointToInteger(advances[glyphIndex]);
- }
+ // Iterate through the glyphs in logical order, seeing whether targetX falls between the previous
+ // position and halfway through the current glyph.
+ // FIXME: this code probably belongs in ComplexTextController.
+ int lastX = controller.rtl() ? controller.width() : 0;
+ for (int glyphIndex = 0; static_cast<unsigned>(glyphIndex) < controller.length(); ++glyphIndex) {
+ int advance = truncateFixedPointToInteger(controller.advances()[glyphIndex]);
+ int nextX = static_cast<int>(controller.xPositions()[glyphIndex]) + advance / 2;
+ if (std::min(nextX, lastX) <= targetX && targetX <= std::max(nextX, lastX))
+ return glyphIndex;
+ lastX = nextX;
}
- return glyphIndex;
+ return controller.length() - 1;
}
// Return the code point index for the given |x| offset into the text run.
@@ -345,20 +338,16 @@ FloatRect Font::selectionRectForComplexText(const TextRun& run,
const FloatPoint& point, int height,
int from, int to) const
{
- int fromX = -1, toX = -1, fromAdvance = -1, toAdvance = -1;
+ int fromX = -1, toX = -1;
ComplexTextController controller(run, 0, this);
controller.setWordSpacingAdjustment(wordSpacing());
controller.setLetterSpacingAdjustment(letterSpacing());
- // Base will point to the x offset for the current script run. Note that, in
+ // Base will point to the x offset for the start of the current script run. Note that, in
// the LTR case, width will be 0.
int base = controller.rtl() ? controller.widthOfFullRun() : 0;
- const int leftEdge = base;
-
- // We want to enumerate the script runs in code point order in the following
- // code. This call also resets |controller|.
- controller.setBackwardsIteration(false);
+ controller.reset();
while (controller.nextScriptRun() && (fromX == -1 || toX == -1)) {
// ComplexTextController will helpfully accululate the x offsets for different
// script runs for us. For this code, however, we always want the x offsets
@@ -374,14 +363,16 @@ FloatRect Font::selectionRectForComplexText(const TextRun& run,
// position.
int glyph = controller.logClusters()[from];
fromX = base + controller.xPositions()[glyph];
- fromAdvance = controller.advances()[glyph];
+ if (controller.rtl())
+ fromX += truncateFixedPointToInteger(controller.advances()[glyph]);
} else
from -= controller.numCodePoints();
if (toX == -1 && to >= 0 && static_cast<unsigned>(to) < controller.numCodePoints()) {
int glyph = controller.logClusters()[to];
toX = base + controller.xPositions()[glyph];
- toAdvance = controller.advances()[glyph];
+ if (controller.rtl())
+ toX += truncateFixedPointToInteger(controller.advances()[glyph]);
} else
to -= controller.numCodePoints();
@@ -390,14 +381,11 @@ FloatRect Font::selectionRectForComplexText(const TextRun& run,
}
// The position in question might be just after the text.
- const int rightEdge = base;
+ const int endEdge = base;
if (fromX == -1 && !from)
- fromX = leftEdge;
- else if (controller.rtl())
- fromX += truncateFixedPointToInteger(fromAdvance);
-
+ fromX = endEdge;
if (toX == -1 && !to)
- toX = rightEdge;
+ toX = endEdge;
ASSERT(fromX != -1 && toX != -1);
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list