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.
<rdar://problem/74879289>
Created attachment 422153 [details] Patch v1.0
Created attachment 422154 [details] Screenshot of Patch v1.0
Created attachment 422155 [details] Screenshot of Patch v1.0
Created attachment 422322 [details] Patch v1.1 - Cleanup
Created attachment 422323 [details] Screenshot of Patch v1.1
Punting on perspective transforms for the moment, and adding logic to return early if such a transform is applied. A majority of this logic should hold true for perspective transformed grids, as it uses relative lengths for constructing lines between sides of the grid, which may not form a parallelogram when a perspective transform is present.
Created attachment 422342 [details] Patch v2.0 - Use renderGrid to transform points
Created attachment 422344 [details] Screenshot of Patch v2.0 (Scale, Translate, & Rotate)
Created attachment 422345 [details] Screenshot of Patch v2.0 (Perspective)
Created attachment 422346 [details] Screenshot of Patch v2.0 (Perspective /w Area Names)
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 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?
Created attachment 422584 [details] Patch v2.1 - Review notes, make FloatLine standalone class
Committed r274096: <https://commits.webkit.org/r274096> All reviewed patches have been landed. Closing bug and clearing flags on attachment 422584 [details].