Skip to content

Commit

Permalink
chore(engine): Refine Model.props.disableWarnings (#2031)
Browse files Browse the repository at this point in the history
  • Loading branch information
ibgreen authored Mar 13, 2024
1 parent f4af7b6 commit 3154cf8
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 23 deletions.
5 changes: 4 additions & 1 deletion modules/core/src/adapter/resources/render-pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,10 @@ export abstract class RenderPipeline extends Resource<RenderPipelineProps> {
}

/** Set bindings (stored on pipeline and set before each call) */
abstract setBindings(bindings: Record<string, Binding>): void;
abstract setBindings(
bindings: Record<string, Binding>,
options?: {disableWarnings?: boolean}
): void;

/** Draw call. Returns false if the draw call was aborted (due to resources still initializing) */
abstract draw(options: {
Expand Down
35 changes: 17 additions & 18 deletions modules/engine/src/model/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,9 @@ export type ModelProps = Omit<RenderPipelineProps, 'vs' | 'fs' | 'bindings'> & {
attributes?: Record<string, Buffer>;
/** */
constantAttributes?: Record<string, TypedArray>;
/** Some applications intentionally supply unused attributes */
ignoreUnknownAttributes?: boolean;

/** Some applications intentionally supply unused attributes and bindings, and want to disable warnings */
disableWarnings?: boolean;

/** @internal For use with {@link TransformFeedback}, WebGL only. */
varyings?: string[];
Expand Down Expand Up @@ -119,7 +120,7 @@ export class Model {
shaderAssembler: ShaderAssembler.getDefaultShaderAssembler(),

debugShaders: undefined!,
ignoreUnknownAttributes: undefined!
disableWarnings: undefined!
};

readonly device: Device;
Expand Down Expand Up @@ -282,16 +283,12 @@ export class Model {
if (props.instanceCount) {
this.setInstanceCount(props.instanceCount);
}
// @ts-expect-error
if (props.indices) {
throw new Error('Model.props.indices removed. Use props.indexBuffer');
}
if (props.indexBuffer) {
this.setIndexBuffer(props.indexBuffer);
}
if (props.attributes) {
this.setAttributes(props.attributes, {
ignoreUnknownAttributes: props.ignoreUnknownAttributes
disableWarnings: props.disableWarnings
});
}
if (props.constantAttributes) {
Expand Down Expand Up @@ -369,7 +366,9 @@ export class Model {
// Any caching needs to be done inside the pipeline functions
// TODO this is a busy initialized check for all bindings every frame
const syncBindings = this._getBindings();
this.pipeline.setBindings(syncBindings);
this.pipeline.setBindings(syncBindings, {
disableWarnings: this.props.disableWarnings
});
if (!isObjectEmpty(this.uniforms)) {
this.pipeline.setUniformsWebGL(this.uniforms);
}
Expand Down Expand Up @@ -536,10 +535,7 @@ export class Model {
* Sets attributes (buffers)
* @note Overrides any attributes previously set with the same name
*/
setAttributes(
buffers: Record<string, Buffer>,
options?: {ignoreUnknownAttributes?: boolean}
): void {
setAttributes(buffers: Record<string, Buffer>, options?: {disableWarnings?: boolean}): void {
if (buffers.indices) {
log.warn(
`Model:${this.id} setAttributes() - indexBuffer should be set using setIndexBuffer()`
Expand All @@ -564,7 +560,7 @@ export class Model {
set = true;
}
}
if (!set && !(options?.ignoreUnknownAttributes || this.props.ignoreUnknownAttributes)) {
if (!set && !(options?.disableWarnings || this.props.disableWarnings)) {
log.warn(
`Model(${this.id}): Ignoring buffer "${buffer.id}" for unknown attribute "${bufferName}"`
)();
Expand All @@ -581,12 +577,15 @@ export class Model {
* is read from the context's global constant value for that attribute location.
* @param constantAttributes
*/
setConstantAttributes(attributes: Record<string, TypedArray>): void {
setConstantAttributes(
attributes: Record<string, TypedArray>,
options?: {disableWarnings?: boolean}
): void {
for (const [attributeName, value] of Object.entries(attributes)) {
const attributeInfo = this._attributeInfos[attributeName];
if (attributeInfo) {
this.vertexArray.setConstantWebGL(attributeInfo.location, value);
} else {
} else if (!(options?.disableWarnings || this.props.disableWarnings)) {
log.warn(
`Model "${this.id}: Ignoring constant supplied for unknown attribute "${attributeName}"`
)();
Expand Down Expand Up @@ -678,8 +677,8 @@ export class Model {
// TODO - delete previous geometry?
this.vertexCount = gpuGeometry.vertexCount;
this.setIndexBuffer(gpuGeometry.indices || null);
this.setAttributes(gpuGeometry.attributes, {ignoreUnknownAttributes: true});
this.setAttributes(attributes, {ignoreUnknownAttributes: this.props.ignoreUnknownAttributes});
this.setAttributes(gpuGeometry.attributes, {disableWarnings: true});
this.setAttributes(attributes, {disableWarnings: this.props.disableWarnings});

this.setNeedsRedraw('geometry attributes');
}
Expand Down
10 changes: 6 additions & 4 deletions modules/webgl/src/adapter/resources/webgl-render-pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ export class WEBGLRenderPipeline extends RenderPipeline {
* Bindings include: textures, samplers and uniform buffers
* @todo needed for portable model
*/
setBindings(bindings: Record<string, Binding>): void {
setBindings(bindings: Record<string, Binding>, options?: {disableWarnings?: boolean}): void {
// if (log.priority >= 2) {
// checkUniformValues(uniforms, this.id, this._uniformSetters);
// }
Expand All @@ -110,9 +110,11 @@ export class WEBGLRenderPipeline extends RenderPipeline {
const validBindings = this.shaderLayout.bindings
.map(binding => `"${binding.name}"`)
.join(', ');
log.warn(
`Unknown binding "${name}" in render pipeline "${this.id}", expected one of ${validBindings}`
)();
if (options?.disableWarnings) {
log.warn(
`Unknown binding "${name}" in render pipeline "${this.id}", expected one of ${validBindings}`
)();
}
continue; // eslint-disable-line no-continue
}
if (!value) {
Expand Down

0 comments on commit 3154cf8

Please sign in to comment.