Changeset 273185 in webkit

Timestamp:

Feb 19, 2021 4:55:59 PM (17 months ago)

Author:

Patrick Angle

Message:

Web Inspector: Implement Grid Overlay "Show line names" drawing
​https://bugs.webkit.org/show_bug.cgi?id=222006
<rdar://problem/74409722>

Reviewed by BJ Burg.

Followup for r273155 related to WTF::String::append performance.

• inspector/InspectorOverlay.cpp:

(WebCore::gridLineNames):

• Use const Vector<String>& to avoid copying the entire Vector and churning the reference count on the strings.

(WebCore::InspectorOverlay::drawGridOverlay):

• Use WTF::StringBuilder instead of WTF::String::append for line label contents.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• inspector/InspectorOverlay.cpp (modified) (3 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-02-19  Patrick Angle  <pangle@apple.com>
+
+        Web Inspector: Implement Grid Overlay "Show line names" drawing
+        https://bugs.webkit.org/show_bug.cgi?id=222006
+        <rdar://problem/74409722>
+
+        Reviewed by BJ Burg.
+
+        Followup for r273155 related to `WTF::String::append` performance.
+
+        * inspector/InspectorOverlay.cpp:
+        (WebCore::gridLineNames):
+        - Use `const Vector<String>&` to avoid copying the entire Vector and churning the reference count on the strings.
+        (WebCore::InspectorOverlay::drawGridOverlay):
+        - Use `WTF::StringBuilder` instead of `WTF::String::append` for line label contents.
+
 2021-02-19  Wenson Hsieh  <wenson_hsieh@apple.com>
 

trunk/Source/WebCore/inspector/InspectorOverlay.cpp

@@ -1339 +1339 @@
     
     OrderedNamedGridLinesMap combinedGridLineNames;
-    auto appendLineNames = [&](unsigned index, Vector<String> newNames) {
+    auto appendLineNames = [&](unsigned index, const Vector<String>& newNames) {
         if (combinedGridLineNames.contains(index)) {
             auto names = combinedGridLineNames.take(index);
@@ -1479 +1479 @@
         context.strokePath(columnPaths);
         
-        Vector<String> lineLabelParts;
-        if (gridOverlay.config.showLineNumbers) {
-            lineLabelParts.append(String::number(i + 1));
-            lineLabelParts.append(String::number(-static_cast<int>(columnPositions.size() - i)));
-        }
-        if (gridOverlay.config.showLineNames && columnLineNames.contains(i))
-            lineLabelParts.appendVector(columnLineNames.get(i));
-        if (lineLabelParts.size()) {
-            auto lineLabel = lineLabelParts[0];
-            for (size_t i = 1; i < lineLabelParts.size(); ++i) {
-                lineLabel.append(thinSpace);
-                lineLabel.append(bullet);
-                lineLabel.append(thinSpace);
-                lineLabel.append(lineLabelParts[i]);
+        StringBuilder lineLabel;
+        if (gridOverlay.config.showLineNumbers)
+            lineLabel.append(i + 1, thinSpace, bullet, thinSpace, -static_cast<int>(columnPositions.size() - i));
+        if (gridOverlay.config.showLineNames && columnLineNames.contains(i)) {
+            for (auto lineName : columnLineNames.get(i)) {
+                if (!lineLabel.isEmpty())
+                    lineLabel.append(thinSpace, bullet, thinSpace);
+                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.
-            drawLayoutLabel(context, lineLabel, FloatPoint(labelX, gridBoundingBox.y()), LabelArrowDirection::Down);
-        }
+        }
+        // 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())
+            drawLayoutLabel(context, lineLabel.toString(), FloatPoint(labelX, gridBoundingBox.y()), LabelArrowDirection::Down);
     }
 
@@ -1544 +1539 @@
         context.strokePath(rowPaths);
         
-        Vector<String> lineLabelParts;
-        if (gridOverlay.config.showLineNumbers) {
-            lineLabelParts.append(String::number(i + 1));
-            lineLabelParts.append(String::number(-static_cast<int>(rowPositions.size() - i)));
-        }
-        if (gridOverlay.config.showLineNames && rowLineNames.contains(i))
-            lineLabelParts.appendVector(rowLineNames.get(i));
-        if (lineLabelParts.size()) {
-            auto lineLabel = lineLabelParts[0];
-            for (size_t i = 1; i < lineLabelParts.size(); ++i) {
-                lineLabel.append(thinSpace);
-                lineLabel.append(bullet);
-                lineLabel.append(thinSpace);
-                lineLabel.append(lineLabelParts[i]);
+        StringBuilder lineLabel;
+        if (gridOverlay.config.showLineNumbers)
+            lineLabel.append(i + 1, thinSpace, bullet, thinSpace, -static_cast<int>(rowPositions.size() - i));
+        if (gridOverlay.config.showLineNames && rowLineNames.contains(i)) {
+            for (auto lineName : rowLineNames.get(i)) {
+                if (!lineLabel.isEmpty())
+                    lineLabel.append(thinSpace, bullet, thinSpace);
+                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.
-            drawLayoutLabel(context, lineLabel, FloatPoint(gridBoundingBox.x(), labelY), LabelArrowDirection::Right);
-        }
+        }
+        // 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())
+            drawLayoutLabel(context, lineLabel.toString(), FloatPoint(gridBoundingBox.x(), labelY), LabelArrowDirection::Right);
     }