[Webkit-unassigned] [Bug 218093] [css-logical] Implement logical border-radius

[bugzilla-daemon at webkit.org]

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

--- Comment #35 from Oriol Brufau <obrufau at igalia.com> ---
Comment on attachment 416519
  --> https://bugs.webkit.org/attachment.cgi?id=416519
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=416519&action=review

> Source/WebCore/platform/text/WritingMode.h:183
> +inline BoxCorner mapLogicalCornerToPhysicalCorner(TextFlow textflow, WritingMode writingMode, LogicalBoxCorner logicalBoxCorner)

I guess this function should be marked constexpr, like the others.

And it seems strange to pass both TextFlow and WritingMode as arguments.
I would pass the TextFlow only, though you may need a new isFlippedLinesTextFlow function.

> Source/WebCore/platform/text/WritingMode.h:185
> +    BoxCorner physicalCorner = static_cast<BoxCorner>(logicalBoxCorner);

physicalCorner doesn't seem necessary.

> Source/WebCore/platform/text/WritingMode.h:186
> +    switch (logicalBoxCorner) {

This switch seems very long. I think you can simplify the cases as such:

    switch (logicalBoxCorner) {
    case LogicalBoxCorner::StartStart:
        if (isFlippedTextFlow(textflow)) {
            if (isReversedTextFlow(textflow))
                return BoxCorner::BottomRight;
        } else if (!isReversedTextFlow(textflow)) {
           return BoxCorner::TopLeft;
        }
        if (isFlippedLinesTextFlow(textflow))
            return BoxCorner::BottomLeft;
        return BoxCorner::TopRight;
    case LogicalBoxCorner::StartEnd:
        if (isFlippedTextFlow(textflow)) {
            if (!isReversedTextFlow(textflow))
                return BoxCorner::BottomRight;
        } else if (isReversedTextFlow(textflow)) {
            return BoxCorner::TopLeft;
        }
        if (isFlippedLinesTextFlow(textflow))
            return BoxCorner::BottomLeft;
        return BoxCorner::TopRight;
    case LogicalBoxCorner::EndStart:
        if (isFlippedTextFlow(textflow)) {
            if (!isReversedTextFlow(textflow))
                return BoxCorner::TopLeft;
        } else if (isReversedTextFlow(textflow)) {
            return BoxCorner::BottomRight;
        }
        if (isFlippedLinesTextFlow(textflow))
            return BoxCorner::TopRight;
        return BoxCorner::BottomLeft;
    case LogicalBoxCorner::EndEnd:
        if (isFlippedTextFlow(textflow)) {
            if (isReversedTextFlow(textflow))
                return BoxCorner::TopLeft;
        } else if (!isReversedTextFlow(textflow)) {
           return BoxCorner::BottomRight;
        }
        if (isFlippedLinesTextFlow(textflow))
            return BoxCorner::TopRight;
        return BoxCorner::BottomLeft;
    }


Or even remove the switch and use

    bool isBlockStart = logicalBoxCorner == LogicalBoxCorner::StartStart || logicalBoxCorner == LogicalBoxCorner::StartEnd;
    bool isInlineStart = logicalBoxCorner == LogicalBoxCorner::StartStart || logicalBoxCorner == LogicalBoxCorner::EndStart;
    if (isBlockStart == isFlippedTextFlow(textflow)) {
        if (isInlineStart == isReversedTextFlow(textflow))
            return BoxCorner::BottomRight;
    } else if (isInlineStart != isReversedTextFlow(textflow)) {
       return BoxCorner::TopLeft;
    }
    if (isBlockStart == isFlippedLinesTextFlow(textflow))
        return BoxCorner::BottomLeft;
    return BoxCorner::TopRight;

-- 
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/20201220/d5dea514/attachment.htm>