[Webkit-unassigned] [Bug 37044] MathML Support for mroot & msqrt

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Apr 13 02:32:05 PDT 2010


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


Alex Milowski <alex at milowski.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |alex at milowski.com




--- Comment #1 from Alex Milowski <alex at milowski.com>  2010-04-13 02:32:05 PST ---
> Index: WebCore/mathml/RenderMathMLRoot.cpp
> ===================================================================
> --- WebCore/mathml/RenderMathMLRoot.cpp	(revision 0)
> +++ WebCore/mathml/RenderMathMLRoot.cpp	(revision 0)

> +
> +const int radicalBasePad = 3;
> +const float thresholdBaseHeight = 1.5;

This should probable be prefixed with a 'g':

   gRadicalBasePad
   gThresholdBaseHeight

Any comment describing these constants would be good.

> +
> +RenderMathMLRoot::RenderMathMLRoot(Node *expression) 
> +: RenderMathMLBlock(expression) 
> +{
> +}
> +
> +void RenderMathMLRoot::addChild(RenderObject* child, RenderObject* )
> +{
> +    if (isEmpty()) {
> +        // Add a block for the index
> +        RenderBlock* block = new (renderArena()) RenderBlock(node());
> +        RefPtr<RenderStyle> indexStyle = makeBlockStyle();
> +        indexStyle->setDisplay(INLINE_BLOCK);
> +        block->setStyle(indexStyle.release());
> +        RenderBlock::addChild(block);
> +        
> +        // this is the base, so wrap it so we can pad it
> +        block = new (renderArena()) RenderBlock(node());
> +        RefPtr<RenderStyle> baseStyle = makeBlockStyle();
> +        baseStyle->setDisplay(INLINE_BLOCK);
> +        baseStyle->setPaddingLeft(Length(0.5 * 0.75 , Percent));

You should make these numbers constants.



> +void RenderMathMLRoot::paint(PaintInfo& info, int tx, int ty)
> +{
> +    RenderMathMLBlock::paint(info , tx , ty);
> +    
> +    tx += x();
> +    ty += y();
> +    
> +    RenderBoxModelObject* indexBox = toRenderBoxModelObject(lastChild());
> +    
> +    int maxHeight = indexBox->offsetHeight();
> +    if (!maxHeight) {
> +        // default to the font size in pixels if we're empty
> +        maxHeight = style()->fontSize();
> +    }
> +    int width = indexBox->offsetWidth();
> +    
> +    int indexWidth = 0;
> +    RenderObject* current = firstChild();
> +    while (current != lastChild()) {
> +        if (current->isBoxModelObject()) {
> +            RenderBoxModelObject* box = toRenderBoxModelObject(current);
> +            indexWidth += box->offsetWidth();
> +        }
> +        current = current->nextSibling();
> +    }
> +    
> +    int frontWidth = static_cast<int>(style()->fontSize() * 0.75);

Make the number a constant as well.

> +    int topStartShift = 0;
> +    // Base height above which the shape of the root changes
> +    int thresholdHeight = static_cast<int>(thresholdBaseHeight * style()->fontSize());
> +    
> +    if (maxHeight > thresholdHeight && thresholdHeight) {
> +        float shift = (maxHeight - thresholdHeight) / static_cast<float>(thresholdHeight);
> +        if (shift > 1.)
> +            shift = 1.;
> +        topStartShift = static_cast<int>(0.5 * frontWidth * shift);
> +    }

Same here with the 0.5

> +    
> +    width += topStartShift;
> +    
> +    int start = tx + indexWidth + radicalBasePad + style()->paddingLeft().value() - static_cast<int>(0.2 * style()->fontSize());
> +    ty += style()->paddingTop().value() - static_cast<int>(0.2 * style()->fontSize());
> +    
> +    FloatPoint topStart(start - topStartShift, ty);
> +    FloatPoint bottomLeft(start - 0.5 * frontWidth , ty + maxHeight + radicalBasePad);
> +    FloatPoint topLeft(start - 0.8 * frontWidth , ty + 0.625 * maxHeight);
> +    FloatPoint leftEnd(start - frontWidth , topLeft.y() + 0.05 * style()->fontSize());

Similarly, make the numbers named constants.

> +    
> +    info.context->save();
> +    
> +    info.context->setStrokeThickness(0.02 * style()->fontSize());

Same here.

> +    info.context->setStrokeStyle(SolidStroke);
> +    info.context->setStrokeColor(style()->color(), sRGBColorSpace);
> +    info.context->setLineJoin(MiterJoin);
> +    info.context->setMiterLimit(style()->fontSize());
> +    
> +    Path root;
> +    
> +    root.moveTo(FloatPoint(topStart.x() + width , ty));

No space before comma.

> +    // draw top
> +    root.addLineTo(topStart);
> +    // draw from top left corner to bottom point of radical
> +    root.addLineTo(bottomLeft);
> +    // draw from bottom point to top of left part of radical base "pocket"
> +    root.addLineTo(topLeft);
> +    // draw to end
> +    root.addLineTo(leftEnd);
> +    
> +    info.context->beginPath();
> +    info.context->addPath(root);
> +    info.context->strokePath();
> +    
> +    info.context->save();
> +    
> +    // Build a mask to draw the thick part of the root.
> +    Path mask;
> +    
> +    mask.moveTo(topStart);
> +    mask.addLineTo(bottomLeft);
> +    mask.addLineTo(topLeft);
> +    mask.addLineTo(FloatPoint(2 * topLeft.x() - leftEnd.x(), 2 * topLeft.y() - leftEnd.y()));

Make the '2' a constant.

> +    
> +    info.context->beginPath();
> +    info.context->addPath(mask);
> +    info.context->clip(mask);
> +    
> +    // Draw the thick part of the root.
> +    info.context->setStrokeThickness(0.1 * style()->fontSize());

Same here.

> +    info.context->setLineCap(SquareCap);
> +    
> +    Path line;
> +    
> +    line = line.createLine(bottomLeft, topLeft);
> +    
> +    info.context->beginPath();
> +    info.context->addPath(line);
> +    info.context->strokePath();
> +    
> +    info.context->restore();
> +    
> +    info.context->restore();
> +
> +}
> +
> +void RenderMathMLRoot::layout()
> +{
> +    RenderBlock::layout();
> +    
> +    int maxHeight = toRenderBoxModelObject(lastChild())->offsetHeight();
> +    
> +    RenderObject* current = lastChild()->firstChild();
> +    
> +    toRenderMathMLBlock(current)->style()->setVerticalAlign(BASELINE);
> +    
> +    if (!maxHeight)
> +        maxHeight = style()->fontSize();
> +    
> +    // Base height above which the shape of the root changes
> +    int thresholdHeight = static_cast<int>(thresholdBaseHeight * style()->fontSize());
> +    int topStartShift = 0;
> +    
> +    if (maxHeight > thresholdHeight && thresholdHeight) {
> +        float shift = (maxHeight - thresholdHeight) / static_cast<float>(thresholdHeight);
> +        if (shift > 1.)
> +            shift = 1.;
> +        topStartShift = static_cast<int>(0.5 * static_cast<int>(style()->fontSize() * 0.75) * shift);
> +        
> +        style()->setPaddingBottom(Length(static_cast<int>(0.2 * style()->fontSize()), Fixed));

More named constants here.

> +    }
> +    
> +    // Positioning of the index
> +    RenderBoxModelObject* indexBox = toRenderBoxModelObject(firstChild()->firstChild());
> +    
> +    int indexShift = indexBox->offsetWidth() + topStartShift;
> +    int rootMarginTop = static_cast<int>(0.375 * maxHeight) + style()->paddingBottom().value() + indexBox->offsetHeight() - (maxHeight + static_cast<int>(0.2 * style()->fontSize()));
> +    
> +    style()->setPaddingLeft(Length(indexShift, Fixed));
> +    if (rootMarginTop > 0)
> +        style()->setPaddingTop(Length(rootMarginTop + static_cast<int>(0.2 * style()->fontSize()), Fixed));

Same here.

> +    
> +    setNeedsLayoutAndPrefWidthsRecalc();
> +    markContainingBlocksForLayout();
> +    RenderBlock::layout();
> +    
> +    indexBox->style()->setBottom(Length(static_cast<int>(0.375 * (maxHeight)) + style()->paddingBottom().value(), Fixed));

Same here.

> +    
> +    indexBox->layout();
> +}
> +    
> +}
> +
> +#endif // ENABLE(MATHML)
> +
> +
> Index: WebCore/mathml/RenderMathMLSquareRoot.cpp
> ===================================================================
> --- WebCore/mathml/RenderMathMLSquareRoot.cpp	(revision 0)
> +++ WebCore/mathml/RenderMathMLSquareRoot.cpp	(revision 0)

> +
> +const int radicalBasePad = 3;
> +const float thresholdBaseHeight = 1.5;

Change the names to be prefixed with a 'g' as before.

> +    
> +RenderMathMLSquareRoot::RenderMathMLSquareRoot(Node *expression) 
> +    : RenderMathMLBlock(expression) 
> +{
> +}
> +
> +void RenderMathMLSquareRoot::paint(PaintInfo& info, int tx, int ty)
> +{
> +    RenderMathMLBlock::paint(info, tx, ty);
> +   
> +    tx += x();
> +    ty += y();
> +
> +    int maxHeight = 0;
> +    int width = 0;
> +    RenderObject* current = firstChild();
> +    while (current) {
> +        if (current->isBoxModelObject()) {
> +            
> +            RenderBoxModelObject* box = toRenderBoxModelObject(current);
> +            
> +            // Check to see if this box has a larger height
> +            if (box->offsetHeight() > maxHeight)
> +                maxHeight = box->offsetHeight();
> +            width += box->offsetWidth();
> +        }
> +        current = current->nextSibling();
> +    }
> +    // default to the font size in pixels if we're empty
> +    if (!maxHeight)
> +        maxHeight = style()->fontSize();
> +    
> +    int frontWidth = static_cast<int>(style()->fontSize() * 0.75);

use named constants.

> +    int topStartShift = 0;
> +    // Base height above which the shape of the root changes
> +    int thresholdHeight = static_cast<int>(thresholdBaseHeight * style()->fontSize());
> +    
> +    if (maxHeight > thresholdHeight && thresholdHeight) {
> +        float shift = (maxHeight - thresholdHeight) / static_cast<float>(thresholdHeight);
> +        if (shift > 1.)
> +            shift = 1.;
> +        topStartShift = static_cast<int>(0.5 * frontWidth * shift);
> +    }
> +    
> +    width += topStartShift;
> +    
> +    FloatPoint topStart(tx + frontWidth - topStartShift, ty);
> +    FloatPoint bottomLeft(tx + frontWidth * 0.5 , ty + maxHeight + radicalBasePad);
> +    FloatPoint topLeft(tx + frontWidth * 0.2 , ty + 0.5 * maxHeight);
> +    FloatPoint leftEnd(tx , topLeft.y() + 0.05 * style()->fontSize());
> +    
> +    info.context->save();
> +    
> +    info.context->setStrokeThickness(0.02 * style()->fontSize());

Same in all of the above.


> +    mask.addLineTo(FloatPoint(2 * topLeft.x() - leftEnd.x(), 2 * topLeft.y() - leftEnd.y()));

Same above.

> +    
> +    info.context->beginPath();
> +    info.context->addPath(mask);
> +    info.context->clip(mask);
> +    
> +    // Draw the thick part of the root.
> +    info.context->setStrokeThickness(0.1 * style()->fontSize());

Same above.


> +void RenderMathMLSquareRoot::layout()
> +{
> +    int maxHeight = 0;
> +    
> +    RenderObject* current = firstChild();
> +    while (current) {
> +        if (current->isBoxModelObject()) {
> +            RenderBoxModelObject* box = toRenderBoxModelObject(current);
> +            
> +            if (box->offsetHeight() > maxHeight)
> +                maxHeight = box->offsetHeight();
> +            
> +            box->style()->setVerticalAlign(BASELINE);
> +        }
> +        current = current->nextSibling();
> +    }
> +    
> +    if (!maxHeight)
> +        maxHeight = style()->fontSize();
> +
> +    
> +    if (maxHeight > static_cast<int>(thresholdBaseHeight * style()->fontSize()))
> +        style()->setPaddingBottom(Length(static_cast<int>(0.2 * style()->fontSize()), Fixed));

Same above.


In general, you should try to use global constants for all your calculations. 
I went
back through that in the rest of the MathML code and tried to make that change.
 Try
to use names like "gPaddingBottomFactor" rather than something more specific
like
"gDoubleHeight".

Looks good otherwise.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.


More information about the webkit-unassigned mailing list