Commit 1b0578db authored by mitz@apple.com's avatar mitz@apple.com
Browse files

REGRESSION (r76743): Uneven spacing in right-to-left justified text

https://bugs.webkit.org/show_bug.cgi?id=53225

Reviewed by Sam Weinig.

Fixes failure in fast/text/atsui-spacing-features.html

There was an inconsistency between rendering code and font code in the interpretation of
'after expansion' and 'trailing expansion'. Changed all code to interpret these in terms of
visual order rather than logical.

* platform/graphics/Font.cpp:
(WebCore::Font::expansionOpportunityCount): Added a text direction parameter and changed to
iterate in visual order accordingly.
* platform/graphics/Font.h:
* platform/graphics/WidthIterator.cpp:
(WebCore::WidthIterator::WidthIterator): Pass the run direction to expansionOpportunityCount().
(WebCore::WidthIterator::advance): For right-to-left runs, evaluate the trailing expansion
condition with respect to the first character, which is the trailing character in visual order.
* platform/graphics/mac/ComplexTextController.cpp:
(WebCore::ComplexTextController::ComplexTextController): Pass the run direction to
expansionOpportunityCount().
* rendering/RenderBlockLineLayout.cpp:
(WebCore::RenderBlock::computeInlineDirectionPositionsForLine): Ditto.



git-svn-id: http://svn.webkit.org/repository/webkit/trunk@76768 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent a3be8f70
2011-01-27 Dan Bernstein <mitz@apple.com>
Reviewed by Sam Weinig.
REGRESSION (r76743): Uneven spacing in right-to-left justified text
https://bugs.webkit.org/show_bug.cgi?id=53225
Fixes failure in fast/text/atsui-spacing-features.html
There was an inconsistency between rendering code and font code in the interpretation of
'after expansion' and 'trailing expansion'. Changed all code to interpret these in terms of
visual order rather than logical.
* platform/graphics/Font.cpp:
(WebCore::Font::expansionOpportunityCount): Added a text direction parameter and changed to
iterate in visual order accordingly.
* platform/graphics/Font.h:
* platform/graphics/WidthIterator.cpp:
(WebCore::WidthIterator::WidthIterator): Pass the run direction to expansionOpportunityCount().
(WebCore::WidthIterator::advance): For right-to-left runs, evaluate the trailing expansion
condition with respect to the first character, which is the trailing character in visual order.
* platform/graphics/mac/ComplexTextController.cpp:
(WebCore::ComplexTextController::ComplexTextController): Pass the run direction to
expansionOpportunityCount().
* rendering/RenderBlockLineLayout.cpp:
(WebCore::RenderBlock::computeInlineDirectionPositionsForLine): Ditto.
2011-01-26 Adam Roben <aroben@apple.com> 2011-01-26 Adam Roben <aroben@apple.com>
   
Don't create the Direct3D device before it's first needed Don't create the Direct3D device before it's first needed
...@@ -458,29 +458,52 @@ bool Font::isCJKIdeographOrSymbol(UChar32 c) ...@@ -458,29 +458,52 @@ bool Font::isCJKIdeographOrSymbol(UChar32 c)
return isCJKIdeograph(c); return isCJKIdeograph(c);
} }
unsigned Font::expansionOpportunityCount(const UChar* characters, size_t length, bool& isAfterExpansion) unsigned Font::expansionOpportunityCount(const UChar* characters, size_t length, TextDirection direction, bool& isAfterExpansion)
{ {
static bool expandAroundIdeographs = canExpandAroundIdeographsInComplexText(); static bool expandAroundIdeographs = canExpandAroundIdeographsInComplexText();
unsigned count = 0; unsigned count = 0;
for (size_t i = 0; i < length; ++i) { if (direction == LTR) {
UChar32 character = characters[i]; for (size_t i = 0; i < length; ++i) {
if (treatAsSpace(character)) { UChar32 character = characters[i];
count++; if (treatAsSpace(character)) {
isAfterExpansion = true; count++;
continue; isAfterExpansion = true;
} continue;
if (U16_IS_LEAD(character) && i + 1 < length && U16_IS_TRAIL(characters[i + 1])) { }
character = U16_GET_SUPPLEMENTARY(character, characters[i + 1]); if (U16_IS_LEAD(character) && i + 1 < length && U16_IS_TRAIL(characters[i + 1])) {
i++; character = U16_GET_SUPPLEMENTARY(character, characters[i + 1]);
i++;
}
if (expandAroundIdeographs && isCJKIdeograph(character)) {
if (!isAfterExpansion)
count++;
count++;
isAfterExpansion = true;
continue;
}
isAfterExpansion = false;
} }
if (expandAroundIdeographs && isCJKIdeograph(character)) { } else {
if (!isAfterExpansion) for (size_t i = length; i > 0; --i) {
UChar32 character = characters[i - 1];
if (treatAsSpace(character)) {
count++; count++;
count++; isAfterExpansion = true;
isAfterExpansion = true; continue;
continue; }
if (U16_IS_TRAIL(character) && i > 1 && U16_IS_LEAD(characters[i - 2])) {
character = U16_GET_SUPPLEMENTARY(characters[i - 2], character);
i--;
}
if (expandAroundIdeographs && isCJKIdeograph(character)) {
if (!isAfterExpansion)
count++;
count++;
isAfterExpansion = true;
continue;
}
isAfterExpansion = false;
} }
isAfterExpansion = false;
} }
return count; return count;
} }
......
...@@ -29,6 +29,7 @@ ...@@ -29,6 +29,7 @@
#include "FontDescription.h" #include "FontDescription.h"
#include "FontFallbackList.h" #include "FontFallbackList.h"
#include "SimpleFontData.h" #include "SimpleFontData.h"
#include "TextDirection.h"
#include "TypesettingFeatures.h" #include "TypesettingFeatures.h"
#include <wtf/HashMap.h> #include <wtf/HashMap.h>
#include <wtf/HashSet.h> #include <wtf/HashSet.h>
...@@ -143,7 +144,7 @@ public: ...@@ -143,7 +144,7 @@ public:
static bool isCJKIdeograph(UChar32); static bool isCJKIdeograph(UChar32);
static bool isCJKIdeographOrSymbol(UChar32); static bool isCJKIdeographOrSymbol(UChar32);
static unsigned expansionOpportunityCount(const UChar*, size_t length, bool& isAfterExpansion); static unsigned expansionOpportunityCount(const UChar*, size_t length, TextDirection, bool& isAfterExpansion);
#if PLATFORM(QT) #if PLATFORM(QT)
QFont font() const; QFont font() const;
......
...@@ -64,7 +64,7 @@ WidthIterator::WidthIterator(const Font* font, const TextRun& run, HashSet<const ...@@ -64,7 +64,7 @@ WidthIterator::WidthIterator(const Font* font, const TextRun& run, HashSet<const
m_expansionPerOpportunity = 0; m_expansionPerOpportunity = 0;
else { else {
bool isAfterExpansion = true; bool isAfterExpansion = true;
unsigned expansionOpportunityCount = Font::expansionOpportunityCount(m_run.characters(), m_end, isAfterExpansion); unsigned expansionOpportunityCount = Font::expansionOpportunityCount(m_run.characters(), m_end, m_run.ltr() ? LTR : RTL, isAfterExpansion);
if (isAfterExpansion && !m_run.allowsTrailingExpansion()) if (isAfterExpansion && !m_run.allowsTrailingExpansion())
expansionOpportunityCount--; expansionOpportunityCount--;
...@@ -177,7 +177,8 @@ void WidthIterator::advance(int offset, GlyphBuffer* glyphBuffer) ...@@ -177,7 +177,8 @@ void WidthIterator::advance(int offset, GlyphBuffer* glyphBuffer)
bool treatAsSpace = Font::treatAsSpace(c); bool treatAsSpace = Font::treatAsSpace(c);
if (treatAsSpace || (expandAroundIdeographs && Font::isCJKIdeograph(c))) { if (treatAsSpace || (expandAroundIdeographs && Font::isCJKIdeograph(c))) {
// Distribute the run's total expansion evenly over all expansion opportunities in the run. // Distribute the run's total expansion evenly over all expansion opportunities in the run.
if (m_expansion && (m_run.allowsTrailingExpansion() || currentCharacter + clusterLength < static_cast<size_t>(m_run.length()))) { if (m_expansion && (m_run.allowsTrailingExpansion() || (m_run.ltr() && currentCharacter + clusterLength < static_cast<size_t>(m_run.length()))
|| (m_run.rtl() && currentCharacter))) {
float previousExpansion = m_expansion; float previousExpansion = m_expansion;
if (!treatAsSpace && !m_isAfterExpansion) { if (!treatAsSpace && !m_isAfterExpansion) {
// Take the expansion opportunity before this ideograph. // Take the expansion opportunity before this ideograph.
......
...@@ -84,7 +84,7 @@ ComplexTextController::ComplexTextController(const Font* font, const TextRun& ru ...@@ -84,7 +84,7 @@ ComplexTextController::ComplexTextController(const Font* font, const TextRun& ru
m_expansionPerOpportunity = 0; m_expansionPerOpportunity = 0;
else { else {
bool isAfterExpansion = true; bool isAfterExpansion = true;
unsigned expansionOpportunityCount = Font::expansionOpportunityCount(m_run.characters(), m_end, isAfterExpansion); unsigned expansionOpportunityCount = Font::expansionOpportunityCount(m_run.characters(), m_end, m_run.ltr() ? LTR : RTL, isAfterExpansion);
if (isAfterExpansion && !m_run.allowsTrailingExpansion()) if (isAfterExpansion && !m_run.allowsTrailingExpansion())
expansionOpportunityCount--; expansionOpportunityCount--;
......
...@@ -324,7 +324,7 @@ void RenderBlock::computeInlineDirectionPositionsForLine(RootInlineBox* lineBox, ...@@ -324,7 +324,7 @@ void RenderBlock::computeInlineDirectionPositionsForLine(RootInlineBox* lineBox,
RenderText* rt = toRenderText(r->m_object); RenderText* rt = toRenderText(r->m_object);
if (textAlign == JUSTIFY && r != trailingSpaceRun) { if (textAlign == JUSTIFY && r != trailingSpaceRun) {
unsigned opportunitiesInRun = Font::expansionOpportunityCount(rt->characters() + r->m_start, r->m_stop - r->m_start, isAfterExpansion); unsigned opportunitiesInRun = Font::expansionOpportunityCount(rt->characters() + r->m_start, r->m_stop - r->m_start, r->m_box->direction(), isAfterExpansion);
expansionOpportunities.append(opportunitiesInRun); expansionOpportunities.append(opportunitiesInRun);
expansionOpportunityCount += opportunitiesInRun; expansionOpportunityCount += opportunitiesInRun;
} }
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment