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

Prevent scatter3d to only display the first point on hover: issue 3258 #3301

Merged
merged 11 commits into from Dec 7, 2018

Conversation

Projects
None yet
2 participants
@archmoj
Copy link
Collaborator

commented Dec 2, 2018

Fixes #3258 for Ubuntu.
Bug was related to linking issues within gl-scatter3d module...
@etpinard
@alexcjohnson

archmoj added some commits Dec 2, 2018

@etpinard

This comment has been minimized.

Copy link
Member

commented Dec 3, 2018

Thanks for investigating this bug @archmoj !

It seems Linux tries to optimise the performance by removing the alpha channel (sometimes after warm up)

sounds like it was very painful to debug 💪


Your fix does make things work on my Ubuntu machine. Now, it does strike me as a little bit hacky. I mean an opacity of 0.99 isn't really an opacity of 1, and probably makes things render more slowly as some of the aforementioned optimization is now skipped.

I wonder if we could instead pass a flag into gl-scatter3d's pickHover routine telling it "all points in this trace have opacity: 1"?

archmoj added some commits Dec 3, 2018

@@ -80,7 +80,7 @@
"gl-plot2d": "^1.4.0",
"gl-plot3d": "^1.6.0",
"gl-pointcloud2d": "^1.0.1",
"gl-scatter3d": "^1.1.0",
"gl-scatter3d": "git://github.com/gl-vis/gl-scatter3d.git#c252e1cd723aed657bdc8b022efe6befcfe9f729",

This comment has been minimized.

Copy link
@archmoj
@archmoj

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 3, 2018

Thank @etpinard for encouraging! It was quite a bad bug. Different approach is used in this revision. No need to tweak the opacity and the baselines didn't changed.

archmoj added some commits Dec 7, 2018

Show resolved Hide resolved log.txt Outdated
@etpinard

This comment has been minimized.

Copy link
Member

commented Dec 7, 2018

Nicely done 💃 for gl-vis/gl-scatter3d#13 and this PR after gl-scatter3d version bump.

@archmoj archmoj merged commit 3ea5c82 into master Dec 7, 2018

8 checks passed

ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: publish Your tests passed on CircleCI!
Details
ci/circleci: test-image Your tests passed on CircleCI!
Details
ci/circleci: test-image2 Your tests passed on CircleCI!
Details
ci/circleci: test-jasmine Your tests passed on CircleCI!
Details
ci/circleci: test-jasmine2 Your tests passed on CircleCI!
Details
ci/circleci: test-syntax Your tests passed on CircleCI!
Details
continuous-integration/appveyor/branch AppVeyor build succeeded
Details

@archmoj archmoj deleted the issue-3258 branch Dec 7, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.