feat(engine): ShaderModule.getBindings() #2099
Conversation
| @@ -43,6 +44,7 @@ export type ShaderModule< | |||
|
|
|||
| /** uniform buffers, textures, samplers, storage, ... */ | |||
| bindings?: Record<keyof BindingsT, {location: number; type: 'texture' | 'sampler' | 'uniforms'}>; | |||
| getBindings?: (settings?: any, prevBindings?: any) => Record<string, Texture | Sampler>; | |||
There was a problem hiding this comment.
nit: Types could be a little more strict here I think, with Record<keyof BindingsT, ...>? Should prevBindings have the same type?
|
@Pessimistress I'm not sure I'm convinced about the scale of the problem. Usually the texture is just a prop that is passed through as a binding: https://github.com/visgl/deck.gl/blob/f1a163a172c4cda5aeccaa11c518798bfe8779b1/modules/layers/src/bitmap-layer/bitmap-layer.ts#L298 I would say we go with this approach, migrate the deck layers and if we see too much duplication we can revise the API. Having a combined callback has drawbacks, if we use |
There was a problem hiding this comment.
There is a utility function splitUniformsAndBindings() in core. I would probably have looked into use something like that here and avoid requiring shader modules to define two get... functions (overall I would prefer if most shader modules were "pure objects" and didn't define functions).
| @@ -505,6 +505,7 @@ export class Model { | |||
| /** Update uniform buffers from the model's shader inputs */ | |||
| updateShaderInputs(): void { | |||
| this._uniformStore.setUniforms(this.shaderInputs.getUniformValues()); | |||
| this.setBindings(this.shaderInputs.getBindings()); | |||
There was a problem hiding this comment.
Nit: not much reason for this comment other than symmetry:
| this.setBindings(this.shaderInputs.getBindings()); | |
| this.setBindings(this.shaderInputs.getBindingValues()); |
| @@ -43,6 +44,7 @@ export type ShaderModule< | |||
|
|
|||
| /** uniform buffers, textures, samplers, storage, ... */ | |||
| bindings?: Record<keyof BindingsT, {location: number; type: 'texture' | 'sampler' | 'uniforms'}>; | |||
| getBindings?: (settings?: any, prevBindings?: any) => Record<string, Texture | Sampler>; | |||
Background
ShaderModulehas no support for bindings, which are needed to complete the port over to UBOs. If this approach looks OK I can make a PR on master, but doing this on v9 in order to test against deck.glChange List
getBindingstoShaderModuletypeShaderInputs&Modelclasses