[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 23:17:10 UTC 2011
The following commit has been merged in the webkit-1.3 branch:
commit 1642e005a72e8db3549d2e50eb6545643c38e13a
Author: evan at chromium.org <evan at chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Wed Jan 19 18:29:15 2011 +0000
2011-01-18 Evan Martin <evan at chromium.org>
Reviewed by Tony Chang.
[chromium] simplify complex text code, fixing a handful of layout tests
https://bugs.webkit.org/show_bug.cgi?id=52661
* platform/chromium/test_expectations.txt: Mark 11 tests as expected to pass.
2011-01-18 Evan Martin <evan at chromium.org>
Reviewed by Tony Chang.
[chromium] simplify complex text code, fixing a handful of layout tests
https://bugs.webkit.org/show_bug.cgi?id=52661
Change ComplexTextControllerLinux to lay out RTL text to the left from
the starting point. (Previously it always went to the right.) This allows
us to map pixel offsets more directly into offsets within the text runs,
simplifying a few of the text-fiddling functions (they no longer need to
track the current position, as ComplexTextController now does it).
* platform/graphics/chromium/ComplexTextControllerLinux.cpp:
(WebCore::ComplexTextController::ComplexTextController):
(WebCore::ComplexTextController::reset):
(WebCore::ComplexTextController::setGlyphXPositions):
* platform/graphics/chromium/ComplexTextControllerLinux.h:
(WebCore::ComplexTextController::offsetX):
* platform/graphics/chromium/FontLinux.cpp:
(WebCore::Font::drawComplexText):
(WebCore::Font::floatWidthForComplexText):
(WebCore::glyphIndexForXPositionInScriptRun):
(WebCore::Font::offsetForPositionForComplexText):
(WebCore::Font::selectionRectForComplexText):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@76137 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 4f12f06..2ec1846 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,12 @@
+2011-01-18 Evan Martin <evan at chromium.org>
+
+ Reviewed by Tony Chang.
+
+ [chromium] simplify complex text code, fixing a handful of layout tests
+ https://bugs.webkit.org/show_bug.cgi?id=52661
+
+ * platform/chromium/test_expectations.txt: Mark 11 tests as expected to pass.
+
2011-01-19 Michael Saboff <msaboff at apple.com>
Reviewed by Darin Adler.
diff --git a/LayoutTests/platform/chromium/test_expectations.txt b/LayoutTests/platform/chromium/test_expectations.txt
index 38cb868..f8b4abb 100644
--- a/LayoutTests/platform/chromium/test_expectations.txt
+++ b/LayoutTests/platform/chromium/test_expectations.txt
@@ -3079,19 +3079,6 @@ BUGCR69411 LINUX : svg/custom/linking-a-03-b-transform.svg = IMAGE PASS
// New test added in r75720
BUGCR69571 : plugins/destroy-on-setwindow.html = CRASH
-// Failing after r75720
-BUGCR69612 LINUX : editing/selection/extend-to-line-boundary.html = TEXT
-BUGCR69612 LINUX : fast/text/atsui-negative-spacing-features.html = IMAGE
-BUGCR69612 LINUX : fast/text/atsui-spacing-features.html = IMAGE
-BUGCR69612 LINUX : fast/text/drawBidiText.html = IMAGE
-BUGCR69612 LINUX : fast/text/international/bidi-AN-after-empty-run.html = IMAGE
-BUGCR69612 LINUX : fast/text/international/bidi-CS-after-AN.html = IMAGE
-BUGCR69612 LINUX : fast/text/international/bidi-linebreak-002.html = IMAGE
-BUGCR69612 LINUX : fast/text/international/bidi-linebreak-003.html = IMAGE
-BUGCR69612 LINUX : fast/text/international/bidi-mirror-he-ar.html = IMAGE
-BUGCR69612 LINUX : fast/text/international/bidi-neutral-run.html = IMAGE
-BUGCR69612 LINUX : fast/text/international/hebrew-vowels.html = IMAGE
-
// Failing after r75768
BUGCR69639 : http/tests/loading/cross-origin-XHR-willLoadRequest.html = TEXT
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index fbca136..e428d33 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,29 @@
+2011-01-18 Evan Martin <evan at chromium.org>
+
+ Reviewed by Tony Chang.
+
+ [chromium] simplify complex text code, fixing a handful of layout tests
+ https://bugs.webkit.org/show_bug.cgi?id=52661
+
+ Change ComplexTextControllerLinux to lay out RTL text to the left from
+ the starting point. (Previously it always went to the right.) This allows
+ us to map pixel offsets more directly into offsets within the text runs,
+ simplifying a few of the text-fiddling functions (they no longer need to
+ track the current position, as ComplexTextController now does it).
+
+ * platform/graphics/chromium/ComplexTextControllerLinux.cpp:
+ (WebCore::ComplexTextController::ComplexTextController):
+ (WebCore::ComplexTextController::reset):
+ (WebCore::ComplexTextController::setGlyphXPositions):
+ * platform/graphics/chromium/ComplexTextControllerLinux.h:
+ (WebCore::ComplexTextController::offsetX):
+ * platform/graphics/chromium/FontLinux.cpp:
+ (WebCore::Font::drawComplexText):
+ (WebCore::Font::floatWidthForComplexText):
+ (WebCore::glyphIndexForXPositionInScriptRun):
+ (WebCore::Font::offsetForPositionForComplexText):
+ (WebCore::Font::selectionRectForComplexText):
+
2011-01-19 Pavel Feldman <pfeldman at chromium.org>
Reviewed by Yury Semikhatsky.
diff --git a/Source/WebCore/platform/graphics/chromium/ComplexTextControllerLinux.cpp b/Source/WebCore/platform/graphics/chromium/ComplexTextControllerLinux.cpp
index 99159e6..4685628 100644
--- a/Source/WebCore/platform/graphics/chromium/ComplexTextControllerLinux.cpp
+++ b/Source/WebCore/platform/graphics/chromium/ComplexTextControllerLinux.cpp
@@ -47,8 +47,6 @@ static int truncateFixedPointToInteger(HB_Fixed value)
ComplexTextController::ComplexTextController(const TextRun& run, unsigned startingX, const Font* font)
: m_font(font)
- , m_startingX(startingX)
- , m_offsetX(m_startingX)
, m_run(getNormalizedTextRun(run, m_normalizedRun, m_normalizedBuffer))
, m_wordSpacingAdjustment(0)
, m_padding(0)
@@ -75,7 +73,7 @@ ComplexTextController::ComplexTextController(const TextRun& run, unsigned starti
m_item.string = m_run.characters();
m_item.stringLength = m_run.length();
- reset();
+ reset(startingX);
}
ComplexTextController::~ComplexTextController()
@@ -137,10 +135,10 @@ void ComplexTextController::setPadding(int padding)
m_padPerWordBreak = 0;
}
-void ComplexTextController::reset()
+void ComplexTextController::reset(unsigned offset)
{
m_indexOfNextScriptRun = 0;
- m_offsetX = m_startingX;
+ m_offsetX = offset;
}
// Advance to the next script run, returning false when the end of the
@@ -277,8 +275,7 @@ void ComplexTextController::setGlyphXPositions(bool isRTL)
int logClustersIndex = 0;
// Iterate through the glyphs in logical order, flipping for RTL where necessary.
- // In RTL mode all variables are positive except m_xPositions, which starts from m_offsetX and runs negative.
- // It is fixed up in a second pass below.
+ // Glyphs are positioned starting from m_offsetX; in RTL mode they go leftwards from there.
for (size_t i = 0; i < m_item.num_glyphs; ++i) {
while (static_cast<unsigned>(logClustersIndex) < m_item.item.length && logClusters()[logClustersIndex] < i)
logClustersIndex++;
@@ -303,16 +300,8 @@ void ComplexTextController::setGlyphXPositions(bool isRTL)
position += advance;
}
- const double width = position;
-
- // Now that we've computed the total width, do another pass to fix positioning for RTL.
- if (isRTL) {
- for (size_t i = 0; i < m_item.num_glyphs; ++i)
- m_xPositions[i] += width;
- }
-
- m_pixelWidth = std::max(width, 0.0);
- m_offsetX += m_pixelWidth;
+ m_pixelWidth = std::max(position, 0.0);
+ m_offsetX += m_pixelWidth * rtlFlip;
}
void ComplexTextController::normalizeSpacesAndMirrorChars(const UChar* source, bool rtl, UChar* destination, int length)
diff --git a/Source/WebCore/platform/graphics/chromium/ComplexTextControllerLinux.h b/Source/WebCore/platform/graphics/chromium/ComplexTextControllerLinux.h
index e264b99..a2aea60 100644
--- a/Source/WebCore/platform/graphics/chromium/ComplexTextControllerLinux.h
+++ b/Source/WebCore/platform/graphics/chromium/ComplexTextControllerLinux.h
@@ -68,7 +68,7 @@ public:
// setPadding sets a number of pixels to be distributed across the TextRun.
// WebKit uses this to justify text.
void setPadding(int);
- void reset();
+ void reset(unsigned offset);
// Advance to the next script run, returning false when the end of the
// TextRun has been reached.
bool nextScriptRun();
@@ -86,7 +86,6 @@ public:
// Set the x offset for the next script run. This affects the values in
// |xPositions|
- void setXOffsetToZero() { m_offsetX = 0; }
bool rtl() const { return m_run.rtl(); }
const uint16_t* glyphs() const { return m_glyphs16; }
@@ -114,6 +113,9 @@ public:
// return the number of code points in the current script run
const unsigned numCodePoints() const { return m_numCodePoints; }
+ // Return the current pixel position of the controller.
+ const unsigned offsetX() const { return m_offsetX; }
+
const FontPlatformData* fontPlatformDataForScriptRun() { return reinterpret_cast<FontPlatformData*>(m_item.font->userData); }
private:
@@ -137,7 +139,6 @@ private:
uint16_t* m_glyphs16; // A vector of 16-bit glyph ids.
SkScalar* m_xPositions; // A vector of x positions for each glyph.
ssize_t m_indexOfNextScriptRun; // Indexes the script run in |m_run|.
- const unsigned m_startingX; // Offset in pixels of the first script run.
unsigned m_offsetX; // Offset in pixels to the start of the next script run.
unsigned m_pixelWidth; // Width (in px) of the current script run.
unsigned m_numCodePoints; // Code points in current script run.
diff --git a/Source/WebCore/platform/graphics/chromium/FontLinux.cpp b/Source/WebCore/platform/graphics/chromium/FontLinux.cpp
index f1eadf2..822bbbb 100644
--- a/Source/WebCore/platform/graphics/chromium/FontLinux.cpp
+++ b/Source/WebCore/platform/graphics/chromium/FontLinux.cpp
@@ -206,6 +206,16 @@ void Font::drawComplexText(GraphicsContext* gc, const TextRun& run,
controller.setLetterSpacingAdjustment(letterSpacing());
controller.setPadding(run.padding());
+ if (run.rtl()) {
+ // FIXME: this causes us to shape the text twice -- once to compute the width and then again
+ // below when actually rendering. Change ComplexTextController to match platform/mac and
+ // platform/chromium/win by having it store the shaped runs, so we can reuse the results.
+ controller.reset(point.x() + controller.widthOfFullRun());
+ // We need to set the padding again because ComplexTextController layout consumed the value.
+ // Fixing the above problem would help here too.
+ controller.setPadding(run.padding());
+ }
+
while (controller.nextScriptRun()) {
if (fill) {
controller.fontPlatformDataForScriptRun()->setupPaint(&fillPaint);
@@ -231,6 +241,7 @@ float Font::floatWidthForComplexText(const TextRun& run, HashSet<const SimpleFon
ComplexTextController controller(run, 0, this);
controller.setWordSpacingAdjustment(wordSpacing());
controller.setLetterSpacingAdjustment(letterSpacing());
+ controller.setPadding(run.padding());
return controller.widthOfFullRun();
}
@@ -239,7 +250,7 @@ static int glyphIndexForXPositionInScriptRun(const ComplexTextController& contro
// 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;
+ int lastX = controller.offsetX() - (controller.rtl() ? -controller.width() : controller.width());
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;
@@ -257,53 +268,29 @@ int Font::offsetForPositionForComplexText(const TextRun& run, float xFloat,
{
// FIXME: This truncation is not a problem for HTML, but only affects SVG, which passes floating-point numbers
// to Font::offsetForPosition(). Bug http://webkit.org/b/40673 tracks fixing this problem.
- int x = static_cast<int>(xFloat);
+ int targetX = static_cast<int>(xFloat);
// (Mac code ignores includePartialGlyphs, and they don't know what it's
// supposed to do, so we just ignore it as well.)
ComplexTextController controller(run, 0, this);
controller.setWordSpacingAdjustment(wordSpacing());
controller.setLetterSpacingAdjustment(letterSpacing());
-
- // If this is RTL text, the first glyph from the left is actually the last
- // code point. So we need to know how many code points there are total in
- // order to subtract. This is different from the length of the TextRun
- // because UTF-16 surrogate pairs are a single code point, but 32-bits long.
- // In LTR we leave this as 0 so that we get the correct value for
- // |basePosition|, below.
- unsigned totalCodePoints = 0;
- if (controller.rtl()) {
- ssize_t offset = 0;
- while (offset < run.length()) {
- utf16_to_code_point(run.characters(), run.length(), &offset);
- totalCodePoints++;
- }
+ controller.setPadding(run.padding());
+ if (run.rtl()) {
+ // See FIXME in drawComplexText.
+ controller.reset(controller.widthOfFullRun());
+ controller.setPadding(run.padding());
}
- unsigned basePosition = totalCodePoints;
-
- // For RTL:
- // code-point order: abcd efg hijkl
- // on screen: lkjih gfe dcba
- // ^ ^
- // | |
- // basePosition--| |
- // totalCodePoints----|
- // Since basePosition is currently the total number of code-points, the
- // first thing we do is decrement it so that it's pointing to the start of
- // the current script-run.
- //
- // For LTR, basePosition is zero so it already points to the start of the
- // first script run.
+ unsigned basePosition = 0;
+
+ int x = controller.offsetX();
while (controller.nextScriptRun()) {
- if (controller.rtl())
- basePosition -= controller.numCodePoints();
+ int nextX = controller.offsetX();
- if (x >= 0 && static_cast<unsigned>(x) < controller.width()) {
- // The x value in question is within this script run. We consider
- // each glyph in presentation order and stop when we find the one
- // covering this position.
- const int glyphIndex = glyphIndexForXPositionInScriptRun(controller, x);
+ if (std::min(x, nextX) <= targetX && targetX <= std::max(x, nextX)) {
+ // The x value in question is within this script run.
+ const int glyphIndex = glyphIndexForXPositionInScriptRun(controller, targetX);
// Now that we have a glyph index, we have to turn that into a
// code-point index. Because of ligatures, several code-points may
@@ -324,10 +311,7 @@ int Font::offsetForPositionForComplexText(const TextRun& run, float xFloat,
return basePosition + controller.numCodePoints() - 1;
}
- x -= controller.width();
-
- if (!controller.rtl())
- basePosition += controller.numCodePoints();
+ basePosition += controller.numCodePoints();
}
return basePosition;
@@ -342,27 +326,21 @@ FloatRect Font::selectionRectForComplexText(const TextRun& run,
ComplexTextController controller(run, 0, this);
controller.setWordSpacingAdjustment(wordSpacing());
controller.setLetterSpacingAdjustment(letterSpacing());
+ controller.setPadding(run.padding());
+ if (run.rtl()) {
+ // See FIXME in drawComplexText.
+ controller.reset(controller.widthOfFullRun());
+ controller.setPadding(run.padding());
+ }
- // 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;
-
- controller.reset();
+ // Iterate through the script runs in logical order, searching for the run covering the positions of interest.
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
- // to start from zero so we call this before each script run.
- controller.setXOffsetToZero();
-
- if (controller.rtl())
- base -= controller.width();
-
if (fromX == -1 && from >= 0 && static_cast<unsigned>(from) < controller.numCodePoints()) {
// |from| is within this script run. So we index the clusters log to
// find which glyph this code-point contributed to and find its x
// position.
int glyph = controller.logClusters()[from];
- fromX = base + controller.xPositions()[glyph];
+ fromX = controller.xPositions()[glyph];
if (controller.rtl())
fromX += truncateFixedPointToInteger(controller.advances()[glyph]);
} else
@@ -370,22 +348,18 @@ FloatRect Font::selectionRectForComplexText(const TextRun& run,
if (toX == -1 && to >= 0 && static_cast<unsigned>(to) < controller.numCodePoints()) {
int glyph = controller.logClusters()[to];
- toX = base + controller.xPositions()[glyph];
+ toX = controller.xPositions()[glyph];
if (controller.rtl())
toX += truncateFixedPointToInteger(controller.advances()[glyph]);
} else
to -= controller.numCodePoints();
-
- if (!controller.rtl())
- base += controller.width();
}
// The position in question might be just after the text.
- const int endEdge = base;
- if (fromX == -1 && !from)
- fromX = endEdge;
- if (toX == -1 && !to)
- toX = endEdge;
+ if (fromX == -1)
+ fromX = controller.offsetX();
+ if (toX == -1)
+ toX = controller.offsetX();
ASSERT(fromX != -1 && toX != -1);
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list