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

Break out scenegraph module (with glTF code) #982

Merged
merged 5 commits into from Mar 19, 2019

Conversation

Projects
None yet
4 participants
@ibgreen
Copy link
Contributor

ibgreen commented Mar 13, 2019

For #724

Background

  • The scenegraph code is growing, especially combined with glTF instantation and presumably support for a growing list of glTF extensions in the future.
  • A main complication is that Model is a of ScenegraphNode and needs to be "split"
  • That will break examples so that also needs to be updated.

Change List

},
"scripts": {
"clean": "rm -fr dist && mkdir -p dist/es5 dist/esm dist/es6",
"build": "npm run clean && npm run build-es6 && npm run build-esm && npm run build-es5",

This comment has been minimized.

Copy link
@Pessimistress

Pessimistress Mar 14, 2019

Contributor

These should no longer be necessary

This comment has been minimized.

Copy link
@ibgreen

ibgreen Mar 15, 2019

Author Contributor

These should no longer be necessary

They are present in all modules, let's update in separate diff given the size of this one.

"build-esm": "BABEL_ENV=esm babel src --config-file ../../babel.config.js --out-dir dist/esm --source-maps --ignore 'node_modules/'",
"build-es5": "BABEL_ENV=es5 babel src --config-file ../../babel.config.js --out-dir dist/es5 --source-maps --ignore 'node_modules/'"
},
"dependencies": {

This comment has been minimized.

Copy link
@Pessimistress

Pessimistress Mar 14, 2019

Contributor

peerDependencies?

This comment has been minimized.

Copy link
@ibgreen

ibgreen Mar 15, 2019

Author Contributor

peerDependencies?

Would need to be changed in all modules, let's update in separate diff given the size of this one.

export {default as Group} from './scenegraph/group';
export {default as Camera} from './scenegraph/camera';
export {default as Model} from './scenegraph/model';
export {default as createGLTFObjects} from './scenegraph/gltf/create-gltf-objects';

This comment has been minimized.

Copy link
@Pessimistress

Pessimistress Mar 14, 2019

Contributor

There is no longer a Model export from core?

This comment has been minimized.

Copy link
@ibgreen

ibgreen Mar 14, 2019

Author Contributor

There is no longer a Model export from core?

There will be, this is WIP.

The Model split is the main problem that needs to be solved. Naturally, the whole point is that Model remains in core so it can be used without the scenegraph module. To do that, it must not inherit from ScenegraphNode, so we need a separate ScenegraphNode subclass (that imports and uses Model from core).

And then update examples that depend on Model scenegraph methods, update docs etc.

Should not be particularly difficult, just a bunch of work, will iterate on this for a while.

This comment has been minimized.

Copy link
@ibgreen

ibgreen Mar 15, 2019

Author Contributor

There is no longer a Model export from core?

Model has now been split!

@ibgreen ibgreen force-pushed the ib/scenegraph-module branch from a059e31 to 494d971 Mar 15, 2019

@ibgreen ibgreen marked this pull request as ready for review Mar 15, 2019


statsWidget.update();

/* TODO - picking temporarily disabled

This comment has been minimized.

Copy link
@Pessimistress

Pessimistress Mar 15, 2019

Contributor

What happened here?

This comment has been minimized.

Copy link
@ibgreen

ibgreen Mar 15, 2019

Author Contributor

pickModels depends on knowledge of scenegraph GroupNode which is no longer part of core.

pickModels was already broken, and this adds to the effort required to fix it we need to make a decision on whether to temporarily cut picking in luma.gl or invest in restoring it.

@ibgreen ibgreen force-pushed the ib/scenegraph-module branch from 494d971 to b3e1e1b Mar 19, 2019

@coveralls

This comment has been minimized.

Copy link

coveralls commented Mar 19, 2019

Coverage Status

Coverage decreased (-0.03%) to 44.905% when pulling b3e1e1b on ib/scenegraph-module into 75e2bc8 on master.

@ibgreen ibgreen merged commit dd15dc5 into master Mar 19, 2019

4 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.03%) to 44.905%
Details
license/cla Contributor License Agreement is signed.
Details

@ibgreen ibgreen deleted the ib/scenegraph-module branch Mar 19, 2019

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.