Bug 222565 - Web Inspector: Grid overlay does not adjust for element transforms (rotation, scale, etc.)
Summary: Web Inspector: Grid overlay does not adjust for element transforms (rotation,...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Patrick Angle
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-03-01 11:05 PST by Patrick Angle
Modified: 2021-03-08 12:55 PST (History)
7 users (show)

See Also:


Attachments
Screenshot of Issue (1.77 MB, image/png)
2021-03-01 11:05 PST, Patrick Angle
no flags Details
Patch v1.0 (25.11 KB, patch)
2021-03-03 15:04 PST, Patrick Angle
no flags Details | Formatted Diff | Diff
Screenshot of Patch v1.0 (1.24 MB, image/png)
2021-03-03 15:05 PST, Patrick Angle
no flags Details
Screenshot of Patch v1.0 (1.45 MB, image/png)
2021-03-03 15:12 PST, Patrick Angle
no flags Details
Patch v1.1 - Cleanup (28.31 KB, patch)
2021-03-04 19:19 PST, Patrick Angle
no flags Details | Formatted Diff | Diff
Screenshot of Patch v1.1 (1.46 MB, image/png)
2021-03-04 19:20 PST, Patrick Angle
no flags Details
Patch v2.0 - Use renderGrid to transform points (25.70 KB, patch)
2021-03-05 00:58 PST, Patrick Angle
no flags Details | Formatted Diff | Diff
Screenshot of Patch v2.0 (Scale, Translate, & Rotate) (1.20 MB, image/png)
2021-03-05 00:58 PST, Patrick Angle
no flags Details
Screenshot of Patch v2.0 (Perspective) (1.31 MB, image/png)
2021-03-05 00:59 PST, Patrick Angle
no flags Details
Screenshot of Patch v2.0 (Perspective /w Area Names) (1.24 MB, image/png)
2021-03-05 00:59 PST, Patrick Angle
no flags Details
Patch v2.1 - Review notes, make FloatLine standalone class (34.40 KB, patch)
2021-03-08 10:50 PST, Patrick Angle
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Patrick Angle 2021-03-01 11:05:12 PST
Created attachment 421846 [details]
Screenshot of Issue

Description:
For elements with a transform (or whose parent is transformed), the overlay to not appropriately transformed.

Steps to Reproduce:
1. Go to labs.jensimmons.com
2. Inspect the page and turn on the grid overlay for `div.header-wrapper`
3. Observe the grid being drawn in the wrong place.

Expected Result:
Grid would be drawn where layout actually occurred, including rotation, scale, etc.
Comment 1 Radar WebKit Bug Importer 2021-03-01 11:05:33 PST
<rdar://problem/74879289>
Comment 2 Patrick Angle 2021-03-03 15:04:32 PST
Created attachment 422153 [details]
Patch v1.0
Comment 3 Patrick Angle 2021-03-03 15:05:00 PST
Created attachment 422154 [details]
Screenshot of Patch v1.0
Comment 4 Patrick Angle 2021-03-03 15:12:51 PST
Created attachment 422155 [details]
Screenshot of Patch v1.0
Comment 5 Patrick Angle 2021-03-04 19:19:54 PST
Created attachment 422322 [details]
Patch v1.1 - Cleanup
Comment 6 Patrick Angle 2021-03-04 19:20:19 PST
Created attachment 422323 [details]
Screenshot of Patch v1.1
Comment 7 Patrick Angle 2021-03-04 19:23:04 PST Comment hidden (obsolete)
Comment 8 Patrick Angle 2021-03-05 00:58:03 PST
Created attachment 422342 [details]
Patch v2.0 - Use renderGrid to transform points
Comment 9 Patrick Angle 2021-03-05 00:58:59 PST
Created attachment 422344 [details]
Screenshot of Patch v2.0 (Scale, Translate, & Rotate)
Comment 10 Patrick Angle 2021-03-05 00:59:15 PST
Created attachment 422345 [details]
Screenshot of Patch v2.0 (Perspective)
Comment 11 Patrick Angle 2021-03-05 00:59:38 PST
Created attachment 422346 [details]
Screenshot of Patch v2.0 (Perspective /w Area Names)
Comment 12 BJ Burg 2021-03-05 10:46:34 PST
Comment on attachment 422342 [details]
Patch v2.0 - Use renderGrid to transform points

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

r=me

Please test this with a grid inside an iframe. File a followup or new patch revision if localToContainer is not enough-it may require localToRoot.

> Source/WebCore/inspector/InspectorOverlay.cpp:1183
> +        for (float y = -topSide.length(); y < leftSide.length(); y += hatchSpacing) {

This is way easier to grok than the scrollOffset version.

> Source/WebCore/inspector/InspectorOverlay.cpp:1456
> +            auto extendedLine = columnStartLine.extendedToBounds(viewportBounds);

Very nice.

> Source/WebCore/inspector/InspectorOverlay.cpp:1614
> +            // If any two lines are coincident with each other, they will not have an intersection, which can occur with extreme `transform: perspective(...)` values.

Wow!
Comment 13 Devin Rousso 2021-03-05 15:34:17 PST
Comment on attachment 422342 [details]
Patch v2.0 - Use renderGrid to transform points

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

r=me as well, nice work!

> Source/WebCore/inspector/InspectorOverlay.cpp:1415
> +    auto* renderGrid = downcast<RenderGrid>(renderer);

NIT: I'd `*downcast<RenderGrid>(renderer)` so you don't have to keep dereferencing

> Source/WebCore/inspector/InspectorOverlay.cpp:1547
> +        if (i < rowHeights.size() && i + 1 != rowPositions.size()) {

NIT: I usually prefer using `i < rowPositions.size()` just in case `i` somehow gets beyond `rowPositions.size()`

> Source/WebCore/inspector/InspectorOverlay.h:170
> +    class FloatLine {

IMO maybe this is worth moving out of inspector code?  I feel like it could be useful elsewhere :)

> Source/WebCore/inspector/InspectorOverlay.h:173
> +        FloatLine(FloatPoint start, FloatPoint end)

`const FloatPoint&`

> Source/WebCore/inspector/InspectorOverlay.h:181
> +        const FloatPoint start() const { return m_start; }
> +        const FloatPoint end() const { return m_end; }

`const FloatPoint&`

> Source/WebCore/inspector/InspectorOverlay.h:201
> +        const FloatLine extendedToBounds(const FloatRect bounds) const

`const FloatRect&`

> Source/WebCore/inspector/InspectorOverlay.h:227
> +            return makeOptional(FloatPoint(

NIT: Would `{{ ... }}` instead of `makeOptional(FloatPoint(...))` here?
Comment 14 Patrick Angle 2021-03-08 10:50:54 PST
Created attachment 422584 [details]
Patch v2.1 - Review notes, make FloatLine standalone class
Comment 15 EWS 2021-03-08 12:55:50 PST
Committed r274096: <https://commits.webkit.org/r274096>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 422584 [details].