Bug 54915 - Remove unused JPEGImageEncoder code
Summary: Remove unused JPEGImageEncoder code
Status: RESOLVED DUPLICATE of bug 186365
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 54522
Blocks:
  Show dependency treegraph
 
Reported: 2011-02-21 16:24 PST by Alice Boxhall
Modified: 2022-06-02 09:05 PDT (History)
9 users (show)

See Also:


Attachments
Patch (7.30 KB, patch)
2011-02-21 16:25 PST, Alice Boxhall
abarth: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alice Boxhall 2011-02-21 16:24:59 PST
Remove unused JPEGImageEncoder code
Comment 1 Alice Boxhall 2011-02-21 16:25:26 PST
Created attachment 83238 [details]
Patch
Comment 2 Peter Kasting 2011-02-21 16:50:56 PST
Do any build files (e.g. *.gyp) need to be touched?
Comment 3 Alice Boxhall 2011-02-21 17:02:40 PST
A grep through WebCore found nothing referring to it.
Comment 4 Adam Barth 2011-02-21 18:12:35 PST
Why was it added?
Comment 5 Adam Barth 2011-02-21 18:13:14 PST
Added in http://trac.webkit.org/changeset/67606
Comment 6 Adam Barth 2011-02-21 18:13:52 PST
@yong & @staikos: Do we need this code?  It seems unused.
Comment 7 Peter Kasting 2011-02-21 23:51:51 PST
(In reply to comment #6)
> @yong & @staikos: Do we need this code?  It seems unused.

Also: Is the PNG encoder unused as well, or is someone using that?
Comment 8 Eric Seidel (no email) 2011-02-24 03:11:31 PST
Comment on attachment 83238 [details]
Patch

rs=me, assuming you didn't break any builds.
Comment 9 George Staikos 2011-02-24 06:21:36 PST
We'll likely be re-adding this commit again, as we are using.  Still in the process of merging.  I'm not sure why the urge to remove it.
Comment 10 Yong Li 2011-02-24 08:12:43 PST
Why do we want to remove it? It allows ImageBuffer to easily support saving images with image/jpeg format. The new ImageBufferOpenVG.cpp patch will use it, which is still in process. https://bugs.webkit.org/show_bug.cgi?id=39230 (the old patch is obsolete). Or this feature can be put into ImageBuffer.cpp to be shared by platforms.
Comment 11 Peter Kasting 2011-02-24 10:22:25 PST
(In reply to comment #10)
> It allows ImageBuffer to easily support saving images with image/jpeg format.

Why is this useful?  How will this be exposed to users?
Comment 12 Yong Li 2011-02-24 12:41:26 PST
(In reply to comment #11)
> (In reply to comment #10)
> > It allows ImageBuffer to easily support saving images with image/jpeg format.
> 
> Why is this useful?  How will this be exposed to users?

canvas.toDataURL(type);

A web page can use this feature to convert canvas to iamge. A sample:

http://www.nihilogic.dk/labs/canvas2image/
Comment 13 Peter Kasting 2011-02-24 12:43:33 PST
(In reply to comment #12)
> (In reply to comment #11)
> > (In reply to comment #10)
> > > It allows ImageBuffer to easily support saving images with image/jpeg format.
> > 
> > Why is this useful?  How will this be exposed to users?
> 
> canvas.toDataURL(type);

Hmm.  I'm pretty sure Chrome supports this already, I wonder how it's implemented there.  Not sure who would know; abarth is already CCed, adding mihaip.
Comment 14 Yong Li 2011-02-24 12:50:11 PST
(In reply to comment #13)
> (In reply to comment #12)
> > (In reply to comment #11)
> > > (In reply to comment #10)
> > > > It allows ImageBuffer to easily support saving images with image/jpeg format.
> > > 
> > > Why is this useful?  How will this be exposed to users?
> > 
> > canvas.toDataURL(type);
> 
> Hmm.  I'm pretty sure Chrome supports this already, I wonder how it's implemented there.  Not sure who would know; abarth is already CCed, adding mihaip.

Seems most ports either leave it unimplemented or rely on (graphics)platform feature. skia port is using a PNG encoder in image-encoders/skia. The jpeg/png encoders in image-encoders are platform independent, and can provide a shared implementation for other ports.
Comment 15 Mihai Parparita 2011-02-24 12:52:06 PST
(In reply to comment #14)
> Seems most ports either leave it unimplemented or rely on (graphics)platform feature. skia port is using a PNG encoder in image-encoders/skia. The jpeg/png encoders in image-encoders are platform independent, and can provide a shared implementation for other ports.

Based on http://trac.webkit.org/changeset/75488 the Skia ports are using image-encoders/skia for JPEG too.
Comment 16 Peter Kasting 2011-02-24 13:02:38 PST
(In reply to comment #15)
> (In reply to comment #14)
> > Seems most ports either leave it unimplemented or rely on (graphics)platform feature. skia port is using a PNG encoder in image-encoders/skia. The jpeg/png encoders in image-encoders are platform independent, and can provide a shared implementation for other ports.
> 
> Based on http://trac.webkit.org/changeset/75488 the Skia ports are using image-encoders/skia for JPEG too.

Is there a compelling reason to choose a platform-dependent versus platform-independent implementation?  It seems like in principle it would be better to be consistent: either change various ports to all use the common code or have the OpenVG port (or whatever) use a mechanism more similar to the other ports.

I have no idea how this stuff works so I can't say which is better.  I'm biased towards cross-platform implementations if they don't come with disadvantages, though.
Comment 17 Adam Barth 2011-03-01 00:37:56 PST
Comment on attachment 83238 [details]
Patch

ok.  Sounds like we shouldn't remove this code quite yet.  We should either start using it or decide not to use it.  If it's still unused after some moderate period of time, we should probably remove then.
Comment 18 Ahmad Saleem 2022-06-02 08:05:27 PDT
I think this has been taken care of based on this commit - https://github.com/WebKit/WebKit/commit/142915fea4abca2327bd6ed1e651a3e8671f5d03

Since I am not able to find any "code" with JPEGImageEncoder on Github Webkit mirror, only past commits and above one is latest. Thanks!
Comment 19 Alexey Proskuryakov 2022-06-02 09:05:58 PDT
Thank you for checking!

*** This bug has been marked as a duplicate of bug 186365 ***