Skip to content

Improve rendering of scattergl, splom and parcoords by implementing plotGlPixelRatio for those traces #5500

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

Merged
merged 39 commits into from
Jul 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
004c401
Take config.plotGlPixelRatio into account for scattergl trace
jonmmease Feb 12, 2021
aa67a7f
Take config.plotGlPixelRatio into account for parcoords trace
jonmmease Feb 12, 2021
55ee4b3
Take config.plotGlPixelRatio into account for splom trace
jonmmease Feb 12, 2021
f03bce0
scattergl size fixes.
jonmmease Feb 13, 2021
ef92fe6
Scale parcoords line width with plotGlPixelRatio
jonmmease Feb 13, 2021
c755979
parcoords: Use paper_bgcolor constraint border and text shadows to su…
jonmmease Feb 13, 2021
b4b60e4
Fix splom grid scaling
jonmmease Feb 13, 2021
4c26380
Fix regl check
jonmmease Feb 13, 2021
73be4e1
lint
jonmmease Feb 13, 2021
992b50e
update canvas size in responsive. Factor of 2 from plotGlPixelRatio
jonmmease Feb 13, 2021
d666f09
Fix responsive tests
jonmmease Feb 13, 2021
89bf4db
Update scattergl_test pixel coordinates.
jonmmease Feb 13, 2021
c14ab5f
Update approximate marker padding calculation
jonmmease Feb 13, 2021
f4c9e6e
Fix approximate marker size padding calculation for SPLOM as well
jonmmease Feb 13, 2021
9a0af27
Update SPLOM tests
jonmmease Feb 13, 2021
5e63feb
update coordinates
jonmmease Feb 13, 2021
3e5f9b7
Fix gl clear context test
jonmmease Feb 14, 2021
2108515
Update some reviewed baselines
jonmmease Feb 14, 2021
f3ce630
Add dark background parcoords baseline
jonmmease Feb 14, 2021
b4abfb2
Fix scattergl line dash lengths
jonmmease Feb 14, 2021
e0eba2b
Fix splom grid spacing
jonmmease Feb 14, 2021
849db07
Update glpolar_style baseline
jonmmease Feb 14, 2021
69582cb
update glpolar_subplots baseline
jonmmease Feb 14, 2021
d6bddd1
Update glpolar_scatter baseline
jonmmease Feb 14, 2021
5f351b5
parcoords linewidth of 1 for browser compatibility
jonmmease Feb 17, 2021
22362fb
Try removing regl size check to see if it changes image export behavior
jonmmease Feb 18, 2021
4860893
Revert "Try removing regl size check to see if it changes image expor…
archmoj Feb 19, 2021
8ea156c
Merge remote-tracking branch 'origin/master' into scattergl_glpixelratio
archmoj Feb 19, 2021
abaf46e
Merge remote-tracking branch 'origin/master' into scattergl_glpixelratio
archmoj Jun 26, 2021
cde7a0b
fix unused variable
archmoj Jun 26, 2021
800de99
update baselines using new system
archmoj Jun 26, 2021
5242b99
Merge remote-tracking branch 'origin/master' into scattergl_glpixelratio
archmoj Jul 5, 2021
549701f
Merge remote-tracking branch 'origin/master' into scattergl_glpixelratio
archmoj Jul 7, 2021
4e65600
provide draft log for PR 5500
archmoj Jul 7, 2021
0147edc
ensure parcoords plotGlPixelRatio is within supported range of the de…
archmoj Jul 9, 2021
c0880c3
adjust jasmine tests
archmoj Jul 9, 2021
6764876
fixup changes to splom grid made in b4b60e4a6dabf5e7cd9ed572cfb29333e…
archmoj Jul 9, 2021
73fc99e
isolate and adjust parcoord hover test
archmoj Jul 9, 2021
89e2437
refactor splom base plot
archmoj Jul 9, 2021
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
1 change: 1 addition & 0 deletions draftlogs/5500_fix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Improve rendering of scattergl, splom and parcoords by implementing plotGlPixelRatio for those traces [[#5500](https://github.com/plotly/plotly.js/pull/5500)]
11 changes: 7 additions & 4 deletions src/plot_api/plot_api.js
Original file line number Diff line number Diff line change
Expand Up @@ -222,17 +222,20 @@ function _doPlot(gd, data, layout, config) {
});
}

var plotGlPixelRatio = gd._context.plotGlPixelRatio;
if(fullLayout._glcanvas) {
fullLayout._glcanvas
.attr('width', fullLayout.width)
.attr('height', fullLayout.height);
.attr('width', fullLayout.width * plotGlPixelRatio)
.attr('height', fullLayout.height * plotGlPixelRatio)
.style('width', fullLayout.width + 'px')
.style('height', fullLayout.height + 'px');

var regl = fullLayout._glcanvas.data()[0].regl;
if(regl) {
// Unfortunately, this can happen when relayouting to large
// width/height on some browsers.
if(Math.floor(fullLayout.width) !== regl._gl.drawingBufferWidth ||
Math.floor(fullLayout.height) !== regl._gl.drawingBufferHeight
if(Math.floor(fullLayout.width * plotGlPixelRatio) !== regl._gl.drawingBufferWidth ||
Math.floor(fullLayout.height * plotGlPixelRatio) !== regl._gl.drawingBufferHeight
) {
var msg = 'WebGL context buffer and canvas dimensions do not match due to browser/WebGL bug.';
if(drawFrameworkCalls) {
Expand Down
4 changes: 2 additions & 2 deletions src/plots/cartesian/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -596,8 +596,8 @@ exports.toSVG = function(gd) {
preserveAspectRatio: 'none',
x: 0,
y: 0,
width: canvas.width,
height: canvas.height
width: canvas.style.width,
height: canvas.style.height
});
}

Expand Down
4 changes: 2 additions & 2 deletions src/traces/parcoords/base_plot.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ exports.toSVG = function(gd) {
preserveAspectRatio: 'none',
x: 0,
y: 0,
width: canvas.width,
height: canvas.height
width: canvas.style.width,
height: canvas.style.height
});
}

Expand Down
44 changes: 33 additions & 11 deletions src/traces/parcoords/lines.js
Original file line number Diff line number Diff line change
Expand Up @@ -160,17 +160,27 @@ function emptyAttributes(regl) {
return attributes;
}

function makeItem(model, leftmost, rightmost, itemNumber, i0, i1, x, y, panelSizeX, panelSizeY, crossfilterDimensionIndex, drwLayer, constraints) {
function makeItem(
model, leftmost, rightmost, itemNumber, i0, i1, x, y, panelSizeX, panelSizeY,
crossfilterDimensionIndex, drwLayer, constraints, plotGlPixelRatio
) {
var dims = [[], []];
for(var k = 0; k < 64; k++) {
dims[0][k] = (k === i0) ? 1 : 0;
dims[1][k] = (k === i1) ? 1 : 0;
}

var overdrag = model.lines.canvasOverdrag;
x *= plotGlPixelRatio;
y *= plotGlPixelRatio;
panelSizeX *= plotGlPixelRatio;
panelSizeY *= plotGlPixelRatio;
var overdrag = model.lines.canvasOverdrag * plotGlPixelRatio;
var domain = model.domain;
var canvasWidth = model.canvasWidth;
var canvasHeight = model.canvasHeight;
var canvasWidth = model.canvasWidth * plotGlPixelRatio;
var canvasHeight = model.canvasHeight * plotGlPixelRatio;
var padL = model.pad.l * plotGlPixelRatio;
var padB = model.pad.b * plotGlPixelRatio;
var layoutHeight = model.layoutHeight * plotGlPixelRatio;
var layoutWidth = model.layoutWidth * plotGlPixelRatio;

var deselectedLinesColor = model.deselectedLines.color;

Expand Down Expand Up @@ -201,13 +211,13 @@ function makeItem(model, leftmost, rightmost, itemNumber, i0, i1, x, y, panelSiz
Math.max(1 / 255, Math.pow(1 / model.lines.color.length, 1 / 3))
],

scissorX: (itemNumber === leftmost ? 0 : x + overdrag) + (model.pad.l - overdrag) + model.layoutWidth * domain.x[0],
scissorX: (itemNumber === leftmost ? 0 : x + overdrag) + (padL - overdrag) + layoutWidth * domain.x[0],
scissorWidth: (itemNumber === rightmost ? canvasWidth - x + overdrag : panelSizeX + 0.5) + (itemNumber === leftmost ? x + overdrag : 0),
scissorY: y + model.pad.b + model.layoutHeight * domain.y[0],
scissorY: y + padB + layoutHeight * domain.y[0],
scissorHeight: panelSizeY,

viewportX: model.pad.l - overdrag + model.layoutWidth * domain.x[0],
viewportY: model.pad.b + model.layoutHeight * domain.y[0],
viewportX: padL - overdrag + layoutWidth * domain.x[0],
viewportY: padB + layoutHeight * domain.y[0],
viewportWidth: canvasWidth,
viewportHeight: canvasHeight
}, constraints);
Expand All @@ -231,6 +241,16 @@ module.exports = function(canvasGL, d) {
var isPick = d.pick;

var regl = d.regl;
var gl = regl._gl;
var supportedLineWidth = gl.getParameter(gl.ALIASED_LINE_WIDTH_RANGE);
// ensure here that plotGlPixelRatio is within supported range; otherwise regl throws error
var plotGlPixelRatio = Math.max(
supportedLineWidth[0],
Math.min(
supportedLineWidth[1],
d.viewModel.plotGlPixelRatio
)
);

var renderState = {
currentRafs: {},
Expand Down Expand Up @@ -307,7 +327,7 @@ module.exports = function(canvasGL, d) {
frag: fragmentShaderSource,

primitive: 'lines',
lineWidth: 1,
lineWidth: plotGlPixelRatio,
attributes: attributes,
uniforms: {
resolution: regl.prop('resolution'),
Expand Down Expand Up @@ -454,6 +474,7 @@ module.exports = function(canvasGL, d) {
var x = p.canvasX;
var y = p.canvasY;
var nextX = x + p.panelSizeX;
var plotGlPixelRatio = p.plotGlPixelRatio;
if(setChanged ||
!prevAxisOrder[i0] ||
prevAxisOrder[i0][0] !== x ||
Expand All @@ -467,7 +488,8 @@ module.exports = function(canvasGL, d) {
p.panelSizeX, p.panelSizeY,
p.dim0.crossfilterDimensionIndex,
isContext ? 0 : isPick ? 2 : 1,
constraints
constraints,
plotGlPixelRatio
);

renderState.clearOnly = clearOnly;
Expand Down
11 changes: 7 additions & 4 deletions src/traces/parcoords/parcoords.js
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ function calcTilt(angle, position) {
};
}

function updatePanelLayout(yAxis, vm) {
function updatePanelLayout(yAxis, vm, plotGlPixelRatio) {
var panels = vm.panels || (vm.panels = []);
var data = yAxis.data();
for(var i = 0; i < data.length - 1; i++) {
Expand All @@ -383,6 +383,7 @@ function updatePanelLayout(yAxis, vm) {
p.panelSizeY = vm.model.canvasHeight;
p.y = 0;
p.canvasY = 0;
p.plotGlPixelRatio = plotGlPixelRatio;
}
}

Expand Down Expand Up @@ -433,6 +434,7 @@ module.exports = function parcoords(gd, cdModule, layout, callbacks) {
var fullLayout = gd._fullLayout;
var svg = fullLayout._toppaper;
var glContainer = fullLayout._glcontainer;
var plotGlPixelRatio = gd._context.plotGlPixelRatio;
var paperColor = gd._fullLayout.paper_bgcolor;

calcAllTicks(cdModule);
Expand All @@ -452,6 +454,7 @@ module.exports = function parcoords(gd, cdModule, layout, callbacks) {
.each(function(d) {
// FIXME: figure out how to handle multiple instances
d.viewModel = vm[0];
d.viewModel.plotGlPixelRatio = plotGlPixelRatio;
d.viewModel.paperColor = paperColor;
d.model = d.viewModel ? d.viewModel.model : null;
});
Expand Down Expand Up @@ -536,7 +539,7 @@ module.exports = function parcoords(gd, cdModule, layout, callbacks) {
.classed(c.cn.yAxis, true);

parcoordsControlView.each(function(p) {
updatePanelLayout(yAxis, p);
updatePanelLayout(yAxis, p, plotGlPixelRatio);
});

glLayers
Expand Down Expand Up @@ -575,7 +578,7 @@ module.exports = function parcoords(gd, cdModule, layout, callbacks) {
e.canvasX = e.x * e.model.canvasPixelRatio;
});

updatePanelLayout(yAxis, p);
updatePanelLayout(yAxis, p, plotGlPixelRatio);

yAxis.filter(function(e) { return Math.abs(d.xIndex - e.xIndex) !== 0; })
.attr('transform', function(d) { return strTranslate(d.xScale(d.xIndex), 0); });
Expand All @@ -588,7 +591,7 @@ module.exports = function parcoords(gd, cdModule, layout, callbacks) {
var p = d.parent;
d.x = d.xScale(d.xIndex);
d.canvasX = d.x * d.model.canvasPixelRatio;
updatePanelLayout(yAxis, p);
updatePanelLayout(yAxis, p, plotGlPixelRatio);
d3.select(this)
.attr('transform', function(d) { return strTranslate(d.x, 0); });
p.contextLayer && p.contextLayer.render(p.panels, false, !someFiltersActive(p));
Expand Down
7 changes: 5 additions & 2 deletions src/traces/scatter/make_bubble_size_func.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ var isNumeric = require('fast-isnumeric');

// used in the drawing step for 'scatter' and 'scattegeo' and
// in the convert step for 'scatter3d'
module.exports = function makeBubbleSizeFn(trace) {
module.exports = function makeBubbleSizeFn(trace, factor) {
if(!factor) {
factor = 2;
}
var marker = trace.marker;
var sizeRef = marker.sizeref || 1;
var sizeMin = marker.sizemin || 0;
Expand All @@ -21,7 +24,7 @@ module.exports = function makeBubbleSizeFn(trace) {
// TODO add support for position/negative bubbles?
// TODO add 'sizeoffset' attribute?
return function(v) {
var baseSize = baseFn(v / 2);
var baseSize = baseFn(v / factor);

// don't show non-numeric and negative sizes
return (isNumeric(baseSize) && (baseSize > 0)) ?
Expand Down
2 changes: 1 addition & 1 deletion src/traces/scattergl/calc.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ module.exports = function calc(gd, trace) {
if(!hasTooManyPoints) {
ppad = calcMarkerSize(trace, len);
} else if(opts.marker) {
ppad = 2 * (opts.marker.sizeAvg || Math.max(opts.marker.size, 3));
ppad = opts.marker.sizeAvg || Math.max(opts.marker.size, 3);
}
calcAxisExpansion(gd, trace, xa, ya, x, y, ppad);
if(opts.errorX) expandForErrorBars(trace, xa, opts.errorX);
Expand Down
30 changes: 17 additions & 13 deletions src/traces/scattergl/convert.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ function convertStyle(gd, trace) {
textUnsel: undefined
};

var plotGlPixelRatio = gd._context.plotGlPixelRatio;

if(trace.visible !== true) return opts;

if(subTypes.hasText(trace)) {
Expand All @@ -64,24 +66,24 @@ function convertStyle(gd, trace) {
if(subTypes.hasLines(trace)) {
opts.line = {
overlay: true,
thickness: trace.line.width,
thickness: trace.line.width * plotGlPixelRatio,
color: trace.line.color,
opacity: trace.opacity
};

var dashes = (constants.DASHES[trace.line.dash] || [1]).slice();
for(i = 0; i < dashes.length; ++i) {
dashes[i] *= trace.line.width;
dashes[i] *= trace.line.width * plotGlPixelRatio;
}
opts.line.dashes = dashes;
}

if(trace.error_x && trace.error_x.visible) {
opts.errorX = convertErrorBarStyle(trace, trace.error_x);
opts.errorX = convertErrorBarStyle(trace, trace.error_x, plotGlPixelRatio);
}

if(trace.error_y && trace.error_y.visible) {
opts.errorY = convertErrorBarStyle(trace, trace.error_y);
opts.errorY = convertErrorBarStyle(trace, trace.error_y, plotGlPixelRatio);
}

if(!!trace.fill && trace.fill !== 'none') {
Expand All @@ -106,6 +108,7 @@ function convertTextStyle(gd, trace) {
var tff = textfontIn.family;
var optsOut = {};
var i;
var plotGlPixelRatio = gd._context.plotGlPixelRatio;

var texttemplate = trace.texttemplate;
if(texttemplate) {
Expand Down Expand Up @@ -191,13 +194,13 @@ function convertTextStyle(gd, trace) {
Array.isArray(tfs) ? (
isNumeric(tfs[i]) ? tfs[i] : 0
) : tfs
);
) * plotGlPixelRatio;

fonti.family = Array.isArray(tff) ? tff[i] : tff;
}
} else {
// if both are single values, make render fast single-value
optsOut.font = {size: tfs, family: tff};
optsOut.font = {size: tfs * plotGlPixelRatio, family: tff};
}

return optsOut;
Expand Down Expand Up @@ -283,7 +286,8 @@ function convertMarkerStyle(trace) {
}

// prepare sizes
var markerSizeFunc = makeBubbleSizeFn(trace);
var sizeFactor = 1;
var markerSizeFunc = makeBubbleSizeFn(trace, sizeFactor);
var s;

if(multiSize || multiLineWidth) {
Expand All @@ -308,10 +312,10 @@ function convertMarkerStyle(trace) {
// See https://github.com/plotly/plotly.js/pull/1781#discussion_r121820798
if(multiLineWidth) {
for(i = 0; i < count; i++) {
borderSizes[i] = optsIn.line.width[i] / 2;
borderSizes[i] = optsIn.line.width[i];
}
} else {
s = optsIn.line.width / 2;
s = optsIn.line.width;
for(i = 0; i < count; i++) {
borderSizes[i] = s;
}
Expand All @@ -335,7 +339,7 @@ function convertMarkerSelection(trace, target) {
if(target.marker && target.marker.symbol) {
optsOut = convertMarkerStyle(Lib.extendFlat({}, optsIn, target.marker));
} else if(target.marker) {
if(target.marker.size) optsOut.size = target.marker.size / 2;
if(target.marker.size) optsOut.size = target.marker.size;
if(target.marker.color) optsOut.colors = target.marker.color;
if(target.marker.opacity !== undefined) optsOut.opacity = target.marker.opacity;
}
Expand Down Expand Up @@ -365,10 +369,10 @@ function convertTextSelection(gd, trace, target) {
return optsOut;
}

function convertErrorBarStyle(trace, target) {
function convertErrorBarStyle(trace, target, plotGlPixelRatio) {
var optsOut = {
capSize: target.width * 2,
lineWidth: target.thickness,
capSize: target.width * 2 * plotGlPixelRatio,
lineWidth: target.thickness * plotGlPixelRatio,
color: target.color
};

Expand Down
26 changes: 17 additions & 9 deletions src/traces/scattergl/plot.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,23 @@ var linkTraces = require('../scatter/link_traces');

var styleTextSelection = require('./edit_style').styleTextSelection;

function getViewport(fullLayout, xaxis, yaxis) {

function getViewport(fullLayout, xaxis, yaxis, plotGlPixelRatio) {
var gs = fullLayout._size;
var width = fullLayout.width;
var height = fullLayout.height;
var width = fullLayout.width * plotGlPixelRatio;
var height = fullLayout.height * plotGlPixelRatio;

var l = gs.l * plotGlPixelRatio;
var b = gs.b * plotGlPixelRatio;
var r = gs.r * plotGlPixelRatio;
var t = gs.t * plotGlPixelRatio;
var w = gs.w * plotGlPixelRatio;
var h = gs.h * plotGlPixelRatio;
return [
gs.l + xaxis.domain[0] * gs.w,
gs.b + yaxis.domain[0] * gs.h,
(width - gs.r) - (1 - xaxis.domain[1]) * gs.w,
(height - gs.t) - (1 - yaxis.domain[1]) * gs.h
l + xaxis.domain[0] * w,
b + yaxis.domain[0] * h,
(width - r) - (1 - xaxis.domain[1]) * w,
(height - t) - (1 - yaxis.domain[1]) * h
];
}

Expand Down Expand Up @@ -59,7 +67,7 @@ module.exports = function plot(gd, subplot, cdata) {
scene.line2d = createLine(regl);
}
if(scene.scatter2d === true) {
scene.scatter2d = createScatter(regl, { constPointSize: true });
scene.scatter2d = createScatter(regl);
}
if(scene.fill2d === true) {
scene.fill2d = createLine(regl);
Expand Down Expand Up @@ -330,7 +338,7 @@ module.exports = function plot(gd, subplot, cdata) {

// provide viewport and range
var vpRange0 = {
viewport: getViewport(fullLayout, xaxis, yaxis),
viewport: getViewport(fullLayout, xaxis, yaxis, gd._context.plotGlPixelRatio),
// TODO do we need those fallbacks?
range: [
(xaxis._rl || xaxis.range)[0],
Expand Down
Loading