Skip to content

Shared canvases #1993

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

Closed
wants to merge 19 commits into from
Closed

Shared canvases #1993

wants to merge 19 commits into from

Conversation

dy
Copy link
Contributor

@dy dy commented Sep 6, 2017

Comes after #1989.

Makes use of pre-created 3-canvas strategy, so that no new canvases created.

@etpinard
Copy link
Contributor

etpinard commented Sep 7, 2017

Hmm. Looks like a bunch of tests are failing after 8915392. By the looks of it, gl contexts aren't getting cleaned up properly, which makes me concerned about the strategy proposed in #1989.

@dfcreative have you tried running npm run test-jasmine locally?

@dy
Copy link
Contributor Author

dy commented Sep 7, 2017

@etpinard I see that there is only one image diff, that is https://4958-45646037-gh.circle-artifacts.com/0/home/ubuntu/plotly.js/build/test_images_diff/diff-gl3d_mirror-ticks.png, and the diff is in antialias.
Although karma cannot run tests in some reason, gotta figure out why.

@etpinard
Copy link
Contributor

etpinard commented Sep 7, 2017

I see that there is only one image diff

image

Nop, actually about the image tests blow up after ~200 of ~400 mocks to test. So something very problematic is happening.

@dy
Copy link
Contributor Author

dy commented Sep 7, 2017

@etpinard to run jasmine I had to do this fix: #1994, please check.
Gotta figure out the rest.

@dy
Copy link
Contributor Author

dy commented Sep 7, 2017

@etpinard now tests pass. Should I create a separate PR for just shared canvases, skipping parcoords changes, or just cherry-pick changes into regl- components? It does not matter much though, anyways I have to complete regl-summer :)

@dy dy requested a review from etpinard September 7, 2017 23:03
@dy dy mentioned this pull request Nov 14, 2017
@etpinard
Copy link
Contributor

Superseded by #2159

@etpinard etpinard closed this Nov 17, 2017
@etpinard etpinard deleted the shared-gl branch November 17, 2017 16:22
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

Successfully merging this pull request may close these issues.

3 participants