Skip to content

Commit

Permalink
fix: texture integer coordinates, resize utils (#2167)
Browse files Browse the repository at this point in the history
  • Loading branch information
ibgreen authored Aug 7, 2024
1 parent 563bbcf commit 68fb5b1
Show file tree
Hide file tree
Showing 13 changed files with 81 additions and 54 deletions.
8 changes: 1 addition & 7 deletions modules/core/src/adapter/device.ts
Original file line number Diff line number Diff line change
Expand Up @@ -389,13 +389,7 @@ export abstract class Device {
abstract createBuffer(props: BufferProps | ArrayBuffer | ArrayBufferView): Buffer;

/** Create a texture */
abstract _createTexture(props: TextureProps): Texture;
createTexture(props: TextureProps): Texture;
// createTexture(data: Promise<TextureData>): Texture;

createTexture(props: TextureProps): Texture {
return this._createTexture(props);
}
abstract createTexture(props: TextureProps): Texture;

/** Create a temporary texture view of a video source */
abstract createExternalTexture(props: ExternalTextureProps): ExternalTexture;
Expand Down
6 changes: 2 additions & 4 deletions modules/core/src/adapter/resources/framebuffer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,7 @@ export abstract class Framebuffer extends Resource<FramebufferProps> {
protected resizeAttachments(width: number, height: number): void {
for (let i = 0; i < this.colorAttachments.length; ++i) {
if (this.colorAttachments[i]) {
const resizedTexture = this.device._createTexture({
...this.colorAttachments[i].texture.props,
const resizedTexture = this.colorAttachments[i].texture.createResizedTexture({
width,
height
});
Expand All @@ -147,8 +146,7 @@ export abstract class Framebuffer extends Resource<FramebufferProps> {
}

if (this.depthStencilAttachment) {
const resizedTexture = this.device._createTexture({
...this.depthStencilAttachment.texture.props,
const resizedTexture = this.depthStencilAttachment.texture.createResizedTexture({
width,
height
});
Expand Down
23 changes: 23 additions & 0 deletions modules/core/src/adapter/resources/texture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,16 @@ export abstract class Texture extends Resource<TextureProps> {
/** Copy external image data into the texture */
abstract copyExternalImage(options: CopyExternalImageOptions): {width: number; height: number};

/**
* Create a new texture with the same parameters but a different size
*
* @note Textures are immutable and cannot be resized after creation, but we can create a similar texture with the same parameters but a new size.
* @note Does not copy contents of the texture
*/
createResizedTexture(size: {width: number; height: number}): Texture {
return this.device.createTexture({...this.props, ...size});
}

/** Default options */
protected static defaultCopyExternalImageOptions: Required<CopyExternalImageOptions> = {
image: undefined!,
Expand All @@ -358,4 +368,17 @@ export abstract class Texture extends Resource<TextureProps> {
colorSpace: 'srgb',
premultipliedAlpha: false
};

/** Ensure we have integer coordinates */
protected static _fixProps(props: TextureProps): TextureProps {
const newProps = {...props};
const {width, height} = newProps;
if (typeof width === 'number') {
newProps.width = Math.max(1, Math.ceil(width));
}
if (typeof height === 'number') {
newProps.height = Math.max(1, Math.ceil(height));
}
return newProps;
}
}
19 changes: 14 additions & 5 deletions modules/engine/src/async-texture/async-texture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,11 +120,20 @@ export class AsyncTexture {
this.destroyed = true;
}

// We could implement resize by replacing the texture
// resize(width: number, height: number): boolean {
// throw new Error('Not implemented');
// // return false;
// }
/**
* Textures are immutable and cannot be resized after creation,
* but we can create a similar texture with the same parameters but a new size.
* @note Does not copy contents of the texture
* @todo Abort pending promise and create a texture with the new size?
*/
resize(size: {width: number; height: number}): void {
if (!this.isReady) {
throw new Error('Cannot resize texture before it is ready');
}
if (this.texture) {
this.texture = this.texture.createResizedTexture(size);
}
}
}

// HELPERS
Expand Down
2 changes: 1 addition & 1 deletion modules/test-utils/src/null-device/null-device.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ export class NullDevice extends Device {
return new NullRenderPass(this, {});
}

_createTexture(props: TextureProps): NullTexture {
createTexture(props: TextureProps): NullTexture {
return new NullTexture(this, props);
}

Expand Down
2 changes: 2 additions & 0 deletions modules/test-utils/src/null-device/resources/null-texture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ export class NullTexture extends Texture {
view: NullTextureView;

constructor(device: NullDevice, props: TextureProps) {
props = Texture._fixProps(props);

super(device, props);

this.device = device;
Expand Down
62 changes: 31 additions & 31 deletions modules/webgl/src/adapter/resources/webgl-framebuffer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,37 +102,6 @@ export class WEBGLFramebuffer extends Framebuffer {
});
}

/**
* Attachment resize is expected to be a noop if size is same
*
protected override resizeAttachments(width: number, height: number): this {
// for default framebuffer, just update the stored size
if (this.handle === null) {
// assert(width === undefined && height === undefined);
this.width = this.gl.drawingBufferWidth;
this.height = this.gl.drawingBufferHeight;
return this;
}
if (width === undefined) {
width = this.gl.drawingBufferWidth;
}
if (height === undefined) {
height = this.gl.drawingBufferHeight;
}
// TODO Not clear that this is better than default destroy/create implementation
for (const colorAttachment of this.colorAttachments) {
colorAttachment.texture.clone({width, height});
}
if (this.depthStencilAttachment) {
this.depthStencilAttachment.texture.resize({width, height});
}
return this;
}
*/

/**
* @param attachment
* @param texture
Expand Down Expand Up @@ -205,3 +174,34 @@ function _getFrameBufferStatus(status: GL) {
return `${status}`;
}
}

/**
* Attachment resize is expected to be a noop if size is same
*
protected override resizeAttachments(width: number, height: number): this {
// for default framebuffer, just update the stored size
if (this.handle === null) {
// assert(width === undefined && height === undefined);
this.width = this.gl.drawingBufferWidth;
this.height = this.gl.drawingBufferHeight;
return this;
}
if (width === undefined) {
width = this.gl.drawingBufferWidth;
}
if (height === undefined) {
height = this.gl.drawingBufferHeight;
}
// TODO Not clear that this is better than default destroy/create implementation
for (const colorAttachment of this.colorAttachments) {
colorAttachment.texture.clone({width, height});
}
if (this.depthStencilAttachment) {
this.depthStencilAttachment.texture.resize({width, height});
}
return this;
}
*/
2 changes: 2 additions & 0 deletions modules/webgl/src/adapter/resources/webgl-texture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,8 @@ export class WEBGLTexture extends Texture {
} | null = null;

constructor(device: Device, props: TextureProps) {
props = Texture._fixProps(props);

// Note: Clear out `props.data` so that we don't hold a reference to any big memory chunks
super(device, {...Texture.defaultProps, ...props, data: undefined!});

Expand Down
3 changes: 1 addition & 2 deletions modules/webgl/src/adapter/webgl-device.ts
Original file line number Diff line number Diff line change
Expand Up @@ -216,8 +216,7 @@ export class WebGLDevice extends Device {
return new WEBGLBuffer(this, newProps);
}

// _createTexture(props: TextureProps): WEBGLTexture {
_createTexture(props: TextureProps): Texture {
createTexture(props: TextureProps): WEBGLTexture {
return new WEBGLTexture(this, props);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import {
getGLParameters,
setGLParameters,
resetGLParameters,
WEBGLTexture,
WebGLStateTracker
} from '@luma.gl/webgl';

Expand Down Expand Up @@ -219,7 +218,7 @@ test('WebGLStateTracker#not cached parameters', t => {

t.is(gl.getParameter(gl.TEXTURE_BINDING_2D), null, 'no bound texture');

const tex = device.createTexture({}) as WEBGLTexture;
const tex = device.createTexture({});
tex.bind();
t.is(gl.getParameter(gl.TEXTURE_BINDING_2D), tex.handle, 'bound texture');

Expand Down
1 change: 1 addition & 0 deletions modules/webgpu/src/adapter/resources/webgpu-texture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ export class WebGPUTexture extends Texture {
// }

constructor(device: WebGPUDevice, props: TextureProps) {
props = Texture._fixProps(props);
super(device, props);
this.device = device;
this.initialize(props);
Expand Down
2 changes: 1 addition & 1 deletion modules/webgpu/src/adapter/webgpu-canvas-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ export class WebGPUCanvasContext extends CanvasContext {

/** Wrap the current canvas context texture in a luma.gl texture */
getCurrentTexture(): WebGPUTexture {
return this.device._createTexture({
return this.device.createTexture({
id: `${this.id}#color-texture`,
handle: this.gpuCanvasContext.getCurrentTexture(),
format: this.format
Expand Down
2 changes: 1 addition & 1 deletion modules/webgpu/src/adapter/webgpu-device.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ export class WebGPUDevice extends Device {
return new WebGPUBuffer(this, newProps);
}

_createTexture(props: TextureProps): WebGPUTexture {
createTexture(props: TextureProps): WebGPUTexture {
return new WebGPUTexture(this, props);
}

Expand Down

0 comments on commit 68fb5b1

Please sign in to comment.