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

.draw() doesn't skip over ids with undefined options #35

Closed
etpinard opened this issue Jun 28, 2018 · 12 comments
Closed

.draw() doesn't skip over ids with undefined options #35

etpinard opened this issue Jun 28, 2018 · 12 comments

Comments

@etpinard
Copy link
Member

@dy

let regl = require('regl')({extensions: 'angle_instanced_arrays'})
let line2d = require('./')(regl)

line2d.update([
  { thickness: 4, points: [0,0, 1,1, 1,0], close: true, color: 'red' },
  undefined,
  { thickness: 4, points: [0,1, 1,1, 0,0], close: true, color: 'blue' }
])
line2d.draw(0)
line2d.draw(2)

only draws the red triangle.


Example in plotly.js https://codepen.io/etpinard/pen/bKQjMe

@ErwanMAS
Copy link

ErwanMAS commented Sep 3, 2018

First from here we assume that at index i of the array passes , we found element with id i
https://github.com/a-vis/regl-line2d/blob/0c25f299abab2e714b7ce68fd585df6d10e533cb/index.js#L407-L408)

But here we remove empty cell from the array passes , so at index i i dont have any more element with id i
https://github.com/a-vis/regl-line2d/blob/0c25f299abab2e714b7ce68fd585df6d10e533cb/index.js#L692

so when we call draw with a number
https://github.com/a-vis/regl-line2d/blob/0c25f299abab2e714b7ce68fd585df6d10e533cb/index.js#L333

we can draw the wrong element

@etpinard
Copy link
Member Author

etpinard commented Sep 4, 2018

@ErwanMAS thanks for the investigation.

I haven't looked at this bug in a while now, but I suspect the best what to fix it would be to implement whether regl-scatter2d does for skipping empty passes.

@etpinard
Copy link
Member Author

etpinard commented Sep 4, 2018

@dy could you help us out?

ErwanMAS added a commit to ErwanMAS/regl-line2d that referenced this issue Sep 9, 2018
@ErwanMAS ErwanMAS mentioned this issue Sep 9, 2018
@dy
Copy link
Member

dy commented Sep 10, 2018

3.0.11 should fix that

@ErwanMAS
Copy link

Hey ,
The patch solve only one case but not other more complex case .

can you test against https://codepen.io/erwanmas/pen/NLwXab ?

@etpinard
Copy link
Member Author

@ErwanMAS using regl-line2d@3.0.11, I'm getting:

image

vs

image

where the fill pattern in the red trace is wrong, but the rest appears right.

@etpinard
Copy link
Member Author

PR for plotly.js -> plotly/plotly.js#2990 with corrected baselines.

@ErwanMAS
Copy link

@dy @etpinard i am still thinking , this is not the right solution .

I applied only the patch and tested and this does not work
https://codepen.io/erwanmas/pen/JapLZb

@etpinard
Copy link
Member Author

Thanks @ErwanMAS !

I confirm your observations on the plotly/plotly.js#2990 branch. The problem seems to only be apparent on "fills" traces. Dima's patch seems like a step in the right direction.

@ErwanMAS can you replicate the problem outside of plotly.js with just regl-line2d?

For future reference, here's a more simple codepen https://codepen.io/etpinard/pen/WgzrVw?editors=1010 (with a legend and the default trace colors making it easier to tell which trace is which).

@etpinard
Copy link
Member Author

@ErwanMAS the problem was in plotly.js.

I was able to fix it in plotly/plotly.js@045a162

Thanks for your help!

@ErwanMAS
Copy link

ErwanMAS commented Sep 11, 2018

How i can run test outside of plotly.js ? @etpinard

@dy
Copy link
Member

dy commented Oct 1, 2018

I guess this issue is fixed via plotly/plotly.js#2990

@dy dy closed this as completed Oct 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants