w3c / svgwg

SVG Working Group specifications
Other
704 stars 132 forks source link

SVGTransform's setMatrix method should take a DOMMatrix2DInit #326

Open zcorpan opened 7 years ago

zcorpan commented 7 years ago

https://svgwg.org/svg2-draft/coords.html#InterfaceSVGTransform

void setMatrix(DOMMatrixReadOnly matrix);

This should be

void setMatrix(optional DOMMatrixInit matrix);

The prose can then invoke https://drafts.fxtf.org/geometry/#create-a-dommatrix-from-the-dictionary if you want an actual DOMMatrix to work with, but working with a dictionary works as well. In the latter case, https://drafts.fxtf.org/geometry/#matrix-validate-and-fixup should be invoked on the dictionary first.

zcorpan commented 7 years ago

Same thing for https://svgwg.org/svg2-draft/coords.html#__svg__SVGTransformList__createSVGTransformFromMatrix

dstorey commented 6 years ago

@dirkschulze this is relate to your recent PR #414. Can you handle this one too?

dirkschulze commented 6 years ago

@dstorey Thought it is a similar approach, I don't think that the previous resolution covers DOMMatrixInit as well. Though I do support the change!

@zcorpan Why the change to optional?

zcorpan commented 6 years ago

@dirkschulze WebIDL dictionaries must be optional.

If the type of an argument is a dictionary type or a union type that has a dictionary as one of its flattened member types, and that dictionary type and its ancestors have no required members, and the argument is either the final argument or is followed only by optional arguments, then the argument must be specified as optional. Such arguments are always considered to have a default value of an empty dictionary, unless otherwise specified.

https://heycam.github.io/webidl/#ref-for-idl-dictionary%E2%91%A1

dirkschulze commented 6 years ago

@zcorpan Thanks!

boggydigital commented 6 years ago

Not blocking updated 2.0 CR publication - assigning 2.0 Recommendation milestone to clean this up before 2.0 REC

css-meeting-bot commented 5 years ago

The SVG Working Group just discussed SVGTransform's setMatrix method should take a DOMMatrix2DInit, and agreed to the following:

The full IRC log of that discussion <myles> TOPIC: SVGTransform's setMatrix method should take a DOMMatrix2DInit
<myles> GitHub: https://github.com/w3c/svgwg/issues/326
<myles> krit: we already changed the interface from DOMPointInit, that would pretty much be the same thing, except DomMatrixReadOnly to DOMMatrix2DInit
<myles> AmeliaBR: the only question: Whether those methods copy the matrix or create a new matrix with the same data
<myles> AmeliaBR: whether they reference the exact same data or create a copy of it
<myles> krit: they currently do not reference the object
<myles> krit: in your terms, it would be a copy
<myles> AmeliaBR: it says it's do a new matrix, so it's not about preserving the exact object. so in that case no there's no reason not to be able to use a dictionary with the same properties as a matrix, it doesn't need to be a particular class type. Sounds good to me. There was another message .... ?
<myles> AmeliaBR: CreateTransformFrom <inaudible> okay, that sounds sane
<myles> RESOLVED: Replace DOMPoint with DOMPoint2DInit with SetMatrix() and createSVGTransformFromMatrix()
longsonr commented 4 years ago

Can we confirm that...

a) I pass { a: 1, m11: 2 } we should throw a TypeError. b) I pass {a: Infinity } and given that DOMMatrix2DInit takes unrestricted doubles unlike an SVGMatrix that should also result in a TypeError.

zcorpan commented 3 years ago

Why was this specced to use DOMMatrix2DInit and not DOMMatrixInit (as suggested in OP)?

The steps that checks "is2D()" is a no-op as specced, I believe, since there won't be any members in the dictionary that can cause it to return false.

@longsonr the tests in https://github.com/web-platform-tests/wpt/tree/master/css/geometry might be useful. (IIRC, Infinity doesn't throw.)