Re: [whatwg] Canvas Path.addPath SVGMatrix not optimal?

2014-08-25 Thread Rik Cabanier
On Mon, Mar 24, 2014 at 7:31 AM, Justin Novosad ju...@google.com wrote:

 On Sat, Mar 22, 2014 at 4:20 AM, Dirk Schulze dschu...@adobe.com wrote:

  So can we agree on:
 
  addPath(Path, optional SVGMatrix)
 

 lgtm


Firefox [1], WebKit [2] and Blink [3] implemented addPath with the matrix
as an optional parameter.
Can the spec be updated to reflect this?

1: https://bugzilla.mozilla.org/show_bug.cgi?id=985801
2: https://bugs.webkit.org/show_bug.cgi?id=130461
3: https://codereview.chromium.org/170503002


Re: [whatwg] Canvas Path.addPath SVGMatrix not optimal?

2014-03-24 Thread Justin Novosad
On Sat, Mar 22, 2014 at 4:20 AM, Dirk Schulze dschu...@adobe.com wrote:

 So can we agree on:

 addPath(Path, optional SVGMatrix)


lgtm


Re: [whatwg] Canvas Path.addPath SVGMatrix not optimal?

2014-03-22 Thread Dirk Schulze
So can we agree on:

addPath(Path, optional SVGMatrix)

(Independent of the discussion about CanvasPathMethods.)

Does some one think it would be necessary to make SVGMatrix nullable (optional 
SVGMatrix?)? I think it would be superfluous.

Greetings,
Dirk

On Mar 21, 2014, at 8:02 PM, Joe Gregorio 
jcgrego...@google.commailto:jcgrego...@google.com wrote:




On Wed, Mar 19, 2014 at 4:46 PM, Dirk Schulze 
dschu...@adobe.commailto:dschu...@adobe.com wrote:
Hi,

I just looked at the definition of Path.addPath[1]:

void addPath(Path path, SVGMatrix? transformation);

SVGMatrix is nullable but can not be omitted all together. Why isn't it 
optional as well? I think it should be optional, especially because creating an 
SVGMatrix at the moment means writing:

var matrix = 
document.createElementNS('http://www.w3.org/2000/svg','svg').createSVGMatrix();

Agreed, that's painful, +1 for making it optional.

  -joe


Greetings,
Dirk

[1] 
http://www.whatwg.org/specs/web-apps/current-work/multipage/the-canvas-element.html#path



Re: [whatwg] Canvas Path.addPath SVGMatrix not optimal?

2014-03-21 Thread Joe Gregorio
On Wed, Mar 19, 2014 at 4:46 PM, Dirk Schulze dschu...@adobe.com wrote:

 Hi,

 I just looked at the definition of Path.addPath[1]:

 void addPath(Path path, SVGMatrix? transformation);

 SVGMatrix is nullable but can not be omitted all together. Why isn’t it
 optional as well? I think it should be optional, especially because
 creating an SVGMatrix at the moment means writing:

 var matrix = document.createElementNS('http://www.w3.org/2000/svg
 ','svg').createSVGMatrix();


Agreed, that's painful, +1 for making it optional.

  -joe



 Greetings,
 Dirk

 [1]
 http://www.whatwg.org/specs/web-apps/current-work/multipage/the-canvas-element.html#path


Re: [whatwg] Canvas Path.addPath SVGMatrix not optimal?

2014-03-20 Thread Justin Novosad
On Wed, Mar 19, 2014 at 5:47 PM, Rik Cabanier caban...@gmail.com wrote:


 On Wed, Mar 19, 2014 at 2:22 PM, Justin Novosad ju...@google.com wrote:


 I agree it should be optional, but just to play devil's advocate : you can
 create an identity SVGMatrix with a simpler bit of code. Just do this
 right
 after creating a canvas rendering context: var identity =
 context.currentTransform;


 Hi Justin,

 did Blink already expose this property?


It is implemented, but not exposed (hidden behind the experimental canvas
features flag)


 As currently specified, this must return a live SVGMatrix object, meaning
 that as you change the CTM on the 2d context, your reference to the
 SVGMatrix should change as well. [1]


D'oh! I totally missed that when I reviewed the implementation. In fact,
the implementer even went to great length to ensure the opposite behavior
(making a copy).
https://codereview.chromium.org/24233004
I'll make sure that gets fixed.


 It's unlikely that you actually want this...
 This API should be renamed to get/setCurrentTransform() and return a copy.


Yes, making a copy felt like the most desirable behavior, so I did not
think twice about the fact that the implementation performs a copy.
Anyways, thanks for catching this.


 1:
 http://www.whatwg.org/specs/web-apps/current-work/multipage/the-canvas-element.html#dom-context-2d-currenttransform



Re: [whatwg] Canvas Path.addPath SVGMatrix not optimal?

2014-03-20 Thread Rik Cabanier
On Thu, Mar 20, 2014 at 7:01 AM, Justin Novosad ju...@google.com wrote:




 On Wed, Mar 19, 2014 at 5:47 PM, Rik Cabanier caban...@gmail.com wrote:


 On Wed, Mar 19, 2014 at 2:22 PM, Justin Novosad ju...@google.com wrote:


 I agree it should be optional, but just to play devil's advocate : you
 can
 create an identity SVGMatrix with a simpler bit of code. Just do this
 right
 after creating a canvas rendering context: var identity =
 context.currentTransform;


 Hi Justin,

 did Blink already expose this property?


 It is implemented, but not exposed (hidden behind the experimental canvas
 features flag)


 As currently specified, this must return a live SVGMatrix object, meaning
 that as you change the CTM on the 2d context, your reference to the
 SVGMatrix should change as well. [1]


 D'oh! I totally missed that when I reviewed the implementation. In fact,
 the implementer even went to great length to ensure the opposite behavior
 (making a copy).
 https://codereview.chromium.org/24233004
 I'll make sure that gets fixed.


By fixed, do you mean you will return a reference or change the name of
the API? :-)


 It's unlikely that you actually want this...
 This API should be renamed to get/setCurrentTransform() and return a copy.


 Yes, making a copy felt like the most desirable behavior, so I did not
 think twice about the fact that the implementation performs a copy.
 Anyways, thanks for catching this.



Re: [whatwg] Canvas Path.addPath SVGMatrix not optimal?

2014-03-20 Thread Justin Novosad
On Thu, Mar 20, 2014 at 11:23 AM, Rik Cabanier caban...@gmail.com wrote:




 On Thu, Mar 20, 2014 at 7:01 AM, Justin Novosad ju...@google.com wrote:




 On Wed, Mar 19, 2014 at 5:47 PM, Rik Cabanier caban...@gmail.com wrote:


 On Wed, Mar 19, 2014 at 2:22 PM, Justin Novosad ju...@google.comwrote:


 I agree it should be optional, but just to play devil's advocate : you
 can
 create an identity SVGMatrix with a simpler bit of code. Just do this
 right
 after creating a canvas rendering context: var identity =
 context.currentTransform;


 Hi Justin,

 did Blink already expose this property?


 It is implemented, but not exposed (hidden behind the experimental canvas
 features flag)


 As currently specified, this must return a live SVGMatrix object,
 meaning that as you change the CTM on the 2d context, your reference to the
 SVGMatrix should change as well. [1]


 D'oh! I totally missed that when I reviewed the implementation. In fact,
 the implementer even went to great length to ensure the opposite behavior
 (making a copy).
 https://codereview.chromium.org/24233004
 I'll make sure that gets fixed.


 By fixed, do you mean you will return a reference or change the name of
 the API? :-)


I have no overwhelmingly strong motives to pick one over the other, so what
I meant by fix is for the implementation follow the spec, whether we
change it or not.  I'll make sure we don't ship it until this discussion is
over.
If no browser has exposed this yet, we can still go back without causing
anyone any pain.


[whatwg] Canvas Path.addPath SVGMatrix not optimal?

2014-03-19 Thread Dirk Schulze
Hi, 

I just looked at the definition of Path.addPath[1]:

void addPath(Path path, SVGMatrix? transformation);

SVGMatrix is nullable but can not be omitted all together. Why isn’t it 
optional as well? I think it should be optional, especially because creating an 
SVGMatrix at the moment means writing:

var matrix = 
document.createElementNS('http://www.w3.org/2000/svg','svg').createSVGMatrix();

Greetings,
Dirk

[1] 
http://www.whatwg.org/specs/web-apps/current-work/multipage/the-canvas-element.html#path

[whatwg] Canvas Path.addPath SVGMatrix not optimal?

2014-03-19 Thread Dirk Schulze
Hi, 

I just looked at the definition of Path.addPath[1]:

   void addPath(Path path, SVGMatrix? transformation);

SVGMatrix is nullable but can not be omitted all together. Why isn’t it 
optional as well? I think it should be optional, especially because creating an 
SVGMatrix at the moment means writing:

   var matrix = 
document.createElementNS('http://www.w3.org/2000/svg','svg').createSVGMatrix();

I would even argue that SVGMatrix should just be optional.

Greetings,
Dirk

[1] 
http://www.whatwg.org/specs/web-apps/current-work/multipage/the-canvas-element.html#path

Re: [whatwg] Canvas Path.addPath SVGMatrix not optimal?

2014-03-19 Thread Justin Novosad
On Wed, Mar 19, 2014 at 4:46 PM, Dirk Schulze dschu...@adobe.com wrote:

 Hi,

 I just looked at the definition of Path.addPath[1]:

 void addPath(Path path, SVGMatrix? transformation);

 SVGMatrix is nullable but can not be omitted all together. Why isn’t it
 optional as well? I think it should be optional, especially because
 creating an SVGMatrix at the moment means writing:

 var matrix = document.createElementNS('http://www.w3.org/2000/svg
 ','svg').createSVGMatrix();

 Greetings,
 Dirk

 [1]
 http://www.whatwg.org/specs/web-apps/current-work/multipage/the-canvas-element.html#path


I agree it should be optional, but just to play devil's advocate : you can
create an identity SVGMatrix with a simpler bit of code. Just do this right
after creating a canvas rendering context: var identity =
context.currentTransform;

-Justin


Re: [whatwg] Canvas Path.addPath SVGMatrix not optimal?

2014-03-19 Thread Rik Cabanier
On Wed, Mar 19, 2014 at 2:22 PM, Justin Novosad ju...@google.com wrote:

 On Wed, Mar 19, 2014 at 4:46 PM, Dirk Schulze dschu...@adobe.com wrote:

  Hi,
 
  I just looked at the definition of Path.addPath[1]:
 
  void addPath(Path path, SVGMatrix? transformation);
 
  SVGMatrix is nullable but can not be omitted all together. Why isn't it
  optional as well? I think it should be optional, especially because
  creating an SVGMatrix at the moment means writing:
 
  var matrix = document.createElementNS('http://www.w3.org/2000/svg
  ','svg').createSVGMatrix();
 
  Greetings,
  Dirk
 
  [1]
 
 http://www.whatwg.org/specs/web-apps/current-work/multipage/the-canvas-element.html#path


 I agree it should be optional, but just to play devil's advocate : you can
 create an identity SVGMatrix with a simpler bit of code. Just do this right
 after creating a canvas rendering context: var identity =
 context.currentTransform;


Hi Justin,

did Blink already expose this property?
As currently specified, this must return a live SVGMatrix object, meaning
that as you change the CTM on the 2d context, your reference to the
SVGMatrix should change as well. [1]

It's unlikely that you actually want this...
This API should be renamed to get/setCurrentTransform() and return a copy.

1:
http://www.whatwg.org/specs/web-apps/current-work/multipage/the-canvas-element.html#dom-context-2d-currenttransform


Re: [whatwg] Canvas Path.addPath SVGMatrix not optimal?

2014-03-19 Thread Dirk Schulze


On Mar 19, 2014, at 10:48 PM, Rik Cabanier 
caban...@gmail.commailto:caban...@gmail.com wrote:




On Wed, Mar 19, 2014 at 2:22 PM, Justin Novosad 
ju...@google.commailto:ju...@google.com wrote:
On Wed, Mar 19, 2014 at 4:46 PM, Dirk Schulze 
dschu...@adobe.commailto:dschu...@adobe.com wrote:

 Hi,

 I just looked at the definition of Path.addPath[1]:

 void addPath(Path path, SVGMatrix? transformation);

 SVGMatrix is nullable but can not be omitted all together. Why isn't it
 optional as well? I think it should be optional, especially because
 creating an SVGMatrix at the moment means writing:

 var matrix = document.createElementNS('http://www.w3.org/2000/svg
 ','svg').createSVGMatrix();

 Greetings,
 Dirk

 [1]
 http://www.whatwg.org/specs/web-apps/current-work/multipage/the-canvas-element.html#path


I agree it should be optional, but just to play devil's advocate : you can
create an identity SVGMatrix with a simpler bit of code. Just do this right
after creating a canvas rendering context: var identity =
context.currentTransform;

Hi Justin,

did Blink already expose this property?
As currently specified, this must return a live SVGMatrix object, meaning that 
as you change the CTM on the 2d context, your reference to the SVGMatrix should 
change as well. [1]

It's unlikely that you actually want this...
This API should be renamed to get/setCurrentTransform() and return a copy.

It could be called getCTM() which SVG offers on SVGElements and also returns an 
SVGMatrix object. But I agree that it would be easier if we return a copy 
instead of a live object.

What does Blink do?

Greetings,
Dirk


1: 
http://www.whatwg.org/specs/web-apps/current-work/multipage/the-canvas-element.html#dom-context-2d-currenttransform