Bug 54915 – Remove unused JPEGImageEncoder code

• Component: New Bugs
• Status: RESOLVED DUPLICATE

[Alice Boxhall]

Remove unused JPEGImageEncoder code

[Alice Boxhall]

Created attachment 83238 [details]
Patch

[Peter Kasting]

Do any build files (e.g. *.gyp) need to be touched?

[Alice Boxhall]

A grep through WebCore found nothing referring to it.

[Adam Barth]

Why was it added?

[Adam Barth]

Added in http://trac.webkit.org/changeset/67606

[Adam Barth]

@yong & @staikos: Do we need this code?  It seems unused.

[Peter Kasting]

(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?

[Eric Seidel (no email)]

Comment on attachment 83238 [details]
Patch

rs=me, assuming you didn't break any builds.

[George Staikos]

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.

[Yong Li]

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.

[Peter Kasting]

(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?

[Yong Li]

(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/

[Peter Kasting]

(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.

[Yong Li]

(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.

[Mihai Parparita]

(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.

[Peter Kasting]

(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.

[Adam Barth]

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.

[Ahmad Saleem]

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!

[Alexey Proskuryakov]

Thank you for checking!

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