Bug 89130 – avoid min and max check in Color constructor when needed.

• Status: RESOLVED
• Resolution: WONTFIX
• Component: WebCore Misc.

[arno.]

Hi,

Color constructor accepts integers arguments and checks that those integers are between 0 and 255.
But sometimes, Color constructor is called with unsigned char, and those checks are therefore unneeded (for example in cairo ImageBuffer::putByteArray)

So, I'm thinking a specialized constructor for Color accepting unsigned char, and not performing min/max checks would improve performance of Color initialization in those cases.

[arno.]

Created attachment 147662 [details]
patch proposal

patch proposal. I get a 5% to 15% speedup in CanvasRenderingContext2D.putImageData in the gtk port. Color constructor does not seem to be called with signed char nor unsigned int, but makeRGBA is called with unsigned int. So, I still added Color constructor for char, unsigned char and unsigned int, just in case

[WebKit Review Bot]

Attachment 147662 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebCore/platform/graphics/Color.h:63:  The parameter name "r" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/platform/graphics/Color.h:63:  The parameter name "g" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/platform/graphics/Color.h:64:  The parameter name "r" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/platform/graphics/Color.h:64:  The parameter name "g" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/platform/graphics/Color.h:64:  The parameter name "a" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/platform/graphics/Color.h:65:  The parameter name "r" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/platform/graphics/Color.h:66:  The parameter name "r" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/platform/graphics/Color.h:66:  The parameter name "a" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/platform/graphics/Color.h:67:  The parameter name "g" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/platform/graphics/Color.h:68:  The parameter name "g" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 10 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[arno.]

(In reply to comment #2)
> Attachment 147662 [details] did not pass style-queue:

I've used this style to match RGBA32 makeRGB(int r, int g, int b) and RGBA32 makeRGBA(int r, int g, int b, int a) declarations.

[Build Bot]

Comment on attachment 147662 [details]
patch proposal

Attachment 147662 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/12951763

[Build Bot]

Comment on attachment 147662 [details]
patch proposal

Attachment 147662 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/12964316

[arno.]

Actually, I need either to add more function overloads, or modify code in many places to use makeRGBA32FromFloats instead of makeRGBA(float, float, float, float).

Alternatively, as cairo putByteArray would be the main beneficiary of the patch, I can let Color.h and Color.cpp untouched and make changes in cairo port only. So, closing this bug for now.

[arno.]

For reference, I have opened bug #89138 for a cairo-only version of this bug.