Skip to content

Commit

Permalink
chore(engine): strict null checks (#2025)
Browse files Browse the repository at this point in the history
  • Loading branch information
ibgreen authored Mar 13, 2024
1 parent 33de6f1 commit 8ab6e67
Show file tree
Hide file tree
Showing 15 changed files with 86 additions and 48 deletions.
6 changes: 4 additions & 2 deletions modules/engine/src/animation-loop/animation-loop.ts
Original file line number Diff line number Diff line change
Expand Up @@ -356,14 +356,16 @@ export class AnimationLoop {

// Initialize the object that will be passed to app callbacks
_initializeAnimationProps(): void {
if (!this.device) {
const canvas = this.device?.canvasContext?.canvas;

if (!this.device || !canvas) {
throw new Error('loop');
}
this.animationProps = {
animationLoop: this,

device: this.device,
canvas: this.device?.canvasContext?.canvas,
canvas,
timeline: this.timeline,

// Initial values
Expand Down
9 changes: 7 additions & 2 deletions modules/engine/src/async-texture/async-texture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,16 +52,20 @@ type AsyncTextureData = AsyncTextureProps['data'];
export class AsyncTexture {
readonly device: Device;

// TODO - should we type these as possibly `null`? It will make usage harder?
// @ts-expect-error
texture: Texture;
// @ts-expect-error
sampler: Sampler;
// @ts-expect-error
view: TextureView;

readonly ready: Promise<void>;
isReady: boolean = false;
destroyed: boolean = false;

protected resolveReady: () => void;
protected rejectReady: (error: Error) => void;
protected resolveReady: () => void = () => {};
protected rejectReady: (error: Error) => void = () => {};

constructor(device: Device, props: AsyncTextureProps) {
this.device = device;
Expand Down Expand Up @@ -107,6 +111,7 @@ export class AsyncTexture {
destroy(): void {
if (this.texture) {
this.texture.destroy();
// @ts-expect-error
this.texture = null;
}
this.destroyed = true;
Expand Down
10 changes: 7 additions & 3 deletions modules/engine/src/computation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,15 +84,18 @@ export class Computation {
/** Bindings (textures, samplers, uniform buffers) */
bindings: Record<string, Binding> = {};

/** The underlying GPU "program". @note May be recreated if parameters change */
/** The underlying GPU pipeline. */
pipeline: ComputePipeline;
/** Assembled compute shader source */
source: string;
/** the underlying compiled compute shader */
// @ts-ignore Set in function called from constructor
shader: Shader;
source: string;

/** ShaderInputs instance */
shaderInputs: ShaderInputs;

// @ts-ignore Set in function called from constructor
_uniformStore: UniformStore;

_pipelineNeedsUpdate: string | false = 'newly created';
Expand All @@ -118,6 +121,7 @@ export class Computation {
const moduleMap = Object.fromEntries(
this.props.modules?.map(module => [module.name, module]) || []
);
// @ts-ignore TODO - fix up typing?
this.shaderInputs = props.shaderInputs || new ShaderInputs(moduleMap);
this.setShaderInputs(this.shaderInputs);

Expand All @@ -143,7 +147,7 @@ export class Computation {
});

this.source = source;

// @ts-ignore
this._getModuleUniforms = getUniforms;

// Create the pipeline
Expand Down
5 changes: 4 additions & 1 deletion modules/engine/src/debug/copy-texture-to-image.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,12 @@ export function copyTextureToDataUrl(
canvas.width = width;
canvas.height = height;
const context = canvas.getContext('2d');
if (!context) {
throw new Error('Failed to create context');
}

// Copy the pixels to a 2D canvas
const imageData = context.createImageData(width, height);
const imageData = context?.createImageData(width, height);
imageData.data.set(data);
context.putImageData(imageData, 0, 0);

Expand Down
26 changes: 14 additions & 12 deletions modules/engine/src/debug/debug-framebuffer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,17 +58,19 @@ export function debugFramebuffer(
// ctx.drawImage(image, 0, 0);

const color = fbo.device.readPixelsToArrayWebGL(fbo);
const imageData = ctx.createImageData(fbo.width, fbo.height);
// Full map
const offset = 0;
// if (color.some((v) => v > 0)) {
// console.error('THERE IS NON-ZERO DATA IN THE FBO!');
// }
for (let i = 0; i < color.length; i += 4) {
imageData.data[offset + i + 0] = color[i + 0] * rgbaScale;
imageData.data[offset + i + 1] = color[i + 1] * rgbaScale;
imageData.data[offset + i + 2] = color[i + 2] * rgbaScale;
imageData.data[offset + i + 3] = opaque ? 255 : color[i + 3] * rgbaScale;
const imageData = ctx?.createImageData(fbo.width, fbo.height);
if (imageData) {
// Full map
const offset = 0;
// if (color.some((v) => v > 0)) {
// console.error('THERE IS NON-ZERO DATA IN THE FBO!');
// }
for (let i = 0; i < color.length; i += 4) {
imageData.data[offset + i + 0] = color[i + 0] * rgbaScale;
imageData.data[offset + i + 1] = color[i + 1] * rgbaScale;
imageData.data[offset + i + 2] = color[i + 2] * rgbaScale;
imageData.data[offset + i + 3] = opaque ? 255 : color[i + 3] * rgbaScale;
}
ctx?.putImageData(imageData, 0, 0);
}
ctx.putImageData(imageData, 0, 0);
}
4 changes: 2 additions & 2 deletions modules/engine/src/geometry/geometry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,14 +134,14 @@ export class Geometry {
return this;
}

_calculateVertexCount(attributes: GeometryAttributes, indices: GeometryAttribute): number {
_calculateVertexCount(attributes: GeometryAttributes, indices?: GeometryAttribute): number {
if (indices) {
return indices.value.length;
}
let vertexCount = Infinity;
for (const attribute of Object.values(attributes)) {
const {value, size, constant} = attribute;
if (!constant && value && size >= 1) {
if (!constant && value && size !== undefined && size >= 1) {
vertexCount = Math.min(vertexCount, value.length / size);
}
}
Expand Down
14 changes: 10 additions & 4 deletions modules/engine/src/geometry/gpu-geometry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ export class GPUGeometry {
}

getIndexes(): Buffer | null {
return this.indices;
return this.indices || null;
}

_calculateVertexCount(positions: Buffer): number {
Expand Down Expand Up @@ -128,9 +128,15 @@ export function getAttributeBuffersFromGeometry(
name = 'colors';
break;
}
attributes[name] = device.createBuffer({data: attribute.value, id: `${attributeName}-buffer`});
const {value, size, normalized} = attribute;
bufferLayout.push({name, format: getVertexFormatFromAttribute(value, size, normalized)});
if (attribute) {
attributes[name] = device.createBuffer({
data: attribute.value,
id: `${attributeName}-buffer`
});
const {value, size, normalized} = attribute;
// @ts-expect-error
bufferLayout.push({name, format: getVertexFormatFromAttribute(value, size, normalized)});
}
}

const vertexCount = geometry._calculateVertexCount(geometry.attributes, geometry.indices);
Expand Down
2 changes: 1 addition & 1 deletion modules/engine/src/lib/pipeline-factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ export class PipelineFactory {

/** Calculate a hash based on all the inputs for a render pipeline */
private _hashRenderPipeline(props: RenderPipelineProps): string {
const vsHash = this._getHash(props.vs?.source);
const vsHash = props.vs ? this._getHash(props.vs.source) : 0;
const fsHash = props.fs ? this._getHash(props.fs.source) : 0;

// WebGL specific
Expand Down
30 changes: 19 additions & 11 deletions modules/engine/src/model/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ export type ModelProps = Omit<RenderPipelineProps, 'vs' | 'fs' | 'bindings'> & {
export class Model {
static defaultProps: Required<ModelProps> = {
...RenderPipeline.defaultProps,
source: null,
source: undefined!,
vs: null,
fs: null,
id: 'unnamed',
Expand All @@ -115,17 +115,20 @@ export class Model {
shaderInputs: undefined!,
pipelineFactory: undefined!,
shaderFactory: undefined!,
transformFeedback: undefined,
transformFeedback: undefined!,
shaderAssembler: ShaderAssembler.getDefaultShaderAssembler(),

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

readonly device: Device;
readonly id: string;
// @ts-expect-error assigned in function called from constructor
readonly source: string;
// @ts-expect-error assigned in function called from constructor
readonly vs: string;
// @ts-expect-error assigned in function called from constructor
readonly fs: string;
readonly pipelineFactory: PipelineFactory;
readonly shaderFactory: ShaderFactory;
Expand Down Expand Up @@ -173,8 +176,9 @@ export class Model {
pipeline: RenderPipeline;

/** ShaderInputs instance */
// @ts-expect-error Assigned in function called by constructor
shaderInputs: ShaderInputs;

// @ts-expect-error Assigned in function called by constructor
_uniformStore: UniformStore;

_attributeInfos: Record<string, AttributeInfo> = {};
Expand All @@ -201,13 +205,15 @@ export class Model {
const moduleMap = Object.fromEntries(
this.props.modules?.map(module => [module.name, module]) || []
);
// @ts-expect-error Fix typings
this.setShaderInputs(props.shaderInputs || new ShaderInputs(moduleMap));

// Setup shader assembler
const platformInfo = getPlatformInfo(device);

// Extract modules from shader inputs if not supplied
const modules =
// @ts-expect-error shaderInputs is assigned in setShaderInputs above.
(this.props.modules?.length > 0 ? this.props.modules : this.shaderInputs?.getModules()) || [];

const isWebGPU = this.device.type === 'webgpu';
Expand All @@ -224,6 +230,7 @@ export class Model {
modules
});
this.source = source;
// @ts-expect-error
this._getModuleUniforms = getUniforms;
} else {
// GLSL
Expand All @@ -235,6 +242,7 @@ export class Model {

this.vs = vs;
this.fs = fs;
// @ts-expect-error
this._getModuleUniforms = getUniforms;
}

Expand Down Expand Up @@ -377,7 +385,7 @@ export class Model {
vertexCount: this.vertexCount,
instanceCount: this.instanceCount,
indexCount,
transformFeedback: this.transformFeedback
transformFeedback: this.transformFeedback || undefined
});
} finally {
this._logDrawCallEnd();
Expand Down Expand Up @@ -669,7 +677,7 @@ export class Model {

// TODO - delete previous geometry?
this.vertexCount = gpuGeometry.vertexCount;
this.setIndexBuffer(gpuGeometry.indices);
this.setIndexBuffer(gpuGeometry.indices || null);
this.setAttributes(gpuGeometry.attributes, {ignoreUnknownAttributes: true});
this.setAttributes(attributes, {ignoreUnknownAttributes: this.props.ignoreUnknownAttributes});

Expand Down Expand Up @@ -800,13 +808,13 @@ export class Model {
_getAttributeDebugTable(): Record<string, Record<string, unknown>> {
const table: Record<string, Record<string, unknown>> = {};
for (const [name, attributeInfo] of Object.entries(this._attributeInfos)) {
const values = this.vertexArray.attributes[attributeInfo.location];
table[attributeInfo.location] = {
name,
type: attributeInfo.shaderType,
values: this._getBufferOrConstantValues(
this.vertexArray.attributes[attributeInfo.location],
attributeInfo.bufferDataType
)
values: values
? this._getBufferOrConstantValues(values, attributeInfo.bufferDataType)
: 'null'
};
}
if (this.vertexArray.indexBuffer) {
Expand Down
10 changes: 5 additions & 5 deletions modules/engine/src/scenegraph/model-node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,6 @@ export class ModelNode extends ScenegraphNode {
this.setProps(props);
}

override getBounds(): [number[], number[]] | null {
return this.bounds;
}

override destroy(): void {
if (this.model) {
this.model.destroy();
Expand All @@ -46,8 +42,12 @@ export class ModelNode extends ScenegraphNode {
this.managedResources = [];
}

override getBounds(): [number[], number[]] | null {
return this.bounds;
}

// Expose model methods
draw(renderPass?: RenderPass) {
draw(renderPass: RenderPass) {
// Return value indicates if something was actually drawn
return this.model.draw(renderPass);
}
Expand Down
6 changes: 3 additions & 3 deletions modules/engine/src/scenegraph/scenegraph-node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,9 +184,9 @@ export class ScenegraphNode {
*/

_setScenegraphNodeProps(props: ScenegraphNodeProps): void {
if ('display' in props) {
this.display = props.display;
}
// if ('display' in props) {
// this.display = props.display;
// }

if ('position' in props) {
this.setPosition(props.position);
Expand Down
4 changes: 3 additions & 1 deletion modules/engine/src/shader-inputs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ export class ShaderInputs<
* The map of modules
* @todo should should this include the resolved dependencies?
*/
// @ts-expect-error Fix typings
modules: Readonly<{[P in keyof ShaderPropsT]: ShaderModuleInputs<ShaderPropsT[P]>}>;

/** Stores the uniform values for each module */
Expand All @@ -59,6 +60,7 @@ export class ShaderInputs<
* Create a new UniformStore instance
* @param modules
*/
// @ts-expect-error Fix typings
constructor(modules: {[P in keyof ShaderPropsT]: ShaderModuleInputs<ShaderPropsT[P]>}) {
// TODO - get all dependencies from modules
const allModules = _resolveModules(Object.values(modules));
Expand Down Expand Up @@ -92,7 +94,7 @@ export class ShaderInputs<
setProps(props: Partial<{[P in keyof ShaderPropsT]?: Partial<ShaderPropsT[P]>}>): void {
for (const name of Object.keys(props)) {
const moduleName = name as keyof ShaderPropsT;
const moduleProps = props[moduleName];
const moduleProps = props[moduleName] || {};
const module = this.modules[moduleName];
if (!module) {
// Ignore props for unregistered modules
Expand Down
4 changes: 4 additions & 0 deletions modules/engine/src/transform/buffer-transform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ export class BufferTransform {

this.transformFeedback = this.device.createTransformFeedback({
layout: this.model.pipeline.shaderLayout,
// @ts-expect-error TODO
buffers: props.feedbackBuffers
});

Expand Down Expand Up @@ -88,6 +89,9 @@ export class BufferTransform {

readAsync(varyingName: string): Promise<Uint8Array> {
const result = this.getBuffer(varyingName);
if (!result) {
throw new Error('BufferTransform#getBuffer');
}
if (result instanceof Buffer) {
return result.readAsync();
}
Expand Down
3 changes: 2 additions & 1 deletion modules/engine/src/transform/texture-transform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ export class TextureTransform {
return targetTexture;
}

getFramebuffer(): Framebuffer {
getFramebuffer(): Framebuffer | undefined {
const currentResources = this.bindings[this.currentIndex];
return currentResources.framebuffer;
}
Expand All @@ -134,6 +134,7 @@ export class TextureTransform {
binding = {
sourceBuffers: {},
sourceTextures: {},
// @ts-expect-error
targetTexture: null
};
}
Expand Down
Loading

0 comments on commit 8ab6e67

Please sign in to comment.