<!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>[192259] 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/192259">192259</a></dd>
<dt>Author</dt> <dd>mmaxfield@apple.com</dd>
<dt>Date</dt> <dd>2015-11-10 12:05:01 -0800 (Tue, 10 Nov 2015)</dd>
</dl>

<h3>Log Message</h3>
<pre>Tatechuyoko text is not vertically centered in its vertical advance
https://bugs.webkit.org/show_bug.cgi?id=151074
&lt;rdar://problem/20074305&gt;

Reviewed by David Hyatt.

During paint time, the run origin of tatechuyoko needs to be adjusted to compensate for the
rotation of the writing mode. The calculation which performed this adjustment was incorrect.

It is incorrect for two reasons:
1. It used the existing text origin, which had the font's ascent incorporated in it, but did
not compensate by either inspecting the overflow bounds' ascent nor the font's ascent proper.
2. It did not distinguish between the overflow bounds' ascent vs. descent. Instead, it added
them together and treated both values together.

No new tests yet. I need to make a font to test this.

* rendering/InlineTextBox.cpp:
(WebCore::InlineTextBox::paint):
* rendering/RenderCombineText.cpp:
(WebCore::RenderCombineText::computeTextOrigin):
(WebCore::RenderCombineText::combineText):
(WebCore::RenderCombineText::adjustTextOrigin): Deleted.
* rendering/RenderCombineText.h:</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorerenderingInlineTextBoxcpp">trunk/Source/WebCore/rendering/InlineTextBox.cpp</a></li>
<li><a href="#trunkSourceWebCorerenderingRenderCombineTextcpp">trunk/Source/WebCore/rendering/RenderCombineText.cpp</a></li>
<li><a href="#trunkSourceWebCorerenderingRenderCombineTexth">trunk/Source/WebCore/rendering/RenderCombineText.h</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (192258 => 192259)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog        2015-11-10 19:20:33 UTC (rev 192258)
+++ trunk/Source/WebCore/ChangeLog        2015-11-10 20:05:01 UTC (rev 192259)
</span><span class="lines">@@ -1,3 +1,30 @@
</span><ins>+2015-11-10  Myles C. Maxfield  &lt;mmaxfield@apple.com&gt;
+
+        Tatechuyoko text is not vertically centered in its vertical advance
+        https://bugs.webkit.org/show_bug.cgi?id=151074
+        &lt;rdar://problem/20074305&gt;
+
+        Reviewed by David Hyatt.
+
+        During paint time, the run origin of tatechuyoko needs to be adjusted to compensate for the
+        rotation of the writing mode. The calculation which performed this adjustment was incorrect.
+
+        It is incorrect for two reasons:
+        1. It used the existing text origin, which had the font's ascent incorporated in it, but did
+        not compensate by either inspecting the overflow bounds' ascent nor the font's ascent proper.
+        2. It did not distinguish between the overflow bounds' ascent vs. descent. Instead, it added
+        them together and treated both values together.
+
+        No new tests yet. I need to make a font to test this.
+
+        * rendering/InlineTextBox.cpp:
+        (WebCore::InlineTextBox::paint):
+        * rendering/RenderCombineText.cpp:
+        (WebCore::RenderCombineText::computeTextOrigin):
+        (WebCore::RenderCombineText::combineText):
+        (WebCore::RenderCombineText::adjustTextOrigin): Deleted.
+        * rendering/RenderCombineText.h:
+
</ins><span class="cx"> 2015-11-10  Youenn Fablet  &lt;youenn.fablet@crf.canon.fr&gt;
</span><span class="cx"> 
</span><span class="cx">         XMLHttpRequestUpload should support ontimeout event handler
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingInlineTextBoxcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/InlineTextBox.cpp (192258 => 192259)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/InlineTextBox.cpp        2015-11-10 19:20:33 UTC (rev 192258)
+++ trunk/Source/WebCore/rendering/InlineTextBox.cpp        2015-11-10 20:05:01 UTC (rev 192259)
</span><span class="lines">@@ -602,8 +602,10 @@
</span><span class="cx">     const ShadowData* textShadow = (paintInfo.forceTextColor()) ? nullptr : lineStyle.textShadow();
</span><span class="cx"> 
</span><span class="cx">     FloatPoint textOrigin(boxOrigin.x(), boxOrigin.y() + font.fontMetrics().ascent());
</span><del>-    if (combinedText)
-        combinedText-&gt;adjustTextOrigin(textOrigin, boxRect);
</del><ins>+    if (combinedText) {
+        if (auto newOrigin = combinedText-&gt;computeTextOrigin(boxRect))
+            textOrigin = newOrigin.value();
+    }
</ins><span class="cx"> 
</span><span class="cx">     if (isHorizontal())
</span><span class="cx">         textOrigin.setY(roundToDevicePixel(LayoutUnit(textOrigin.y()), renderer().document().deviceScaleFactor()));
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingRenderCombineTextcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderCombineText.cpp (192258 => 192259)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/RenderCombineText.cpp        2015-11-10 19:20:33 UTC (rev 192258)
+++ trunk/Source/WebCore/rendering/RenderCombineText.cpp        2015-11-10 20:05:01 UTC (rev 192259)
</span><span class="lines">@@ -66,10 +66,17 @@
</span><span class="cx">     return RenderText::width(from, length, font, xPosition, fallbackFonts, glyphOverflow);
</span><span class="cx"> }
</span><span class="cx"> 
</span><del>-void RenderCombineText::adjustTextOrigin(FloatPoint&amp; textOrigin, const FloatRect&amp; boxRect) const
</del><ins>+Optional&lt;FloatPoint&gt; RenderCombineText::computeTextOrigin(const FloatRect&amp; boxRect) const
</ins><span class="cx"> {
</span><del>-    if (m_isCombined)
-        textOrigin.move(boxRect.height() / 2 - ceilf(m_combinedTextSize.width()) / 2, boxRect.width() + (boxRect.width() - m_combinedTextSize.height()) / 2);
</del><ins>+    if (!m_isCombined)
+        return Nullopt;
+
+    // Visually center m_combinedTextWidth/Ascent/Descent within boxRect
+    FloatPoint result = boxRect.minXMaxYCorner();
+    FloatSize combinedTextSize(m_combinedTextWidth, m_combinedTextAscent + m_combinedTextDescent);
+    result.move((boxRect.size().transposedSize() - combinedTextSize) / 2);
+    result.move(0, m_combinedTextAscent);
+    return result;
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> void RenderCombineText::getStringToRender(int start, String&amp; string, int&amp; length) const
</span><span class="lines">@@ -147,7 +154,9 @@
</span><span class="cx">     if (m_isCombined) {
</span><span class="cx">         static NeverDestroyed&lt;String&gt; objectReplacementCharacterString(&amp;objectReplacementCharacter, 1);
</span><span class="cx">         RenderText::setRenderedText(objectReplacementCharacterString.get());
</span><del>-        m_combinedTextSize = FloatSize(combinedTextWidth, glyphOverflow.bottom + glyphOverflow.top);
</del><ins>+        m_combinedTextWidth = combinedTextWidth;
+        m_combinedTextAscent = glyphOverflow.top;
+        m_combinedTextDescent = glyphOverflow.bottom;
</ins><span class="cx">     }
</span><span class="cx"> }
</span><span class="cx"> 
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingRenderCombineTexth"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/RenderCombineText.h (192258 => 192259)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/RenderCombineText.h        2015-11-10 19:20:33 UTC (rev 192258)
+++ trunk/Source/WebCore/rendering/RenderCombineText.h        2015-11-10 20:05:01 UTC (rev 192259)
</span><span class="lines">@@ -35,7 +35,7 @@
</span><span class="cx">     Text&amp; textNode() const { return downcast&lt;Text&gt;(nodeForNonAnonymous()); }
</span><span class="cx"> 
</span><span class="cx">     void combineText();
</span><del>-    void adjustTextOrigin(FloatPoint&amp; textOrigin, const FloatRect&amp; boxRect) const;
</del><ins>+    Optional&lt;FloatPoint&gt; computeTextOrigin(const FloatRect&amp; boxRect) const;
</ins><span class="cx">     void getStringToRender(int, String&amp;, int&amp; length) const;
</span><span class="cx">     bool isCombined() const { return m_isCombined; }
</span><span class="cx">     float combinedTextWidth(const FontCascade&amp; font) const { return font.size(); }
</span><span class="lines">@@ -52,7 +52,9 @@
</span><span class="cx">     virtual void setRenderedText(const String&amp;) override;
</span><span class="cx"> 
</span><span class="cx">     RefPtr&lt;RenderStyle&gt; m_combineFontStyle;
</span><del>-    FloatSize m_combinedTextSize;
</del><ins>+    float m_combinedTextWidth { 0 };
+    float m_combinedTextAscent { 0 };
+    float m_combinedTextDescent { 0 };
</ins><span class="cx">     bool m_isCombined : 1;
</span><span class="cx">     bool m_needsFontUpdate : 1;
</span><span class="cx"> };
</span></span></pre>
</div>
</div>

</body>
</html>