Summary: | Web Inspector: Grid overlay does not adjust for element transforms (rotation, scale, etc.) | ||
---|---|---|---|
Product: | WebKit | Reporter: | Patrick Angle <pangle> |
Component: | Web Inspector | Assignee: | Patrick Angle <pangle> |
Status: | RESOLVED FIXED | ||
Severity: | Normal | CC: | bburg, ews-watchlist, hi, inspector-bugzilla-changes, joepeck, pangle, webkit-bug-importer |
Priority: | P2 | Keywords: | InRadar |
Version: | WebKit Nightly Build | ||
Hardware: | All | ||
OS: | All | ||
Attachments: |
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]. |
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.