Avoid calling Math.pow if possible. #11963
Conversation
tamuratak
changed the title
tamuratak
force-pushed
the
srgb_conv
branch
2 times, most recently
from
dd974cd to
5a267a1
Compare
There was a problem hiding this comment.
While #11963 (comment) contains a lot of nice information/context, please make sure that that information is also present in the actual commit message as well.
That way the information is generally available on e.g. the Git command line too (which is often useful when working with the code locally or when bisecting something). Also, explicitly mentioning that this applies to the CalRGB colorspace in the title/commit message probably doesn't hurt either :-)
| @@ -1057,6 +1057,9 @@ const CalRGBCS = (function CalRGBCSClosure() { | |||
| if (color <= 0.0031308) { | |||
| return adjustToRange(0, 1, 12.92 * color); | |||
| } | |||
| if (color >= 0.99554525) { | |||
There was a problem hiding this comment.
Please add a comment above this line, explaining exactly how this value was determined!
As-is, this line looks quite wrong until you've actually read #11963 (comment)
|
Looks good with the comment above addressed. |
Avoid calling Math.pow if possible when calculating the transfer function of the CalRGB color space since calling Math.pow is expensive. If the value of color is larger than the threshold, 0.99554525, the final result of the transform is larger that 254.5 since ((1 + 0.055) * 0.99554525 ** (1 / 2.4) - 0.055) * 255 === 254.50000003134699
|
Thank you for your responses. I have edited as requested. |
|
/botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @timvandermeij received. Current queue size: 0 Live output at: http://54.67.70.0:8877/b79975484f4a0ad/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @timvandermeij received. Current queue size: 0 Live output at: http://54.215.176.217:8877/18d9e571f1c3cf1/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.67.70.0:8877/b79975484f4a0ad/output.txt Total script time: 25.67 mins
Image differences available at: http://54.67.70.0:8877/b79975484f4a0ad/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://54.215.176.217:8877/18d9e571f1c3cf1/output.txt Total script time: 29.96 mins
Image differences available at: http://54.215.176.217:8877/18d9e571f1c3cf1/reftest-analyzer.html#web=eq.log |
|
/botio-linux preview |
From: Bot.io (Linux m4)ReceivedCommand cmd_preview from @timvandermeij received. Current queue size: 0 Live output at: http://54.67.70.0:8877/e647bb4d96be1fc/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/e647bb4d96be1fc/output.txt Total script time: 3.41 mins Published |
|
Nice work; thank you for figuring this out! |
Related to #9581.
When loading a high-resolution image, I have found that the most time-consuming part is converting sRGB to RGB. Especially calling
Math.powis expensive. The following profile of Chrome is the one when loading the PDF file of #9581.In this PR, we avoid calling
Math.powif the value ofcoloris close enough to1. The threshold is determined to assure that the result is larger than254.5.The benchmark result for the PDF file of #9581 is significant.
One of other possible solutions is to avoid converting sRGB to RGB if not necessary. I think it is not necessary when we just display images without transparency operations.