Bug 16610 – <canvas> restore() removes path segments created before it

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Ilmari Heikkinen]

A ctx.restore() removes the path segments created before it. This makes it difficult to create transformed paths using ctx.save(); create_path(); ctx.restore()

E.g. 
ctx.save(); 
  ctx.scale(0.5, 1); 
  ctx.arc(0,0,10,0,Math.PI*2,false); 
ctx.restore(); 
ctx.stroke(); 
draws nothing (instead of the expected ellipse.)

[Ilmari Heikkinen]

Created attachment 18116 [details]
Testcase for beginPath(); save(); arc(); restore(); stroke()

[Oliver Hunt]

Hmm, technically this is an easy fix, but i find myself wondering wtf firefox is doing -- it is not mathematically correct and appears to be completely detached from reality :-/

[Oliver Hunt]

Erk, it would appear that firefox attempts to maintain the screen position and size of a path across save/restore boundaries.  In effect, firefox appears to be trying to make:
save();
translate(100,100);
rect(0,0,10,10);
restore()

produce the same thing as
translate(100,100);
rect(0,0,10,10);
translate(-100,-100);

In practice this means that we have to maintain the transform for each level of the state stack -- this makes me unhappy, and i feel is not spec behaviour.  My reading of the spec would indicate that the opera behaviour is correct -- after restore() drawing the path should result in a circle, whereas firefox produces an ellipse.

[Oliver Hunt]

Created attachment 19567 [details]
Patch to implement bizarre firefox behaviour

*sigh*  This is what would be necessary to match the firefox behaviour.  Matching Opera -- and thus being correct relative to html5 by my reading -- is a smaller and downright trivial patch, that would just mean removing all the state transform from this patch.

[Oliver Hunt]

Comment on attachment 19567 [details]
Patch to implement bizarre firefox behaviour

Okay, i've talked about this with Hixie.  It would appear that firefox behaviour is that expeted by the spec, which makes me cry.  Therefore i'm marking the logic change of the patch for review.  I'll bang out tests and changelog tomorrow

[Darin Adler]

Comment on attachment 19567 [details]
Patch to implement bizarre firefox behaviour

Looks fine, r=me.

But where are the new regression tests?

[Oliver Hunt]

Layout tests reviewed by Mark Rowe, landed r30891