Modified: trunk/Source/WebCore/ChangeLog (200186 => 200187)
--- trunk/Source/WebCore/ChangeLog 2016-04-28 12:18:23 UTC (rev 200186)
+++ trunk/Source/WebCore/ChangeLog 2016-04-28 12:20:51 UTC (rev 200187)
@@ -1,3 +1,28 @@
+2016-04-28 Frederic Wang <[email protected]>
+
+ RenderMathMLOperator refactoring: introduce getBaseGlyph and remove parameter from getDisplayStyleLargeOperator
+ https://bugs.webkit.org/show_bug.cgi?id=156910
+
+ Reviewed by Alejandro G. Castro.
+
+ No new tests, the behavior is not changed.
+
+ * rendering/mathml/RenderMathMLOperator.cpp:
+ (WebCore::RenderMathMLOperator::italicCorrection): We do not need to pass m_textContent to
+ getDisplayStyleLargeOperator.
+ (WebCore::RenderMathMLOperator::computePreferredLogicalWidths): We use getBaseGlyph and do
+ not pass m_textContent to getDisplayStyleLargeOperator or findStretchyData.
+ (WebCore::RenderMathMLOperator::getBaseGlyph): Introduce a helper function to retrieve the
+ base glyph and do some validity checks.
+ (WebCore::RenderMathMLOperator::getDisplayStyleLargeOperator): We remove the character
+ parameter as it is always m_textContent.
+ We use getBaseGlyph and replace primaryFont with baseGlyph.font.
+ (WebCore::RenderMathMLOperator::findStretchyData): Ditto.
+ (WebCore::RenderMathMLOperator::updateStyle): We do not pass m_textContent to
+ getDisplayStyleLargeOperator or findStretchyData.
+ * rendering/mathml/RenderMathMLOperator.h: Declare getBaseGlyph and remove the parameter
+ from getDisplayStyleLargeOperator and findStretchyData.
+
2016-04-28 Commit Queue <[email protected]>
Unreviewed, rolling out r200185.
Modified: trunk/Source/WebCore/rendering/mathml/RenderMathMLOperator.cpp (200186 => 200187)
--- trunk/Source/WebCore/rendering/mathml/RenderMathMLOperator.cpp 2016-04-28 12:18:23 UTC (rev 200186)
+++ trunk/Source/WebCore/rendering/mathml/RenderMathMLOperator.cpp 2016-04-28 12:20:51 UTC (rev 200187)
@@ -207,7 +207,7 @@
if (isLargeOperatorInDisplayStyle()) {
const auto& primaryFont = style().fontCascade().primaryFont();
if (auto* mathData = primaryFont.mathData()) {
- StretchyData largeOperator = getDisplayStyleLargeOperator(m_textContent);
+ StretchyData largeOperator = getDisplayStyleLargeOperator();
return mathData->getItalicCorrection(primaryFont, largeOperator.variant().glyph);
}
}
@@ -306,8 +306,8 @@
return;
}
- GlyphData data = "" !style().isLeftToRightDirection());
- float maximumGlyphWidth = advanceWidthForGlyph(data);
+ GlyphData baseGlyph;
+ float maximumGlyphWidth = getBaseGlyph(style(), baseGlyph) ? advanceWidthForGlyph(baseGlyph) : 0;
if (!m_isVertical) {
if (maximumGlyphWidth < stretchSize())
maximumGlyphWidth = stretchSize();
@@ -317,12 +317,12 @@
}
if (isLargeOperatorInDisplayStyle()) {
// Large operators in STIX Word have incorrect advance width, causing misplacement of superscript, so we use the glyph bound instead (http://sourceforge.net/p/stixfonts/tracking/49/).
- StretchyData largeOperator = getDisplayStyleLargeOperator(m_textContent);
+ StretchyData largeOperator = getDisplayStyleLargeOperator();
if (largeOperator.mode() == DrawSizeVariant)
maximumGlyphWidth = boundsForGlyph(largeOperator.variant()).width();
} else {
// FIXME: some glyphs (e.g. the one for "FRACTION SLASH" in the STIX Math font or large operators) have a width that depends on the height, resulting in large gaps (https://bugs.webkit.org/show_bug.cgi?id=130326).
- findStretchyData(m_textContent, &maximumGlyphWidth);
+ findStretchyData(&maximumGlyphWidth);
}
m_maxPreferredLogicalWidth = m_minPreferredLogicalWidth = m_leadingSpace + maximumGlyphWidth + m_trailingSpace;
}
@@ -386,6 +386,12 @@
updateOperatorProperties();
}
+bool RenderMathMLOperator::getBaseGlyph(const RenderStyle& style, GlyphData& baseGlyph) const
+{
+ baseGlyph = style.fontCascade().glyphDataForCharacter(m_textContent, !style.isLeftToRightDirection());
+ return baseGlyph.isValid() && baseGlyph.font == &style.fontCascade().primaryFont();
+}
+
bool RenderMathMLOperator::getGlyphAssemblyFallBack(Vector<OpenTypeMathData::AssemblyPart> assemblyParts, StretchyData& stretchyData) const
{
GlyphData top;
@@ -496,30 +502,29 @@
return true;
}
-RenderMathMLOperator::StretchyData RenderMathMLOperator::getDisplayStyleLargeOperator(UChar character) const
+RenderMathMLOperator::StretchyData RenderMathMLOperator::getDisplayStyleLargeOperator() const
{
StretchyData data;
ASSERT(m_isVertical && isLargeOperatorInDisplayStyle());
- const auto& primaryFont = style().fontCascade().primaryFont();
- GlyphData baseGlyph = style().fontCascade().glyphDataForCharacter(character, !style().isLeftToRightDirection());
- if (!primaryFont.mathData() || baseGlyph.font != &primaryFont || !baseGlyph.font || !baseGlyph.glyph)
+ GlyphData baseGlyph;
+ if (!getBaseGlyph(style(), baseGlyph) || !baseGlyph.font->mathData())
return data;
Vector<Glyph> sizeVariants;
Vector<OpenTypeMathData::AssemblyPart> assemblyParts;
// The value of displayOperatorMinHeight is sometimes too small, so we ensure that it is at least \sqrt{2} times the size of the base glyph.
- float displayOperatorMinHeight = std::max(baseGlyph.font->boundsForGlyph(baseGlyph.glyph).height() * sqrtOfTwoFloat, primaryFont.mathData()->getMathConstant(primaryFont, OpenTypeMathData::DisplayOperatorMinHeight));
+ float displayOperatorMinHeight = std::max(baseGlyph.font->boundsForGlyph(baseGlyph.glyph).height() * sqrtOfTwoFloat, baseGlyph.font->mathData()->getMathConstant(*baseGlyph.font, OpenTypeMathData::DisplayOperatorMinHeight));
- primaryFont.mathData()->getMathVariants(baseGlyph.glyph, true, sizeVariants, assemblyParts);
+ baseGlyph.font->mathData()->getMathVariants(baseGlyph.glyph, true, sizeVariants, assemblyParts);
// We choose the first size variant that is larger than the expected displayOperatorMinHeight and otherwise fallback to the largest variant.
for (auto& variant : sizeVariants) {
GlyphData sizeVariant;
sizeVariant.glyph = variant;
- sizeVariant.font = &primaryFont;
+ sizeVariant.font = baseGlyph.font;
data.setSizeVariantMode(sizeVariant);
if (boundsForGlyph(sizeVariant).height() >= displayOperatorMinHeight)
return data;
@@ -527,25 +532,26 @@
return data;
}
-RenderMathMLOperator::StretchyData RenderMathMLOperator::findStretchyData(UChar character, float* maximumGlyphWidth)
+RenderMathMLOperator::StretchyData RenderMathMLOperator::findStretchyData(float* maximumGlyphWidth)
{
ASSERT(!maximumGlyphWidth || m_isVertical);
StretchyData data;
StretchyData assemblyData;
- const auto& primaryFont = style().fontCascade().primaryFont();
- GlyphData baseGlyph = style().fontCascade().glyphDataForCharacter(character, !style().isLeftToRightDirection());
+ GlyphData baseGlyph;
+ if (!getBaseGlyph(style(), baseGlyph))
+ return data;
- if (primaryFont.mathData() && baseGlyph.font == &primaryFont) {
+ if (baseGlyph.font->mathData()) {
Vector<Glyph> sizeVariants;
Vector<OpenTypeMathData::AssemblyPart> assemblyParts;
- primaryFont.mathData()->getMathVariants(baseGlyph.glyph, m_isVertical, sizeVariants, assemblyParts);
+ baseGlyph.font->mathData()->getMathVariants(baseGlyph.glyph, m_isVertical, sizeVariants, assemblyParts);
// We verify the size variants.
for (auto& variant : sizeVariants) {
GlyphData sizeVariant;
sizeVariant.glyph = variant;
- sizeVariant.font = &primaryFont;
+ sizeVariant.font = baseGlyph.font;
if (maximumGlyphWidth)
*maximumGlyphWidth = std::max(*maximumGlyphWidth, advanceWidthForGlyph(sizeVariant));
else {
@@ -567,7 +573,7 @@
const StretchyCharacter* stretchyCharacter = nullptr;
const unsigned maxIndex = WTF_ARRAY_LENGTH(stretchyCharacters);
for (unsigned index = 0; index < maxIndex; ++index) {
- if (stretchyCharacters[index].character == character) {
+ if (stretchyCharacters[index].character == m_textContent) {
stretchyCharacter = &stretchyCharacters[index];
if (!style().isLeftToRightDirection() && index < leftRightPairsCount * 2) {
// If we are in right-to-left direction we select the mirrored form by adding -1 or +1 according to the parity of index.
@@ -641,14 +647,14 @@
return;
if (m_isVertical && isLargeOperatorInDisplayStyle())
- m_stretchyData = getDisplayStyleLargeOperator(m_textContent);
+ m_stretchyData = getDisplayStyleLargeOperator();
else {
// We do not stretch if the base glyph is large enough.
GlyphData baseGlyph = style().fontCascade().glyphDataForCharacter(m_textContent, !style().isLeftToRightDirection());
float baseSize = m_isVertical ? heightForGlyph(baseGlyph) : advanceWidthForGlyph(baseGlyph);
if (stretchSize() <= baseSize)
return;
- m_stretchyData = findStretchyData(m_textContent, nullptr);
+ m_stretchyData = findStretchyData(nullptr);
}
if (m_isVertical && m_stretchyData.mode() == DrawSizeVariant) {
Modified: trunk/Source/WebCore/rendering/mathml/RenderMathMLOperator.h (200186 => 200187)
--- trunk/Source/WebCore/rendering/mathml/RenderMathMLOperator.h 2016-04-28 12:18:23 UTC (rev 200186)
+++ trunk/Source/WebCore/rendering/mathml/RenderMathMLOperator.h 2016-04-28 12:20:51 UTC (rev 200187)
@@ -141,9 +141,10 @@
bool shouldAllowStretching() const;
+ bool getBaseGlyph(const RenderStyle&, GlyphData&) const;
bool getGlyphAssemblyFallBack(Vector<OpenTypeMathData::AssemblyPart>, StretchyData&) const;
- StretchyData getDisplayStyleLargeOperator(UChar) const;
- StretchyData findStretchyData(UChar, float* maximumGlyphWidth);
+ StretchyData getDisplayStyleLargeOperator() const;
+ StretchyData findStretchyData(float* maximumGlyphWidth);
enum GlyphPaintTrimming {
TrimTop,