<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[201862] trunk/Source/WebCore</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/201862">201862</a></dd>
<dt>Author</dt> <dd>fred.wang@free.fr</dd>
<dt>Date</dt> <dd>2016-06-09 07:50:19 -0700 (Thu, 09 Jun 2016)</dd>
</dl>

<h3>Log Message</h3>
<pre>Introduce MathOperator::Type
https://bugs.webkit.org/show_bug.cgi?id=156950

Patch by Frederic Wang &lt;fwang@igalia.com&gt; on 2016-06-09
Reviewed by Sergio Villar Senin.

No new tests, behavior is not change.

An enum Type is introduced in MathOperator in order to indicate
which kind of stretching is requested. In follow-up work, this will
allow to just call setOperator and stretchTo without having to
explicitly call calculateDisplayStyleLargeOperator or calculateStretchyData.

* rendering/mathml/MathOperator.cpp:
(WebCore::MathOperator::setOperator): Use Type instead of a boolean.
(WebCore::MathOperator::setGlyphAssembly): Add an assert to ensure that the function is correctly used.
(WebCore::MathOperator::calculateDisplayStyleLargeOperator): Ditto, this makes the assert more accurate.
(WebCore::MathOperator::calculateStretchyData): Ditto and replace m_isVertical with a local isVertical variable.
(WebCore::MathOperator::fillWithVerticalExtensionGlyph): Ditto.
(WebCore::MathOperator::fillWithHorizontalExtensionGlyph): Ditto.
(WebCore::MathOperator::paintVerticalGlyphAssembly): Ditto.
(WebCore::MathOperator::paintHorizontalGlyphAssembly): Ditto.
* rendering/mathml/MathOperator.h: Add the Type enum.
(WebCore::MathOperator::stretchSize): Use Type instead of a boolean and add an
assert to ensure that the function is correctly used.
* rendering/mathml/RenderMathMLOperator.cpp:
(WebCore::RenderMathMLOperator::computePreferredLogicalWidths): Call setOperator with the correct value.
(WebCore::RenderMathMLOperator::updateStyle): Ditto.</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorerenderingmathmlMathOperatorcpp">trunk/Source/WebCore/rendering/mathml/MathOperator.cpp</a></li>
<li><a href="#trunkSourceWebCorerenderingmathmlMathOperatorh">trunk/Source/WebCore/rendering/mathml/MathOperator.h</a></li>
<li><a href="#trunkSourceWebCorerenderingmathmlRenderMathMLOperatorcpp">trunk/Source/WebCore/rendering/mathml/RenderMathMLOperator.cpp</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (201861 => 201862)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog        2016-06-09 12:59:40 UTC (rev 201861)
+++ trunk/Source/WebCore/ChangeLog        2016-06-09 14:50:19 UTC (rev 201862)
</span><span class="lines">@@ -1,3 +1,33 @@
</span><ins>+2016-06-09  Frederic Wang  &lt;fwang@igalia.com&gt;
+
+        Introduce MathOperator::Type
+        https://bugs.webkit.org/show_bug.cgi?id=156950
+
+        Reviewed by Sergio Villar Senin.
+
+        No new tests, behavior is not change.
+
+        An enum Type is introduced in MathOperator in order to indicate
+        which kind of stretching is requested. In follow-up work, this will
+        allow to just call setOperator and stretchTo without having to
+        explicitly call calculateDisplayStyleLargeOperator or calculateStretchyData.
+
+        * rendering/mathml/MathOperator.cpp:
+        (WebCore::MathOperator::setOperator): Use Type instead of a boolean.
+        (WebCore::MathOperator::setGlyphAssembly): Add an assert to ensure that the function is correctly used.
+        (WebCore::MathOperator::calculateDisplayStyleLargeOperator): Ditto, this makes the assert more accurate.
+        (WebCore::MathOperator::calculateStretchyData): Ditto and replace m_isVertical with a local isVertical variable.
+        (WebCore::MathOperator::fillWithVerticalExtensionGlyph): Ditto.
+        (WebCore::MathOperator::fillWithHorizontalExtensionGlyph): Ditto.
+        (WebCore::MathOperator::paintVerticalGlyphAssembly): Ditto.
+        (WebCore::MathOperator::paintHorizontalGlyphAssembly): Ditto.
+        * rendering/mathml/MathOperator.h: Add the Type enum.
+        (WebCore::MathOperator::stretchSize): Use Type instead of a boolean and add an
+        assert to ensure that the function is correctly used.
+        * rendering/mathml/RenderMathMLOperator.cpp:
+        (WebCore::RenderMathMLOperator::computePreferredLogicalWidths): Call setOperator with the correct value.
+        (WebCore::RenderMathMLOperator::updateStyle): Ditto.
+
</ins><span class="cx"> 2016-06-09  Commit Queue  &lt;commit-queue@webkit.org&gt;
</span><span class="cx"> 
</span><span class="cx">         Unreviewed, rolling out r201810.
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingmathmlMathOperatorcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/mathml/MathOperator.cpp (201861 => 201862)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/mathml/MathOperator.cpp        2016-06-09 12:59:40 UTC (rev 201861)
+++ trunk/Source/WebCore/rendering/mathml/MathOperator.cpp        2016-06-09 14:50:19 UTC (rev 201862)
</span><span class="lines">@@ -75,12 +75,18 @@
</span><span class="cx">     { 0x222b, 0x2320, 0x23ae, 0x2321, 0x0    } // integral sign
</span><span class="cx"> };
</span><span class="cx"> 
</span><del>-void MathOperator::setOperator(UChar baseCharacter, bool isVertical)
</del><ins>+void MathOperator::setOperator(UChar baseCharacter, Type operatorType)
</ins><span class="cx"> {
</span><span class="cx">     m_baseCharacter = baseCharacter;
</span><del>-    m_isVertical = isVertical;
</del><ins>+    m_operatorType = operatorType;
</ins><span class="cx"> }
</span><span class="cx"> 
</span><ins>+LayoutUnit MathOperator::stretchSize() const
+{
+    ASSERT(m_operatorType == Type::VerticalOperator || m_operatorType == Type::HorizontalOperator);
+    return m_operatorType == Type::VerticalOperator ? m_ascent + m_descent : m_width;
+}
+
</ins><span class="cx"> bool MathOperator::getBaseGlyph(const RenderStyle&amp; style, GlyphData&amp; baseGlyph) const
</span><span class="cx"> {
</span><span class="cx">     baseGlyph = style.fontCascade().glyphDataForCharacter(m_baseCharacter, !style.isLeftToRightDirection());
</span><span class="lines">@@ -97,13 +103,14 @@
</span><span class="cx"> 
</span><span class="cx"> void MathOperator::setGlyphAssembly(const GlyphAssemblyData&amp; assemblyData)
</span><span class="cx"> {
</span><ins>+    ASSERT(m_operatorType == Type::VerticalOperator || m_operatorType == Type::HorizontalOperator);
</ins><span class="cx">     m_stretchType = StretchType::GlyphAssembly;
</span><span class="cx">     m_assembly = assemblyData;
</span><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> void MathOperator::calculateDisplayStyleLargeOperator(const RenderStyle&amp; style)
</span><span class="cx"> {
</span><del>-    ASSERT(m_isVertical);
</del><ins>+    ASSERT(m_operatorType == Type::DisplayOperator);
</ins><span class="cx"> 
</span><span class="cx">     GlyphData baseGlyph;
</span><span class="cx">     if (!getBaseGlyph(style, baseGlyph) || !baseGlyph.font-&gt;mathData())
</span><span class="lines">@@ -230,7 +237,9 @@
</span><span class="cx"> 
</span><span class="cx"> void MathOperator::calculateStretchyData(const RenderStyle&amp; style, float* maximumGlyphWidth, LayoutUnit targetSize)
</span><span class="cx"> {
</span><del>-    ASSERT(!maximumGlyphWidth || m_isVertical);
</del><ins>+    ASSERT(m_operatorType == Type::VerticalOperator || m_operatorType == Type::HorizontalOperator);
+    ASSERT(!maximumGlyphWidth || m_operatorType == Type::VerticalOperator);
+    bool isVertical = m_operatorType == Type::VerticalOperator;
</ins><span class="cx"> 
</span><span class="cx">     GlyphData baseGlyph;
</span><span class="cx">     if (!getBaseGlyph(style, baseGlyph))
</span><span class="lines">@@ -238,7 +247,7 @@
</span><span class="cx"> 
</span><span class="cx">     if (!maximumGlyphWidth) {
</span><span class="cx">         // We do not stretch if the base glyph is large enough.
</span><del>-        float baseSize = m_isVertical ? heightForGlyph(baseGlyph) : advanceWidthForGlyph(baseGlyph);
</del><ins>+        float baseSize = isVertical ? heightForGlyph(baseGlyph) : advanceWidthForGlyph(baseGlyph);
</ins><span class="cx">         if (targetSize &lt;= baseSize)
</span><span class="cx">             return;
</span><span class="cx">     }
</span><span class="lines">@@ -247,7 +256,7 @@
</span><span class="cx">     if (baseGlyph.font-&gt;mathData()) {
</span><span class="cx">         Vector&lt;Glyph&gt; sizeVariants;
</span><span class="cx">         Vector&lt;OpenTypeMathData::AssemblyPart&gt; assemblyParts;
</span><del>-        baseGlyph.font-&gt;mathData()-&gt;getMathVariants(baseGlyph.glyph, m_isVertical, sizeVariants, assemblyParts);
</del><ins>+        baseGlyph.font-&gt;mathData()-&gt;getMathVariants(baseGlyph.glyph, isVertical, sizeVariants, assemblyParts);
</ins><span class="cx">         // We verify the size variants.
</span><span class="cx">         for (auto&amp; sizeVariant : sizeVariants) {
</span><span class="cx">             GlyphData glyphData(sizeVariant, baseGlyph.font);
</span><span class="lines">@@ -255,7 +264,7 @@
</span><span class="cx">                 *maximumGlyphWidth = std::max(*maximumGlyphWidth, advanceWidthForGlyph(glyphData));
</span><span class="cx">             else {
</span><span class="cx">                 setSizeVariant(glyphData);
</span><del>-                float size = m_isVertical ? heightForGlyph(glyphData) : advanceWidthForGlyph(glyphData);
</del><ins>+                float size = isVertical ? heightForGlyph(glyphData) : advanceWidthForGlyph(glyphData);
</ins><span class="cx">                 if (size &gt;= targetSize)
</span><span class="cx">                     return;
</span><span class="cx">             }
</span><span class="lines">@@ -265,7 +274,7 @@
</span><span class="cx">         if (!calculateGlyphAssemblyFallback(style, assemblyParts, assemblyData))
</span><span class="cx">             return;
</span><span class="cx">     } else {
</span><del>-        if (!m_isVertical)
</del><ins>+        if (!isVertical)
</ins><span class="cx">             return;
</span><span class="cx"> 
</span><span class="cx">         // If the font does not have a MATH table, we fallback to the Unicode-only constructions.
</span><span class="lines">@@ -303,7 +312,7 @@
</span><span class="cx">     }
</span><span class="cx"> 
</span><span class="cx">     // We ensure that the size is large enough to avoid glyph overlaps.
</span><del>-    float minSize = m_isVertical ?
</del><ins>+    float minSize = isVertical ?
</ins><span class="cx">         heightForGlyph(assemblyData.topOrRight) + heightForGlyph(assemblyData.middle) + heightForGlyph(assemblyData.bottomOrLeft)
</span><span class="cx">         : advanceWidthForGlyph(assemblyData.bottomOrLeft) + advanceWidthForGlyph(assemblyData.middle) + advanceWidthForGlyph(assemblyData.topOrRight);
</span><span class="cx">     if (minSize &gt; targetSize)
</span><span class="lines">@@ -366,7 +375,7 @@
</span><span class="cx"> 
</span><span class="cx"> void MathOperator::fillWithVerticalExtensionGlyph(const RenderStyle&amp; style, PaintInfo&amp; info, const LayoutPoint&amp; from, const LayoutPoint&amp; to)
</span><span class="cx"> {
</span><del>-    ASSERT(m_isVertical);
</del><ins>+    ASSERT(m_operatorType == Type::VerticalOperator);
</ins><span class="cx">     ASSERT(m_stretchType == StretchType::GlyphAssembly);
</span><span class="cx">     ASSERT(m_assembly.extension.isValid());
</span><span class="cx">     ASSERT(from.y() &lt;= to.y());
</span><span class="lines">@@ -404,7 +413,7 @@
</span><span class="cx"> 
</span><span class="cx"> void MathOperator::fillWithHorizontalExtensionGlyph(const RenderStyle&amp; style, PaintInfo&amp; info, const LayoutPoint&amp; from, const LayoutPoint&amp; to)
</span><span class="cx"> {
</span><del>-    ASSERT(!m_isVertical);
</del><ins>+    ASSERT(m_operatorType == Type::HorizontalOperator);
</ins><span class="cx">     ASSERT(m_stretchType == StretchType::GlyphAssembly);
</span><span class="cx">     ASSERT(m_assembly.extension.isValid());
</span><span class="cx">     ASSERT(from.x() &lt;= to.x());
</span><span class="lines">@@ -440,7 +449,7 @@
</span><span class="cx"> 
</span><span class="cx"> void MathOperator::paintVerticalGlyphAssembly(const RenderStyle&amp; style, PaintInfo&amp; info, const LayoutPoint&amp; paintOffset)
</span><span class="cx"> {
</span><del>-    ASSERT(m_isVertical);
</del><ins>+    ASSERT(m_operatorType == Type::VerticalOperator);
</ins><span class="cx">     ASSERT(m_stretchType == StretchType::GlyphAssembly);
</span><span class="cx">     ASSERT(m_assembly.topOrRight.isValid());
</span><span class="cx">     ASSERT(m_assembly.bottomOrLeft.isValid());
</span><span class="lines">@@ -472,7 +481,7 @@
</span><span class="cx"> 
</span><span class="cx"> void MathOperator::paintHorizontalGlyphAssembly(const RenderStyle&amp; style, PaintInfo&amp; info, const LayoutPoint&amp; paintOffset)
</span><span class="cx"> {
</span><del>-    ASSERT(!m_isVertical);
</del><ins>+    ASSERT(m_operatorType == Type::HorizontalOperator);
</ins><span class="cx">     ASSERT(m_stretchType == StretchType::GlyphAssembly);
</span><span class="cx">     ASSERT(m_assembly.bottomOrLeft.isValid());
</span><span class="cx">     ASSERT(m_assembly.topOrRight.isValid());
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingmathmlMathOperatorh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/mathml/MathOperator.h (201861 => 201862)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/mathml/MathOperator.h        2016-06-09 12:59:40 UTC (rev 201861)
+++ trunk/Source/WebCore/rendering/mathml/MathOperator.h        2016-06-09 14:50:19 UTC (rev 201862)
</span><span class="lines">@@ -40,7 +40,8 @@
</span><span class="cx"> class MathOperator {
</span><span class="cx"> public:
</span><span class="cx">     MathOperator() { }
</span><del>-    void setOperator(UChar baseCharacter, bool isVertical);
</del><ins>+    enum class Type { UndefinedOperator, DisplayOperator, VerticalOperator, HorizontalOperator };
+    void setOperator(UChar baseCharacter, Type);
</ins><span class="cx"> 
</span><span class="cx">     LayoutUnit italicCorrection() const { return m_italicCorrection; }
</span><span class="cx"> 
</span><span class="lines">@@ -65,7 +66,7 @@
</span><span class="cx">         TrimLeftAndRight
</span><span class="cx">     };
</span><span class="cx"> 
</span><del>-    LayoutUnit stretchSize() const { return m_isVertical ? m_ascent + m_descent : m_width; };
</del><ins>+    LayoutUnit stretchSize() const;
</ins><span class="cx">     bool getBaseGlyph(const RenderStyle&amp;, GlyphData&amp;) const;
</span><span class="cx">     void setSizeVariant(const GlyphData&amp;);
</span><span class="cx">     void setGlyphAssembly(const GlyphAssemblyData&amp;);
</span><span class="lines">@@ -80,7 +81,7 @@
</span><span class="cx">     void paintHorizontalGlyphAssembly(const RenderStyle&amp;, PaintInfo&amp;, const LayoutPoint&amp;);
</span><span class="cx"> 
</span><span class="cx">     UChar m_baseCharacter = 0;
</span><del>-    bool m_isVertical = false;
</del><ins>+    Type m_operatorType { Type::UndefinedOperator };
</ins><span class="cx">     StretchType m_stretchType = StretchType::Unstretched;
</span><span class="cx">     union {
</span><span class="cx">         GlyphData m_variant;
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingmathmlRenderMathMLOperatorcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/mathml/RenderMathMLOperator.cpp (201861 => 201862)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/mathml/RenderMathMLOperator.cpp        2016-06-09 12:59:40 UTC (rev 201861)
+++ trunk/Source/WebCore/rendering/mathml/RenderMathMLOperator.cpp        2016-06-09 14:50:19 UTC (rev 201862)
</span><span class="lines">@@ -267,7 +267,12 @@
</span><span class="cx">         return;
</span><span class="cx">     }
</span><span class="cx"> 
</span><del>-    m_mathOperator.setOperator(m_textContent, m_isVertical);
</del><ins>+    MathOperator::Type type;
+    if (isLargeOperatorInDisplayStyle())
+        type = MathOperator::Type::DisplayOperator;
+    else
+        type = m_isVertical ? MathOperator::Type::VerticalOperator : MathOperator::Type::HorizontalOperator;
+    m_mathOperator.setOperator(m_textContent, type);
</ins><span class="cx">     GlyphData baseGlyph;
</span><span class="cx">     float maximumGlyphWidth = m_mathOperator.getBaseGlyph(style(), baseGlyph) ? advanceWidthForGlyph(baseGlyph) : 0;
</span><span class="cx">     if (!m_isVertical) {
</span><span class="lines">@@ -369,7 +374,12 @@
</span><span class="cx">     if (!shouldAllowStretching())
</span><span class="cx">         return;
</span><span class="cx"> 
</span><del>-    m_mathOperator.setOperator(m_textContent, m_isVertical);
</del><ins>+    MathOperator::Type type;
+    if (isLargeOperatorInDisplayStyle())
+        type = MathOperator::Type::DisplayOperator;
+    else
+        type = m_isVertical ? MathOperator::Type::VerticalOperator : MathOperator::Type::HorizontalOperator;
+    m_mathOperator.setOperator(m_textContent, type);
</ins><span class="cx">     if (m_isVertical &amp;&amp; isLargeOperatorInDisplayStyle())
</span><span class="cx">         m_mathOperator.calculateDisplayStyleLargeOperator(style());
</span><span class="cx">     else {
</span></span></pre>
</div>
</div>

</body>
</html>