Bug 17719 – Wx require toDataURL implementations

• Component: DOM
• Status: RESOLVED WONTFIX

[Sam Weinig]

Now that there is a CG implementation of canvas's toDataURL method, we should also implement it for the rest of the ports.  For Qt, it should be trivial to us QImageWriter.  For the other ports we will probably use at the very least libpng.

[Sam Weinig]

Created attachment 19603 [details]
Entirely untested implementation for Qt

[Alp Toker]

For Cairo I think we can get away with using this:

cairo_status_t      cairo_surface_write_to_png_stream   (cairo_surface_t *surface,
                                                         cairo_write_func_t write_func,
                                                         void *closure);

Mozilla's toDataURL implementation has a full-blown libpng-based encoder we could borrow from if this strategy doesn't work out.

[Dirk Schulze]

Created attachment 23463 [details]
toDataURL implementation in Cairo

This is the implementation of toDataURL in Cairo. It only supports png at the moment. We can try to support more formats later.

[Oliver Hunt]

Comment on attachment 23463 [details]
toDataURL implementation in Cairo

This patch basically looks correct the issue is that you ignore the mimeType argument to toDataURL, and always encode as image/png, but do not report this

i think you could get away with:
+    ASSERT(mimeType == "image/png");
+    return String::format("data:image/png;base64,%s", out.data());

[Dirk Schulze]

Created attachment 23464 [details]
toDataURL implementation in Cairo

Fixed the issue with mimetypes.

[Eric Seidel (no email)]

Comment on attachment 23464 [details]
toDataURL implementation in Cairo

Looks good to me.

static cairo_status_t writeFunction(void *closure, const unsigned char *data, unsigned int length)
98102 {
99      notImplemented();
100      return String();
 103     Vector<char> *in = (Vector<char> *) closure;
 104     in->append(data, length);
 105     return CAIRO_STATUS_SUCCESS;
 106 }

Has style issues.  * placement, and the c-style cast (Vector<char> *) should be a reinterpret_cast<Vector<char>*> instead.  Unfortunately you don't yet have commit-bit :( so I'll ask you to post another patch (which I don't need to see, but is needed for someone else to land).

[Dirk Schulze]

Created attachment 23465 [details]
toDataURL implementation in Cairo

fixes the c-style issue.

[Oliver Hunt]

Comment on attachment 23465 [details]
toDataURL implementation in Cairo

looks good, r=me

[Alp Toker]

(In reply to comment #8)
> (From update of attachment 23465 [details] [edit])
> looks good, r=me
> 

Cairo patch landed in r36508 (needed a couple of tweaks to build and not assert in debug builds, and also added the necessary entry in supportedImageMIMETypesForEncoding).

Can we close this bug?

[Dirk Schulze]

(In reply to comment #9)
> Can we close this bug?

No, the Qt and wx-implementations are missing.

[Holger Freyther]

There is a Qt implementation now. WX is missing.

[Darin Adler]

Comment on attachment 23465 [details]
toDataURL implementation in Cairo

Clearing the review flag since this patch was landed.

[Jan Alonzo]

Changed subject and removed Cairo, Gtk keyword as the patches for these have already landed. Adding in Wx instead.

[Martin Robinson]

The Wx port is no longer in trunk.