333613 - update canvas on branch

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, defect)

Description

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

Two bits of functionality that we should give to web developers on the branch:

1. toDataURL bits
   - bug 245684 (add image encoder support)
   - bug 291218 (toDataURL)
   - bug 293244 (security checks)
2. canvas speedup and API fixes
   - bug 333177

Comment 1

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

Attachment: moz18-canvas-update.patch

1.8 version of my patch from bug 333177; the only change outside of canvas bits is an API change in nsIRenderingContext.h -- we were returing "native graphic data" as a PRUint32, when in most cases the actual data was a pointer (this function was unused in most of the RC backends, but I needed to get at the underlying native thing for canvas).  This seems like a very bad thing to do on 64-bit platforms, so I changed this to return a |void *| instead.

Comment 2

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

Oh, that patch does not have image encoders moved over, though the infrastructure is in the canvas code -- once they're moved over the bits can just be uncommented out.

Comment 3

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

1.8.1, boy howdy.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Do we really want all the MOZ_CAIRO_GFX code? Is it worth it to get more consistent diffs between branch and trunk?

Comment 5

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

I think branch/trunk sync is virtuous here, yeah.

Comment 6

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

I'm writing the exact same code for trunk/branch right now; I don't see any value to pulling out one or the other.  Sometime after Fx2 ships, I'll pull out the MOZILLA_1_8_* stuff from the trunk, because at that point it will be much harder to get simultaneous changes in; but for now, it seems easier to do a patch if it can apply directly to branch and trunk.

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 220169 [details] [diff] [review]
moz18-canvas-update.patch

sr+ with toDataURLAs disabled on branch.

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 220169 [details] [diff] [review]
moz18-canvas-update.patch

resetting requests since it looks like we'll want to make it toDataURL(format) as per the WHATWG spec.

Comment 9

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

Attachment: new patch, with updated todataURL stuff

New patch, with updated toDataURL API.

Comment 10

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 220710 [details] [diff] [review]
new patch, with updated todataURL stuff

branch-approved, let's get this mother on.

Comment 11

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

Checked in 

Comment 12

[Roy Tam]

(In reply to comment #11)
> Checked in 
> 

It breaks the gdiplus SVG rendering.

mozilla\layout\svg\renderer\src\gdiplus\nsSVGGDIPlusCanvas.cpp(140) : error C2664: 'nsIRenderingContext::RetrieveCurrentNativeGraphicData' : cannot convert parameter 1 from 'PRUint32 *' to 'void ** '

Comment 13

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Can you file a bug on that breakage, and mark it as blocking this one?  Thanks.  It looks like the obvious fix (following the new pattern in nsCanvasRenderingContext2D::Render) would work, but I don't have an environment to test that in.

Comment 14

[Roy Tam]

(In reply to comment #13)
> Can you file a bug on that breakage, and mark it as blocking this one?  Thanks.
>  It looks like the obvious fix (following the new pattern in
> nsCanvasRenderingContext2D::Render) would work, but I don't have an environment
> to test that in.
> 

bug 337214 is filed.

Comment 15

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Doesn't affect the 1.8.0.x branch, marking bug blocking appropriately.  (But do we use GDI SVG anywhere on the 1.8 branch?  I thought that was all cairo now.)

Comment 16

[Roy Tam]

(In reply to comment #15)
> Doesn't affect the 1.8.0.x branch, marking bug blocking appropriately.  (But do
> we use GDI SVG anywhere on the 1.8 branch?  I thought that was all cairo now.)
> 

Sorry for mis-marking this. Originally I want to mark blocking Firefox 2.0a1 but there's no option for that.

Comment 17

[tor]

(In reply to comment #15)
> Doesn't affect the 1.8.0.x branch, marking bug blocking appropriately.  (But do
> we use GDI SVG anywhere on the 1.8 branch?  I thought that was all cairo now.)

While we didn't end up shipping it, the GDI+ backend code is still in the 1.8 branches.

Comment 18

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

And we've already shipped alpha1 some weeks ago, so...

(More in the new bug.)

Comment 19

[Adam Guthrie]

*** Bug 337319 has been marked as a duplicate of this bug. ***

Comment 20

[Mike Beltzner [:beltzner, not reading bugmail]]

Er, wait - I just marked this [checkin needed] but vlad's comment 11 makes me think it might be checked in already. If it is, please mark fixed1.8.1 in the keywords.

Comment 21

[Doug Shelton]

BeOS branch builds have same bustage as OS/2.  Bug 343365 created.