-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Update docs for getShaders()
#876
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent, thanks!
Were there no places where this was an issue in the existing layers (including sample-layers)?
docs/advanced/subclassed-layers.md
Outdated
This makes it much easier for others to subclass your layer and make small | ||
changes to the shaders. | ||
|
||
Note: When overwriting `getShaders()` you pass down any unmodified shader(s) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you pass
=> you should pass
... See code example below.
docs/advanced/subclassed-layers.md
Outdated
vs: super.getShaders().vs, | ||
fs: fragmentShader | ||
} | ||
const shaders = Object.assign({}, super.getShaders(), { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a comment -
// use object.assign to make sure we don't overwrite existing fields like `vs`, `modules`...
@@ -59,17 +59,22 @@ export default MultiColorPathLayer extends PathLayer { | |||
|
|||
You can replace the shaders used in a layer by overriding the `getShaders()` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we add any links to luma.gl shader modules docs (or maybe they are still pretty thin?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
luma.gl's shader module documentation talks more about assemberShaders
and Model
class, not about shader modules in general, probably not a good link.
Verified sample-layers, most extend Layer. Fixed in few examples. |
Fixes #867