[cssom-view-1] Added definition of getBoxQuads()#10538
Conversation
svgeesus
changed the title
|
One issues atm: |
SebastianZ
changed the title
|
I think it makes more sense to add another option to I imagine something like Sebastian |
should be fixed in current polyfill |
it's in a extra method, cause it is used from other functions in the spec (like convertPointFromNode). And convertPointFromNode use it multiple times |
emilio
left a comment
emilio
left a comment
There was a problem hiding this comment.
Thanks, this looks like a good start. Please look at how other specs (specially recent ones) use bikeshed in order to get the formatting and references correct. Also this needs to work in terms of nodes and boxes, not elements.
|
|
||
| The <dfn method for=GeometryUtils lt="getBoxQuads(options)|getBoxQuads()">getBoxQuads(<var>options</var>)</dfn> method must run the following steps: | ||
|
|
||
| The helper method getCompleteTransform(anchestor) must run the following steps: |
There was a problem hiding this comment.
You need to get the element as a parameter, and use <div algorithm> for this for example:
<div algorithm="get the complete transform">
In order to <dfn export>get the complete transform</dfn> of an node <var>node</var> relative to a node <var>ancestor</ancestor> ...
</div>
There was a problem hiding this comment.
Added node to helper function
|
|
||
| test block in inline | ||
| 1. let currentElement be the element getCompleteTransform() called on. | ||
| 2. let transformationMatrix be the current transformation matrix of currentElement (https://www.w3.org/TR/css-transforms-2/#ctm) |
There was a problem hiding this comment.
Please use bikeshed references for this.
There was a problem hiding this comment.
Where should I use a bikeshed reference? (Is there an editor wich helps in editing bs files)
|
|
||
| <ol> | ||
| <li><p class=issue>... | ||
| 1. get the resulting matrix between the element and the document.defaultView via the algorithm defined in getCompleteTransform |
There was a problem hiding this comment.
All these algorithms need to be in terms of nodes, as GeometryUtils also applies to text nodes and such.
There was a problem hiding this comment.
Is node correct? Or should it be "GeometryNode"?
| - 'border': no change | ||
| - 'margin': x - margin.left, y - margin.top | ||
| - 'padding': x + border.leftWidth, y + border.topWidth | ||
| - 'content': x + border.leftWidth + padding.left, y + border.topWidth + padding.top |
There was a problem hiding this comment.
This is not actually based on the computed style but on the actual used padding box etc. Those boxes are defined already elsewhere and we should reuse the definitions.
There was a problem hiding this comment.
I don't really find where I should link to.
maybe: https://drafts.csswg.org/css2/#Computing_widths_and_margins
There was a problem hiding this comment.
I don't really find where I should link to. maybe: https://drafts.csswg.org/css2/#Computing_widths_and_margins
Pretty sure those boxes are defined here: https://drafts.csswg.org/css-box-4/#box-edges
There was a problem hiding this comment.
Yeah, but I need to link to the Algorythm how they are calculated, or is this enough?
There was a problem hiding this comment.
Yeah, but I need to link to the Algorythm how they are calculated, or is this enough?
There is no algorithm like that for these. They are defined in English instead. Browsers have algorithms internally that match those descriptions and can call into those when a spec says to do something with one of these boxes, so linking to their definitions is enough to unambiguously tell an implementer what needs to happen.
More modern specs like to define their concepts in terms of algorithms to derive them, because that makes implementation easier and leaves less room for creative (mis-)interpretation of the definition.
TL;DR: You want to link to a concept's definition when using it. Whether that is an algorithm or not doesn't matter.
|
Hi @jogibear9988, would you maybe have the time to follow-up on the review from Emilio? Thanks |
|
@emilio |
emilio
left a comment
emilio
left a comment
There was a problem hiding this comment.
This still needs a bit of work to get the spec into a reasonable shape, but looks in the right direction. Thanks!
| points are flattened (3d transform), z=0. like getClientRect | ||
|
|
||
| test block in inline | ||
| 1. let currentElement be node. |
There was a problem hiding this comment.
currentNode, right? I think this should use <var>current</var> and other bikeshed markup to display properly.
| 1. let currentElement be node. | ||
| 2. let transformationMatrix be the current transformation matrix of currentElement (https://www.w3.org/TR/css-transforms-2/#ctm) | ||
| 3. while currentElement is not null do | ||
| <ol> |
There was a problem hiding this comment.
Do you really need the <ol>?
|
|
||
| The <dfn method for=GeometryUtils lt="getBoxQuads(options)|getBoxQuads()">getBoxQuads(<var>options</var>)</dfn> method must run the following steps: | ||
|
|
||
| The helper method getCompleteTransform(node, anchestor) must run the following steps: |
There was a problem hiding this comment.
I think you should probably add a <dfn> so that you can reference this properly elsewhere.
|
|
||
| viewport boxes are all the same | ||
| <ol> | ||
| 1. create an array as result container |
There was a problem hiding this comment.
Let |result| be an empty [=list=] of [=DOMQuad=] objects. etc, please look at existing bikeshed files to see how to make this work.
| viewport boxes are all the same | ||
| <ol> | ||
| 1. create an array as result container | ||
| 2. for each css-fragment of the element run the following steps |
There was a problem hiding this comment.
of what element? Probably of this? Also fragment should link to a definition.
| - 'padding': https://drafts.csswg.org/css-box-4/#box-edges - use the padding-box | ||
| - 'content': https://drafts.csswg.org/css-box-4/#box-edges - use the content-box | ||
| 3. create a DomQuad from the rect via DOMQuad.fromRect | ||
| 4. Calculate the resulting transformation matrix between the element and options.realtiveTo (if relativeTo is unset use window.defaultView) via the algorithm defined in getCompleteTransform |
There was a problem hiding this comment.
window.defaultView is not a thing, also s/realtiveTo/relativeTo.
This should be untransforming the rect to the layout viewport if there's no relativeTo.
|
|
||
| <ol> | ||
| <li><p class=issue>... | ||
| 1. transform each point of the DOMQuad via convertPointFromNode |
There was a problem hiding this comment.
This should link to convertPointFromNode, but also probably should have more precise details like:
* Let <var>p1</var> be the result of calling [=convertFromFromNode=] with <var>quad</var>.p1, <var>from</var> and <var>options</var>.
* ...
* Return a new [=DOMQuad=] with <var>p1</var>...
|
|
||
| <ol> | ||
| <li><p class=issue>... | ||
| 1. get the resulting matrix between the node and the document.defaultView via the algorithm defined in getCompleteTransform |
There was a problem hiding this comment.
Give these meaningful names like:
* Let <var>thisTransformToViewport</var> be the result of calling [=getCompleteTransform=] with <var>this</var> and <var>this</var>' [=associated document=].
* Let <var>fromTransformToViewport</var> be the result of ...
* Invert <var>fromTransformToViewport</var>.
* ...
Then reference those.
| - 'border': no change | ||
| - 'margin': x - margin.left, y - margin.top | ||
| - 'padding': x + border.leftWidth, y + border.topWidth | ||
| - 'content': x + border.leftWidth + padding.left, y + border.topWidth + padding.top |
There was a problem hiding this comment.
Same, refer to standard terms. It seems ResizeObserver refers to the CSS2 border box area, etc.
| 1. get the resulting matrix between the node and the document.defaultView via the algorithm defined in getCompleteTransform | ||
| 2. get the resulting matrix between the from and the document.defaultView via the algorithm defined in getCompleteTransform | ||
| 3. inverse the matrix in 2. | ||
| 4. Depending on the value of options.fromBox modify the point with the calculated style values of the element. |
There was a problem hiding this comment.
This should use <var>point</var> etc.
|
Hi @jogibear9988. Would you maybe have the time to update this PR based on the review feedback from Emilio? Thanks a lot! |
|
Sorry, I've read them, and hopefully fix in the next days |
|
Hi @jogibear9988. I wanted to check back with you about the status of this PR. Is it still something you can continue working on? Thanks! |
|
@whimboo |
|
Also I think the spec needs an upgrade to also include a function to calculate the complete transformation matrix between two elements. |
This would need a separate PR – one that would need a CSS WG Resolution first to get the group’s consensus to adding it. |
|
Hello @jogibear9988. I want to check back with you if you may have some time that this PR could be moved forward? Or is it blocked based on the last comment? Thanks! |
Rewrite getCompleteTransform, getBoxQuads, convertQuadFromNode,
convertRectFromNode, and convertPointFromNode algorithm definitions
using proper bikeshed markup:
- Use <div algorithm> block with <dfn> for getCompleteTransform
- Use <var> tags for all variables throughout
- Use {{}} bikeshed notation for DOM types (DOMQuad, DOMMatrix, etc.)
- Reference css-transforms-1 CTM definition properly
- Handle Document (layout viewport) vs Element/Text (box fragments)
- Use proper <a spec=css-break>box fragment</a> references
- Reference margin/padding/border/content edges correctly
- Fix relativeTo (not realtiveTo) and use layout viewport as fallback
- Give meaningful variable names (thisTransformToViewport, etc.)
- Link convertQuadFromNode to convertPointFromNode explicitly
- Reference used values of CSS properties for box adjustments
Co-authored-by: jogibear9988 <364896+jogibear9988@users.noreply.github.com>
…ible matrix note - Spell out how margin/padding/content box adjustments work using specific CSS property used values - Add note about non-invertible transformation matrices - Add link-defaults for border-right-width and border-bottom-width Co-authored-by: jogibear9988 <364896+jogibear9988@users.noreply.github.com>
Co-authored-by: jogibear9988 <364896+jogibear9988@users.noreply.github.com> Agent-Logs-Url: https://github.com/jogibear9988/csswg-drafts/sessions/2cc807ab-9305-4dd8-b0f7-dafb7f3c6be2
[cssom-view] Specify GeometryUtils algorithms (getBoxQuads, convertQuadFromNode, convertRectFromNode, convertPointFromNode)
…-from-pull-10538 # Conflicts: # cssom-view-1/Overview.bs
Resolve merge conflicts with updated patch-1 base branch
…10538 Copilot/fix issues from pull 10538
The main changes are: - rewrote getBoxQuads() in terms of nodes and box fragments, with layout-viewport coordinates as the default when relativeTo is omitted - added explicit Document and Text handling - moved the box-space wording to helper algorithms and removed the margin/padding/content edge arithmetic based on used values - reused convertQuadFromNode()/convertPointFromNode() for the actual coordinate conversion path I also validated the edited file locally. A remote Bikeshed run still hits unrelated existing markdown errors later in cssom-view-1, but it did not report new fatal issues in the GeometryUtils block.
|
also added some description to my polyfill: https://github.com/jogibear9988/getBoxQuadsPolyfill/blob/main/csswg-getBoxQuads-calculation.md |
|
I also have a new library wich uses getBoxQuads API, to be able to create screenshots or vector exports of HTML pages: https://github.com/node-projects/layout2vector |
Fixes #10537
A linked issue:
#10514
A firefox issue with a little bit of description:
https://bugzilla.mozilla.org/show_bug.cgi?id=1107559
A polyfill for the API
https://github.com/jogibear9988/getBoxQuadsPolyfill