-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Fix color clustering #2377
Fix color clustering #2377
Changes from 12 commits
5de6918
ce01ea7
20559c0
7c944c9
4fada56
87b22b2
64f112f
f2e80a7
1154589
d604a76
9532210
7cb3295
ba40db9
20c3a00
eae6da2
f8b4e52
48e1124
0e746bf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -648,7 +648,6 @@ describe('@gl Test gl2d plots', function() { | |||||||||||||||||||||||||||||
.then(done); | ||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
it('should not scroll document while panning', function(done) { | ||||||||||||||||||||||||||||||
var mock = { | ||||||||||||||||||||||||||||||
data: [ | ||||||||||||||||||||||||||||||
|
@@ -773,4 +772,64 @@ describe('@gl Test gl2d plots', function() { | |||||||||||||||||||||||||||||
.catch(fail) | ||||||||||||||||||||||||||||||
.then(done); | ||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
it('should remove fill2d', function(done) { | ||||||||||||||||||||||||||||||
var mock = require('@mocks/gl2d_axes_labels2.json'); | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
Plotly.plot(gd, mock.data, mock.layout) | ||||||||||||||||||||||||||||||
.then(delay(1000)) | ||||||||||||||||||||||||||||||
.then(function() { | ||||||||||||||||||||||||||||||
expect(readPixel(gd.querySelector('.gl-canvas-context'), 100, 80)[0]).not.toBe(0); | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
return Plotly.restyle(gd, {fill: 'none'}); | ||||||||||||||||||||||||||||||
}) | ||||||||||||||||||||||||||||||
.then(function() { | ||||||||||||||||||||||||||||||
expect(readPixel(gd.querySelector('.gl-canvas-context'), 100, 80)[0]).toBe(0); | ||||||||||||||||||||||||||||||
}) | ||||||||||||||||||||||||||||||
.catch(fail) | ||||||||||||||||||||||||||||||
.then(done); | ||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
it('should be able to draw more than 4096 colors', function(done) { | ||||||||||||||||||||||||||||||
var gd = createGraphDiv(); | ||||||||||||||||||||||||||||||
var x = []; | ||||||||||||||||||||||||||||||
var color = []; | ||||||||||||||||||||||||||||||
var N = 1e5; | ||||||||||||||||||||||||||||||
var w = 500; | ||||||||||||||||||||||||||||||
var h = 500; | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
for(var i = 0; i < N; i++) { | ||||||||||||||||||||||||||||||
x.push(i); | ||||||||||||||||||||||||||||||
color.push(Math.random()); | ||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe I'm being paranoid, but I still don't like plotly.js/src/traces/box/plot.js Lines 17 to 30 in ce5bc97
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ha right. Good call! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
Plotly.newPlot(gd, [{ | ||||||||||||||||||||||||||||||
type: 'scattergl', | ||||||||||||||||||||||||||||||
mode: 'markers', | ||||||||||||||||||||||||||||||
x: x, | ||||||||||||||||||||||||||||||
y: color, | ||||||||||||||||||||||||||||||
marker: { | ||||||||||||||||||||||||||||||
color: color, | ||||||||||||||||||||||||||||||
colorscale: [ | ||||||||||||||||||||||||||||||
[0, 'rgb(255, 0, 0)'], | ||||||||||||||||||||||||||||||
[0.5, 'rgb(0, 255, 0)'], | ||||||||||||||||||||||||||||||
[1.0, 'rgb(0, 0, 255)'] | ||||||||||||||||||||||||||||||
] | ||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||
}], { | ||||||||||||||||||||||||||||||
width: w, | ||||||||||||||||||||||||||||||
height: h, | ||||||||||||||||||||||||||||||
margin: {l: 0, t: 0, b: 0, r: 0} | ||||||||||||||||||||||||||||||
}) | ||||||||||||||||||||||||||||||
.then(function() { | ||||||||||||||||||||||||||||||
var total = readPixel(gd.querySelector('.gl-canvas-context'), 0, 0, w, h) | ||||||||||||||||||||||||||||||
.reduce(function(acc, v) { return acc + v; }, 0); | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
// the total value was 3762487 before PR | ||||||||||||||||||||||||||||||
// https://github.com/plotly/plotly.js/pull/2377 | ||||||||||||||||||||||||||||||
expect(total).toBeGreaterThan(4e6); | ||||||||||||||||||||||||||||||
}) | ||||||||||||||||||||||||||||||
.catch(fail) | ||||||||||||||||||||||||||||||
.then(done); | ||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you briefly explain why this fixes #2376 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That fixes #2354. That is internal
regl-line2d
behavior: when we doline2d.update([opts1, undefined, opts2, ...rest])
, the second line in batch is ignored instead of being removed. I made it acceptnull
, indicating that batch item should be removed.