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

Move WebGL Marks in a separate package #1389

Merged
merged 1 commit into from Oct 21, 2021

Conversation

martinRenou
Copy link
Member

@martinRenou martinRenou commented Jul 19, 2021

Move WebGL marks to the bqplot-gl package

@martinRenou martinRenou changed the title Get rid of WebGL related stuffs Get rid of WebGL related marks Jul 19, 2021
@martinRenou martinRenou force-pushed the get_rid_webgl branch 2 times, most recently from aec46ca to 607abb2 Compare July 20, 2021 13:48
@martinRenou martinRenou changed the title Get rid of WebGL related marks Move WebGL Marks in a separate package Jul 20, 2021
@martinRenou martinRenou force-pushed the get_rid_webgl branch 4 times, most recently from c0d34d1 to dacff09 Compare July 21, 2021 06:59
@martinRenou martinRenou force-pushed the get_rid_webgl branch 2 times, most recently from c3c0d24 to a6a038a Compare August 30, 2021 07:32
@martinRenou martinRenou marked this pull request as ready for review August 30, 2021 14:36
@martinRenou
Copy link
Member Author

martinRenou commented Aug 30, 2021

PR tested with https://github.com/bqplot/bqplot-gl in bqplot/bqplot-gl#7 (adding visual tests with Galata and current JS tests soon)

@maartenbreddels
Copy link
Member

What about creating a webgl canvas, and letting the marks create a threejs renderer if needed?

@martinRenou
Copy link
Member Author

@maartenbreddels thanks for the review :)

What about creating a webgl canvas, and letting the marks create a threejs renderer if needed?

What would be the upside of this?

@maartenbreddels
Copy link
Member

Most of the code in the Figure class can stay as it is (only the line/lines that creates the rendered will change), and we don't need any Figure subclass.

@martinRenou
Copy link
Member Author

Right! Yeah that would be cool. I'll try it this way then.

@maartenbreddels
Copy link
Member

maartenbreddels commented Sep 22, 2021

This means you can write a threejs, raw WebGL, or Regl marks and mix them.

@martinRenou martinRenou force-pushed the get_rid_webgl branch 7 times, most recently from 728a8aa to f5a122a Compare October 4, 2021 14:17
@martinRenou
Copy link
Member Author

@maartenbreddels the webgl context is now created here in bqplot and we don't need to subclass the Figure

@maartenbreddels
Copy link
Member

Yes, very cool!
The only worry I have is that now we don't lazily create the context, like before using createWebGLRenderer (which now should be called createWebGLCanvas I think. Otherwise, we'll create these contexts too often, and the web-browser will show this 'broken' image because we created too many WebGL context.

@martinRenou martinRenou force-pushed the get_rid_webgl branch 3 times, most recently from 177a357 to 3b6e2a3 Compare October 21, 2021 09:06
@martinRenou
Copy link
Member Author

The only worry I have is that now we don't lazily create the context, like before using createWebGLRenderer (which now should be called createWebGLCanvas I think. Otherwise, we'll create these contexts too often, and the web-browser will show this 'broken' image because we created too many WebGL context.

This should be resolved now :)

@martinRenou martinRenou merged commit dbc6398 into bqplot:master Oct 21, 2021
@martinRenou martinRenou deleted the get_rid_webgl branch October 21, 2021 10:03
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.

None yet

2 participants