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.

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

Introduces attribute source to image traces #5075

Merged
merged 26 commits into from
Aug 31, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
1c893e2
Introduces attribute "source" to image traces featuring fast rendering
antoinerg Jun 26, 2020
d45496d
Ignores image trace's source attribute that are not data URIs
antoinerg Aug 13, 2020
f03c7ae
Adds image_labuda_droplets_source to mock_test.js
antoinerg Aug 14, 2020
2a687f2
Add "z" variable to hovertemplate in image defined by source attr
antoinerg Aug 14, 2020
aaf7cbc
Append promise of image loading to the plot api so it can wait for it
antoinerg Aug 14, 2020
e693446
In image traces, delete source attribute when it is invalid
antoinerg Aug 14, 2020
130593e
Fixes image_test to reflect that invalid source are deleted
antoinerg Aug 17, 2020
09eb1d0
Improve code style in the plot routine of image traces
antoinerg Aug 17, 2020
9b8a843
Add a workaround to image's onload event not firing when image is cached
antoinerg Aug 17, 2020
2ba0998
Introduces Lib.isIOS() to detect Apple mobile devices
antoinerg Aug 17, 2020
a2ea331
Fall back to legacy rendering of image traces for iOS devices
antoinerg Aug 17, 2020
4f807c5
Add missing semicolon
antoinerg Aug 18, 2020
7dd3b68
Fall back to legacy rendering of images traces for IE
antoinerg Aug 18, 2020
f2caed8
Loads images into an <img> instead of an <image> for image traces
antoinerg Aug 18, 2020
d91e0c1
Reintroduce image traces with source attribute in the `test-image` suite
antoinerg Aug 18, 2020
fe1d208
Improves code readability for image traces
antoinerg Aug 18, 2020
2960e55
Removes broken test following changes in how image traces check userA…
antoinerg Aug 18, 2020
3717cbe
Rename _isFrom(Z|Source) to _has(Z|Source)
antoinerg Aug 19, 2020
b586a86
Replace Labuda's photography of droplets by one from public domain
antoinerg Aug 19, 2020
d73f4c4
Add image_astronaut_source to mock_test.js
antoinerg Aug 19, 2020
18133c9
Update description of attribute `source` in images traces
antoinerg Aug 20, 2020
19ce21d
Introduce colormodel "rgba256" to image traces
antoinerg Aug 21, 2020
74574b6
Update description of attribute colormodel in image traces
antoinerg Aug 31, 2020
8c736f7
Update description of attribute source in image traces
antoinerg Aug 31, 2020
3b75921
Prefer z data over source in image traces and improve code readability
antoinerg Aug 31, 2020
66c58f6
:hocho: unreachable code in image trace defaults routine
antoinerg Aug 31, 2020
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
9 changes: 9 additions & 0 deletions src/traces/image/attributes.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,15 @@ for(var i = 0; i < cm.length; i++) {
}

module.exports = extendFlat({
source: {
valType: 'string',
role: 'info',
editType: 'calc',
description: [
'Specifies the data URI of the image to be visualized.',
'The URI consists of "data:[<media type>][;base64],<data>"'
archmoj marked this conversation as resolved.
Show resolved Hide resolved
].join(' ')
},
z: {
valType: 'data_array',
role: 'info',
Expand Down
23 changes: 21 additions & 2 deletions src/traces/image/calc.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,27 @@ var constants = require('./constants');
var isNumeric = require('fast-isnumeric');
var Axes = require('../../plots/cartesian/axes');
var maxRowLength = require('../../lib').maxRowLength;
var sizeOf = require('image-size');
var dataUri = require('../../snapshot/helpers').IMAGE_URL_PREFIX;
var Buffer = require('buffer/').Buffer; // note: the trailing slash is important!

module.exports = function calc(gd, trace) {
var h;
var w;
if(trace._isFromZ) {
h = trace.z.length;
w = maxRowLength(trace.z);
} else if(trace._isFromSource) {
var size = getImageSize(trace.source);
h = size.height;
w = size.width;
}

var xa = Axes.getFromId(gd, trace.xaxis || 'x');
var ya = Axes.getFromId(gd, trace.yaxis || 'y');

var x0 = xa.d2c(trace.x0) - trace.dx / 2;
var y0 = ya.d2c(trace.y0) - trace.dy / 2;
var h = trace.z.length;
var w = maxRowLength(trace.z);

// Set axis range
var i;
Expand Down Expand Up @@ -84,3 +96,10 @@ function makeScaler(trace) {
return c;
};
}

// Get image size
function getImageSize(src) {
var data = src.replace(dataUri, '');
var buff = new Buffer(data, 'base64');
return sizeOf(buff);
}
20 changes: 16 additions & 4 deletions src/traces/image/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,19 @@
var Lib = require('../../lib');
var attributes = require('./attributes');
var constants = require('./constants');
var dataUri = require('../../snapshot/helpers').IMAGE_URL_PREFIX;

module.exports = function supplyDefaults(traceIn, traceOut) {
function coerce(attr, dflt) {
return Lib.coerce(traceIn, traceOut, attributes, attr, dflt);
}
coerce('source');
if(traceOut.source && !traceOut.source.match(dataUri)) traceOut.source = null;
antoinerg marked this conversation as resolved.
Show resolved Hide resolved
traceOut._isFromSource = !!traceOut.source;

var z = coerce('z');
if(z === undefined || !z.length || !z[0] || !z[0].length) {
traceOut._isFromZ = !(z === undefined || !z.length || !z[0] || !z[0].length);
if(!traceOut._isFromZ && !traceOut._isFromSource) {
archmoj marked this conversation as resolved.
Show resolved Hide resolved
traceOut.visible = false;
return;
}
Expand All @@ -26,10 +32,16 @@ module.exports = function supplyDefaults(traceIn, traceOut) {
coerce('y0');
coerce('dx');
coerce('dy');
var colormodel = coerce('colormodel');

coerce('zmin', constants.colormodel[colormodel].min);
coerce('zmax', constants.colormodel[colormodel].max);
if(traceOut._isFromZ) {
coerce('colormodel');
archmoj marked this conversation as resolved.
Show resolved Hide resolved
coerce('zmin', constants.colormodel[traceOut.colormodel].min);
coerce('zmax', constants.colormodel[traceOut.colormodel].max);
} else if(traceOut._isFromSource) {
traceOut.colormodel = 'rgba';
Copy link
Contributor

@archmoj archmoj Aug 18, 2020

Choose a reason for hiding this comment

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

Hmm...
We need to update description for colormodel

colormodel: {
valType: 'enumerated',
values: cm,
dflt: 'rgb',
role: 'info',
editType: 'calc',
description: 'Color model used to map the numerical color components described in `z` into colors.'
},

Or even better if you not set this parameter this way.
What about adding an underscore variable e.g.

traceOut._colormodel = 'rgba';

here and then

var colormodel = trace._colormodel || trace.colormodel;

in calc and plot?

Copy link
Contributor

Choose a reason for hiding this comment

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

Wondering also what happens if one pass a jpeg (often has no alpha channel) instead of a png?
In that case the default should be rgba or simply rgb?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Codepen for jpeg: https://codepen.io/MojtabaSamimi/pen/zYqKOKy

This seems correct to me.

cc @alexcjohnson

Copy link
Contributor

Choose a reason for hiding this comment

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

Thinking more about this automatic default behaviour.
I still believe we should respect colormodel to be defined by the user.
And we must remove colormodel.dflt and set it during supplyDefaults.

if(traceOut._isFromZ) {
  coerce('colormodel', 'rgb');
  ...
} else if(traceOut._isFromSource) {
  coerce('colormodel', isJPEG(traceOut.source) ? 'rgb' : 'rgba');
  ...
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I still believe we should respect colormodel to be defined by the user.

Do you mean supporting colormodel hsl with say a PNG image? That seems like a weird thing to do and support. Also, it would be slow: now we would have to read all the pixels one by one and convert them into the new colorspace. cc @archmoj

Copy link
Contributor Author

Choose a reason for hiding this comment

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

traceOut.zmin = constants.colormodel[traceOut.colormodel].min;
traceOut.zmax = constants.colormodel[traceOut.colormodel].max;
}

coerce('text');
coerce('hovertext');
Expand Down
1 change: 1 addition & 0 deletions src/traces/image/event_data.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,6 @@ module.exports = function eventData(out, pt) {
if(pt.ya) out.yaxis = pt.ya;
out.color = pt.color;
out.colormodel = pt.trace.colormodel;
if(!out.z) out.z = pt.color;
return out;
};
13 changes: 10 additions & 3 deletions src/traces/image/hover.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,15 @@ module.exports = function hoverPoints(pointData, xval, yval) {
var nx = Math.floor((xval - cd0.x0) / trace.dx);
var ny = Math.floor(Math.abs(yval - cd0.y0) / trace.dy);

var pixel;
if(trace._isFromZ) {
pixel = cd0.z[ny][nx];
} else if(trace._isFromSource) {
pixel = trace._canvas.getContext('2d').getImageData(nx, ny, 1, 1).data;
}

// return early if pixel is undefined
if(!cd0.z[ny][nx]) return;
if(!pixel) return;

var hoverinfo = cd0.hi || trace.hoverinfo;
var fmtColor;
Expand All @@ -41,7 +48,7 @@ module.exports = function hoverPoints(pointData, xval, yval) {

var colormodel = trace.colormodel;
var dims = colormodel.length;
var c = trace._scaler(cd0.z[ny][nx]);
var c = trace._scaler(pixel);
var s = constants.colormodel[colormodel].suffix;

var colorstring = [];
Expand All @@ -64,7 +71,7 @@ module.exports = function hoverPoints(pointData, xval, yval) {
var py = ya.c2p(cd0.y0 + (ny + 0.5) * trace.dy);
var xVal = cd0.x0 + (nx + 0.5) * trace.dx;
var yVal = cd0.y0 + (ny + 0.5) * trace.dy;
var zLabel = '[' + cd0.z[ny][nx].slice(0, trace.colormodel.length).join(', ') + ']';
var zLabel = '[' + pixel.slice(0, trace.colormodel.length).join(', ') + ']';
return [Lib.extendFlat(pointData, {
index: [ny, nx],
x0: xa.c2p(cd0.x0 + nx * trace.dx),
Expand Down
155 changes: 119 additions & 36 deletions src/traces/image/plot.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,24 @@ var Lib = require('../../lib');
var xmlnsNamespaces = require('../../constants/xmlns_namespaces');
var constants = require('./constants');

function compatibleAxis(ax) {
return ax.type === 'linear' &&
// y axis must be reversed but x axis mustn't be
((ax.range[1] > ax.range[0]) === (ax._id.charAt(0) === 'x'));
}

module.exports = function plot(gd, plotinfo, cdimage, imageLayer) {
var xa = plotinfo.xaxis;
var ya = plotinfo.yaxis;

var supportsPixelatedImage = !Lib.isSafari() && !gd._context._exportedPlot;
antoinerg marked this conversation as resolved.
Show resolved Hide resolved

Lib.makeTraceGroups(imageLayer, cdimage, 'im').each(function(cd) {
var plotGroup = d3.select(this);
var cd0 = cd[0];
var trace = cd0.trace;
var fastImage = supportsPixelatedImage && trace._isFromSource && compatibleAxis(xa) && compatibleAxis(ya);
trace._fastImage = fastImage;
archmoj marked this conversation as resolved.
Show resolved Hide resolved

var z = cd0.z;
var x0 = cd0.x0;
Expand Down Expand Up @@ -66,11 +76,14 @@ module.exports = function plot(gd, plotinfo, cdimage, imageLayer) {
}

// Reduce image size when zoomed in to save memory
var extra = 0.5; // half the axis size
left = Math.max(-extra * xa._length, left);
right = Math.min((1 + extra) * xa._length, right);
top = Math.max(-extra * ya._length, top);
bottom = Math.min((1 + extra) * ya._length, bottom);
if(!fastImage) {
var extra = 0.5; // half the axis size
archmoj marked this conversation as resolved.
Show resolved Hide resolved
left = Math.max(-extra * xa._length, left);
right = Math.min((1 + extra) * xa._length, right);
top = Math.max(-extra * ya._length, top);
bottom = Math.min((1 + extra) * ya._length, bottom);
}

var imageWidth = Math.round(right - left);
var imageHeight = Math.round(bottom - top);

Expand All @@ -82,48 +95,118 @@ module.exports = function plot(gd, plotinfo, cdimage, imageLayer) {
return;
}

// Draw each pixel
var canvas = document.createElement('canvas');
canvas.width = imageWidth;
canvas.height = imageHeight;
var context = canvas.getContext('2d');

var ipx = function(i) {return Lib.constrain(Math.round(xa.c2p(x0 + i * dx) - left), 0, imageWidth);};
var jpx = function(j) {return Lib.constrain(Math.round(ya.c2p(y0 + j * dy) - top), 0, imageHeight);};

var fmt = constants.colormodel[trace.colormodel].fmt;
var c;
for(i = 0; i < cd0.w; i++) {
var ipx0 = ipx(i); var ipx1 = ipx(i + 1);
if(ipx1 === ipx0 || isNaN(ipx1) || isNaN(ipx0)) continue;
for(var j = 0; j < cd0.h; j++) {
var jpx0 = jpx(j); var jpx1 = jpx(j + 1);
if(jpx1 === jpx0 || isNaN(jpx1) || isNaN(jpx0) || !z[j][i]) continue;
c = trace._scaler(z[j][i]);
if(c) {
context.fillStyle = trace.colormodel + '(' + fmt(c).join(',') + ')';
} else {
// Return a transparent pixel
context.fillStyle = 'rgba(0,0,0,0)';
// Create a new canvas and draw magnified pixels on it
function drawMagnifiedPixelsOnCanvas(readPixel) {
var colormodel = trace.colormodel;
var canvas = document.createElement('canvas');
canvas.width = imageWidth;
canvas.height = imageHeight;
var context = canvas.getContext('2d');

var ipx = function(i) {return Lib.constrain(Math.round(xa.c2p(x0 + i * dx) - left), 0, imageWidth);};
archmoj marked this conversation as resolved.
Show resolved Hide resolved
var jpx = function(j) {return Lib.constrain(Math.round(ya.c2p(y0 + j * dy) - top), 0, imageHeight);};
archmoj marked this conversation as resolved.
Show resolved Hide resolved

var fmt = constants.colormodel[colormodel].fmt;
var c;
for(i = 0; i < cd0.w; i++) {
var ipx0 = ipx(i); var ipx1 = ipx(i + 1);
if(ipx1 === ipx0 || isNaN(ipx1) || isNaN(ipx0)) continue;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why we need to test for NaNs?
Could we skip this process?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why we need to test for NaNs?
Could we skip this process?

I'm not sure at the moment.

However, this PR didn't change this logic: I simply moved it into a function. Could we maybe revisit this topic in a later PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

But they still be called in the case of non-fast-image!
So let's add some logic to skip these isNaN calls when source is present.

if(ipx1 === ipx0 || (
  !hasSource && (isNaN(ipx1) || isNaN(ipx0))
)) continue;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not confident about this change. Checking for NaN might be necessary even when the image is defined via source if there are a logarithmic scale and negative values involved (ie. Math.log(-10) is NaN).

cc @archmoj

Copy link
Contributor

Choose a reason for hiding this comment

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

Alright. Let's skip this for now.

for(var j = 0; j < cd0.h; j++) {
var jpx0 = jpx(j); var jpx1 = jpx(j + 1);
if(jpx1 === jpx0 || isNaN(jpx1) || isNaN(jpx0) || !readPixel(i, j)) continue;
c = trace._scaler(readPixel(i, j));
if(c) {
context.fillStyle = colormodel + '(' + fmt(c).join(',') + ')';
} else {
// Return a transparent pixel
context.fillStyle = 'rgba(0,0,0,0)';
}
context.fillRect(ipx0, jpx0, ipx1 - ipx0, jpx1 - jpx0);
}
context.fillRect(ipx0, jpx0, ipx1 - ipx0, jpx1 - jpx0);
}

return canvas;
}

function sizeImage(image) {
image.attr({
height: imageHeight,
width: imageWidth,
x: left,
y: top
});
}

var data = (trace._isFromSource && !fastImage) ? [cd, {hidden: true}] : [cd];
var image3 = plotGroup.selectAll('image')
.data(cd);
.data(data);

image3.enter().append('svg:image').attr({
xmlns: xmlnsNamespaces.svg,
preserveAspectRatio: 'none'
});

image3.attr({
height: imageHeight,
width: imageWidth,
x: left,
y: top,
'xlink:href': canvas.toDataURL('image/png')
if(fastImage) sizeImage(image3);
image3.exit().remove();

// Pixelated image rendering
// http://phrogz.net/tmp/canvas_image_zoom.html
// https://developer.mozilla.org/en-US/docs/Web/CSS/image-rendering
image3
.attr('style', 'image-rendering: optimizeSpeed; image-rendering: -moz-crisp-edges; image-rendering: -o-crisp-edges; image-rendering: -webkit-optimize-contrast; image-rendering: optimize-contrast; image-rendering: crisp-edges; image-rendering: pixelated;');
antoinerg marked this conversation as resolved.
Show resolved Hide resolved

new Promise(function(resolve) {
if(trace._isFromZ) {
resolve();
} else if(trace._isFromSource) {
// Transfer image to a canvas to access pixel information
trace._canvas = trace._canvas || document.createElement('canvas');
trace._canvas.width = w;
trace._canvas.height = h;
var context = trace._canvas.getContext('2d');

var sel;
if(fastImage) {
// Use the displayed image
sel = image3;
} else {
// Use the hidden image
sel = d3.select(image3[0][1]);
}

var image = sel.node();
image.onload = function() {
context.drawImage(image, 0, 0);
// we need to wait for the image to be loaded in order to redraw it from the canvas
if(!fastImage) resolve();
};
sel.attr('xlink:href', trace.source);
if(fastImage) resolve();
}
})
.then(function() {
if(!fastImage) {
var canvas;
if(trace._isFromZ) {
canvas = drawMagnifiedPixelsOnCanvas(function(i, j) {return z[j][i];});
} else if(trace._isFromSource) {
var context = trace._canvas.getContext('2d');
var data = context.getImageData(0, 0, w, h).data;
canvas = drawMagnifiedPixelsOnCanvas(function(i, j) {
var index = 4 * (j * w + i);
return [
data[index + 0],
archmoj marked this conversation as resolved.
Show resolved Hide resolved
data[index + 1],
data[index + 2],
data[index + 3]
];
});
}
var href = canvas.toDataURL('image/png');
var displayImage = d3.select(image3[0][0]);
archmoj marked this conversation as resolved.
Show resolved Hide resolved
sizeImage(displayImage);
displayImage.attr('xlink:href', href);
}
});
});
};
3 changes: 2 additions & 1 deletion src/traces/image/style.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ var d3 = require('d3');
module.exports = function style(gd) {
d3.select(gd).selectAll('.im image')
.style('opacity', function(d) {
return d.trace.opacity;
if(d && d.hidden) return 0;
return d[0].trace.opacity;
archmoj marked this conversation as resolved.
Show resolved Hide resolved
});
};
Binary file modified test/image/baselines/image_axis_reverse.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/image_axis_type.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/image_cat.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/image_colormodel.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/image_non_numeric.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/image_with_heatmap.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/image/baselines/image_zmin_zmax.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 2 additions & 1 deletion test/image/compare_pixels_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,8 @@ if(allMock || argv.filter) {
allMockList = allMockList.filter(function(mockName) {
var cond = !(
mockName === 'font-wishlist' ||
mockName.indexOf('mapbox_') !== -1
mockName.indexOf('mapbox_') !== -1 ||
mockName.match(/image_.*source/)
antoinerg marked this conversation as resolved.
Show resolved Hide resolved
);
if(!cond) console.log(' -', mockName);
return cond;
Expand Down
1 change: 1 addition & 0 deletions test/image/mocks/image_labuda_droplets_source.json

Large diffs are not rendered by default.

Loading