[Webkit-unassigned] [Bug 38845] Canvas color property getters should serialize them according to spec

[bugzilla-daemon at webkit.org]

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





--- Comment #12 from Dirk Schulze <krit at webkit.org>  2010-05-11 23:09:02 PST ---
(From update of attachment 55797)
The color parsing code looks better now.

> +String serializedCanvasColor(const Color& color)
> +{
> +    // FIXME: What should we do with invalid colors?
> +    if (!color.isValid())
> +        return String("rgba(0, 0, 0, 0.0)");
> +
> +    if (color.hasAlpha()) {
> +        // Match Gecko (0.0 for zero, 5 decimals for anything else)
> +        if (!color.alpha())
> +            return String::format("rgba(%u, %u, %u, 0.0)", color.red(), color.green(), color.blue());
> +        return String::format("rgba(%u, %u, %u, %.5f)", color.red(), color.green(), color.blue(), color.alpha() / 255.0f);
>      }
> +
> +    return String::format("#%02x%02x%02x", color.red(), color.green(), color.blue());
I'm sorry, I missread the spec. The spec talks about 1 _or more digits_ for fractional part. Nevertheless, if alpha is zero, you just use one digit after the point, if alpha is >0 but <1 you use 5? Shouldn't we have the same count of digits? What is FF and Opera doing here? Maybe we should aline to them here.
In my opinion, the fractional part should allways have the same count of digits, this makes the parsing easier for the user. If you want, you can write a mail to whatwg at lists.whatwg.org for clarification.
Another point is String::format(). This is not language aware on some posix-systems (point or comma on fractional numbers). See bug 18994. You can also ask Evan Martin for a better solution here.

> +String serializedCanvasColor(const Color& color);
> +
>      class CanvasGradient;
>      class CanvasPattern;
>      class GraphicsContext;
>  
>      class CanvasStyle : public RefCounted<CanvasStyle> {
>      public:
> +        static PassRefPtr<CanvasStyle> create(const Color& color) { return adoptRef(new CanvasStyle(color)); }
>          static PassRefPtr<CanvasStyle> create(const String& color) { return adoptRef(new CanvasStyle(color)); }
>          static PassRefPtr<CanvasStyle> create(float grayLevel) { return adoptRef(new CanvasStyle(grayLevel)); }
>          static PassRefPtr<CanvasStyle> create(const String& color, float alpha) { return adoptRef(new CanvasStyle(color, alpha)); }
> @@ -45,14 +50,17 @@ namespace WebCore {
>          static PassRefPtr<CanvasStyle> create(PassRefPtr<CanvasGradient> gradient) { return adoptRef(new CanvasStyle(gradient)); }
>          static PassRefPtr<CanvasStyle> create(PassRefPtr<CanvasPattern> pattern) { return adoptRef(new CanvasStyle(pattern)); }
>  
> -        String color() const { return m_color; }
> +        String color() const { return serializedCanvasColor(m_color); }
>          CanvasGradient* canvasGradient() const { return m_gradient.get(); }
>          CanvasPattern* canvasPattern() const { return m_pattern.get(); }
>  
>          void applyFillColor(GraphicsContext*);
>          void applyStrokeColor(GraphicsContext*);
>  
> +        bool isValid() const;
...
the complete class shouldn't be indented, please also put serializedCanvasColor after the class definition.

The code looks much better. I would like to see Oliver to give some comments on this bug.

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