[Webkit-unassigned] [Bug 179682] Incorrect bounds inside <mover>/<munder> when a stretchy operator is present

[bugzilla-daemon at webkit.org]

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

--- Comment #27 from Minsheng Liu <lambda at liu.ms> ---
I must admit that I am confused with your "skip non-stretchy operators".
The skipping code you gave was:

if (toHorizontalStretchyOperator(child))
    continue;

which seems more like "skip stretchy operators" than non-stretchy ones.
I try to summarize your points (based on my understanding) to make sure that
we are on the same page.

I checked your original algorithm:
> 1) If none of the child is a horizontal stretchy operators, then just call layoutIfNeeded() on children and exit.
> 2) If there is at least one horizontal stretchy operator, then
>    * reset the stretch size for all of them and force them to relayout.
>    * call layoutIfNeeded on other children.
>    * calculate the target width as the maximum of all child->logicalWidth() [note that at this point, the operators are unstretched]
>    * call stretchTo on horizontal stretchy operators.
>    * force them to relayout again with the correct size [probably you can skip that for direct <mo> children, as you did here].

Compare that with your review comment, I think you are suggesting this
algorithm. I would then take your "skip non-stretchy operators" as skipping
those operators in step 2.1. However, in my latest patch, I merely combined
all step 1, 2.1, 2.2, 2.3 into one loop.

In my code, the first loop traverses all children.
By default, layoutIfNeeded is called on each child.
In addition to that, if a child has a stretchy operator (actually, our code
really says that if a child is an embellished horizontal stretchy operator,
more on this later.), then the stretch size is reset and the child is forced
to relayout. I add some protections so that the code work as expected
in recursive cases.

Step 2.4 and 2.5 is just the second loop.

However, I have to point out that this could not really cover the case of
stretchy under/over and a non-stretchy base. You see, fundamentllay speaking,
the issue is that stretch width is different for each child.
If we take the standard literally, the stretch width for some child x is the
maximal width of all children other than x. As a result, I have to set
stretch width to something different for each child in the second loop.

Now, I realize that the vertical stretching case is actually simpler than
our situation. There, the only thing we need to worry about is to let
stretchy operators to cover the whole row. Here, things get complicated.
Operators in the base have to cover the whole element, while
in the case of non-stretchy base and stretchy braces the stretchy brace
must cover the base only. Therefore, I made the following three rules:

1. stretch width for op in base = max(width of base, width of over, widht of under)
   standard says: max(width of over, width of under)
2. stretch width for op in under = width of base
  standard says: max(width of over, width of base)
2. stretch width for op in over = width of base
  standard says: max(width of under, width of base)

New code with other parts removed and no hack:

void RenderMathMLUnderOver::stretchHorizontalOperatorsAndLayoutChildren()
{
    // omitted code

    LayoutUnit stretchWidthForBase = 0;
    // The loop layouts each sub-expression's width with
    // embellished stretchy operators' width set to 0,
    // unless the stretchy operators' stretch widths have been locked.
    for (auto* child = firstChildBox(); child; child = child->nextSiblingBox()) {
        // omitted code

        stretchWidthForBase = std::max(stretchWidthForBase, child->logicalWidth());
    }

    LayoutUnit stretchWidthForOthers = firstChildBox()->logicalWidth();

    for (auto* child = firstChildBox(); child; child = child->nextSiblingBox()) {
        LayoutUnit stretchWidth = child == firstChildBox() ? stretchWidthForBase : stretchWidthForOthers;

        if (auto renderOperator = toHorizontalStretchyOperator(child)) {
            if (!renderOperator->isStretchWidthLocked()) {
                renderOperator->resetStretchSize();
                renderOperator->stretchTo(stretchWidth);

                renderOperator->setStretchWidthLocked(true);
                child->setNeedsLayout();
                child->layout();
                renderOperator->setStretchWidthLocked(false);
            }
        }
    }
}

I do not quite understand the comment "Skipping the embellished op does not work for nested
structures like <munder><mover><mo>_</mo>...</mover> <mo>_</mo></munder>"
but my alogrithm does handle that case well.


I hope I have understood you correctly and explain myself clearly.
If not, could you explain your view, especially those parts I misunderstood you?
Thanks.

And, our code so far, when detecting the strechy operators, merely detects
those embellished. That might be an issue. Consider the following strcutures:

<munder><mover><mtext>...</mtext><mo>_</mo></mover><mo>_</mo></munder>

The inner <mo> is not an embellished operator, so the code will layout it
as if it is not stretchy. When <mo> contains leading/trailing space,
this becomes an issue. If the inner <mo> is not stretched, the outer <mo>
is stretched to the same width of <mtext>. However, since <mo> is not
an embellished operator, the outer <mo> will stretch to <mtext> + leading spaces
for <mo>. Without significant change to the code I cannot handle this case.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20171128/54dac373/attachment-0001.html>