Fix lessons #1028
Fix lessons #1028
Conversation
| @@ -3,6 +3,11 @@ import {AnimationLoop, Texture2D, loadFile, setParameters} from '@luma.gl/core'; | |||
| import {addEvents} from '@luma.gl/addons'; | |||
| import {Matrix4, radians} from 'math.gl'; | |||
| import {loadWorldGeometry, World} from './world'; | |||
| /* eslint-disable complexity */ | |||
There was a problem hiding this comment.
Use // eslint-disable-next-line complexity in front of each function/method instead of globally disabling it.
|
|
||
| /* | ||
| Cave texture from: http://texturelib.com/texture/?path=/Textures/rock/cave/rock_cave_0019 | ||
| */ |
There was a problem hiding this comment.
Add a line about license for textures from this lib?
examples/lessons/10/package.json
Outdated
| @@ -8,8 +8,10 @@ | |||
| "start-local": "webpack-dev-server --env.local --progress --hot --open -d" | |||
| }, | |||
| "dependencies": { | |||
| "@luma.gl/addons": "^7.0.0-alpha", | |||
| "@luma.gl/constants": "^7.0.0-alpha.3", | |||
There was a problem hiding this comment.
Use same version for all luma deps?
examples/lessons/13/app.js
Outdated
| @@ -233,6 +244,7 @@ const animationLoop = new AnimationLoop({ | |||
|
|
|||
| moon | |||
| .setUniforms({ | |||
| uSampler: moonTexture, | |||
There was a problem hiding this comment.
Do we need to set this again if we are no longer sharing programs?
Teaching best practices would be to only set uniforms that vary each frame?
There was a problem hiding this comment.
No, that was to work around the program-sharing issue.
examples/lessons/13/app.js
Outdated
| @@ -166,38 +169,46 @@ const animationLoop = new AnimationLoop({ | |||
| }), | |||
| program: fragmentLightingProgram, | |||
There was a problem hiding this comment.
Should we stop making programs in advance and just pass in the shaders to the Model constructors to avoid the sharing issue (which seems to be an overly ambitious optimization unrelated to the main exposition of this example, i.e. fragment lighting)?
| @@ -274,54 +287,6 @@ function animate(state) { | |||
| state.lastTime = timeNow; | |||
| } | |||
|
|
|||
| /* global document */ | |||
| // eslint-disable-next-line complexity | |||
| function getControlValues() { | |||
There was a problem hiding this comment.
Instead of deleting these, should we restore the HTML elements above instead, they seem to have been cut by mistake in a previous cleanup.
There was a problem hiding this comment.
It actually looks like that logic has been moved around for a while, but hasn't been hooked into any actual UI for a long while:
https://github.com/uber/luma.gl/blob/5.0-release/examples/lessons/13/app.js
https://github.com/uber/luma.gl/blob/6.0-release/examples/lessons/13/app.js
I'll leave this for now, and we can discuss we want to flesh out with controls.
Fixes to lesson examples bases on #756