[Webkit-unassigned] [Bug 34347] MathML Support for mrow and Stretchy Operators
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Mar 2 05:09:46 PST 2010
https://bugs.webkit.org/show_bug.cgi?id=34347
Kenneth Rohde Christiansen <kenneth at webkit.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #48392|review? |review-
Flag| |
--- Comment #3 from Kenneth Rohde Christiansen <kenneth at webkit.org> 2010-03-02 05:09:46 PST ---
(From update of attachment 48392)
> + int index = -1;
> + if (firstChar) {
> + for (index++; stretchy[index][0]; index++) {
> + if (stretchy[index][0] == firstChar)
> + break;
> + }
> + }
> +
> + bool noStretch = false;
> + if (mo) {
> + AtomicString stretchyAttr = mo->getAttribute(MathMLNames::stretchyAttr);
> + noStretch = equalIgnoringCase(stretchyAttr, "false");
> + }
So you are checking if we are allowed to stretch? If so, couldn't this be above
the other if (firstChar)?
> + // either we have no characters, the stretch height is small, or we have a non-stretchable character
> + // If so, then we just set the font size and wrap the text
> + bool canStretch = index >= 0 && stretchy[index][0];
You only need the above index if you are going to stretch. Couldn't you
separate out the code in a private method, isStretchable or so?
The below if cases are huge, so it would be good to have them separated out in
methods as well.
> + if (noStretch || !firstChar || m_stretchHeight <= 24 || !canStretch) {
> +
> + m_isStacked = false;
> + RenderBlock* container = new (renderArena()) RenderBlock(node());
> +
> + RefPtr<RenderStyle> newStyle = RenderStyle::create();
> + newStyle->inheritFrom(style());
> + newStyle->setDisplay(BLOCK);
> +
> + int currentFontSize = style()->fontSize();
> + if (!noStretch && m_stretchHeight > 0 && m_stretchHeight <= 24 && m_stretchHeight > currentFontSize && canStretch) {
24?
> + FontDescription* desc = new FontDescription();
> + desc->setIsAbsoluteSize(true);
> + desc->setSpecifiedSize(m_stretchHeight);
> + desc->setComputedSize(m_stretchHeight);
> + newStyle->setFontDescription(*desc);
> + newStyle->font().update(newStyle->font().fontSelector());
> + }
> +
> + newStyle->setVerticalAlign(BASELINE);
> + container->setStyle(newStyle.release());
> + addChild(container);
> +
> + RenderText* text = new (renderArena()) RenderText(node(), m_operator ? StringImpl::create(&m_operator, 1) : StringImpl::create(mo->textContent()));
> + text->setStyle(container->style());
> + container->addChild(text);
> +
> + } else {
> + // build stretchable characters
> + m_isStacked = true;
> +
> + if (stretchy[index][4]) {
> +
> + // we have a middle glyph (e.g. a curly bracket)
> +
> + int half = (m_stretchHeight - 10) / 2;
You use a lot of constants in the code around here. Where do they come from? I
see 10, -2 and -4 in many places.
> + if (half <= 10) {
> + // we only have enough space for the top & bottom glyph
> + makeGlyph(stretchy[index][1], half);
> + makeGlyph(stretchy[index][4], 10, -2);
> + makeGlyph(stretchy[index][3], 0, -4);
> + } else {
> + // we have to extend both the top and bottom to the middle
> + makeGlyph(stretchy[index][1], 10);
> + int remaining = half-10;
Missing spaces around -
> + while (remaining > 0) {
> + if (remaining < 10) {
> + makeGlyph(stretchy[index][2], remaining);
> + remaining = 0;
> + } else {
> + makeGlyph(stretchy[index][2], 10);
> + remaining -= 10;
> + }
> + }
> +
> + // The middle glyph
> + makeGlyph(stretchy[index][4], 10, -2);
> +
> + remaining = half-10;
same thing
> + // make sure we have the full height in case the height is odd
> + if (m_stretchHeight % 2 == 1)
> + remaining++;
> +
> + while (remaining > 0) {
> + if (remaining < 10) {
> + makeGlyph(stretchy[index][2], remaining);
> + remaining = 0;
> + } else {
> + makeGlyph(stretchy[index][2], 10);
> + remaining -= 10;
> + }
> + }
> + makeGlyph(stretchy[index][3], 0, -4);
For these indices 2, 3 etc, would it make sense using an enum?
> + }
> + } else {
> +
> + // we do not have a middle glyph
> +
Excessive newlining above
> + int remaining = m_stretchHeight - 20;
> + makeGlyph(stretchy[index][1], 10);
> + while (remaining > 0) {
> + if (remaining < 10) {
> + makeGlyph(stretchy[index][2], remaining);
> + remaining = 0;
> + } else {
> + makeGlyph(stretchy[index][2], 10);
> + remaining -= 10;
> + }
> + }
> + makeGlyph(stretchy[index][3], 0, -4);
> + }
> + }
> +
> +}
> +
> +RefPtr<RenderStyle> RenderMathMLOperator::makeStackableStyle(int size, int topRelative)
> +{
> + RefPtr<RenderStyle> newStyle = RenderStyle::create();
> + newStyle->inheritFrom(style());
> + newStyle->setDisplay(BLOCK);
> +
> + // 14px is the size we'll use for all characters
ok, why?
> + FontDescription* desc = new FontDescription();
> + desc->setIsAbsoluteSize(true);
> + desc->setSpecifiedSize(14);
> + desc->setComputedSize(14);
> + newStyle->setFontDescription(*desc);
> + newStyle->font().update(newStyle->font().fontSelector());
> + newStyle->setLineHeight(Length(12, Fixed));
and 12?
> + virtual void layout();
> + virtual RefPtr<RenderStyle> makeStackableStyle(int size, int topRelative);
create* is more common for RefPtr in WebKit, I guess.
> + } else if (current->isBoxModelObject()) {
> +
> + RenderBoxModelObject* box = toRenderBoxModelObject(current);
excessive newlining
> +
> + // Check to see if this box has a larger height
> + if (box->offsetHeight() > maxHeight)
> + maxHeight = box->offsetHeight();
> +
> + }
Why a newline above?
> +
> + }
> + return maxHeight;
> +}
> +
> +
> +
> +void RenderMathMLRow::layout()
3 newlines? :-)
> + if (!block->hasBase() && !block->isRenderMathMLOperator()) {
> +
> + // Check to see if this box has a larger height
Newline
> + if (block->offsetHeight() > maxHeight)
> + maxHeight = block->offsetHeight();
> + }
> + if (block->isRenderMathMLOperator())
> + if (block->offsetHeight() > operatorHeight)
> + operatorHeight = block->offsetHeight();
> + } else if (current->isBoxModelObject()) {
> +
> + RenderBoxModelObject* box = toRenderBoxModelObject(current);
> +
> + // Check to see if this box has a larger height
enwlines
--
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