[Webkit-unassigned] [Bug 49365] [chromium] Add canvas.toDataURL("image/jpeg", quality) support

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=49365


Stephen White <senorblanco at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |senorblanco at chromium.org




--- Comment #16 from Stephen White <senorblanco at chromium.org>  2010-12-02 10:56:12 PST ---
Looks good. (Comments below are more conversational in nature :).

(In reply to comment #15)
> (In reply to comment #8)
> 
> > I don't know if it helps for this particular test, but some of the canvas/philip tests require a different unpremultiply that the one implemented in SkUnPreMultiply, in order to achieve roundtrip lossless conversion on a certain set of colours.  See http://trac.webkit.org/changeset/71760, function mulDiv255Ceil() in ImageBufferSkia.cpp.  The test was canvas/philip/tests/2d.imageData.put.unchanged.html.  If this does fix the problem, it would be
> ideal if you could upstream this function to Skia.
> 
> So mulDiv255Ceil() appears to premultiply, yes?  In my case, I'm doing the opposite -- I must unpremultiply much like getImageData().

Oh, you're right.  In that case, forget everything I said.  :)

> I note that
> getImageData() uses color-component * 255 / alpha to unpremultiply RGB values.  I believe SkUnPreMultiply does exactly that using a lookup table to
> avoid integer division.  Please check, because I wondered why getImageData() uses an integer divide.

I think I'm to blame, actually:  see r50408, where I ripped out SkPMColorToColor() and replaced it with integer divides (as the CG path does).  If I recall correctly, SkPMColorToColor() rounds, while the CG path truncates, and the latter behaviour was required to get us to pass the layout test.  (It also looks like the LUT in SkPMColorToColor is only accurate to with one lsb, but I could be wrong about that).  It should be possible to implement a rounding LUT that behaves the same as the CG path, but it would probably need to be bigger to be fully accurate.

> The test I mentioned, canvas/philip/tests/toDataURL.jpeg.alpha.html, suggests we should not unpremultiply at all, and instead ignore the alpha
> channel and JPEG encode the premultiplied RGB.  Firefox and Safari unpremultiply first and JPEG encode the unpremultiplied RGB.  Don't know who's
> right here, I asked ian to clarify the spec - http://www.w3.org/Bugs/Public/show_bug.cgi?id=11431
> 
> 
> > > WebCore/platform/image-encoders/skia/JPGImageEncoder.cpp:107
> > > +            SkColor unmultiplied = SkUnPreMultiply::PMColorToColor(*pixel++);
> > 
> > See above:  check if mulDiv255Ceil() fixes the JPEG+alpha case.
> 
> see above.
> 
> > > WebCore/platform/image-encoders/skia/JPGImageEncoder.cpp:117
> > > +        out->m_buffer.resize(8192);
> > 
> > This magic value should be in a constant.
> 
> done.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.