583931 - CanvasRenderingContext2D.arcTo causes strange draw with Scale and Translate

Status: NEW

(Core :: Graphics: Canvas2D, defect)

Description

[Makoto Kato [:m_kato]]

Attachment: testcase

After scale and translate, arcTo() is invalid elliptical arc.

When scale is comment out, it works fine.


- Example
  var g = document.getElementById("canvas1").getContext("2d");

  g.scale(0.9, 0.9); // It works if this line is comment out

  g.translate(100, 100);
  g.strokeStyle = "#888";
  g.lineWidth = 20;

  g.beginPath();
  g.lineCap = "round";
  g.moveTo(0,0);
  g.arcTo(0,0, 50,0, 50);
  g.stroke();
  g.restore();

Comment 1

[Joe Drew (not getting mail)]

Can we get a screenshot of this (it looks OK on Mac, I think)? Is it a regression from 3.6?

I don't think this should block as-is, though.

Comment 2

[Boris Zbarsky [:bzbarsky]]

This is definitely broken for me on Mac trunk.  fx3.6 is also broken.

The correct rendering is a singe small circle.  Are you actually seeing that?

Comment 3

[Joe Drew (not getting mail)]

hoo boy is that EVER not what I get.

Not a regression, though (just tested in 3.6), so I won't block on it. Would totally take a patch!

Comment 4

[Jeff Muizelaar [:jrmuizel]]

Dirk, care to look at this?

Comment 5

[Makoto Kato [:m_kato]]

Attachment: fix

Comment 6

[Jeff Muizelaar [:jrmuizel]]

(In reply to comment #5)
> Created attachment 462621 [details] [diff] [review]
> fix

Can you explain why this fixes it?

Comment 7

[Makoto Kato [:m_kato]]

(In reply to comment #6)
> (In reply to comment #5)
> > Created attachment 462621 [details] [diff] [review] [details]
> > fix
> 
> Can you explain why this fixes it?

0:000> p
eax=0013c5e8 ebx=0f190b70 ecx=0a0d1860 edx=0f190b70 esi=074a3ba0 edi=6cb5f4b0
eip=5ff75b64 esp=0013c594 ebp=0013c5fc iopl=0         nv up ei pl nz na po nc
cs=0023  ss=002b  ds=002b  es=002b  fs=0053  gs=002b             efl=00000202
xul!nsCanvasRenderingContext2D::ArcTo+0x67:
5ff75b64 e8c67afcff      call    xul!gfxContext::CurrentPoint (5ff3d62f)

0:000> p
eax=0013c5e8 ebx=0f190b70 ecx=0013c558 edx=0013c560 esi=074a3ba0 edi=6cb5f4b0
eip=5ff75b69 esp=0013c598 ebp=0013c5fc iopl=0         nv up ei pl nz ac pe nc
cs=0023  ss=002b  ds=002b  es=002b  fs=0053  gs=002b             efl=00000216
xul!nsCanvasRenderingContext2D::ArcTo+0x6c:
5ff75b69 d9450c          fld     dword ptr [ebp+0Ch]  ss:002b:0013c608=00000000

0:000> dt  -r gfxPoint @eax
xul!gfxPoint
   +0x000 x                : 2.6490953928259842e-006 <--- We expect 0.  But by rounding, it isn't 0.0
   +0x008 y                : 2.6490953928259842e-006 <--- We expect 0.  But by rounding, it isn't 0.0

Comment 8

[Boris Zbarsky [:bzbarsky]]

I'm not the right reviewer for this.  I have no idea how any of this graphics stuff works.  Please pick a reviewer who does?

Comment 9

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Comment on attachment 462621 [details] [diff] [review]
fix

I guess?

Comment 10

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Oh -- not a lot of context in the patch, I assume this is saving/restoring the matrix so that we don't actually modify the matrix outside of this function?

Comment 11

[Jeff Muizelaar [:jrmuizel]]

This needs a better explanation of why we aren't getting what we're expecting. I think it's unlikely that this would be the correct fix.

Comment 12

[Joe Drew (not getting mail)]

Philip/Dirk, do you have any idea of what the spec-compliant behaviour is here?