Use 3 param method for converting r,g,b into css string. #5449
Conversation
|
i have the feeling one could avoid a few array allocations in colorspace as well, but that seemed to be way to much change for this PR. |
|
|
||
| // Enlighten the color (70%) | ||
| var r = Math.round(color[0] * 255); | ||
| r = Math.round((255 - r) * 0.7) + r; |
There was a problem hiding this comment.
Is there any reason for the double rounding here? We could just as well use
var r = Math.round(255 * (0.3 * color[0] + 0.7));
There was a problem hiding this comment.
to be hones, I did not check. i just unrolled the loop. I suspect this does not really matter much. its in annotations code. the rounded result could be different however.
There was a problem hiding this comment.
Can you add a constant here, e.g. var BACKGROUND_ENLIGHT = 0.7 (placed after the comment above) to not use magical 0.7 three times?
the rounded result could be different however.
I don't really like having round in color operations. Also 255 multiplication can be moved just before makeCssRgb, e.g.:
var r = BACKGROUND_ENLIGHT * (1.0 - color[0]) + color[0];
var g = BACKGROUND_ENLIGHT * (1.0 - color[1]) + color[1];
var b = BACKGROUND_ENLIGHT * (1.0 - color[2]) + color[2];
content.style.backgroundColor = Util.makeCssRgb((r * 255) | 0, (g * 255) | 0, (b * 255) | 0);
There was a problem hiding this comment.
done. push incoming
This avoids creation of temporary arrays to pass them into the util method. Also using "arguments" is more expensive then passing in 3 parameters.
CodingFabian
force-pushed
the
preferParamsOverArguments
branch
from
918d680
to
ceffeab
Compare
|
/botio-linux preview |
From: Bot.io (Linux)ReceivedCommand cmd_preview from @timvandermeij received. Current queue size: 0 Live output at: http://107.21.233.14:8877/ce5a94888e2a02b/output.txt |
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/ce5a94888e2a02b/output.txt Total script time: 0.77 mins Published
|
|
/botio test |
From: Bot.io (Linux)ReceivedCommand cmd_test from @yurydelendik received. Current queue size: 0 Live output at: http://107.21.233.14:8877/c914bc37da211fb/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @yurydelendik received. Current queue size: 0 Live output at: http://107.22.172.223:8877/99447bb9b37e718/output.txt |
From: Bot.io (Windows)FailedFull output at http://107.22.172.223:8877/99447bb9b37e718/output.txt Total script time: 3.06 mins
Image differences available at: http://107.22.172.223:8877/99447bb9b37e718/reftest-analyzer.html#web=eq.log |
|
/botio-windows test |
From: Bot.io (Windows)ReceivedCommand cmd_test from @yurydelendik received. Current queue size: 0 Live output at: http://107.22.172.223:8877/629a6a8d337cdda/output.txt |
From: Bot.io (Windows)FailedFull output at http://107.22.172.223:8877/629a6a8d337cdda/output.txt Total script time: 16.55 mins
|
From: Bot.io (Linux)FailedFull output at http://107.21.233.14:8877/c914bc37da211fb/output.txt Total script time: 22.64 mins
Image differences available at: http://107.21.233.14:8877/c914bc37da211fb/reftest-analyzer.html#web=eq.log |
|
patch looks good, thanks. (looks like ff36/linux fixed gradient color, non related to this change) /botio makeref |
From: Bot.io (Linux)ReceivedCommand cmd_makeref from @yurydelendik received. Current queue size: 0 Live output at: http://107.21.233.14:8877/0b4ce0ae57d403a/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_makeref from @yurydelendik received. Current queue size: 0 Live output at: http://107.22.172.223:8877/a053d9b93912359/output.txt |
Use 3 param method for converting r,g,b into css string.
From: Bot.io (Windows)SuccessFull output at http://107.22.172.223:8877/a053d9b93912359/output.txt Total script time: 16.69 mins
|
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/0b4ce0ae57d403a/output.txt Total script time: 22.25 mins
|
This avoids creation of temporary arrays to pass them into the util
method. Also using "arguments" is more expensive then passing in 3
parameters.
FYI, here the benchmark which shows the cost of using 'arguments':
http://jsperf.com/arguments-or-explicit-use