Diff
Modified: trunk/Source/WebCore/ChangeLog (275127 => 275128)
--- trunk/Source/WebCore/ChangeLog 2021-03-27 03:09:33 UTC (rev 275127)
+++ trunk/Source/WebCore/ChangeLog 2021-03-27 03:27:27 UTC (rev 275128)
@@ -1,3 +1,34 @@
+2021-03-26 Patrick Angle <[email protected]>
+
+ Web Inspector: Grid layout labels can be drawn outside the viewport
+ https://bugs.webkit.org/show_bug.cgi?id=221972
+
+ Reviewed by BJ Burg.
+
+ Added logic for a best-effort attempt to make sure that layout labels are drawn within the document's bounds, or
+ at least within the grid itself. Labels are measured and adjusted so that if they would be drawn outside the
+ document's bounds, they will be pulled inside the grid. This does not guarantee that all labels will be visible
+ all the time. It is still possible an entire side of a grid will be outside the document's bounds, and this does
+ not attempt to correct for this case, as the desired anchor points for labels will be outside of the visible
+ area of the document.
+
+ * inspector/InspectorOverlay.cpp:
+ (WebCore::InspectorOverlay::fontForLayoutLabel):
+ - Added helper that creates the FontCascade for layout labels, which is done in a few different places.
+ (WebCore::InspectorOverlay::backgroundPathForLayoutLabel):
+ - Added helper to create the Path for the background of layout labels, which is now used in `WebKit::WKInspectorHighlightView`.
+ (WebCore::expectedSizeForLayoutLabel):
+ - Gets the expected size of the label based on the text and arrow direction, and does so without needing to
+ create the entire label background's path.
+ (WebCore::InspectorOverlay::drawLayoutLabel):
+ - Support new `LabelArrowEdgePosition` property
+ (WebCore::InspectorOverlay::drawGridOverlay):
+ (WebCore::buildLabel):
+ (WebCore::InspectorOverlay::buildGridOverlay):
+ * inspector/InspectorOverlay.h:
+ (WebCore::InspectorOverlay::Highlight::GridHighlightOverlay::Label::encode const):
+ (WebCore::InspectorOverlay::Highlight::GridHighlightOverlay::Label::decode):
+
2021-03-26 Zalan Bujtas <[email protected]>
[RenderTreeBuilder] No need to update the counters when the renderer is moved internally
Modified: trunk/Source/WebCore/inspector/InspectorOverlay.cpp (275127 => 275128)
--- trunk/Source/WebCore/inspector/InspectorOverlay.cpp 2021-03-27 03:09:33 UTC (rev 275127)
+++ trunk/Source/WebCore/inspector/InspectorOverlay.cpp 2021-03-27 03:27:27 UTC (rev 275128)
@@ -84,6 +84,9 @@
static constexpr float rulerSubStepIncrement = 5;
static constexpr float rulerSubStepLength = 5;
+static constexpr float layoutLabelPadding = 3;
+static constexpr float layoutLabelArrowSize = 4;
+
static constexpr UChar bullet = 0x2022;
static constexpr UChar ellipsis = 0x2026;
static constexpr UChar multiplicationSign = 0x00D7;
@@ -1199,12 +1202,8 @@
context.strokePath(hatchPath);
}
-void InspectorOverlay::drawLayoutLabel(GraphicsContext& context, String label, FloatPoint point, InspectorOverlay::LabelArrowDirection direction, Color backgroundColor, float maximumWidth)
+FontCascade InspectorOverlay::fontForLayoutLabel()
{
- GraphicsContextStateSaver saver(context);
-
- context.translate(point);
-
FontCascadeDescription fontDescription;
fontDescription.setFamilies({ "system-ui" });
fontDescription.setWeight(FontSelectionValue(500));
@@ -1212,76 +1211,248 @@
FontCascade font(WTFMove(fontDescription), 0, 0);
font.update(nullptr);
+ return font;
+}
- constexpr auto padding = 4;
- constexpr auto arrowSize = 4;
+Path InspectorOverlay::backgroundPathForLayoutLabel(float width, float height, InspectorOverlay::LabelArrowDirection arrowDirection, InspectorOverlay::LabelArrowEdgePosition arrowEdgePosition, float arrowSize)
+{
+ Path path;
+ FloatSize offsetForArrowEdgePosition;
+
+ switch (arrowDirection) {
+ case InspectorOverlay::LabelArrowDirection::Down:
+ path.moveTo({ -(width / 2), -height - arrowSize});
+ path.addLineTo({ -(width / 2), -arrowSize });
+
+ switch (arrowEdgePosition) {
+ case InspectorOverlay::LabelArrowEdgePosition::Leading:
+ path.addLineTo({ -(width / 2), 0 });
+ path.addLineTo({ -(width / 2) + arrowSize, -arrowSize });
+ offsetForArrowEdgePosition = { (width / 2), 0 };
+ break;
+ case InspectorOverlay::LabelArrowEdgePosition::Middle:
+ path.addLineTo({ -arrowSize, -arrowSize });
+ path.addLineTo({ 0, 0 });
+ path.addLineTo({ arrowSize, -arrowSize });
+ break;
+ case InspectorOverlay::LabelArrowEdgePosition::Trailing:
+ path.addLineTo({ (width / 2) - arrowSize, -arrowSize });
+ path.addLineTo({ (width / 2), 0 });
+ offsetForArrowEdgePosition = { -(width / 2), 0 };
+ break;
+ case InspectorOverlay::LabelArrowEdgePosition::None:
+ break;
+ }
+
+ path.addLineTo({ (width / 2), -arrowSize });
+ path.addLineTo({ (width / 2), -height - arrowSize });
+ break;
+ case InspectorOverlay::LabelArrowDirection::Up:
+ path.moveTo({ -(width / 2), height + arrowSize });
+ path.addLineTo({ -(width / 2), arrowSize });
+
+ switch (arrowEdgePosition) {
+ case InspectorOverlay::LabelArrowEdgePosition::Leading:
+ path.addLineTo({ -(width / 2), 0 });
+ path.addLineTo({ -(width / 2) + arrowSize, arrowSize });
+ offsetForArrowEdgePosition = { (width / 2), 0 };
+ break;
+ case InspectorOverlay::LabelArrowEdgePosition::Middle:
+ path.addLineTo({ -arrowSize, arrowSize });
+ path.addLineTo({ 0, 0 });
+ path.addLineTo({ arrowSize, arrowSize });
+ break;
+ case InspectorOverlay::LabelArrowEdgePosition::Trailing:
+ path.addLineTo({ (width / 2) - arrowSize, arrowSize });
+ path.addLineTo({ (width / 2), 0 });
+ offsetForArrowEdgePosition = { -(width / 2), 0 };
+ break;
+ case InspectorOverlay::LabelArrowEdgePosition::None:
+ break;
+ }
+
+ path.addLineTo({ (width / 2), arrowSize });
+ path.addLineTo({ (width / 2), height + arrowSize });
+ break;
+ case InspectorOverlay::LabelArrowDirection::Right:
+ path.moveTo({ -width - arrowSize, (height / 2) });
+ path.addLineTo({ -arrowSize, (height / 2) });
+
+ switch (arrowEdgePosition) {
+ case InspectorOverlay::LabelArrowEdgePosition::Leading:
+ path.addLineTo({ -arrowSize, -(height / 2) + arrowSize });
+ path.addLineTo({ 0, -(height / 2) });
+ offsetForArrowEdgePosition = { 0, (height / 2) };
+ break;
+ case InspectorOverlay::LabelArrowEdgePosition::Middle:
+ path.addLineTo({ -arrowSize, arrowSize });
+ path.addLineTo({ 0, 0 });
+ path.addLineTo({ -arrowSize, -arrowSize });
+ break;
+ case InspectorOverlay::LabelArrowEdgePosition::Trailing:
+ path.addLineTo({ 0, (height / 2) });
+ path.addLineTo({ -arrowSize, (height / 2) - arrowSize });
+ offsetForArrowEdgePosition = { 0, -(height / 2) };
+ break;
+ case InspectorOverlay::LabelArrowEdgePosition::None:
+ break;
+ }
+
+ path.addLineTo({ -arrowSize, -(height / 2) });
+ path.addLineTo({ -width - arrowSize, -(height / 2) });
+ break;
+ case InspectorOverlay::LabelArrowDirection::Left:
+ path.moveTo({ width + arrowSize, (height / 2) });
+ path.addLineTo({ arrowSize, (height / 2) });
+
+ switch (arrowEdgePosition) {
+ case InspectorOverlay::LabelArrowEdgePosition::Leading:
+ path.addLineTo({ arrowSize, -(height / 2) + arrowSize });
+ path.addLineTo({ 0, -(height / 2) });
+ offsetForArrowEdgePosition = { 0, (height / 2) };
+ break;
+ case InspectorOverlay::LabelArrowEdgePosition::Middle:
+ path.addLineTo({ arrowSize, arrowSize });
+ path.addLineTo({ 0, 0 });
+ path.addLineTo({ arrowSize, -arrowSize });
+ break;
+ case InspectorOverlay::LabelArrowEdgePosition::Trailing:
+ path.addLineTo({ 0, (height / 2) });
+ path.addLineTo({ arrowSize, (height / 2) - arrowSize });
+ offsetForArrowEdgePosition = { 0, -(height / 2) };
+ break;
+ case InspectorOverlay::LabelArrowEdgePosition::None:
+ break;
+ }
+
+ path.addLineTo({ arrowSize, -(height / 2) });
+ path.addLineTo({ width + arrowSize, -(height / 2) });
+ break;
+ case InspectorOverlay::LabelArrowDirection::None:
+ path.addLineTo({ 0, height });
+ path.addLineTo({ width, height });
+ path.addLineTo({ width, 0 });
+ break;
+ }
+
+ path.closeSubpath();
+ path.translate(offsetForArrowEdgePosition);
+
+ return path;
+}
+
+static FloatSize expectedSizeForLayoutLabel(String label, InspectorOverlay::LabelArrowDirection direction, float maximumWidth = 0)
+{
+ auto font = InspectorOverlay::fontForLayoutLabel();
+
float textHeight = font.fontMetrics().floatHeight();
+ float textWidth = font.width(TextRun(label));
+ if (maximumWidth && textWidth + (layoutLabelPadding * 2) > maximumWidth)
+ textWidth = maximumWidth;
+
+ switch (direction) {
+ case InspectorOverlay::LabelArrowDirection::Down:
+ case InspectorOverlay::LabelArrowDirection::Up:
+ return { textWidth + (layoutLabelPadding * 2), textHeight + (layoutLabelPadding * 2) + layoutLabelArrowSize };
+ case InspectorOverlay::LabelArrowDirection::Right:
+ case InspectorOverlay::LabelArrowDirection::Left:
+ return { textWidth + (layoutLabelPadding * 2) + layoutLabelArrowSize, textHeight + (layoutLabelPadding * 2) };
+ case InspectorOverlay::LabelArrowDirection::None:
+ return { textWidth + (layoutLabelPadding * 2), textHeight + (layoutLabelPadding * 2) };
+ }
+}
+
+void InspectorOverlay::drawLayoutLabel(GraphicsContext& context, String label, FloatPoint point, InspectorOverlay::LabelArrowDirection arrowDirection, InspectorOverlay::LabelArrowEdgePosition arrowEdgePosition, Color backgroundColor, float maximumWidth)
+{
+ ASSERT(arrowEdgePosition != LabelArrowEdgePosition::None || arrowDirection == LabelArrowDirection::None);
+
+ GraphicsContextStateSaver saver(context);
+
+ context.translate(point);
+
+ auto font = fontForLayoutLabel();
+ float textHeight = font.fontMetrics().floatHeight();
float textDescent = font.fontMetrics().floatDescent();
float textWidth = font.width(TextRun(label));
- if (maximumWidth && textWidth + (padding * 2) > maximumWidth) {
+ if (maximumWidth && textWidth + (layoutLabelPadding * 2) > maximumWidth) {
label.append("..."_s);
- while (textWidth + (padding * 2) > maximumWidth && label.length() >= 4) {
+ while (textWidth + (layoutLabelPadding * 2) > maximumWidth && label.length() >= 4) {
// Remove the fourth from last character (the character before the ellipsis) and remeasure.
label.remove(label.length() - 4);
textWidth = font.width(TextRun(label));
}
}
-
- Path labelPath;
+
FloatPoint textPosition;
-
- switch (direction) {
+ switch (arrowDirection) {
case InspectorOverlay::LabelArrowDirection::Down:
- labelPath.moveTo({ -(textWidth / 2) - padding, -textHeight - (padding * 2) - arrowSize });
- labelPath.addLineTo({ -(textWidth / 2) - padding, -arrowSize });
- labelPath.addLineTo({ -arrowSize, -arrowSize });
- labelPath.addLineTo({ 0, 0 });
- labelPath.addLineTo({ arrowSize, -arrowSize });
- labelPath.addLineTo({ (textWidth / 2) + padding, -arrowSize });
- labelPath.addLineTo({ (textWidth / 2) + padding, -textHeight - (padding * 2) - arrowSize });
- textPosition = FloatPoint(-(textWidth / 2), -textDescent - arrowSize - padding);
+ switch (arrowEdgePosition) {
+ case InspectorOverlay::LabelArrowEdgePosition::Leading:
+ textPosition = FloatPoint(layoutLabelPadding, -textDescent - layoutLabelArrowSize - layoutLabelPadding);
+ break;
+ case InspectorOverlay::LabelArrowEdgePosition::Middle:
+ textPosition = FloatPoint(-(textWidth / 2), -textDescent - layoutLabelArrowSize - layoutLabelPadding);
+ break;
+ case InspectorOverlay::LabelArrowEdgePosition::Trailing:
+ textPosition = FloatPoint(-(textWidth) - layoutLabelPadding, -textDescent - layoutLabelArrowSize - layoutLabelPadding);
+ break;
+ case InspectorOverlay::LabelArrowEdgePosition::None:
+ break;
+ }
break;
case InspectorOverlay::LabelArrowDirection::Up:
- labelPath.moveTo({ -(textWidth / 2) - padding, textHeight + (padding * 2) + arrowSize });
- labelPath.addLineTo({ -(textWidth / 2) - padding, arrowSize });
- labelPath.addLineTo({ -arrowSize, arrowSize });
- labelPath.addLineTo({ 0, 0 });
- labelPath.addLineTo({ arrowSize, arrowSize });
- labelPath.addLineTo({ (textWidth / 2) + padding, arrowSize });
- labelPath.addLineTo({ (textWidth / 2) + padding, textHeight + (padding * 2) + arrowSize });
- textPosition = FloatPoint(-(textWidth / 2), textHeight - textDescent + arrowSize + padding);
+ switch (arrowEdgePosition) {
+ case InspectorOverlay::LabelArrowEdgePosition::Leading:
+ textPosition = FloatPoint(layoutLabelPadding, textHeight - textDescent + layoutLabelArrowSize + layoutLabelPadding);
+ break;
+ case InspectorOverlay::LabelArrowEdgePosition::Middle:
+ textPosition = FloatPoint(-(textWidth / 2), textHeight - textDescent + layoutLabelArrowSize + layoutLabelPadding);
+ break;
+ case InspectorOverlay::LabelArrowEdgePosition::Trailing:
+ textPosition = FloatPoint(-(textWidth) - layoutLabelPadding, textHeight - textDescent + layoutLabelArrowSize + layoutLabelPadding);
+ break;
+ case InspectorOverlay::LabelArrowEdgePosition::None:
+ break;
+ }
break;
case InspectorOverlay::LabelArrowDirection::Right:
- labelPath.moveTo({ -textWidth - (padding * 2) - arrowSize, (textHeight / 2) + padding });
- labelPath.addLineTo({ -arrowSize, (textHeight / 2) + padding });
- labelPath.addLineTo({ -arrowSize, arrowSize });
- labelPath.addLineTo({ 0, 0 });
- labelPath.addLineTo({ -arrowSize, -arrowSize });
- labelPath.addLineTo({ -arrowSize, -(textHeight / 2) - padding });
- labelPath.addLineTo({ -textWidth - (padding * 2) - arrowSize, -(textHeight / 2) - padding });
- textPosition = FloatPoint(-textWidth - arrowSize - padding, (textHeight / 2) - textDescent);
+ switch (arrowEdgePosition) {
+ case InspectorOverlay::LabelArrowEdgePosition::Leading:
+ textPosition = FloatPoint(-textWidth - layoutLabelArrowSize - layoutLabelPadding, layoutLabelPadding + textHeight - textDescent);
+ break;
+ case InspectorOverlay::LabelArrowEdgePosition::Middle:
+ textPosition = FloatPoint(-textWidth - layoutLabelArrowSize - layoutLabelPadding, (textHeight / 2) - textDescent);
+ break;
+ case InspectorOverlay::LabelArrowEdgePosition::Trailing:
+ textPosition = FloatPoint(-textWidth - layoutLabelArrowSize - layoutLabelPadding, -layoutLabelPadding - textDescent);
+ break;
+ case InspectorOverlay::LabelArrowEdgePosition::None:
+ break;
+ }
break;
case InspectorOverlay::LabelArrowDirection::Left:
- labelPath.moveTo({ textWidth + (padding * 2) + arrowSize, (textHeight / 2) + padding });
- labelPath.addLineTo({ arrowSize, (textHeight / 2) + padding });
- labelPath.addLineTo({ arrowSize, arrowSize });
- labelPath.addLineTo({ 0, 0 });
- labelPath.addLineTo({ arrowSize, -arrowSize });
- labelPath.addLineTo({ arrowSize, -(textHeight / 2) - padding });
- labelPath.addLineTo({ textWidth + (padding * 2) + arrowSize, -(textHeight / 2) - padding });
- textPosition = FloatPoint(arrowSize + padding, (textHeight / 2) - textDescent);
+ switch (arrowEdgePosition) {
+ case InspectorOverlay::LabelArrowEdgePosition::Leading:
+ textPosition = FloatPoint(layoutLabelArrowSize + layoutLabelPadding, layoutLabelPadding + textHeight - textDescent);
+ break;
+ case InspectorOverlay::LabelArrowEdgePosition::Middle:
+ textPosition = FloatPoint(layoutLabelArrowSize + layoutLabelPadding, (textHeight / 2) - textDescent);
+ break;
+ case InspectorOverlay::LabelArrowEdgePosition::Trailing:
+ textPosition = FloatPoint(layoutLabelArrowSize + layoutLabelPadding, -layoutLabelPadding - textDescent);
+ break;
+ case InspectorOverlay::LabelArrowEdgePosition::None:
+ break;
+ }
break;
case InspectorOverlay::LabelArrowDirection::None:
- labelPath.addLineTo({ 0, textHeight + (padding * 2) });
- labelPath.addLineTo({ textWidth + (padding * 2), textHeight + (padding * 2) });
- labelPath.addLineTo({ textWidth + (padding * 2), 0 });
- textPosition = FloatPoint(padding, padding + textHeight - textDescent);
+ textPosition = FloatPoint(layoutLabelPadding, layoutLabelPadding + textHeight - textDescent);
break;
}
-
- labelPath.closeSubpath();
-
+
+ Path labelPath = backgroundPathForLayoutLabel(textWidth + (layoutLabelPadding * 2), textHeight + (layoutLabelPadding * 2), arrowDirection, arrowEdgePosition, layoutLabelArrowSize);
+
context.setFillColor(backgroundColor);
context.fillPath(labelPath);
context.strokePath(labelPath);
@@ -1315,10 +1486,10 @@
// Draw labels on top of all other lines.
context.setStrokeThickness(1);
for (auto area : gridOverlay.areas)
- drawLayoutLabel(context, area.name, area.quad.p1(), LabelArrowDirection::None, translucentLabelBackgroundColor, area.quad.boundingBox().width());
+ drawLayoutLabel(context, area.name, area.quad.p1(), LabelArrowDirection::None, LabelArrowEdgePosition::None, translucentLabelBackgroundColor, area.quad.boundingBox().width());
for (auto label : gridOverlay.labels)
- drawLayoutLabel(context, label.text, label.location, label.arrowDirection, label.backgroundColor);
+ drawLayoutLabel(context, label.text, label.location, label.arrowDirection, label.arrowEdgePosition, label.backgroundColor);
}
static Vector<String> authoredGridTrackSizes(Node* node, GridTrackSizingDirection direction, unsigned expectedTrackCount)
@@ -1419,7 +1590,7 @@
return combinedGridLineNames;
}
-static InspectorOverlay::Highlight::GridHighlightOverlay::Label buildLabel(String text, FloatPoint location, Color backgroundColor, InspectorOverlay::LabelArrowDirection arrowDirection)
+static InspectorOverlay::Highlight::GridHighlightOverlay::Label buildLabel(String text, FloatPoint location, Color backgroundColor, InspectorOverlay::LabelArrowDirection arrowDirection, InspectorOverlay::LabelArrowEdgePosition arrowEdgePosition)
{
InspectorOverlay::Highlight::GridHighlightOverlay::Label label;
label.text = text;
@@ -1426,6 +1597,7 @@
label.location = location;
label.backgroundColor = backgroundColor;
label.arrowDirection = arrowDirection;
+ label.arrowEdgePosition = arrowEdgePosition;
return label;
}
@@ -1454,8 +1626,10 @@
if (!pageView)
return { };
FloatRect viewportBounds = { { 0, 0 }, pageView->sizeForVisibleContent() };
+
+ auto scrollPosition = pageView->scrollPosition();
if (offsetBoundsByScroll)
- viewportBounds.setLocation(pageView->scrollPosition());
+ viewportBounds.setLocation(scrollPosition);
auto& renderGrid = *downcast<RenderGrid>(renderer);
auto columnPositions = renderGrid.columnPositions();
@@ -1504,11 +1678,12 @@
gridHighlightOverlay.gridLines.append(columnStartLine);
}
- FloatPoint gapLabelPosition = columnStartLine.start();
+ FloatLine gapLabelLine = columnStartLine;
if (i) {
gridHighlightOverlay.gaps.append({ previousColumnEndLine.start(), columnStartLine.start(), columnStartLine.end(), previousColumnEndLine.end() });
FloatLine lineBetweenColumnTops = { columnStartLine.start(), previousColumnEndLine.start() };
- gapLabelPosition = lineBetweenColumnTops.pointAtRelativeDistance(0.5);
+ FloatLine lineBetweenColumnBottoms = { columnStartLine.end(), previousColumnEndLine.end() };
+ gapLabelLine = { lineBetweenColumnTops.pointAtRelativeDistance(0.5), lineBetweenColumnTops.pointAtRelativeDistance(0.5) };
}
if (i < columnWidths.size() && i < columnPositions.size()) {
@@ -1539,7 +1714,7 @@
}
FloatLine trackTopLine = { columnStartLine.start(), columnEndLine.start() };
- gridHighlightOverlay.labels.append(buildLabel(trackSizeLabel, trackTopLine.pointAtRelativeDistance(0.5), translucentLabelBackgroundColor, LabelArrowDirection::Up));
+ gridHighlightOverlay.labels.append(buildLabel(trackSizeLabel, trackTopLine.pointAtRelativeDistance(0.5), translucentLabelBackgroundColor, LabelArrowDirection::Up, LabelArrowEdgePosition::Middle));
}
} else
previousColumnEndLine = columnStartLine;
@@ -1554,9 +1729,34 @@
lineLabel.append(lineName);
}
}
- // FIXME: <webkit.org/b/221972> Layout labels can be drawn outside the viewport, and a best effort should be made to keep them in the viewport while the grid is in the viewport.
- if (!lineLabel.isEmpty())
- gridHighlightOverlay.labels.append(buildLabel(lineLabel.toString(), gapLabelPosition, Color::white, LabelArrowDirection::Down));
+
+ if (!lineLabel.isEmpty()) {
+ auto text = lineLabel.toString();
+ auto arrowDirection = LabelArrowDirection::Down;
+ auto arrowEdgePosition = LabelArrowEdgePosition::Middle;
+
+ if (!i)
+ arrowEdgePosition = LabelArrowEdgePosition::Leading;
+ else if (i == columnPositions.size() - 1)
+ arrowEdgePosition = LabelArrowEdgePosition::Trailing;
+
+ auto expectedLabelSize = expectedSizeForLayoutLabel(text, arrowDirection);
+ auto gapLabelPosition = gapLabelLine.start();
+
+ // The area under the window's toolbar is drawable, but not meaningfully visible, so we must account for that space.
+ auto topEdgeInset = pageView->topContentInset(ScrollView::TopContentInsetType::WebCoreOrPlatformContentInset);
+ if (gapLabelLine.start().y() - expectedLabelSize.height() - topEdgeInset + scrollPosition.y() - viewportBounds.y() < 0) {
+ arrowDirection = LabelArrowDirection::Up;
+
+ // Special case for the first column to make sure the label will be out of the way of the first row's label.
+ // The label heights will be the same, as they use the same font, so moving down by this label's size will
+ // create enough space for this special circumstance.
+ if (!i)
+ gapLabelPosition = gapLabelLine.pointAtAbsoluteDistance(expectedLabelSize.height());
+ }
+
+ gridHighlightOverlay.labels.append(buildLabel(text, gapLabelPosition, Color::white, arrowDirection, arrowEdgePosition));
+ }
}
auto rowHeights = renderGrid.trackSizesForComputedStyle(GridTrackSizingDirection::ForRows);
@@ -1607,7 +1807,7 @@
}
}
FloatLine trackLeftLine = { rowStartLine.start(), rowEndLine.start() };
- gridHighlightOverlay.labels.append(buildLabel(trackSizeLabel, trackLeftLine.pointAtRelativeDistance(0.5), translucentLabelBackgroundColor, LabelArrowDirection::Left));
+ gridHighlightOverlay.labels.append(buildLabel(trackSizeLabel, trackLeftLine.pointAtRelativeDistance(0.5), translucentLabelBackgroundColor, LabelArrowDirection::Left, LabelArrowEdgePosition::Middle));
}
} else
previousRowEndLine = rowStartLine;
@@ -1622,9 +1822,23 @@
lineLabel.append(lineName);
}
}
- // FIXME: <webkit.org/b/221972> Layout labels can be drawn outside the viewport, and a best effort should be made to keep them in the viewport while the grid is in the viewport.
- if (!lineLabel.isEmpty())
- gridHighlightOverlay.labels.append(buildLabel(lineLabel.toString(), gapLabelPosition, Color::white, LabelArrowDirection::Right));
+
+ if (!lineLabel.isEmpty()) {
+ auto text = lineLabel.toString();
+ auto arrowDirection = LabelArrowDirection::Right;
+ auto arrowEdgePosition = LabelArrowEdgePosition::Middle;
+
+ if (!i)
+ arrowEdgePosition = LabelArrowEdgePosition::Leading;
+ else if (i == rowPositions.size() - 1)
+ arrowEdgePosition = LabelArrowEdgePosition::Trailing;
+
+ auto expectedLabelSize = expectedSizeForLayoutLabel(text, arrowDirection);
+ if (gapLabelPosition.x() - expectedLabelSize.width() + scrollPosition.x() - viewportBounds.x() < 0)
+ arrowDirection = LabelArrowDirection::Left;
+
+ gridHighlightOverlay.labels.append(buildLabel(text, gapLabelPosition, Color::white, arrowDirection, arrowEdgePosition));
+ }
}
if (gridOverlay.config.showAreaNames) {
Modified: trunk/Source/WebCore/inspector/InspectorOverlay.h (275127 => 275128)
--- trunk/Source/WebCore/inspector/InspectorOverlay.h 2021-03-27 03:09:33 UTC (rev 275127)
+++ trunk/Source/WebCore/inspector/InspectorOverlay.h 2021-03-27 03:27:27 UTC (rev 275128)
@@ -52,6 +52,7 @@
namespace WebCore {
+class FontCascade;
class FloatPoint;
class GraphicsContext;
class InspectorClient;
@@ -73,6 +74,13 @@
Right,
};
+ enum class LabelArrowEdgePosition {
+ None,
+ Leading, // Positioned at the left/top side of edge.
+ Middle, // Positioned at the center on the edge.
+ Trailing, // Positioned at the right/bottom side of the edge.
+ };
+
struct Highlight {
WTF_MAKE_STRUCT_FAST_ALLOCATED;
@@ -103,6 +111,7 @@
FloatPoint location;
Color backgroundColor;
LabelArrowDirection arrowDirection;
+ LabelArrowEdgePosition arrowEdgePosition;
#if PLATFORM(IOS_FAMILY)
template<class Encoder> void encode(Encoder&) const;
@@ -209,6 +218,8 @@
Inspector::ErrorStringOr<void> clearGridOverlayForNode(Node&);
void clearAllGridOverlays();
+ WEBCORE_EXPORT static FontCascade fontForLayoutLabel();
+ WEBCORE_EXPORT static Path backgroundPathForLayoutLabel(float, float, InspectorOverlay::LabelArrowDirection, InspectorOverlay::LabelArrowEdgePosition, float arrowSize);
private:
using TimeRectPair = std::pair<MonotonicTime, FloatRect>;
@@ -226,7 +237,7 @@
Path drawElementTitle(GraphicsContext&, Node&, const Highlight::Bounds&);
void drawLayoutHatching(GraphicsContext&, FloatQuad);
- void drawLayoutLabel(GraphicsContext&, String, FloatPoint, LabelArrowDirection, Color backgroundColor = Color::white, float maximumWidth = 0);
+ void drawLayoutLabel(GraphicsContext&, String, FloatPoint, LabelArrowDirection, InspectorOverlay::LabelArrowEdgePosition, Color backgroundColor = Color::white, float maximumWidth = 0);
void drawGridOverlay(GraphicsContext&, const InspectorOverlay::Highlight::GridHighlightOverlay&);
Optional<InspectorOverlay::Highlight::GridHighlightOverlay> buildGridOverlay(const InspectorOverlay::Grid&, bool offsetBoundsByScroll = false);
@@ -289,6 +300,7 @@
encoder << location;
encoder << backgroundColor;
encoder << static_cast<uint32_t>(arrowDirection);
+ encoder << static_cast<uint32_t>(arrowEdgePosition);
}
template<class Decoder> Optional<InspectorOverlay::Highlight::GridHighlightOverlay::Label> InspectorOverlay::Highlight::GridHighlightOverlay::Label::decode(Decoder& decoder)
@@ -306,6 +318,11 @@
return { };
label.arrowDirection = (InspectorOverlay::LabelArrowDirection)arrowDirection;
+ uint32_t arrowEdgePosition;
+ if (!decoder.decode(arrowEdgePosition))
+ return { };
+ label.arrowEdgePosition = (InspectorOverlay::LabelArrowEdgePosition)arrowEdgePosition;
+
return { label };
}
Modified: trunk/Source/WebKit/ChangeLog (275127 => 275128)
--- trunk/Source/WebKit/ChangeLog 2021-03-27 03:09:33 UTC (rev 275127)
+++ trunk/Source/WebKit/ChangeLog 2021-03-27 03:27:27 UTC (rev 275128)
@@ -1,3 +1,18 @@
+2021-03-26 Patrick Angle <[email protected]>
+
+ Web Inspector: Grid layout labels can be drawn outside the viewport
+ https://bugs.webkit.org/show_bug.cgi?id=221972
+
+ Reviewed by BJ Burg.
+
+ Add support for the new `WebCore::InspectorOverlay::LabelArrowEdgePosition` property to grid overlays on iOS.
+
+ * UIProcess/Inspector/ios/WKInspectorHighlightView.mm:
+ (createLayoutLabelLayer):
+ - Support the new `WebCore::InspectorOverlay::LabelArrowEdgePosition`.
+ - Use new helpers in WebCore::InspectorOverlay to reduce code duplication.
+ (-[WKInspectorHighlightView _createGridOverlayLayer:scale:]):
+
2021-03-26 Wenson Hsieh <[email protected]>
Use PUICQuickboardController for text input when HAVE(QUICKBOARD_CONTROLLER) is defined
Modified: trunk/Source/WebKit/UIProcess/Inspector/ios/WKInspectorHighlightView.mm (275127 => 275128)
--- trunk/Source/WebKit/UIProcess/Inspector/ios/WKInspectorHighlightView.mm 2021-03-27 03:09:33 UTC (rev 275127)
+++ trunk/Source/WebKit/UIProcess/Inspector/ios/WKInspectorHighlightView.mm 2021-03-27 03:27:27 UTC (rev 275128)
@@ -318,16 +318,10 @@
return layer;
}
-static CALayer * createLayoutLabelLayer(String label, WebCore::FloatPoint point, WebCore::InspectorOverlay::LabelArrowDirection direction, WebCore::Color backgroundColor, WebCore::Color strokeColor, double scale, float maximumWidth = 0)
+static CALayer * createLayoutLabelLayer(String label, WebCore::FloatPoint point, WebCore::InspectorOverlay::LabelArrowDirection arrowDirection, WebCore::InspectorOverlay::LabelArrowEdgePosition arrowEdgePosition, WebCore::Color backgroundColor, WebCore::Color strokeColor, double scale, float maximumWidth = 0)
{
- WebCore::FontCascadeDescription fontDescription;
- fontDescription.setFamilies({ "system-ui" });
- fontDescription.setWeight(WebCore::FontSelectionValue(500));
- fontDescription.setComputedSize(12);
+ auto font = WebCore::InspectorOverlay::fontForLayoutLabel();
- WebCore::FontCascade font(WTFMove(fontDescription), 0, 0);
- font.update(nullptr);
-
constexpr auto padding = 4;
constexpr auto arrowSize = 4;
float textHeight = font.fontMetrics().floatHeight();
@@ -342,71 +336,88 @@
}
}
- auto labelPath = adoptCF(CGPathCreateMutable());
-
// Note: Implementation Difference - The textPosition is the center of text, unlike WebCore::InspectorOverlay, where the textPosition is leftmost point on the baseline of the text.
WebCore::FloatPoint textPosition;
-
- switch (direction) {
+ switch (arrowDirection) {
case WebCore::InspectorOverlay::LabelArrowDirection::Down:
- CGPathMoveToPoint(labelPath.get(), 0, -(textWidth / 2) - padding, -textHeight - (padding * 2) - arrowSize);
- CGPathAddLineToPoint(labelPath.get(), 0, -(textWidth / 2) - padding, -arrowSize);
- CGPathAddLineToPoint(labelPath.get(), 0, -arrowSize, -arrowSize);
- CGPathAddLineToPoint(labelPath.get(), 0, 0, 0);
- CGPathAddLineToPoint(labelPath.get(), 0, arrowSize, -arrowSize);
- CGPathAddLineToPoint(labelPath.get(), 0, (textWidth / 2) + padding, -arrowSize);
- CGPathAddLineToPoint(labelPath.get(), 0, (textWidth / 2) + padding, -textHeight - (padding * 2) - arrowSize);
- textPosition = WebCore::FloatPoint(0, -(textHeight / 2) - arrowSize - padding);
+ switch (arrowEdgePosition) {
+ case WebCore::InspectorOverlay::LabelArrowEdgePosition::Leading:
+ textPosition = WebCore::FloatPoint((textWidth / 2) + padding, -(textHeight / 2) - arrowSize - padding);
+ break;
+ case WebCore::InspectorOverlay::LabelArrowEdgePosition::Middle:
+ textPosition = WebCore::FloatPoint(0, -(textHeight / 2) - arrowSize - padding);
+ break;
+ case WebCore::InspectorOverlay::LabelArrowEdgePosition::Trailing:
+ textPosition = WebCore::FloatPoint(-(textWidth / 2) - padding, -(textHeight / 2) - arrowSize - padding);
+ break;
+ case WebCore::InspectorOverlay::LabelArrowEdgePosition::None:
+ break;
+ }
break;
case WebCore::InspectorOverlay::LabelArrowDirection::Up:
- CGPathMoveToPoint(labelPath.get(), 0, -(textWidth / 2) - padding, textHeight + (padding * 2) + arrowSize);
- CGPathAddLineToPoint(labelPath.get(), 0, -(textWidth / 2) - padding, arrowSize);
- CGPathAddLineToPoint(labelPath.get(), 0, -arrowSize, arrowSize);
- CGPathAddLineToPoint(labelPath.get(), 0, 0, 0);
- CGPathAddLineToPoint(labelPath.get(), 0, arrowSize, arrowSize);
- CGPathAddLineToPoint(labelPath.get(), 0, (textWidth / 2) + padding, arrowSize);
- CGPathAddLineToPoint(labelPath.get(), 0, (textWidth / 2) + padding, textHeight + (padding * 2) + arrowSize);
- textPosition = WebCore::FloatPoint(0, (textHeight / 2) + arrowSize + padding);
+ switch (arrowEdgePosition) {
+ case WebCore::InspectorOverlay::LabelArrowEdgePosition::Leading:
+ textPosition = WebCore::FloatPoint((textWidth / 2) + padding, (textHeight / 2) + arrowSize + padding);
+ break;
+ case WebCore::InspectorOverlay::LabelArrowEdgePosition::Middle:
+ textPosition = WebCore::FloatPoint(0, (textHeight / 2) + arrowSize + padding);
+ break;
+ case WebCore::InspectorOverlay::LabelArrowEdgePosition::Trailing:
+ textPosition = WebCore::FloatPoint(-(textWidth / 2) - padding, (textHeight / 2) + arrowSize + padding);
+ break;
+ case WebCore::InspectorOverlay::LabelArrowEdgePosition::None:
+ break;
+ }
break;
case WebCore::InspectorOverlay::LabelArrowDirection::Right:
- CGPathMoveToPoint(labelPath.get(), 0, -textWidth - (padding * 2) - arrowSize, (textHeight / 2) + padding);
- CGPathAddLineToPoint(labelPath.get(), 0, -arrowSize, (textHeight / 2) + padding);
- CGPathAddLineToPoint(labelPath.get(), 0, -arrowSize, arrowSize);
- CGPathAddLineToPoint(labelPath.get(), 0, 0, 0);
- CGPathAddLineToPoint(labelPath.get(), 0, -arrowSize, -arrowSize);
- CGPathAddLineToPoint(labelPath.get(), 0, -arrowSize, -(textHeight / 2) - padding);
- CGPathAddLineToPoint(labelPath.get(), 0, -textWidth - (padding * 2) - arrowSize, -(textHeight / 2) - padding);
- textPosition = WebCore::FloatPoint(-(textWidth / 2) - arrowSize - padding, 0);
+ switch (arrowEdgePosition) {
+ case WebCore::InspectorOverlay::LabelArrowEdgePosition::Leading:
+ textPosition = WebCore::FloatPoint(-(textWidth / 2) - arrowSize - padding, (textHeight / 2) + padding);
+ break;
+ case WebCore::InspectorOverlay::LabelArrowEdgePosition::Middle:
+ textPosition = WebCore::FloatPoint(-(textWidth / 2) - arrowSize - padding, 0);
+ break;
+ case WebCore::InspectorOverlay::LabelArrowEdgePosition::Trailing:
+ textPosition = WebCore::FloatPoint(-(textWidth / 2) - arrowSize - padding, -(textHeight / 2) - padding);
+ break;
+ case WebCore::InspectorOverlay::LabelArrowEdgePosition::None:
+ break;
+ }
break;
case WebCore::InspectorOverlay::LabelArrowDirection::Left:
- CGPathMoveToPoint(labelPath.get(), 0, textWidth + (padding * 2) + arrowSize, (textHeight / 2) + padding);
- CGPathAddLineToPoint(labelPath.get(), 0, arrowSize, (textHeight / 2) + padding);
- CGPathAddLineToPoint(labelPath.get(), 0, arrowSize, arrowSize);
- CGPathAddLineToPoint(labelPath.get(), 0, 0, 0);
- CGPathAddLineToPoint(labelPath.get(), 0, arrowSize, -arrowSize);
- CGPathAddLineToPoint(labelPath.get(), 0, arrowSize, -(textHeight / 2) - padding);
- CGPathAddLineToPoint(labelPath.get(), 0, textWidth + (padding * 2) + arrowSize, -(textHeight / 2) - padding);
- textPosition = WebCore::FloatPoint((textWidth / 2) + arrowSize + padding, 0);
+ switch (arrowEdgePosition) {
+ case WebCore::InspectorOverlay::LabelArrowEdgePosition::Leading:
+ textPosition = WebCore::FloatPoint((textWidth / 2) + arrowSize + padding, (textHeight / 2) + padding);
+ break;
+ case WebCore::InspectorOverlay::LabelArrowEdgePosition::Middle:
+ textPosition = WebCore::FloatPoint((textWidth / 2) + arrowSize + padding, 0);
+ break;
+ case WebCore::InspectorOverlay::LabelArrowEdgePosition::Trailing:
+ textPosition = WebCore::FloatPoint((textWidth / 2) + arrowSize + padding, -(textHeight / 2) - padding);
+ break;
+ case WebCore::InspectorOverlay::LabelArrowEdgePosition::None:
+ break;
+ }
break;
case WebCore::InspectorOverlay::LabelArrowDirection::None:
- CGPathMoveToPoint(labelPath.get(), 0, 0, 0);
- CGPathAddLineToPoint(labelPath.get(), 0, 0, textHeight + (padding * 2));
- CGPathAddLineToPoint(labelPath.get(), 0, textWidth + (padding * 2), textHeight + (padding * 2));
- CGPathAddLineToPoint(labelPath.get(), 0, textWidth + (padding * 2), 0);
textPosition = WebCore::FloatPoint(padding + (textWidth / 2), padding + (textHeight / 2));
break;
}
- CGPathCloseSubpath(labelPath.get());
-
CALayer *layer = [CALayer layer];
+#if USE(CG)
+ // WebCore::Path::PlatformPathPtr is only a CGPath* when `USE(CG)` is true.
+ auto labelPath = WebCore::InspectorOverlay::backgroundPathForLayoutLabel(textWidth + (padding * 2), textHeight + (padding * 2), arrowDirection, arrowEdgePosition, arrowSize);
+ CGPath* platformLabelPath = labelPath.ensurePlatformPath();
+
CAShapeLayer *labelPathLayer = [CAShapeLayer layer];
- labelPathLayer.path = labelPath.get();
+ labelPathLayer.path = platformLabelPath;
labelPathLayer.fillColor = cachedCGColor(backgroundColor);
labelPathLayer.strokeColor = cachedCGColor(strokeColor);
labelPathLayer.position = CGPointMake(point.x(), point.y());
[layer addSublayer:labelPathLayer];
+#endif
CATextLayer *textLayer = [CATextLayer layer];
textLayer.frame = CGRectMake(0, 0, textWidth, textHeight);
@@ -453,10 +464,10 @@
constexpr auto translucentLabelBackgroundColor = WebCore::Color::white.colorWithAlphaByte(153);
for (auto area : overlay.areas)
- [layer addSublayer:createLayoutLabelLayer(area.name, area.quad.p1(), WebCore::InspectorOverlay::LabelArrowDirection::None, translucentLabelBackgroundColor, overlay.color, scale, area.quad.boundingBox().width())];
+ [layer addSublayer:createLayoutLabelLayer(area.name, area.quad.p1(), WebCore::InspectorOverlay::LabelArrowDirection::None, WebCore::InspectorOverlay::LabelArrowEdgePosition::None, translucentLabelBackgroundColor, overlay.color, scale, area.quad.boundingBox().width())];
for (auto label : overlay.labels)
- [layer addSublayer:createLayoutLabelLayer(label.text, label.location, label.arrowDirection, label.backgroundColor, overlay.color, scale)];
+ [layer addSublayer:createLayoutLabelLayer(label.text, label.location, label.arrowDirection, label.arrowEdgePosition, label.backgroundColor, overlay.color, scale)];
return layer;
}
