Changeset 275519 in webkit


Ignore:
Timestamp:
Apr 6, 2021 8:56:18 AM (16 months ago)
Author:
Patrick Angle
Message:

Web Inspector: Grid overlay does not honor writing modes and RTL layout direction.
​https://bugs.webkit.org/show_bug.cgi?id=224127

Reviewed by BJ Burg.

Source/WebCore:

Grid overlays did not previous honor writing modes and RTL layout direction correctly. The underlying math was
correct, but the 'origin' of the element's grid, and the direction in which rows and columns increased, was not
handled correctly. This patch resolves this by taking writing mode and direction into account when calculating
row/column line positions as well as correctly orienting layout label arrows based on the current writing
mode/direction.

Area names have been moved to the center of their respective areas, which both helps to make sure the label is
within the area for all writing modes/direction/transformation, as well as improved legibility on smaller grids
where area names were previously behind other labels.

  • inspector/InspectorOverlay.cpp:

(WebCore::InspectorOverlay::backgroundPathForLayoutLabel):
(WebCore::InspectorOverlay::drawLayoutLabel):
(WebCore::InspectorOverlay::drawGridOverlay):

  • backgroundPathForLayoutLabel, drawLayoutLabel, and drawGridOverlay now interpret the None arrow direction

as using the label's location as its center, not top-left.
(WebCore::authoredGridTrackSizes):

  • Drive-by removal of extra whitespace.

(WebCore::InspectorOverlay::buildGridOverlay):

  • Use the element's computed style to determine writing mode and direction (which takes into account the dir attribute).
  • Adjust columnLineAt and rowLineAt calculations based on writing mode and direction.
  • Adjust all arrow directions and positions based on writing mode and direction.

Source/WebKit:

  • UIProcess/Inspector/ios/WKInspectorHighlightView.mm:

(createLayoutLabelLayer):

  • WebCore::backgroundPathForLayoutLabel now treats the None arrow direction as being centered on the label's

location.

Location:
trunk/Source
Files:
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r275518 r275519  
     12021-04-06  Patrick Angle  <pangle@apple.com>
     2
     3        Web Inspector: Grid overlay does not honor writing modes and RTL layout direction.
     4        https://bugs.webkit.org/show_bug.cgi?id=224127
     5
     6        Reviewed by BJ Burg.
     7
     8        Grid overlays did not previous honor writing modes and RTL layout direction correctly. The underlying math was
     9        correct, but the 'origin' of the element's grid, and the direction in which rows and columns increased, was not
     10        handled correctly. This patch resolves this by taking writing mode and direction into account when calculating
     11        row/column line positions as well as correctly orienting layout label arrows based on the current writing
     12        mode/direction.
     13
     14        Area names have been moved to the center of their respective areas, which both helps to make sure the label is
     15        within the area for all writing modes/direction/transformation, as well as improved legibility on smaller grids
     16        where area names were previously behind other labels.
     17
     18        * inspector/InspectorOverlay.cpp:
     19        (WebCore::InspectorOverlay::backgroundPathForLayoutLabel):
     20        (WebCore::InspectorOverlay::drawLayoutLabel):
     21        (WebCore::InspectorOverlay::drawGridOverlay):
     22        - `backgroundPathForLayoutLabel`, `drawLayoutLabel`, and `drawGridOverlay` now interpret the `None` arrow direction
     23        as using the label's location as its center, not top-left.
     24        (WebCore::authoredGridTrackSizes):
     25        - Drive-by removal of extra whitespace.
     26        (WebCore::InspectorOverlay::buildGridOverlay):
     27        - Use the element's computed style to determine writing mode and direction (which takes into account the `dir` attribute).
     28        - Adjust columnLineAt and rowLineAt calculations based on writing mode and direction.
     29        - Adjust all arrow directions and positions based on writing mode and direction.
     30
     31
    1322021-04-06  Stephan Szabo  <stephan.szabo@sony.com>
    233

  • trunk/Source/WebCore/inspector/InspectorOverlay.cpp

    r275293 r275519  
    13311331        break;
    13321332    case InspectorOverlay::LabelArrowDirection::None:
    1333         path.addLineTo({ 0, height });
    1334         path.addLineTo({ width, height });
    1335         path.addLineTo({ width, 0 });
     1333        path.moveTo({ -(width / 2), -(height / 2) });
     1334        path.addLineTo({ -(width / 2), height / 2 });
     1335        path.addLineTo({ width / 2, height / 2 });
     1336        path.addLineTo({ width / 2, -(height / 2) });
    13361337        break;
    13371338    }
    … …  
    14491450        break;
    14501451    case InspectorOverlay::LabelArrowDirection::None:
    1451         textPosition = FloatPoint(layoutLabelPadding, layoutLabelPadding + textHeight - textDescent);
     1452        textPosition = FloatPoint(-(textWidth / 2), (textHeight / 2) - textDescent);
    14521453        break;
    14531454    }
    … …  
    14881489    context.setStrokeThickness(1);
    14891490    for (auto area : gridOverlay.areas)
    1490         drawLayoutLabel(context, area.name, area.quad.p1(), LabelArrowDirection::None, LabelArrowEdgePosition::None, translucentLabelBackgroundColor, area.quad.boundingBox().width());
     1491        drawLayoutLabel(context, area.name, area.quad.center(), LabelArrowDirection::None, LabelArrowEdgePosition::None, translucentLabelBackgroundColor, area.quad.boundingBox().width());
    14911492
    14921493    for (auto label : gridOverlay.labels)
    … …  
    15151516        }
    15161517    }
    1517    
     1518
    15181519    if (!cssValue || !is<CSSValueList>(cssValue))
    15191520        return { };
    … …  
    15251526            trackSizes.append(currentValue.cssText());
    15261527    };
    1527    
     1528
    15281529    for (auto& currentValue : downcast<CSSValueList>(*cssValue)) {
    15291530        if (is<CSSGridAutoRepeatValue>(currentValue)) {
    … …  
    15381539            break;
    15391540        }
    1540        
     1541
    15411542        if (is<CSSGridIntegerRepeatValue>(currentValue)) {
    15421543            size_t repetitions = downcast<CSSGridIntegerRepeatValue>(currentValue.get()).repetitions();
    … …  
    15471548            continue;
    15481549        }
    1549        
     1550
    15501551        handleValueIgnoringLineNames(currentValue);
    15511552    }
    1552    
     1553
    15531554    // Remaining tracks will be `auto`.
    15541555    while (trackSizes.size() < expectedTrackCount)
    … …  
    16531654    FrameView* containingView = containingFrame->view();
    16541655
    1655     auto columnLineAt = [&](int x) -> FloatLine {
     1656    auto computedStyle = node->computedStyle();
     1657    if (!computedStyle)
     1658        return { };
     1659
     1660    auto isHorizontalWritingMode = computedStyle->isHorizontalWritingMode();
     1661    auto isDirectionFlipped = !computedStyle->isLeftToRightDirection();
     1662    auto isWritingModeFlipped = computedStyle->isFlippedBlocksWritingMode();
     1663    auto contentBox = renderGrid.absoluteBoundingBoxRectIgnoringTransforms();
     1664
     1665    auto columnLineAt = [&](float x) -> FloatLine {
     1666        FloatPoint startPoint;
     1667        FloatPoint endPoint;
     1668        if (isHorizontalWritingMode) {
     1669            startPoint = { isDirectionFlipped ? contentBox.width() - x : x, isWritingModeFlipped ? contentBox.height() - gridStartY : gridStartY };
     1670            endPoint = { isDirectionFlipped ? contentBox.width() - x : x, isWritingModeFlipped ? contentBox.height() - gridEndY : gridEndY };
     1671        } else {
     1672            startPoint = { isWritingModeFlipped ? contentBox.width() - gridStartY : gridStartY, isDirectionFlipped ? contentBox.height() - x : x };
     1673            endPoint = { isWritingModeFlipped ? contentBox.width() - gridEndY : gridEndY, isDirectionFlipped ? contentBox.height() - x : x };
     1674        }
    16561675        return {
    1657             localPointToRootPoint(containingView, renderGrid.localToContainerPoint(FloatPoint(x, gridStartY), nullptr)),
    1658             localPointToRootPoint(containingView, renderGrid.localToContainerPoint(FloatPoint(x, gridEndY), nullptr)),
     1676            localPointToRootPoint(containingView, renderGrid.localToContainerPoint(startPoint, nullptr)),
     1677            localPointToRootPoint(containingView, renderGrid.localToContainerPoint(endPoint, nullptr)),
    16591678        };
    16601679    };
    1661     auto rowLineAt = [&](int y) -> FloatLine {
     1680    auto rowLineAt = [&](float y) -> FloatLine {
     1681        FloatPoint startPoint;
     1682        FloatPoint endPoint;
     1683        if (isHorizontalWritingMode) {
     1684            startPoint = { isDirectionFlipped ? contentBox.width() - gridStartX : gridStartX, isWritingModeFlipped ? contentBox.height() - y : y };
     1685            endPoint = { isDirectionFlipped ? contentBox.width() - gridEndX : gridEndX, isWritingModeFlipped ? contentBox.height() - y : y };
     1686        } else {
     1687            startPoint = { isWritingModeFlipped ? contentBox.width() - y : y, isDirectionFlipped ? contentBox.height() - gridStartX : gridStartX };
     1688            endPoint = { isWritingModeFlipped ? contentBox.width() - y : y, isDirectionFlipped ? contentBox.height() - gridEndX : gridEndX };
     1689        }
    16621690        return {
    1663             localPointToRootPoint(containingView, renderGrid.localToContainerPoint(FloatPoint(gridStartX, y), nullptr)),
    1664             localPointToRootPoint(containingView, renderGrid.localToContainerPoint(FloatPoint(gridEndX, y), nullptr)),
     1691            localPointToRootPoint(containingView, renderGrid.localToContainerPoint(startPoint, nullptr)),
     1692            localPointToRootPoint(containingView, renderGrid.localToContainerPoint(endPoint, nullptr)),
    16651693        };
     1694    };
     1695
     1696    auto correctedArrowDirection = [&](LabelArrowDirection direction, GridTrackSizingDirection sizingDirection) -> LabelArrowDirection {
     1697        if ((sizingDirection == GridTrackSizingDirection::ForColumns && isWritingModeFlipped) || (sizingDirection == GridTrackSizingDirection::ForRows && isDirectionFlipped)) {
     1698            switch (direction) {
     1699            case LabelArrowDirection::Down:
     1700                direction = LabelArrowDirection::Up;
     1701                break;
     1702            case LabelArrowDirection::Up:
     1703                direction = LabelArrowDirection::Down;
     1704                break;
     1705            case LabelArrowDirection::Left:
     1706                direction = LabelArrowDirection::Right;
     1707                break;
     1708            case LabelArrowDirection::Right:
     1709                direction = LabelArrowDirection::Left;
     1710                break;
     1711            case LabelArrowDirection::None:
     1712                break;
     1713            }
     1714        }
     1715
     1716        if (!isHorizontalWritingMode) {
     1717            switch (direction) {
     1718            case LabelArrowDirection::Down:
     1719                direction = LabelArrowDirection::Right;
     1720                break;
     1721            case LabelArrowDirection::Up:
     1722                direction = LabelArrowDirection::Left;
     1723                break;
     1724            case LabelArrowDirection::Left:
     1725                direction = LabelArrowDirection::Up;
     1726                break;
     1727            case LabelArrowDirection::Right:
     1728                direction = LabelArrowDirection::Down;
     1729                break;
     1730            case LabelArrowDirection::None:
     1731                break;
     1732            }
     1733        }
     1734
     1735        return direction;
     1736    };
     1737
     1738    auto correctedArrowEdgePosition = [&](LabelArrowEdgePosition edgePosition, GridTrackSizingDirection sizingDirection) -> LabelArrowEdgePosition {
     1739        if ((sizingDirection == GridTrackSizingDirection::ForRows && isWritingModeFlipped) || (sizingDirection == GridTrackSizingDirection::ForColumns && isDirectionFlipped)) {
     1740            if (edgePosition == LabelArrowEdgePosition::Leading)
     1741                return LabelArrowEdgePosition::Trailing;
     1742            if (edgePosition == LabelArrowEdgePosition::Trailing)
     1743                return LabelArrowEdgePosition::Leading;
     1744        }
     1745
     1746        return edgePosition;
    16661747    };
    16671748
    … …  
    17071788                auto trackSizeLabel = String::number(roundf(width));
    17081789                trackSizeLabel.append("px"_s);
    1709                
     1790
    17101791                String authoredTrackSize;
    17111792                if (i < authoredTrackColumnSizes.size()) {
    … …  
    17181799                    }
    17191800                }
    1720                
     1801
    17211802                FloatLine trackTopLine = { columnStartLine.start(), columnEndLine.start() };
    1722                 gridHighlightOverlay.labels.append(buildLabel(trackSizeLabel, trackTopLine.pointAtRelativeDistance(0.5), translucentLabelBackgroundColor, LabelArrowDirection::Up, LabelArrowEdgePosition::Middle));
     1803                gridHighlightOverlay.labels.append(buildLabel(trackSizeLabel, trackTopLine.pointAtRelativeDistance(0.5), translucentLabelBackgroundColor, correctedArrowDirection(LabelArrowDirection::Up, GridTrackSizingDirection::ForColumns), LabelArrowEdgePosition::Middle));
    17231804            }
    17241805        } else
    … …  
    17381819        if (!lineLabel.isEmpty()) {
    17391820            auto text = lineLabel.toString();
    1740             auto arrowDirection = LabelArrowDirection::Down;
    1741             auto arrowEdgePosition = LabelArrowEdgePosition::Middle;
     1821            auto arrowDirection = correctedArrowDirection(LabelArrowDirection::Down, GridTrackSizingDirection::ForColumns);
     1822            auto arrowEdgePosition = correctedArrowEdgePosition(LabelArrowEdgePosition::Middle, GridTrackSizingDirection::ForColumns);
    17421823
    17431824            if (!i)
    1744                 arrowEdgePosition = LabelArrowEdgePosition::Leading;
     1825                arrowEdgePosition = correctedArrowEdgePosition(LabelArrowEdgePosition::Leading, GridTrackSizingDirection::ForColumns);
    17451826            else if (i == columnPositions.size() - 1)
    1746                 arrowEdgePosition = LabelArrowEdgePosition::Trailing;
     1827                arrowEdgePosition = correctedArrowEdgePosition(LabelArrowEdgePosition::Trailing, GridTrackSizingDirection::ForColumns);
    17471828
    17481829            auto expectedLabelSize = expectedSizeForLayoutLabel(text, arrowDirection);
    … …  
    17521833            auto topEdgeInset = pageView->topContentInset(ScrollView::TopContentInsetType::WebCoreOrPlatformContentInset);
    17531834            if (gapLabelLine.start().y() - expectedLabelSize.height() - topEdgeInset + scrollPosition.y() - viewportBounds.y() < 0) {
    1754                 arrowDirection = LabelArrowDirection::Up;
     1835                arrowDirection = correctedArrowDirection(LabelArrowDirection::Up, GridTrackSizingDirection::ForColumns);
    17551836
    17561837                // Special case for the first column to make sure the label will be out of the way of the first row's label.
    … …  
    18121893                    }
    18131894                }
     1895
    18141896                FloatLine trackLeftLine = { rowStartLine.start(), rowEndLine.start() };
    1815                 gridHighlightOverlay.labels.append(buildLabel(trackSizeLabel, trackLeftLine.pointAtRelativeDistance(0.5), translucentLabelBackgroundColor, LabelArrowDirection::Left, LabelArrowEdgePosition::Middle));
     1897                gridHighlightOverlay.labels.append(buildLabel(trackSizeLabel, trackLeftLine.pointAtRelativeDistance(0.5), translucentLabelBackgroundColor, correctedArrowDirection(LabelArrowDirection::Left, GridTrackSizingDirection::ForRows), LabelArrowEdgePosition::Middle));
    18161898            }
    18171899        } else
    … …  
    18311913        if (!lineLabel.isEmpty()) {
    18321914            auto text = lineLabel.toString();
    1833             auto arrowDirection = LabelArrowDirection::Right;
    1834             auto arrowEdgePosition = LabelArrowEdgePosition::Middle;
     1915            auto arrowDirection = correctedArrowDirection(LabelArrowDirection::Right, GridTrackSizingDirection::ForRows);
     1916            auto arrowEdgePosition = correctedArrowEdgePosition(LabelArrowEdgePosition::Middle, GridTrackSizingDirection::ForRows);
    18351917
    18361918            if (!i)
    1837                 arrowEdgePosition = LabelArrowEdgePosition::Leading;
     1919                arrowEdgePosition = correctedArrowEdgePosition(LabelArrowEdgePosition::Leading, GridTrackSizingDirection::ForRows);
    18381920            else if (i == rowPositions.size() - 1)
    1839                 arrowEdgePosition = LabelArrowEdgePosition::Trailing;
     1921                arrowEdgePosition = correctedArrowEdgePosition(LabelArrowEdgePosition::Trailing, GridTrackSizingDirection::ForRows);
    18401922
    18411923            auto expectedLabelSize = expectedSizeForLayoutLabel(text, arrowDirection);
    18421924            if (gapLabelPosition.x() - expectedLabelSize.width() + scrollPosition.x() - viewportBounds.x() < 0)
    1843                 arrowDirection = LabelArrowDirection::Left;
     1925                arrowDirection = correctedArrowDirection(LabelArrowDirection::Left, GridTrackSizingDirection::ForRows);
    18441926
    18451927            gridHighlightOverlay.labels.append(buildLabel(text, gapLabelPosition, Color::white, arrowDirection, arrowEdgePosition));

  • trunk/Source/WebKit/ChangeLog

    r275517 r275519  
     12021-04-06  Patrick Angle  <pangle@apple.com>
     2
     3        Web Inspector: Grid overlay does not honor writing modes and RTL layout direction.
     4        https://bugs.webkit.org/show_bug.cgi?id=224127
     5
     6        Reviewed by BJ Burg.
     7
     8        * UIProcess/Inspector/ios/WKInspectorHighlightView.mm:
     9        (createLayoutLabelLayer):
     10        - WebCore::backgroundPathForLayoutLabel now treats the `None` arrow direction as being centered on the label's
     11        location.
     12
    1132021-04-06  Philippe Normand  <pnormand@igalia.com>
    214

  • trunk/Source/WebKit/UIProcess/Inspector/ios/WKInspectorHighlightView.mm

    r275128 r275519  
    401401        break;
    402402    case WebCore::InspectorOverlay::LabelArrowDirection::None:
    403         textPosition = WebCore::FloatPoint(padding + (textWidth / 2), padding + (textHeight / 2));
     403        // Text position will remain (0, 0).
    404404        break;
    405405    }
Note: See TracChangeset for help on using the changeset viewer.