Summary: | [GTK] Separate image encoding from dataURL construction | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | noel gordon <noel.gordon> | ||||||
Component: | Canvas | Assignee: | noel gordon <noel.gordon> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | mrobinson, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
noel gordon
2012-04-04 06:07:41 PDT
Created attachment 135575 [details]
Patch
Comment on attachment 135575 [details]
Patch
This seems like a nice refactor, but is there some reason that it will be useful in the future?
Comment on attachment 135575 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=135575&action=review > Source/WebCore/platform/graphics/gtk/ImageBufferGtk.cpp:70 > + base64Encode(reinterpret_cast<const char*>(buffer.get()), bufferSize, base64Data); It seems unusual that we need this reinpterpret_cast here. Perhaps you could try removing this before landing? Happy to try removing the reinterpret_cast<> and try landing. Created attachment 136382 [details]
Patch for landing
Other questions: does the SupportedImageMIMETypeForEncoding registry for GTK contain items that don't begin with "image/"? I'd be surprised, and it seems these lines are redundant, due to the ASSERT at the start of ImageBuffer::toDataURL() ... http://trac.webkit.org/browser/trunk/Source/WebCore/platform/graphics/gtk/ImageBufferGtk.cpp#L39 39 if (!mimeType.startsWith("image/")) 40 return "data:,"; Oh, and is gdk_pixbuf_save_to_buffer() thread-safe? I don't see anything in the docs about that. Comment on attachment 136382 [details] Patch for landing Clearing flags on attachment: 136382 Committed r113742: <http://trac.webkit.org/changeset/113742> All reviewed patches have been landed. Closing bug. (In reply to comment #7) > Other questions: does the SupportedImageMIMETypeForEncoding registry for GTK contain items that don't begin with "image/"? I'd be surprised, and it seems these lines are redundant, due to the ASSERT at the start of ImageBuffer::toDataURL() ... > > http://trac.webkit.org/browser/trunk/Source/WebCore/platform/graphics/gtk/ImageBufferGtk.cpp#L39 > > 39 if (!mimeType.startsWith("image/")) > 40 return "data:,"; Filed bug 83657 about that if you'd like to review. |