[Webkit-unassigned] [Bug 42965] CSS: Add fast-path for rgba() color parsing

[bugzilla-daemon at webkit.org]

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


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #62690|review?                     |review-
               Flag|                            |




--- Comment #5 from Darin Adler <darin at apple.com>  2010-07-27 10:22:46 PST ---
(From update of attachment 62690)
> +static inline bool parseAlphaDouble(const UChar*& string, const UChar* end, UChar terminator, double& value)
> +{
> +    int length = end - string;
> +    if (!length)
> +        return false;
> +
> +    Vector<char, 256> bytes(length + 1);
> +    for (int i = 0; i < length; ++i) {
> +        if (!isASCIIDigit(string[i]) && string[i] != '.' && string[i] != ')')
> +            return false;
> +        bytes[i] = string[i];
> +    }
> +    bytes[length] = '\0';
> +    char* foundTerminator;
> +    value = WTF::strtod(bytes.data(), &foundTerminator);
> +    value = max(0.0, min(value, 1.0));
> +    string += (foundTerminator - bytes.data()) + 1;
> +    return *foundTerminator == terminator;
> +}

For the typical alpha values like 0, 1, and the tenths and simple powers of too we can do a *lot* faster than calling the general purpose strtod function every time, perhaps even avoiding floating point math altogether.

> +        rgb = makeRGBA(red, green, blue, static_cast<int>(lroundf(255.0f * static_cast<float>(alpha))));

I don’t think it’s correct to multiply the alpha value by 255. At other call sites you'll see code more like this:

    nextafter(256, 0) * alpha

We need some test cases that cover the edge cases, the boundaries between alpha of 254 and 255 for example, and alpha of 0 and 1. The relevant functions to look at are CSSParser::parseColorParameters and the various functions in the Color.cpp file. There may be some cases where multiplying by 255 is right. Maybe we can also do code sharing.

If the test cases prove that you are not changing behavior, then I suppose you could put this patch up for review again.

review- because of the 255/256 thing.

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