Bug 17719 - Wx require toDataURL implementations
: Wx require toDataURL implementations
Status: NEW
Product: WebKit
Classification: Unclassified
Component: HTML DOM
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To: Nobody
: Wx
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-03-07 19:13 PST by Sam Weinig
Modified: 2010-06-10 16:10 PDT (History)
5 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Sam Weinig 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 Sam Weinig 2008-03-07 19:16:59 PST
Created attachment 19603 [details]
Entirely untested implementation for Qt
Comment 2 Alp Toker 2008-08-11 17:45:00 PDT
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 Dirk Schulze 2008-09-15 23:55:01 PDT
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.
Comment 4 Oliver Hunt 2008-09-16 00:04:39 PDT
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());
Comment 5 Dirk Schulze 2008-09-16 00:09:53 PDT
Created attachment 23464 [details]
toDataURL implementation in Cairo

Fixed the issue with mimetypes.
Comment 6 Eric Seidel 2008-09-16 00:36:02 PDT
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).
Comment 7 Dirk Schulze 2008-09-16 01:03:54 PDT
Created attachment 23465 [details]
toDataURL implementation in Cairo

fixes the c-style issue.
Comment 8 Oliver Hunt 2008-09-16 01:08:10 PDT
Comment on attachment 23465 [details]
toDataURL implementation in Cairo

looks good, r=me
Comment 9 Alp Toker 2008-09-16 10:01:48 PDT
(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 Dirk Schulze 2008-09-16 10:21:09 PDT
(In reply to comment #9)
> Can we close this bug?

No, the Qt and wx-implementations are missing.
Comment 11 Holger Freyther 2008-09-26 13:07:39 PDT
There is a Qt implementation now. WX is missing.
Comment 12 Darin Adler 2008-10-12 15:12:54 PDT
Comment on attachment 23465 [details]
toDataURL implementation in Cairo

Clearing the review flag since this patch was landed.
Comment 13 Jan Alonzo 2008-10-18 20:29:13 PDT
Changed subject and removed Cairo, Gtk keyword as the patches for these have already landed. Adding in Wx instead.