Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve performance canvas heatmap #574

Merged
merged 17 commits into from
May 13, 2016
Merged

Improve performance canvas heatmap #574

Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
e9592b2
Using typed array to increase performance
aashish24 Apr 20, 2016
19557f8
Using event data for transformation
aashish24 Apr 21, 2016
fadaa91
Fix intermediate transforms and update delays on the heatmap.
manthey May 3, 2016
037c106
A minor speed-up can be obtained by directly setting transforms rathe…
manthey May 3, 2016
d0cb87c
Rename origin to gcsOrigin to make it more obvious it isn't in displa…
manthey May 4, 2016
5e23638
Reduce number of function calls for a minor speed improvement.
manthey May 4, 2016
47e461a
Precompute gcs positions for heatmaps.
manthey May 5, 2016
f2c6a68
Loops can be faster than functions.
manthey May 5, 2016
f52faa5
Fix some issues setting max and min intensity.
manthey May 6, 2016
b4801bd
Fixed Endian check
aashish24 May 11, 2016
8f25c0a
Do not use typed array on big-endian systems
aashish24 May 12, 2016
ec76824
Simplify _colorize.
manthey May 12, 2016
a1db91b
Remove opacity as a heatmap feature property.
manthey May 12, 2016
3b8fa85
Switch to a single var statement.
manthey May 12, 2016
ee6500e
Complete coverage of src/heatmapFeature.js.
manthey May 12, 2016
62b0e77
Add tests for _animatePan.
manthey May 12, 2016
10da56c
Merge branch 'master' into improve_performance_canvas_heatmap
aashish24 May 13, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 17 additions & 47 deletions src/canvas/heatmapFeature.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ var canvas_heatmapFeature = function (arg) {
var geo_event = require('../event');

var m_this = this,
m_typedBuffer = null,
m_typedClampedBuffer = null,
m_typedBufferData = null,
m_typedBuffer,
m_typedClampedBuffer,
m_typedBufferData,
m_heatMapPosition,
s_exit = this._exit,
s_init = this._init,
Expand Down Expand Up @@ -124,54 +124,23 @@ var canvas_heatmapFeature = function (arg) {
* @protected
*/
////////////////////////////////////////////////////////////////////////////
this._colorize = function (context2d, width, height, imageData, gradient) {
var isLittleEndian = true, i, j, index, pixels;

// Determine whether Uint32 is little- or big-endian.
if (!m_typedBuffer || (m_typedBuffer.length !== imageData.data.length)) {
m_typedBuffer = new ArrayBuffer(imageData.data.length);
this._colorize = function (pixels, gradient) {
var grad = new Uint32Array(gradient.buffer);
var i, j, k, pixlen = pixels.length;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we remove the other var (to be consistent). Also I was told that fewer vars declarations is better.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll make it so there is just one statement.

I'd be shocked if there is a measurable performance difference between single and multiple var statements.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The one var rule comes from jslint. I think the justification for it is preventing C programmers from misunderstanding how scoping rules work in javascript. My preference is just to keep all of the variable declarations at the top of the function, but I don't think enforcing a one var rule is worthwhile.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Roger that. I think most of our code is one var and declarations are on top of the function. As far as we are consistent, I am good.

if (!m_typedBuffer || m_typedBuffer.length !== pixlen) {
m_typedBuffer = new ArrayBuffer(pixlen);
m_typedClampedBuffer = new Uint8ClampedArray(m_typedBuffer);
m_typedBufferData = new Uint32Array(m_typedBuffer);
}

m_typedBufferData[1] = 0x0a0b0c0d;

isLittleEndian = true;
if (m_typedClampedBuffer[4] === 0x0a &&
m_typedClampedBuffer[5] === 0x0b &&
m_typedClampedBuffer[6] === 0x0c &&
m_typedClampedBuffer[7] === 0x0d) {
isLittleEndian = false;
}

if (isLittleEndian) {
i = 0;
for (j = 0; j < (width * height * 4); j += 4) {
index = imageData.data[j + 3] * 4;
if (index) {
m_typedBufferData[i] =
(gradient[index + 3] << 24) |
(gradient[index + 2] << 16) |
(gradient[index + 1] << 8) |
gradient[index];
}
i += 1;
}
imageData.data.set(m_typedClampedBuffer);
} else {
pixels = imageData.data;
for (i = 0; i < pixels.length; i += 4) {
j = pixels[i + 3] * 4;
if (j) {
pixels[i] = gradient[j];
pixels[i + 1] = gradient[j + 1];
pixels[i + 2] = gradient[j + 2];
pixels[i + 3] = gradient[j + 3]
}
for (i = 3, k = 0; i < pixlen; i += 4, k += 1) {
// Get opacity from the temporary canvas image and look up the final
// value from gradient
j = pixels[i];
if (j) {
m_typedBufferData[k] = grad[j];
}
}

context2d.putImageData(imageData, 0, 0);
pixels.set(m_typedClampedBuffer);
};

////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -212,7 +181,8 @@ var canvas_heatmapFeature = function (arg) {
}
canvas = layer.canvas()[0];
pixelArray = context2d.getImageData(0, 0, canvas.width, canvas.height);
m_this._colorize(context2d, canvas.width, canvas.height, pixelArray, m_this._grad);
m_this._colorize(pixelArray.data, m_this._grad);
context2d.putImageData(pixelArray, 0, 0);

m_heatMapPosition = {
zoom: map.zoom(),
Expand Down