Closed Bug 1355675 Opened 3 years ago Closed 3 years ago

Provide a ChromeOnly API for Element to report its root-relative transform matrix

Categories

(Core :: CSS Parsing and Computation, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: bradwerth, Assigned: bradwerth)

References

Details

Attachments

(4 files)

Devtools draws overlays on top of elements, and most of the time it uses bounding corners of transformed quads to do this. There are some cases where the overlay needs more information than the bounding quad (like drawing grid lines on a display:grid element), and to handle that we need to provide the transform matrix.
Something like https://www.w3.org/TR/SVG/types.html#__svg__SVGLocatable__getTransformToElement but for all elements?

Note that our implementation of getTransformToElement only handles SVG transforms and not CSS transforms currently but it could be extended to do so. Perhaps it could be made Chrome only for elements that don't implement SVGLocatable and content accessible to those that do.

Alternatively perhaps doing a similar thing to getScreenCTM() would more meet your needs.
nsLayoutUtils::GetTransformToAncestor seems helpful for implementing this.
Comment on attachment 8866106 [details]
Bug 1355675 Part 4: Add tests of Element::getTransformTo... methods.

Matteo, is this the sort of thing you're looking for to draw more accurate overlays in dev tools?
Attachment #8866106 - Flags: feedback?(zer0)
Attachment #8871512 - Flags: review?(matt.woodrow)
Attachment #8866104 - Flags: review?(matt.woodrow)
Attachment #8866105 - Flags: review?(matt.woodrow)
Attachment #8866106 - Flags: review?(matt.woodrow)
Matteo, are these methods useful for you?
Flags: needinfo?(zer0)
Comment on attachment 8871512 [details]
Bug 1355675 Part 1: Add optional inCSSUnits parameters to GetTransformMatrix and GetTransformToAncestor.

https://reviewboard.mozilla.org/r/142980/#review147378

::: layout/base/nsLayoutUtils.h:856
(Diff revision 1)
>     * Gets the transform for aFrame relative to aAncestor. Pass null for
>     * aAncestor to go up to the root frame.
>     */
> -  static Matrix4x4 GetTransformToAncestor(nsIFrame *aFrame, const nsIFrame *aAncestor);
> +  static Matrix4x4 GetTransformToAncestor(nsIFrame *aFrame,
> +                                          const nsIFrame *aAncestor,
> +                                          bool aInCSSUnits = false);

Please add a comment about this new parameter.
Attachment #8871512 - Flags: review?(matt.woodrow) → review+
Comment on attachment 8866104 [details]
Bug 1355675 Part 2: Extend DOMMatrixReadOnly to allow instantiation with a Matrix4x4.

https://reviewboard.mozilla.org/r/137706/#review147382
Attachment #8866104 - Flags: review?(matt.woodrow) → review+
Comment on attachment 8866105 [details]
Bug 1355675 Part 3: Add some Chrome-only getTransformTo... methods to Element.

https://reviewboard.mozilla.org/r/137708/#review147384

::: dom/base/Element.cpp:3429
(Diff revision 3)
>  }
>  
> +already_AddRefed<DOMMatrixReadOnly>
> +Element::GetTransformToAncestor(Element& aAncestor)
> +{
> +  nsIFrame* primaryFrame = GetPrimaryFrame();

GetPrimaryFrame is fallible, so you want to check for that, here and elsewhere.
Attachment #8866105 - Flags: review?(matt.woodrow) → review+
Attachment #8866105 - Flags: review?(bugs)
Attachment #8866106 - Flags: review?(matt.woodrow) → review?(bugs)
Comment on attachment 8866106 [details]
Bug 1355675 Part 4: Add tests of Element::getTransformTo... methods.

Matt should probably review this
Attachment #8866106 - Flags: review?(bugs) → review?(matt.woodrow)
Comment on attachment 8866105 [details]
Bug 1355675 Part 3: Add some Chrome-only getTransformTo... methods to Element.

https://reviewboard.mozilla.org/r/137708/#review147490

::: dom/webidl/Element.webidl:171
(Diff revision 3)
>    [ChromeOnly, Pure]
>    sequence<Grid> getGridFragments();
> +
> +  [ChromeOnly]
> +  DOMMatrixReadOnly getTransformToAncestor(Element ancestor);
> +  DOMMatrixReadOnly getTransformToParent();

So you expose getTransformToParent() and 
getTransformToViewport() to the web.

We don't want that.
Attachment #8866105 - Flags: review?(bugs) → review-
Comment on attachment 8866105 [details]
Bug 1355675 Part 3: Add some Chrome-only getTransformTo... methods to Element.

https://reviewboard.mozilla.org/r/137708/#review148050
Attachment #8866105 - Flags: review?(bugs) → review+
Comment on attachment 8866106 [details]
Bug 1355675 Part 4: Add tests of Element::getTransformTo... methods.

I really think Matt should review the tests since he know how the (ChromeOnly) API should work.
Attachment #8866106 - Flags: review?(bugs)
Attachment #8866106 - Flags: review?(matt.woodrow)
Comment on attachment 8866106 [details]
Bug 1355675 Part 4: Add tests of Element::getTransformTo... methods.

https://reviewboard.mozilla.org/r/137710/#review148256
Attachment #8866106 - Flags: review?(matt.woodrow) → review+
Pushed by bwerth@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a204d31127ce
Part 1: Add optional inCSSUnits parameters to GetTransformMatrix and GetTransformToAncestor. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/ad325c236e95
Part 2: Extend DOMMatrixReadOnly to allow instantiation with a Matrix4x4. r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/fcfd741290a4
Part 3: Add some Chrome-only getTransformTo... methods to Element. r=mattwoodrow,smaug
https://hg.mozilla.org/integration/autoland/rev/cb9066e8f9f3
Part 4: Add tests of Element::getTransformTo... methods. r=mattwoodrow
Forgot to remove the ni here, me and Brad discussed that by email and over vidyo, providing the examples needed.
Flags: needinfo?(zer0)
Blocks: 1370278
You need to log in before you can comment on or make changes to this bug.