Skip to content

Commit

Permalink
chore: Enable TypeScript null hecks
Browse files Browse the repository at this point in the history
  • Loading branch information
ibgreen committed Sep 21, 2024
1 parent 89e8581 commit 9f5096d
Show file tree
Hide file tree
Showing 28 changed files with 85 additions and 56 deletions.
6 changes: 3 additions & 3 deletions .github/ISSUE_TEMPLATE/bug-report.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: Bug Report
description: Something does not work as expected
name: Open a Bug Report (GitHub Issue)
description: GitHub Issues are for confirmed bugs or approved feature requests ONLY. Please ask generic questions or request help via GitHub Discussions.
title: "[Bug]"
labels: bug
body:
Expand All @@ -14,7 +14,7 @@ body:
luma.gl primarily uses [GitHub Discussions](https://github.com/visgl/luma.gl/discussions)
and the [Open Visualization OpenJS Foundation slack channels](https://www.openvisualization.org/#get-involved) to interact with users.
GitHub Issues are for confirmed bugs only.
Please start in the Discussions section rather than opening an issue if you are e.g.
- not yet sure if you are hitting a bug or just using the APIs wrong
- having issues with your particular build environment that are not yet clearly root caused,
Expand Down
4 changes: 2 additions & 2 deletions .github/ISSUE_TEMPLATE/config.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
blank_issues_enabled: false
contact_links:
- name: I have a question / I need help
- name: Start a Discussion: Open a GitHub Discussion (I have a question / I need help)
url: https://github.com/visgl/loaders.gl/discussions
about: Please ask generic questions or request help in discussions. Non-specific issues will be closed by maintainers.
about: Please ask generic questions or request help here in discussions. "Issues" are for confirmed bugs or approved feature requests.
2 changes: 1 addition & 1 deletion modules/gltf/src/gltf/create-gltf-model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ export type CreateGLTFModelOptions = {
};

export function createGLTFModel(device: Device, options: CreateGLTFModelOptions): ModelNode {
const {id, geometry, material, vertexCount, materialOptions, modelOptions} = options;
const {id, geometry, material, vertexCount, materialOptions, modelOptions = {}} = options;

const parsedMaterial = parsePBRMaterial(device, material, geometry.attributes, materialOptions);
log.info(4, 'createGLTFModel defines: ', parsedMaterial.defines)();
Expand Down
6 changes: 3 additions & 3 deletions modules/gltf/src/gltf/gltf-animator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// SPDX-License-Identifier: MIT
// Copyright (c) vis.gl contributors

import {log} from '@luma.gl/core';
import {log, TypedArray} from '@luma.gl/core';
import {Matrix4, Quaternion} from '@math.gl/core';

// TODO: import from loaders.gl?
Expand Down Expand Up @@ -102,13 +102,13 @@ function accessorToJsArray(accessor) {
const length = components * accessor.count;
const {buffer, byteOffset} = accessor.bufferView.data;

const array = new ArrayType(buffer, byteOffset + (accessor.byteOffset || 0), length);
const array: TypedArray = new ArrayType(buffer, byteOffset + (accessor.byteOffset || 0), length);

if (components === 1) {
accessor._animation = Array.from(array);
} else {
// Slice array
const slicedArray = [];
const slicedArray: number[][] = [];
for (let i = 0; i < array.length; i += components) {
slicedArray.push(Array.from(array.slice(i, i + components)));
}
Expand Down
3 changes: 2 additions & 1 deletion modules/gltf/src/gltf/gltf-instantiator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export type GLTFInstantiatorOptions = {
const DEFAULT_OPTIONS: GLTFInstantiatorOptions = {
modelOptions: {},
pbrDebug: false,
imageBasedLightingEnvironment: null,
imageBasedLightingEnvironment: undefined,
lights: true,
useTangents: false
};
Expand Down Expand Up @@ -52,6 +52,7 @@ export class GLTFInstantiator {
return new GLTFAnimator(this.gltf);
}

// @ts-ignore TODO - should we create an empty animator that does nothing?
return null;
}

Expand Down
2 changes: 1 addition & 1 deletion modules/gltf/src/pbr/parse-pbr-material.ts
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ function addTexture(
device: Device,
gltfTexture,
uniformName: string,
define = null,
define: string,
parsedMaterial: ParsedPBRMaterial
): void {
const parameters = gltfTexture?.texture?.sampler?.parameters || {};
Expand Down
27 changes: 14 additions & 13 deletions modules/gltf/src/pbr/pbr-environment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,11 @@ export function loadPBREnvironment(device: Device, props: PBREnvironmentProps):
const brdfLutTexture = new AsyncTexture(device, {
id: 'brdfLUT',
sampler: {
wrapS: 'clamp-to-edge',
wrapT: 'clamp-to-edge',
addressModeU: 'clamp-to-edge',
addressModeV: 'clamp-to-edge',
minFilter: 'linear',
maxFilter: 'linear'
} as SamplerProps,
magFilter: 'linear'
} as const satisfies SamplerProps,
// Texture accepts a promise that returns an image as data (Async Textures)
data: loadImageTexture(props.brdfLutUrl)
});
Expand All @@ -36,28 +36,29 @@ export function loadPBREnvironment(device: Device, props: PBREnvironmentProps):
id: 'DiffuseEnvSampler',
getTextureForFace: dir => loadImageTexture(props.getTexUrl('diffuse', dir, 0)),
sampler: {
wrapS: 'clamp-to-edge',
wrapT: 'clamp-to-edge',
addressModeU: 'clamp-to-edge',
addressModeV: 'clamp-to-edge',
minFilter: 'linear',
maxFilter: 'linear'
} as SamplerProps
magFilter: 'linear'
} as const satisfies SamplerProps
});

const specularEnvSampler = makeCube(device, {
id: 'SpecularEnvSampler',
getTextureForFace: (dir: number) => {
const imageArray = [];
const imageArray: Promise<any>[] = [];
// @ts-ignore
for (let lod = 0; lod <= props.specularMipLevels - 1; lod++) {
imageArray.push(loadImageTexture(props.getTexUrl('specular', dir, lod)));
}
return imageArray;
},
sampler: {
wrapS: 'clamp-to-edge',
wrapT: 'clamp-to-edge',
addressModeU: 'clamp-to-edge',
addressModeV: 'clamp-to-edge',
minFilter: 'linear', // [GL.TEXTURE_MIN_FILTER]: GL.LINEAR_MIPMAP_LINEAR,
maxFilter: 'linear'
} as SamplerProps
magFilter: 'linear'
} as const satisfies SamplerProps
});

return {
Expand Down
16 changes: 11 additions & 5 deletions modules/shadertools/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,17 +1,23 @@
{
"extends": "../../tsconfig.json",
"include": ["src/**/*"],
"exclude": ["node_modules", "test"],
"include": [
"src/**/*"
],
"exclude": [
"node_modules",
"test"
],
"compilerOptions": {
"strict": true,
"strictNullChecks": true,
"noImplicitAny": true,
"composite": true,
"rootDir": "src",
"outDir": "dist"
},
"references": [
// TODO: shadertools is almost independent of API, is it worth breaking the dependency?
{"path": "../core"}
{
"path": "../core"
}
]
}
}
8 changes: 5 additions & 3 deletions modules/test-utils/src/create-test-device.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,8 @@ const webgpuDevicePromise = makeWebGPUTestDevice();
export async function getTestDevices(
types: ('webgl' | 'webgpu' | 'null' | 'unknown')[] = ['webgl', 'webgpu']
): Promise<Device[]> {
return [await getNullTestDevice(), await getWebGLTestDevice(), await getWebGPUTestDevice()]
.filter(Boolean)
.filter(device => types.includes(device.type));
return ([await getNullTestDevice(), await getWebGLTestDevice(), await getWebGPUTestDevice()] as Device[])
.filter(device => types.includes(device?.type));
}

/** returns WebGPU device promise, if available */
Expand Down Expand Up @@ -73,6 +72,7 @@ async function makeWebGPUTestDevice(): Promise<WebGPUDevice | null> {
webgpuDeviceResolvers.resolve(webgpuDevice);
} catch (error) {
log.error(String(error))();
// @ts-ignore TODO
webgpuDeviceResolvers.resolve(null);
}
return webgpuDeviceResolvers.promise;
Expand All @@ -93,6 +93,7 @@ async function makeWebGLTestDevice(): Promise<WebGLDevice> {
webglDeviceResolvers.resolve(webglDevice);
} catch (error) {
log.error(String(error))();
// @ts-ignore TODO
webglDeviceResolvers.resolve(null);
}
return webglDeviceResolvers.promise;
Expand All @@ -113,6 +114,7 @@ async function makeNullTestDevice(): Promise<NullDevice> {
nullDeviceResolvers.resolve(nullDevice);
} catch (error) {
log.error(String(error))();
// @ts-ignore TODO
nullDevicePromise = Promise.resolve(null);
}
return nullDeviceResolvers.promise;
Expand Down
2 changes: 2 additions & 0 deletions modules/test-utils/src/deprecated/classic-animation-loop.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// SPDX-License
// Copyright (c) vis.gl contributors

// @ts-nocheck This should be replaced with model animation loop

// TODO - replace createGLContext, instrumentGLContext, resizeGLContext?
// TODO - remove dependency on framebuffer (bundle size impact)
import {luma, Device, DeviceProps, log} from '@luma.gl/core';
Expand Down
2 changes: 1 addition & 1 deletion modules/test-utils/src/null-device/null-canvas-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export class NullCanvasContext extends CanvasContext {
return 'NullCanvasContext';
}

constructor(device: NullDevice, props: CanvasContextProps) {
constructor(device: NullDevice, props?: CanvasContextProps) {
// Note: Base class creates / looks up the canvas (unless under Node.js)
super(props);
this.device = device;
Expand Down
4 changes: 3 additions & 1 deletion modules/test-utils/src/null-device/resources/null-texture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ export class NullTexture extends Texture {
// const data = props.data;
// this.setImageData(props);

this.setSampler(props.sampler);
if (props.sampler) {
this.setSampler(props.sampler);
}

this.view = new NullTextureView(this.device, {
...props,
Expand Down
1 change: 1 addition & 0 deletions modules/test-utils/src/performance-test-runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export class PerformanceTestRunner extends TestRunner {
this._fps?.timeEnd();
this._fps?.timeStart();

// @ts-ignore TODO
if (this._fps.count > this.testOptions.maxFramesToRender) {
animationProps.done();
}
Expand Down
4 changes: 2 additions & 2 deletions modules/webgl/src/adapter/converters/webgl-texture-table.ts
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ export function getDepthStencilAttachmentWebGL(
/** TODO - VERY roundabout legacy way of calculating bytes per pixel */
export function getTextureFormatBytesPerPixel(format: TextureFormat): number {
const formatInfo = getTextureFormatInfo(format);
return formatInfo.bytesPerPixel;
return formatInfo.bytesPerPixel as number;
}

// DATA TYPE HELPERS
Expand Down Expand Up @@ -394,7 +394,7 @@ export function getWebGLPixelDataFormat(
/**
* Map WebGPU style texture format strings to GL constants
*/
function convertTextureFormatToGL(format: TextureFormat): GL | undefined {
function convertTextureFormatToGL(format: TextureFormat): GL {
const formatInfo = WEBGL_TEXTURE_FORMATS[format];
const webglFormat = formatInfo?.gl;
if (webglFormat === undefined) {
Expand Down
2 changes: 2 additions & 0 deletions modules/webgl/src/adapter/helpers/webgl-texture-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// SPDX-License-Identifier: MIT
// Copyright (c) vis.gl contributors

// @ts-nocheck This file will be deleted in upcoming refactor

import type {Buffer, Texture, FramebufferProps} from '@luma.gl/core';
import {Framebuffer, getTypedArrayFromDataType, getDataTypeFromTypedArray} from '@luma.gl/core';
import {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ function _copyTextureToTexture(device: WebGLDevice, options: CopyTextureToTextur
// TODO - support gl.readBuffer (WebGL2 only)
// const prevBuffer = gl.readBuffer(attachment);

let texture: WEBGLTexture = null;
let texture: WEBGLTexture;
let textureTarget: GL;
if (destinationTexture instanceof WEBGLTexture) {
texture = destinationTexture;
Expand Down
6 changes: 5 additions & 1 deletion modules/webgl/src/adapter/resources/webgl-query-set.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,11 @@ export class WEBGLQuerySet extends QuerySet {
throw new Error('WebGL QuerySet can only have one value');
}

this.handle = this.device.gl.createQuery();
const handle = this.device.gl.createQuery();
if (!handle) {
throw new Error('WebGL query not supported');
}
this.handle = handle;
Object.seal(this);
}

Expand Down
13 changes: 8 additions & 5 deletions modules/webgl/src/adapter/resources/webgl-render-pass.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {setGLParameters} from '../../context/parameters/unified-parameter-api';
import {WEBGLQuerySet} from './webgl-query-set';
import {WEBGLFramebuffer} from './webgl-framebuffer';

const COLOR_CHANNELS = [0x1, 0x2, 0x4, 0x8]; // GPUColorWrite RED, GREEN, BLUE, ALPHA
const COLOR_CHANNELS: NumberArray4 = [0x1, 0x2, 0x4, 0x8]; // GPUColorWrite RED, GREEN, BLUE, ALPHA

export class WEBGLRenderPass extends RenderPass {
readonly device: WebGLDevice;
Expand Down Expand Up @@ -44,7 +44,7 @@ export class WEBGLRenderPass extends RenderPass {
// Specify mapping of draw buffer locations to color attachments
const webglFramebuffer = this.props.framebuffer as WEBGLFramebuffer;
// Default framebuffers can only be set to GL.BACK or GL.NONE
if (webglFramebuffer?.handle) {
if (this.props.framebuffer && webglFramebuffer?.handle) {
const drawBuffers = this.props.framebuffer.colorAttachments.map(
(_, i) => GL.COLOR_ATTACHMENT0 + i
);
Expand Down Expand Up @@ -93,7 +93,10 @@ export class WEBGLRenderPass extends RenderPass {
// WebGPU viewports are 6 coordinates (X, Y, Z)
if (parameters.viewport.length >= 6) {
glParameters.viewport = parameters.viewport.slice(0, 4) as NumberArray4;
glParameters.depthRange = [parameters.viewport[4], parameters.viewport[5]];
glParameters.depthRange = [
parameters.viewport[4] as number,
parameters.viewport[5] as number
];
} else {
// WebGL viewports are 4 coordinates (X, Y)
glParameters.viewport = parameters.viewport as NumberArray4;
Expand All @@ -114,9 +117,9 @@ export class WEBGLRenderPass extends RenderPass {
parameters[GL.STENCIL_REF] = parameters.stencilReference;
}

if (parameters.colorMask) {
if ('colorMask' in parameters) {
glParameters.colorMask = COLOR_CHANNELS.map(channel =>
Boolean(channel & parameters.colorMask)
Boolean(channel & parameters.colorMask as number)
);
}

Expand Down
5 changes: 4 additions & 1 deletion modules/webgl/src/adapter/resources/webgl-render-pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,9 @@ export class WEBGLRenderPipeline extends RenderPipeline {
log.timeEnd(3, `RenderPipeline ${this.id} - shaderLayout introspection`)();

// Merge provided layout with introspected layout
this.shaderLayout = mergeShaderLayout(this.introspectedLayout, props.shaderLayout);
if (props.shaderLayout) {
this.shaderLayout = mergeShaderLayout(this.introspectedLayout, props.shaderLayout);
}
}

override destroy(): void {
Expand All @@ -97,6 +99,7 @@ export class WEBGLRenderPipeline extends RenderPipeline {
this.destroyed = true;
// @ts-expect-error
this.handle.destroyed = true;
// @ts-ignore
this.handle = null;
}
}
Expand Down
6 changes: 4 additions & 2 deletions modules/webgl/src/adapter/resources/webgl-texture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,9 @@ export class WEBGLTexture extends Texture {
readonly gl: WebGL2RenderingContext;
handle: WebGLTexture;

sampler: WEBGLSampler = undefined; // TODO - currently unused in WebGL. Create dummy sampler?
view: WEBGLTextureView = undefined; // TODO - currently unused in WebGL. Create dummy view?
// @ts-ignore TODO - currently unused in WebGL. Create dummy sampler?
sampler: WEBGLSampler = undefined;
view: WEBGLTextureView;

/**
* The WebGL target corresponding to the texture type
Expand Down Expand Up @@ -307,6 +308,7 @@ export class WEBGLTexture extends Texture {
}

gl.bindTexture(this.glTarget, this.handle);
// @ts-ignore TODO fix types
return _textureUnit;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ export class WEBGLTransformFeedback extends TransformFeedback {
return location >= 0 ? this.buffers[location] : null;
}

bind(funcOrHandle = this.handle) {
bind(funcOrHandle: (() => void) | WebGLTransformFeedback | null = this.handle) {
if (typeof funcOrHandle !== 'function') {
this.gl.bindTransformFeedback(GL.TRANSFORM_FEEDBACK, funcOrHandle);
return this;
Expand Down Expand Up @@ -148,7 +148,7 @@ export class WEBGLTransformFeedback extends TransformFeedback {
return Number(locationOrName);
}

for (const varying of this.layout.varyings) {
for (const varying of this.layout.varyings || []) {
if (locationOrName === varying.name) {
return varying.location;
}
Expand Down
Loading

0 comments on commit 9f5096d

Please sign in to comment.