Attachment 48598 Details for Bug 34865 – Account for non-monotonic character-to-glyph mapping

[patch] Account for non-monotonic character-to-glyph mapping

7609268_r1.diff (text/plain), 10.59 KB, created by mitz on 2010-02-11 17:36:50 PST

(hide)

Description:

Filename:

MIME Type:

Creator: mitz

Created: 2010-02-11 17:36:50 PST

Size: 10.59 KB

patch

obsolete

>Index: WebCore/ChangeLog
>===================================================================
>--- WebCore/ChangeLog	(revision 54689)
>+++ WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,29 @@
>+2010-02-11  Dan Bernstein  <mitz@apple.com>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        <rdar://problem/7609268> REGRESSION (r50301): Problem selecting text in a Devanagari website
>+        https://bugs.webkit.org/show_bug.cgi?id=34865
>+
>+        Test: fast/text/glyph-reordering.html
>+
>+        * platform/graphics/mac/ComplexTextController.cpp:
>+        (WebCore::ComplexTextController::ComplexTextRun::ComplexTextRun): Initialize m_isMonotonic.
>+        (WebCore::ComplexTextController::ComplexTextRun::setIsMonotonic): Added. Sets m_isMonotonic,
>+        and if the run is not monotonic, populates m_lastInidices with the end offsets of each glyphâs
>+        corresponding character range.
>+        (WebCore::ComplexTextController::advance): For non-monotonic runs, use lastIndexAt() to get
>+        glyphsâ end offsets.
>+        (WebCore::ComplexTextController::adjustGlyphsAndAdvances): Check for monotonicity and call
>+        setIsMonotonic(false) if needed.
>+        * platform/graphics/mac/ComplexTextController.h:
>+        (WebCore::ComplexTextController::ComplexTextRun::lastIndexAt): Added this getter.
>+        (WebCore::ComplexTextController::ComplexTextRun::isMonotonic): Ditto.
>+        * platform/graphics/mac/ComplexTextControllerATSUI.cpp:
>+        (WebCore::ComplexTextController::ComplexTextRun::ComplexTextRun): Initialize m_isMonotonic.
>+        * platform/graphics/mac/ComplexTextControllerCoreText.cpp:
>+        (WebCore::ComplexTextController::ComplexTextRun::ComplexTextRun): Ditto.
>+
> 2010-02-11  Kwang Yul Seo  <skyul@company100.net>
> 
>         Reviewed by Eric Seidel.
>Index: WebCore/platform/graphics/mac/ComplexTextController.cpp
>===================================================================
>--- WebCore/platform/graphics/mac/ComplexTextController.cpp	(revision 54657)
>+++ WebCore/platform/graphics/mac/ComplexTextController.cpp	(working copy)
>@@ -316,6 +316,7 @@ ComplexTextController::ComplexTextRun::C
>     , m_characters(characters)
>     , m_stringLocation(stringLocation)
>     , m_stringLength(stringLength)
>+    , m_isMonotonic(true)
> {
> #if USE(CORE_TEXT) && USE(ATSUI)
>     shouldUseATSUIAPI() ? createTextRunFromFontDataATSUI(ltr) : createTextRunFromFontDataCoreText(ltr);
>@@ -326,6 +327,31 @@ ComplexTextController::ComplexTextRun::C
> #endif
> }
> 
>+void ComplexTextController::ComplexTextRun::setIsMonotonic(bool isMonotonic)
>+{
>+    m_isMonotonic = isMonotonic;
>+    if (isMonotonic)
>+        return;
>+
>+    Vector<bool, 64> mappedIndices(m_stringLength);
>+    for (size_t i = 0; i < m_glyphCount; ++i) {
>+        ASSERT(indexAt(i) < static_cast<CFIndex>(m_stringLength));
>+        mappedIndices[indexAt(i)] = true;
>+    }
>+
>+    m_lastIndices.grow(m_glyphCount);
>+    for (size_t i = 0; i < m_glyphCount; ++i) {
>+        CFIndex nextMappedIndex = m_stringLength;
>+        for (size_t j = indexAt(i) + 1; j < m_stringLength; ++j) {
>+            if (mappedIndices[j]) {
>+                nextMappedIndex = j;
>+                break;
>+            }
>+        }
>+        m_lastIndices[i] = nextMappedIndex;
>+    }
>+}
>+
> void ComplexTextController::advance(unsigned offset, GlyphBuffer* glyphBuffer)
> {
>     if (static_cast<int>(offset) > m_end)
>@@ -348,10 +374,13 @@ void ComplexTextController::advance(unsi
>         while (m_glyphInCurrentRun < glyphCount) {
>             unsigned glyphStartOffset = complexTextRun.indexAt(g);
>             unsigned glyphEndOffset;
>-            if (ltr)
>-                glyphEndOffset = max<unsigned>(glyphStartOffset, g + 1 < glyphCount ? complexTextRun.indexAt(g + 1) : complexTextRun.stringLength());
>-            else
>-                glyphEndOffset = max<unsigned>(glyphStartOffset, g > 0 ? complexTextRun.indexAt(g - 1) : complexTextRun.stringLength());
>+            if (complexTextRun.isMonotonic()) {
>+                if (ltr)
>+                    glyphEndOffset = max<unsigned>(glyphStartOffset, g + 1 < glyphCount ? complexTextRun.indexAt(g + 1) : complexTextRun.stringLength());
>+                else
>+                    glyphEndOffset = max<unsigned>(glyphStartOffset, g > 0 ? complexTextRun.indexAt(g - 1) : complexTextRun.stringLength());
>+            } else
>+                glyphEndOffset = complexTextRun.lastIndexAt(g);
> 
>             CGSize adjustedAdvance = m_adjustedAdvances[k];
> 
>@@ -393,7 +422,7 @@ void ComplexTextController::adjustGlyphs
> {
>     size_t runCount = m_complexTextRuns.size();
>     for (size_t r = 0; r < runCount; ++r) {
>-        const ComplexTextRun& complexTextRun = *m_complexTextRuns[r];
>+        ComplexTextRun& complexTextRun = *m_complexTextRuns[r];
>         unsigned glyphCount = complexTextRun.glyphCount();
>         const SimpleFontData* fontData = complexTextRun.fontData();
> 
>@@ -405,10 +434,18 @@ void ComplexTextController::adjustGlyphs
>         CGFloat roundedSpaceWidth = roundCGFloat(fontData->spaceWidth());
>         bool roundsAdvances = !m_font.isPrinterFont() && fontData->platformData().roundsGlyphAdvances();
>         bool hasExtraSpacing = (m_font.letterSpacing() || m_font.wordSpacing() || m_padding) && !m_run.spacingDisabled();
>-
>+        CFIndex lastCharacterIndex = m_run.ltr() ? numeric_limits<CFIndex>::min() : numeric_limits<CFIndex>::max();
>+        bool isMonotonic = true;
> 
>         for (unsigned i = 0; i < glyphCount; i++) {
>             CFIndex characterIndex = complexTextRun.indexAt(i);
>+            if (m_run.ltr()) {
>+                if (characterIndex < lastCharacterIndex)
>+                    isMonotonic = false;
>+            } else {
>+                if (characterIndex > lastCharacterIndex)
>+                    isMonotonic = false;
>+            }
>             UChar ch = *(cp + characterIndex);
>             bool lastGlyph = lastRun && i + 1 == glyphCount;
>             UChar nextCh;
>@@ -500,7 +537,10 @@ void ComplexTextController::adjustGlyphs
>             advance.height *= -1;
>             m_adjustedAdvances.append(advance);
>             m_adjustedGlyphs.append(glyph);
>+            lastCharacterIndex = characterIndex;
>         }
>+        if (!isMonotonic)
>+            complexTextRun.setIsMonotonic(false);
>     }
> }
> 
>Index: WebCore/platform/graphics/mac/ComplexTextController.h
>===================================================================
>--- WebCore/platform/graphics/mac/ComplexTextController.h	(revision 54657)
>+++ WebCore/platform/graphics/mac/ComplexTextController.h	(working copy)
>@@ -88,8 +88,11 @@ private:
>         unsigned stringLocation() const { return m_stringLocation; }
>         size_t stringLength() const { return m_stringLength; }
>         ALWAYS_INLINE CFIndex indexAt(size_t i) const;
>+        CFIndex lastIndexAt(size_t i) const { ASSERT(!m_isMonotonic); return m_lastIndices[i]; }
>         const CGGlyph* glyphs() const { return m_glyphs; }
>         const CGSize* advances() const { return m_advances; }
>+        bool isMonotonic() const { return m_isMonotonic; }
>+        void setIsMonotonic(bool);
> 
>     private:
> #if USE(CORE_TEXT)
>@@ -124,14 +127,15 @@ private:
> #if USE(ATSUI)
>         Vector<CFIndex, 64> m_atsuiIndices;
> #endif
>+        Vector<CFIndex, 64> m_lastIndices;
>         Vector<CGGlyph, 64> m_glyphsVector;
>         const CGGlyph* m_glyphs;
>         Vector<CGSize, 64> m_advancesVector;
>         const CGSize* m_advances;
> #if USE(ATSUI)
>-        bool m_ltr;
>         bool m_directionalOverride;
> #endif
>+        bool m_isMonotonic;
>     };
> 
>     void collectComplexTextRuns();
>Index: WebCore/platform/graphics/mac/ComplexTextControllerATSUI.cpp
>===================================================================
>--- WebCore/platform/graphics/mac/ComplexTextControllerATSUI.cpp	(revision 54657)
>+++ WebCore/platform/graphics/mac/ComplexTextControllerATSUI.cpp	(working copy)
>@@ -145,8 +145,8 @@ ComplexTextController::ComplexTextRun::C
>     , m_characters(characters)
>     , m_stringLocation(stringLocation)
>     , m_stringLength(stringLength)
>-    , m_ltr(ltr)
>     , m_directionalOverride(directionalOverride)
>+    , m_isMonotonic(true)
> {
>     OSStatus status;
> 
>Index: WebCore/platform/graphics/mac/ComplexTextControllerCoreText.cpp
>===================================================================
>--- WebCore/platform/graphics/mac/ComplexTextControllerCoreText.cpp	(revision 54657)
>+++ WebCore/platform/graphics/mac/ComplexTextControllerCoreText.cpp	(working copy)
>@@ -46,6 +46,7 @@ ComplexTextController::ComplexTextRun::C
>     , m_characters(characters)
>     , m_stringLocation(stringLocation)
>     , m_stringLength(stringLength)
>+    , m_isMonotonic(true)
> {
>     m_glyphCount = CTRunGetGlyphCount(m_coreTextRun.get());
>     m_coreTextIndices = CTRunGetStringIndicesPtr(m_coreTextRun.get());
>Index: LayoutTests/ChangeLog
>===================================================================
>--- LayoutTests/ChangeLog	(revision 54689)
>+++ LayoutTests/ChangeLog	(working copy)
>@@ -1,3 +1,13 @@
>+2010-02-11  Dan Bernstein  <mitz@apple.com>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        <rdar://problem/7609268> REGRESSION (r50301): Problem selecting text in a Devanagari website
>+        https://bugs.webkit.org/show_bug.cgi?id=34865
>+
>+        * fast/text/glyph-reordering-expected.txt: Added.
>+        * fast/text/glyph-reordering.html: Added.
>+
> 2010-02-11  Nikolas Zimmermann  <nzimmermann@rim.com>
> 
>         Not reviewed. Oops, move mac-lepard results for mask-invalidation.svg into the correct folder.
>Index: LayoutTests/fast/text/glyph-reordering-expected.txt
>===================================================================
>--- LayoutTests/fast/text/glyph-reordering-expected.txt	(revision 0)
>+++ LayoutTests/fast/text/glyph-reordering-expected.txt	(revision 0)
>@@ -0,0 +1,4 @@
>+This tests text selection in complex scripts where glyph reordering occurs.
>+
>+à¤¹à¤¿à¤¨à¥à¤¦à¥
>+PASS
>Index: LayoutTests/fast/text/glyph-reordering.html
>===================================================================
>--- LayoutTests/fast/text/glyph-reordering.html	(revision 0)
>+++ LayoutTests/fast/text/glyph-reordering.html	(revision 0)
>@@ -0,0 +1,17 @@
>+<p>
>+    This tests text selection in complex scripts where glyph reordering occurs.
>+</p>
>+<div id="target" style="font-size: 48px;">&#x0939;&#x093F;&#x0928;&#x094D;&#x0926;&#x0940;</div>
>+<p id="result">Test did not run</p>
>+<script>
>+    if (window.layoutTestController)
>+        layoutTestController.dumpAsText();
>+
>+    var textNode = target.firstChild;
>+    var range = document.createRange();
>+    range.setStart(textNode, 0);
>+    range.setEnd(textNode, 2);
>+
>+    var clientRects = range.getClientRects();
>+    document.getElementById("result").innerText = clientRects[0].width === 41 ? "PASS" : "FAIL: width was " + clientRects[0].width;
>+</script>

Flags:

simon.fraser: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 34865: 48598