Modified: trunk/Source/WebCore/ChangeLog (226176 => 226177)
--- trunk/Source/WebCore/ChangeLog 2017-12-20 14:25:50 UTC (rev 226176)
+++ trunk/Source/WebCore/ChangeLog 2017-12-20 14:28:05 UTC (rev 226177)
@@ -1,3 +1,56 @@
+2017-12-20 Frederic Wang <fw...@igalia.com>
+
+ Refactor RenderMathMLFraction to remove members modified during layout
+ https://bugs.webkit.org/show_bug.cgi?id=180151
+
+ Reviewed by Manuel Rego Casasnovas.
+
+ Currently, RenderMathMLFraction has three LayoutUnit members m_defaultLineThickness,
+ m_lineThickness and m_ascent that are set during layout. In the past such members have caused
+ MathML rendering bugs due to update issues. This patch refactors the layout of MathML
+ fractions so that it does not require to store and keep these LayoutUnit members up-to-date.
+ New helper functions are introduced to perform the simple arithmetic calculations required.
+
+ No new tests, behavior unchanged and already covered by existing tests.
+
+ * rendering/mathml/RenderMathMLFraction.cpp: We add new helper functions to calculate line
+ thickness values. This allows to remove updateLineThickness(), m_defaultLineThickness and
+ m_lineThickness. We also introduce the ascentOverHorizontalAxis() helper function to
+ calculate the ascent over the middle of its fraction bar or stack gap. This allows to remove
+ the m_ascent member.
+ (WebCore::RenderMathMLFraction::defaultLineThickness const): Helper function to calculate
+ the default thickness of the fraction bar given in the MATH table or a fallback value.
+ This replaces the use of m_defaultLineThickness.
+ (WebCore::RenderMathMLFraction::lineThickness const): Helper function to resolve the
+ actual thickness based on the @linethickness attribute and the default value. This replaces
+ the use of m_lineThickness.
+ (WebCore::RenderMathMLFraction::relativeLineThickness const): Rewrite this function using
+ the new helper functions.
+ (WebCore::RenderMathMLFraction::fractionParameters const): Make this const and replaces
+ isStack() with !lineThickness().
+ (WebCore::RenderMathMLFraction::stackParameters const): Ditto. Also move from layoutBlock
+ the adjustment of parameters to ensure a minimum gap. Doing so assumes that the fraction is
+ valid so we add an ASSERT.
+ (WebCore::RenderMathMLFraction::horizontalOffset const): Make this a const since it does not
+ mutate anything.
+ (WebCore::RenderMathMLFraction::ascentOverHorizontalAxis const): Move this code from
+ layoutBlock() to determine the middle of the stack gap or of the fraction bar. This helper
+ function replaces m_ascent - mathAxisHeight(). Note that the adjustment of topShiftUp is now
+ done in stackParameters().
+ (WebCore::RenderMathMLFraction::layoutBlock): Remove the call to updateLineThickness().
+ Rely on stackParameters() and ascentOverHorizontalAxis() to perform the necessary calculation
+ of bottomShiftDown and ascent respectively.
+ (WebCore::RenderMathMLFraction::paint): Use lineThickness() and ascentOverHorizontalAxis()
+ instead of m_lineThickness, m_ascent and isStack().
+ (WebCore::RenderMathMLFraction::firstLineBaseline const): Use ascentOverHorizontalAxis() and
+ mathAxisHeight() instead of m_ascent.
+ (WebCore::RenderMathMLFraction::updateLineThickness): Deleted.
+ * rendering/mathml/RenderMathMLFraction.h: Declare new helper functions for line thickness
+ values and ascent and remove the old LayoutUnit members. Make horizontalOffset(),
+ fractionParameter() and stackParameters() const since they do not modify anything and the two
+ last are used in the const function ascentOverHorizontalAxis(), itself used in
+ firstLineBaseline().
+
2017-12-20 Carlos Alberto Lopez Perez <clo...@igalia.com>
[GTK][Clang] Build fix after r226138
Modified: trunk/Source/WebCore/rendering/mathml/RenderMathMLFraction.cpp (226176 => 226177)
--- trunk/Source/WebCore/rendering/mathml/RenderMathMLFraction.cpp 2017-12-20 14:25:50 UTC (rev 226176)
+++ trunk/Source/WebCore/rendering/mathml/RenderMathMLFraction.cpp 2017-12-20 14:28:05 UTC (rev 226177)
@@ -68,25 +68,29 @@
return *firstChildBox()->nextSiblingBox();
}
-void RenderMathMLFraction::updateLineThickness()
+LayoutUnit RenderMathMLFraction::defaultLineThickness() const
{
- // We first determine the default line thickness.
const auto& primaryFont = style().fontCascade().primaryFont();
- const auto* mathData = style().fontCascade().primaryFont().mathData();
- if (mathData)
- m_defaultLineThickness = mathData->getMathConstant(primaryFont, OpenTypeMathData::FractionRuleThickness);
- else
- m_defaultLineThickness = ruleThicknessFallback();
+ if (const auto* mathData = primaryFont.mathData())
+ return mathData->getMathConstant(primaryFont, OpenTypeMathData::FractionRuleThickness);
+ return ruleThicknessFallback();
+}
- // Next we resolve the thickness using m_defaultLineThickness as the default value.
- m_lineThickness = toUserUnits(element().lineThickness(), style(), m_defaultLineThickness);
- if (m_lineThickness < 0)
- m_lineThickness = 0;
+LayoutUnit RenderMathMLFraction::lineThickness() const
+{
+ return std::max<LayoutUnit>(toUserUnits(element().lineThickness(), style(), defaultLineThickness()), 0);
}
-RenderMathMLFraction::FractionParameters RenderMathMLFraction::fractionParameters()
+float RenderMathMLFraction::relativeLineThickness() const
{
- ASSERT(!isStack());
+ if (LayoutUnit defaultThickness = defaultLineThickness())
+ return lineThickness() / defaultThickness;
+ return 0;
+}
+
+RenderMathMLFraction::FractionParameters RenderMathMLFraction::fractionParameters() const
+{
+ ASSERT(lineThickness());
FractionParameters parameters;
// We try and read constants to draw the fraction from the OpenType MATH and use fallback values otherwise.
@@ -111,9 +115,10 @@
return parameters;
}
-RenderMathMLFraction::StackParameters RenderMathMLFraction::stackParameters()
+RenderMathMLFraction::StackParameters RenderMathMLFraction::stackParameters() const
{
- ASSERT(isStack());
+ ASSERT(!lineThickness());
+ ASSERT(isValid());
StackParameters parameters;
// We try and read constants to draw the stack from the OpenType MATH and use fallback values otherwise.
@@ -133,6 +138,17 @@
parameters.bottomShiftDown = 0;
}
+ LayoutUnit numeratorAscent = ascentForChild(numerator());
+ LayoutUnit numeratorDescent = numerator().logicalHeight() - numeratorAscent;
+ LayoutUnit denominatorAscent = ascentForChild(denominator());
+ LayoutUnit gap = parameters.topShiftUp - numeratorDescent + parameters.bottomShiftDown - denominatorAscent;
+ if (gap < parameters.gapMin) {
+ // If the gap is not large enough, we increase the shifts by the same value.
+ LayoutUnit delta = (parameters.gapMin - gap) / 2;
+ parameters.topShiftUp += delta;
+ parameters.bottomShiftDown += delta;
+ }
+
return parameters;
}
@@ -160,7 +176,7 @@
setPreferredLogicalWidthsDirty(false);
}
-LayoutUnit RenderMathMLFraction::horizontalOffset(RenderBox& child, MathMLFractionElement::FractionAlignment align)
+LayoutUnit RenderMathMLFraction::horizontalOffset(RenderBox& child, MathMLFractionElement::FractionAlignment align) const
{
switch (align) {
case MathMLFractionElement::FractionAlignmentRight:
@@ -175,6 +191,22 @@
return LayoutUnit(0);
}
+LayoutUnit RenderMathMLFraction::ascentOverHorizontalAxis() const
+{
+ ASSERT(isValid());
+
+ LayoutUnit numeratorAscent = ascentForChild(numerator());
+ if (LayoutUnit thickness = lineThickness()) {
+ // For normal fraction layout, the axis is the middle of the fraction bar.
+ FractionParameters parameters = fractionParameters();
+ return std::max(numerator().logicalHeight() + parameters.numeratorGapMin + thickness / 2, numeratorAscent + parameters.numeratorMinShiftUp);
+ }
+
+ // For stack layout, the axis is the middle of the gap between numerator and denonimator.
+ StackParameters parameters = stackParameters();
+ return numeratorAscent + parameters.topShiftUp;
+}
+
void RenderMathMLFraction::layoutBlock(bool relayoutChildren, LayoutUnit)
{
ASSERT(needsLayout());
@@ -192,38 +224,26 @@
setLogicalWidth(std::max(numerator().logicalWidth(), denominator().logicalWidth()));
- updateLineThickness();
LayoutUnit verticalOffset = 0; // This is the top of the renderer.
LayoutPoint numeratorLocation(horizontalOffset(numerator(), element().numeratorAlignment()), verticalOffset);
numerator().setLocation(numeratorLocation);
- LayoutUnit numeratorAscent = ascentForChild(numerator());
- LayoutUnit numeratorDescent = numerator().logicalHeight() - numeratorAscent;
LayoutUnit denominatorAscent = ascentForChild(denominator());
LayoutUnit denominatorDescent = denominator().logicalHeight() - denominatorAscent;
- if (isStack()) {
+ LayoutUnit ascent = ascentOverHorizontalAxis();
+ verticalOffset += ascent;
+ if (LayoutUnit thickness = lineThickness()) {
+ FractionParameters parameters = fractionParameters();
+ verticalOffset += std::max(thickness / 2 + parameters.denominatorGapMin, parameters.denominatorMinShiftDown - denominatorAscent);
+ } else {
StackParameters parameters = stackParameters();
- LayoutUnit gap = parameters.topShiftUp - numeratorDescent + parameters.bottomShiftDown - denominatorAscent;
- if (gap < parameters.gapMin) {
- // If the gap is not large enough, we increase the shifts by the same value.
- LayoutUnit delta = (parameters.gapMin - gap) / 2;
- parameters.topShiftUp += delta;
- parameters.bottomShiftDown += delta;
- }
- verticalOffset += numeratorAscent + parameters.topShiftUp; // This is the middle of the stack gap.
- m_ascent = verticalOffset + mathAxisHeight();
verticalOffset += parameters.bottomShiftDown - denominatorAscent;
- } else {
- FractionParameters parameters = fractionParameters();
- verticalOffset += std::max(numerator().logicalHeight() + parameters.numeratorGapMin + m_lineThickness / 2, numeratorAscent + parameters.numeratorMinShiftUp); // This is the middle of the fraction bar.
- m_ascent = verticalOffset + mathAxisHeight();
- verticalOffset += std::max(m_lineThickness / 2 + parameters.denominatorGapMin, parameters.denominatorMinShiftDown - denominatorAscent);
}
LayoutPoint denominatorLocation(horizontalOffset(denominator(), element().denominatorAlignment()), verticalOffset);
denominator().setLocation(denominatorLocation);
- verticalOffset = std::max(verticalOffset + denominator().logicalHeight(), m_ascent + denominatorDescent); // This is the bottom of our renderer.
+ verticalOffset = std::max(verticalOffset + denominator().logicalHeight(), ascent + mathAxisHeight() + denominatorDescent); // This is the bottom of our renderer.
setLogicalHeight(verticalOffset);
layoutPositionedObjects(relayoutChildren);
@@ -234,14 +254,15 @@
void RenderMathMLFraction::paint(PaintInfo& info, const LayoutPoint& paintOffset)
{
RenderMathMLBlock::paint(info, paintOffset);
- if (info.context().paintingDisabled() || info.phase != PaintPhaseForeground || style().visibility() != VISIBLE || !isValid() || isStack())
+ LayoutUnit thickness = lineThickness();
+ if (info.context().paintingDisabled() || info.phase != PaintPhaseForeground || style().visibility() != VISIBLE || !isValid() || !thickness)
return;
- IntPoint adjustedPaintOffset = roundedIntPoint(paintOffset + location() + LayoutPoint(0, m_ascent - mathAxisHeight()));
+ IntPoint adjustedPaintOffset = roundedIntPoint(paintOffset + location() + LayoutPoint(0, ascentOverHorizontalAxis()));
GraphicsContextStateSaver stateSaver(info.context());
- info.context().setStrokeThickness(m_lineThickness);
+ info.context().setStrokeThickness(thickness);
info.context().setStrokeStyle(SolidStroke);
info.context().setStrokeColor(style().visitedDependentColor(CSSPropertyColor));
info.context().drawLine(adjustedPaintOffset, roundedIntPoint(LayoutPoint(adjustedPaintOffset.x() + logicalWidth(), adjustedPaintOffset.y())));
@@ -250,7 +271,7 @@
std::optional<int> RenderMathMLFraction::firstLineBaseline() const
{
if (isValid())
- return std::optional<int>(std::lround(static_cast<float>(m_ascent)));
+ return std::optional<int>(std::lround(static_cast<float>(ascentOverHorizontalAxis() + mathAxisHeight())));
return RenderMathMLBlock::firstLineBaseline();
}
Modified: trunk/Source/WebCore/rendering/mathml/RenderMathMLFraction.h (226176 => 226177)
--- trunk/Source/WebCore/rendering/mathml/RenderMathMLFraction.h 2017-12-20 14:25:50 UTC (rev 226176)
+++ trunk/Source/WebCore/rendering/mathml/RenderMathMLFraction.h 2017-12-20 14:28:05 UTC (rev 226177)
@@ -41,7 +41,9 @@
public:
RenderMathMLFraction(MathMLFractionElement&, RenderStyle&&);
- float relativeLineThickness() const { return m_defaultLineThickness ? m_lineThickness / m_defaultLineThickness : LayoutUnit(0); }
+ LayoutUnit defaultLineThickness() const;
+ LayoutUnit lineThickness() const;
+ float relativeLineThickness() const;
private:
bool isRenderMathMLFraction() const final { return true; }
@@ -55,12 +57,10 @@
MathMLFractionElement& element() const { return static_cast<MathMLFractionElement&>(nodeForNonAnonymous()); }
- bool isStack() const { return !m_lineThickness; }
bool isValid() const;
RenderBox& numerator() const;
RenderBox& denominator() const;
- LayoutUnit horizontalOffset(RenderBox&, MathMLFractionElement::FractionAlignment);
- void updateLineThickness();
+ LayoutUnit horizontalOffset(RenderBox&, MathMLFractionElement::FractionAlignment) const;
struct FractionParameters {
LayoutUnit numeratorGapMin;
LayoutUnit denominatorGapMin;
@@ -67,17 +67,14 @@
LayoutUnit numeratorMinShiftUp;
LayoutUnit denominatorMinShiftDown;
};
- FractionParameters fractionParameters();
+ FractionParameters fractionParameters() const;
struct StackParameters {
LayoutUnit gapMin;
LayoutUnit topShiftUp;
LayoutUnit bottomShiftDown;
};
- StackParameters stackParameters();
-
- LayoutUnit m_ascent;
- LayoutUnit m_defaultLineThickness { 1 };
- LayoutUnit m_lineThickness;
+ StackParameters stackParameters() const;
+ LayoutUnit ascentOverHorizontalAxis() const;
};
} // namespace WebCore
