[Webkit-unassigned] [Bug 37304] WebKit does not support jpeg qulaity in canvas.toDataURL

[bugzilla-daemon at webkit.org]

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





--- Comment #19 from Leo Yang <leo.yang at torchmobile.com.cn>  2010-04-26 19:35:00 PST ---
(In reply to comment #18)
> (From update of attachment 54253 [details])
> > +#include <wtf/MathExtras.h>
> 
> No need for MathExtras.h (see below).
> 
> > +    double quality = 1.0;
> > +    if (args.size() > 1) {
> > +        JSValue v = args.at(1);
> > +        if (v.isNumber())
> > +            quality = v.toNumber(exec);
> > +        if (quality > 1.0 || quality < 0.0 || !isfinite(quality))
> > +            quality = 1.0;
> > +    }
> 
> There is a simpler, cleaner way to write this.
> 
>     double quality = args.at(1).toNumber(exec);
>     if (!(quality >= 0 && quality < 1))
>         quality = 1;
> 
> You don't need all those if statements. This correctly handles NAN, infinity,
> missing arguments, and the like. A NAN will return false from any comparison.
> 
HTML 5 draft spec reuires quality must be with number type. So the simpler way
can not handle one case in which quality is a numeric like string (e.g.
String('0.01'), per the spec, it should be converted to 100% quality)

> > +#include "PlatformString.h"
> >  
> >  #include <wtf/OwnPtr.h>
> >  #include <wtf/Noncopyable.h>
> > @@ -41,7 +43,6 @@ class FloatSize;
> >  class GraphicsContext;
> >  class ImageBuffer;
> >  class IntPoint;
> > -class String;
> 
> There is no need to include the header here in CanvasSurface.h. You can pass a
> const String& through to another const String& without including the definition
> of the String class.
I had thought it need not to include PlatformString.h, but CanvasSurface.cpp
doesn't compile by gcc-4.3.3-Ubuntu. I think the reson is that inline function
return non-reference type. So If I inlcude PlatformString.h in CavasSurface.cpp
before line #include "CanvasSurface.h", it's a bit violation of code style.

> 
> > -        DOMString toDataURL(in [ConvertUndefinedOrNullToNullString] DOMString type)
> > +        // HTML5 draft spec: DOMString toDataURL(DOMString type, ...);
> > +        [Custom] DOMString toDataURL(in [ConvertUndefinedOrNullToNullString] DOMString type)
> >              raises(DOMException);
> 
> I don't understand this comment. You should leave it out unless it has
> something specific to say.
> 
OK. Will remove it

> > -    if (!m_data.m_pixmap.save(&buffer, mimeType.substring(sizeof "image").utf8().data()))
> > +    if (!m_data.m_pixmap.save(&buffer, mimeType.substring(sizeof "image").utf8().data(), quality*100+0.5))
> 
> This is not the correct way to scale a [0,1] floating point value to a [0,100]
> integer value. The correct way is "quality * nextafter(100, 0)".
quality * nextafter(100, 0) is not bad. But quality * 100 + 0.5 should be
faster than the previous one, and the two ways has same precsion. Regard as
correctness, if quality = 0.9995, the result is 99 if it adopts the previous
way, but I think 100 is more resonable.

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