Bug 17719 - Wx require toDataURL implementations
: Wx require toDataURL implementations
Status: NEW
: WebKit
HTML DOM
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To:
:
: Wx
:
:
  Show dependency treegraph
 
Reported: 2008-03-07 19:13 PST by
Modified: 2010-06-10 16:10 PST (History)


Attachments
Entirely untested implementation for Qt (3.01 KB, patch)
2008-03-07 19:16 PST, Sam Weinig
no flags Review Patch | Details | Formatted Diff | Diff
toDataURL implementation in Cairo (2.31 KB, patch)
2008-09-15 23:55 PST, Dirk Schulze
no flags Review Patch | Details | Formatted Diff | Diff
toDataURL implementation in Cairo (2.33 KB, patch)
2008-09-16 00:09 PST, Dirk Schulze
eric: review-
Review Patch | Details | Formatted Diff | Diff
toDataURL implementation in Cairo (2.42 KB, patch)
2008-09-16 01:03 PST, Dirk Schulze
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2008-03-07 19:13:00 PST
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.
------- Comment #1 From 2008-03-07 19:16:59 PST -------
Created an attachment (id=19603) [details]
Entirely untested implementation for Qt
------- Comment #2 From 2008-08-11 17:45:00 PST -------
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.
------- Comment #3 From 2008-09-15 23:55:01 PST -------
Created an attachment (id=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.
------- Comment #4 From 2008-09-16 00:04:39 PST -------
(From update of attachment 23463 [details])
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());
------- Comment #5 From 2008-09-16 00:09:53 PST -------
Created an attachment (id=23464) [details]
toDataURL implementation in Cairo

Fixed the issue with mimetypes.
------- Comment #6 From 2008-09-16 00:36:02 PST -------
(From update of attachment 23464 [details])
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).
------- Comment #7 From 2008-09-16 01:03:54 PST -------
Created an attachment (id=23465) [details]
toDataURL implementation in Cairo

fixes the c-style issue.
------- Comment #8 From 2008-09-16 01:08:10 PST -------
(From update of attachment 23465 [details])
looks good, r=me
------- Comment #9 From 2008-09-16 10:01:48 PST -------
(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?
------- Comment #10 From 2008-09-16 10:21:09 PST -------
(In reply to comment #9)
> Can we close this bug?

No, the Qt and wx-implementations are missing.
------- Comment #11 From 2008-09-26 13:07:39 PST -------
There is a Qt implementation now. WX is missing.
------- Comment #12 From 2008-10-12 15:12:54 PST -------
(From update of attachment 23465 [details])
Clearing the review flag since this patch was landed.
------- Comment #13 From 2008-10-18 20:29:13 PST -------
Changed subject and removed Cairo, Gtk keyword as the patches for these have already landed. Adding in Wx instead.