Attachment #277045 for bug #156906

View Details Raw Unified Return to Bug 156906

| Differences between



2016-04-22  Frederic Wang  <fwang@igalia.com>

        Minor refactoring in RenderMathMLOperator
        https://bugs.webkit.org/show_bug.cgi?id=156906

        Reviewed by NOBODY (OOPS!).

        No new tests, this is only minor refactoring that does not change the code.

        * rendering/mathml/RenderMathMLOperator.cpp:
        (WebCore::RenderMathMLOperator::getGlyphAssemblyFallBack):
        We rename the "state" integer to an "expected" enum indicating the next expected part.
        (WebCore::RenderMathMLOperator::paintGlyph): We add a missing dot at the end of a sequence.
        We also replace ceil(x+1) with ceil(x)+1 to get rid of the temporary variable.

2016-04-21  Frederic Wang  <fwang@igalia.com>

        More improvements and explanations regarding resetting CSS properties on the <math> element



    if (nonExtenderCount > 3)
        return false; // This is not supported: there are too many pieces.

    // We now browse the list of pieces from left to right for horizontal operators and from bottom to top for vertical operators.
    enum PartType {
        Start,
        ExtenderBetweenStartAndMiddle,
        Middle,
        ExtenderBetweenMiddleAndEnd,
        End,
        None
    };
    PartType expected = Start;
    extension.glyph = 0;
    middle.glyph = 0;
    for (auto& part : assemblyParts) {
        if (nonExtenderCount < 3) {
            // If we only have at most two non-extenders then we skip the middle glyph.
            if (expected == ExtenderBetweenStartAndMiddle)
                expected = ExtenderBetweenMiddleAndEnd;
            else if (expected == Middle)
                expected = End;
        }
        if (part.isExtender) {
            if (!extension.glyph)

            else if (extension.glyph != part.glyph)
                return false; // This is not supported: the assembly has different extenders.

            if (expected == Start) {
                // We ignore left/bottom piece and multiple successive extenders.
                expected = ExtenderBetweenStartAndMiddle;
            } else if (expected == Middle) {
                // We ignore middle piece and multiple successive extenders.
                expected = ExtenderBetweenMiddleAndEnd;
            } else if (expected >= End)
                return false; // This is not supported: we got an unexpected extender.
            continue;
        }

        if (expected == Start) {
            // We copy the left/bottom part.
            bottom.glyph = part.glyph;
            expected = ExtenderBetweenStartAndMiddle;
            continue;
        }

        if (expected == ExtenderBetweenStartAndMiddle || expected == Middle) {
            // We copy the middle part.
            middle.glyph = part.glyph;
            expected = ExtenderBetweenMiddleAndEnd;
            continue;
        }

        if (expected == ExtenderBetweenMiddleAndEnd || expected == End) {
            // We copy the right/top part.
            top.glyph = part.glyph;
            expected = None;
        }
    }



    // In order to have glyphs fit snugly with one another we snap the connecting edges to pixel boundaries
    // and trim off one pixel. The pixel trim is to account for fonts that have edge pixels that have less
    // than full coverage. These edge pixels can introduce small seams between connected glyphs.
    FloatRect clipBounds = info.rect;
    switch (trim) {
    case TrimTop:

        glyphPaintRect.shiftMaxYEdgeTo(glyphPaintRect.maxY().floor() - 1);
        clipBounds.shiftMaxYEdgeTo(glyphPaintRect.maxY());
        break;
    case TrimTopAndBottom:
        glyphPaintRect.shiftYEdgeTo(glyphPaintRect.y().ceil() + 1);
        glyphPaintRect.shiftYEdgeTo(temp.ceil());
        glyphPaintRect.shiftMaxYEdgeTo(glyphPaintRect.maxY().floor() - 1);
        clipBounds.shiftYEdgeTo(glyphPaintRect.y());
        clipBounds.shiftMaxYEdgeTo(glyphPaintRect.maxY());
    }
        break;
    case TrimLeft:
        glyphPaintRect.shiftXEdgeTo(glyphPaintRect.x().ceil() + 1);

        glyphPaintRect.shiftMaxXEdgeTo(glyphPaintRect.maxX().floor() - 1);
        clipBounds.shiftMaxXEdgeTo(glyphPaintRect.maxX());
        break;
    case TrimLeftAndRight:
        glyphPaintRect.shiftXEdgeTo(glyphPaintRect.x().ceil() + 1);
        glyphPaintRect.shiftXEdgeTo(temp.ceil());
        glyphPaintRect.shiftMaxXEdgeTo(glyphPaintRect.maxX().floor() - 1);
        clipBounds.shiftXEdgeTo(glyphPaintRect.x());
        clipBounds.shiftMaxXEdgeTo(glyphPaintRect.maxX());
    }
    }

    // Clipping the enclosing IntRect avoids any potential issues at joined edges.
    GraphicsContextStateSaver stateSaver(info.context());

Return to Bug 156906

Lines 406-426 bool RenderMathMLOperator::getGlyphAssemblyFallBack(Vector<OpenTypeMathData::Ass a/Source/WebCore/rendering/mathml/RenderMathMLOperator.cpp_sec1
- a/Source/WebCore/rendering/mathml/RenderMathMLOperator.cpp -32 / +32 lines
406	if (nonExtenderCount > 3)	406	if (nonExtenderCount > 3)
407	return false; // This is not supported: there are too many pieces.	407	return false; // This is not supported: there are too many pieces.
408		408
409	// We now browse the list of pieces.	409	// We now browse the list of pieces from left to right for horizontal operators and from bottom to top for vertical operators.
410	// 1 = look for a left/bottom glyph	410	enum PartType {
411	// 2 = look for an extender between left/bottom and mid	411	Start,
412	// 4 = look for a middle glyph	412	ExtenderBetweenStartAndMiddle,
413	// 5 = look for an extender between middle and right/top	413	Middle,
414	// 5 = look for a right/top glyph	414	ExtenderBetweenMiddleAndEnd,
415	// 6 = no more piece expected	415	End,
416	unsigned state = 1;	416	None
417		417	};
		418	PartType expected = Start;
418	extension.glyph = 0;	419	extension.glyph = 0;
419	middle.glyph = 0;	420	middle.glyph = 0;
420	for (auto& part : assemblyParts) {	421	for (auto& part : assemblyParts) {
421	if ((state == 2 \|\| state == 3) && nonExtenderCount < 3) {	422	if (nonExtenderCount < 3) {
422	// We do not try to find a middle glyph.	423	// If we only have at most two non-extenders then we skip the middle glyph.
423	state += 2;	424	if (expected == ExtenderBetweenStartAndMiddle)
		425	expected = ExtenderBetweenMiddleAndEnd;
		426	else if (expected == Middle)
		427	expected = End;
424	}	428	}
425	if (part.isExtender) {	429	if (part.isExtender) {
426	if (!extension.glyph)	430	if (!extension.glyph)
Lines 428-462 bool RenderMathMLOperator::getGlyphAssemblyFallBack(Vector<OpenTypeMathData::Ass a/Source/WebCore/rendering/mathml/RenderMathMLOperator.cpp_sec2
428	else if (extension.glyph != part.glyph)	432	else if (extension.glyph != part.glyph)
429	return false; // This is not supported: the assembly has different extenders.	433	return false; // This is not supported: the assembly has different extenders.
430		434
431	if (state == 1) {	435	if (expected == Start) {
432	// We ignore left/bottom piece and multiple successive extenders.	436	// We ignore left/bottom piece and multiple successive extenders.
433	state = 2;	437	expected = ExtenderBetweenStartAndMiddle;
434	} else if (state == 3) {	438	} else if (expected == Middle) {
435	// We ignore middle piece and multiple successive extenders.	439	// We ignore middle piece and multiple successive extenders.
436	state = 4;	440	expected = ExtenderBetweenMiddleAndEnd;
437	} else if (state >= 5)	441	} else if (expected >= End)
438	return false; // This is not supported: we got an unexpected extender.	442	return false; // This is not supported: we got an unexpected extender.
439	continue;	443	continue;
440	}	444	}
441		445
442	if (state == 1) {	446	if (expected == Start) {
443	// We copy the left/bottom part.	447	// We copy the left/bottom part.
444	bottom.glyph = part.glyph;	448	bottom.glyph = part.glyph;
445	state = 2;	449	expected = ExtenderBetweenStartAndMiddle;
446	continue;	450	continue;
447	}	451	}
448		452
449	if (state == 2 \|\| state == 3) {	453	if (expected == ExtenderBetweenStartAndMiddle \|\| expected == Middle) {
450	// We copy the middle part.	454	// We copy the middle part.
451	middle.glyph = part.glyph;	455	middle.glyph = part.glyph;
452	state = 4;	456	expected = ExtenderBetweenMiddleAndEnd;
453	continue;	457	continue;
454	}	458	}
455		459
456	if (state == 4 \|\| state == 5) {	460	if (expected == ExtenderBetweenMiddleAndEnd \|\| expected == End) {
457	// We copy the right/top part.	461	// We copy the right/top part.
458	top.glyph = part.glyph;	462	top.glyph = part.glyph;
459	state = 6;	463	expected = None;
460	}	464	}
461	}	465	}
462		466
Lines 708-714 LayoutRect RenderMathMLOperator::paintGlyph(PaintInfo& info, const GlyphData& da a/Source/WebCore/rendering/mathml/RenderMathMLOperator.cpp_sec3
708		712
709	// In order to have glyphs fit snugly with one another we snap the connecting edges to pixel boundaries	713	// In order to have glyphs fit snugly with one another we snap the connecting edges to pixel boundaries
710	// and trim off one pixel. The pixel trim is to account for fonts that have edge pixels that have less	714	// and trim off one pixel. The pixel trim is to account for fonts that have edge pixels that have less
711	// than full coverage. These edge pixels can introduce small seams between connected glyphs	715	// than full coverage. These edge pixels can introduce small seams between connected glyphs.
712	FloatRect clipBounds = info.rect;	716	FloatRect clipBounds = info.rect;
713	switch (trim) {	717	switch (trim) {
714	case TrimTop:	718	case TrimTop:
Lines 719-731 LayoutRect RenderMathMLOperator::paintGlyph(PaintInfo& info, const GlyphData& da a/Source/WebCore/rendering/mathml/RenderMathMLOperator.cpp_sec4
719	glyphPaintRect.shiftMaxYEdgeTo(glyphPaintRect.maxY().floor() - 1);	723	glyphPaintRect.shiftMaxYEdgeTo(glyphPaintRect.maxY().floor() - 1);
720	clipBounds.shiftMaxYEdgeTo(glyphPaintRect.maxY());	724	clipBounds.shiftMaxYEdgeTo(glyphPaintRect.maxY());
721	break;	725	break;
722	case TrimTopAndBottom: {	726	case TrimTopAndBottom:
723	LayoutUnit temp = glyphPaintRect.y() + 1;	727	glyphPaintRect.shiftYEdgeTo(glyphPaintRect.y().ceil() + 1);
724	glyphPaintRect.shiftYEdgeTo(temp.ceil());
725	glyphPaintRect.shiftMaxYEdgeTo(glyphPaintRect.maxY().floor() - 1);	728	glyphPaintRect.shiftMaxYEdgeTo(glyphPaintRect.maxY().floor() - 1);
726	clipBounds.shiftYEdgeTo(glyphPaintRect.y());	729	clipBounds.shiftYEdgeTo(glyphPaintRect.y());
727	clipBounds.shiftMaxYEdgeTo(glyphPaintRect.maxY());	730	clipBounds.shiftMaxYEdgeTo(glyphPaintRect.maxY());
728	}
729	break;	731	break;
730	case TrimLeft:	732	case TrimLeft:
731	glyphPaintRect.shiftXEdgeTo(glyphPaintRect.x().ceil() + 1);	733	glyphPaintRect.shiftXEdgeTo(glyphPaintRect.x().ceil() + 1);
Lines 735-748 LayoutRect RenderMathMLOperator::paintGlyph(PaintInfo& info, const GlyphData& da a/Source/WebCore/rendering/mathml/RenderMathMLOperator.cpp_sec5
735	glyphPaintRect.shiftMaxXEdgeTo(glyphPaintRect.maxX().floor() - 1);	737	glyphPaintRect.shiftMaxXEdgeTo(glyphPaintRect.maxX().floor() - 1);
736	clipBounds.shiftMaxXEdgeTo(glyphPaintRect.maxX());	738	clipBounds.shiftMaxXEdgeTo(glyphPaintRect.maxX());
737	break;	739	break;
738	case TrimLeftAndRight: {	740	case TrimLeftAndRight:
739	LayoutUnit temp = glyphPaintRect.x() + 1;	741	glyphPaintRect.shiftXEdgeTo(glyphPaintRect.x().ceil() + 1);
740	glyphPaintRect.shiftXEdgeTo(temp.ceil());
741	glyphPaintRect.shiftMaxXEdgeTo(glyphPaintRect.maxX().floor() - 1);	742	glyphPaintRect.shiftMaxXEdgeTo(glyphPaintRect.maxX().floor() - 1);
742	clipBounds.shiftXEdgeTo(glyphPaintRect.x());	743	clipBounds.shiftXEdgeTo(glyphPaintRect.x());
743	clipBounds.shiftMaxXEdgeTo(glyphPaintRect.maxX());	744	clipBounds.shiftMaxXEdgeTo(glyphPaintRect.maxX());
744	}	745	}
745	}
746		746
747	// Clipping the enclosing IntRect avoids any potential issues at joined edges.	747	// Clipping the enclosing IntRect avoids any potential issues at joined edges.
748	GraphicsContextStateSaver stateSaver(info.context());	748	GraphicsContextStateSaver stateSaver(info.context());

Lines 1-3 a/Source/WebCore/ChangeLog_sec1
- a/Source/WebCore/ChangeLog +15 lines
		1	2016-04-22 Frederic Wang <fwang@igalia.com>
		2
		3	Minor refactoring in RenderMathMLOperator
		4	https://bugs.webkit.org/show_bug.cgi?id=156906
		5
		6	Reviewed by NOBODY (OOPS!).
		7
		8	No new tests, this is only minor refactoring that does not change the code.
		9
		10	* rendering/mathml/RenderMathMLOperator.cpp:
		11	(WebCore::RenderMathMLOperator::getGlyphAssemblyFallBack):
		12	We rename the "state" integer to an "expected" enum indicating the next expected part.
		13	(WebCore::RenderMathMLOperator::paintGlyph): We add a missing dot at the end of a sequence.
		14	We also replace ceil(x+1) with ceil(x)+1 to get rid of the temporary variable.
		15
1	2016-04-21 Frederic Wang <fwang@igalia.com>	16	2016-04-21 Frederic Wang <fwang@igalia.com>
2		17
3	More improvements and explanations regarding resetting CSS properties on the <math> element	18	More improvements and explanations regarding resetting CSS properties on the <math> element