diff --git a/js/display/Instance.js b/js/display/Instance.js index a6484eb1c..b222f8b21 100644 --- a/js/display/Instance.js +++ b/js/display/Instance.js @@ -77,7 +77,7 @@ class Instance { // @public {boolean} this.isWebGLSupported = display.isWebGLAllowed() && Utils.isWebGLSupported; - // TODO: add display.isVelloAllowed()? + // TODO: add display.isVelloAllowed()? https://github.com/phetsims/scenery/issues/1584 // NOTE: We rely on DeviceContext's checks to be finished before calling here this.isVelloSupported = DeviceContext.isVelloSupportedSync(); diff --git a/js/display/Renderer.js b/js/display/Renderer.js index 3593c24f8..47cb3fe56 100644 --- a/js/display/Renderer.js +++ b/js/display/Renderer.js @@ -84,7 +84,7 @@ Renderer.stripBitmask = function( bitmask ) { return bitmask & Renderer.bitmaskRendererArea; }; -// TODO: presumably we can find a better way of handling this? +// TODO: presumably we can find a better way of handling this? https://github.com/phetsims/scenery/issues/1584 Renderer.createOrderBitmask = function( firstRenderer, secondRenderer, thirdRenderer, fourthRenderer, fifthRenderer ) { firstRenderer = firstRenderer || 0; secondRenderer = secondRenderer || 0; diff --git a/js/display/VelloBlock.js b/js/display/VelloBlock.js index 606abb7b2..94d803e42 100644 --- a/js/display/VelloBlock.js +++ b/js/display/VelloBlock.js @@ -48,7 +48,7 @@ class VelloBlock extends FittedBlock { // Since we saw some jitter on iPad, see #318 and generally expect WebGPU layers to span the entire display // In the future, it would be good to understand what was causing the problem and make webgl consistent // with svg and canvas again. - // TODO: Don't have it be a "Fitted" block then + // TODO: Don't have it be a "Fitted" block then https://github.com/phetsims/scenery/issues/1584 super.initialize( display, renderer, transformRootInstance, FittedBlock.FULL_DISPLAY ); this.filterRootInstance = filterRootInstance; @@ -56,7 +56,7 @@ class VelloBlock extends FittedBlock { // // {boolean} - Whether we pass this flag to the WebGL Context. It will store the contents displayed on the screen, // // so that canvas.toDataURL() will work. It also requires clearing the context manually ever frame. Both incur // // performance costs, so it should be false by default. - // // TODO: This block can be shared across displays, so we need to handle preserveDrawingBuffer separately? + // // TODO: This block can be shared across displays, so we need to handle preserveDrawingBuffer separately? https://github.com/phetsims/scenery/issues/1584 // this.preserveDrawingBuffer = display._preserveDrawingBuffer; // list of {Drawable}s that need to be updated before we update @@ -89,7 +89,7 @@ class VelloBlock extends FittedBlock { this.domElement = this.canvas; - // TODO: handle context restoration/loss + // TODO: handle context restoration/loss https://github.com/phetsims/scenery/issues/1584 this.deviceContext = DeviceContext.getSync(); this.canvasContext = this.deviceContext.getCanvasContext( this.canvas ); } @@ -148,14 +148,14 @@ class VelloBlock extends FittedBlock { } // update the fit BEFORE drawing, since it may change our offset - // TODO: make not fittable + // TODO: make not fittable https://github.com/phetsims/scenery/issues/1584 this.updateFit(); const encoding = this.encoding; encoding.reset( false ); // Iterate through all of our drawables (linked list) - //OHTWO TODO: PERFORMANCE: create an array for faster drawable iteration (this is probably a hellish memory access pattern) + //OHTWO TODO: PERFORMANCE: create an array for faster drawable iteration (this is probably a hellish memory access pattern) https://github.com/phetsims/scenery/issues/1584 this.currentDrawable = null; // we haven't rendered a drawable this frame yet for ( let drawable = this.firstDrawable; drawable !== null; drawable = drawable.nextDrawable ) { // ignore invisible drawables @@ -199,7 +199,7 @@ class VelloBlock extends FittedBlock { } /** - * TODO: code share with Canvas, somehow perhaps? + * TODO: code share with Canvas, somehow perhaps? https://github.com/phetsims/scenery/issues/1584 * Walk down towards the root, popping any clip/opacity effects that were needed. * @private * @@ -253,7 +253,7 @@ class VelloBlock extends FittedBlock { /** * Walk up towards the next leaf, pushing any clip/opacity effects that are needed. - * TODO: code share with Canvas? + * TODO: code share with Canvas? https://github.com/phetsims/scenery/issues/1584 * @private * * @param {PhetEncoding} encoding @@ -325,16 +325,16 @@ class VelloBlock extends FittedBlock { encoding.encodeLineWidth( -1 ); if ( clipArea ) { - // TODO: consolidate tolerance somewhere. Adaptively set this up? ACTUALLY we should really avoid - // TODO: re-encoding the clips like this every frame, right? + // TODO: consolidate tolerance somewhere. Adaptively set this up? ACTUALLY we should really avoid https://github.com/phetsims/scenery/issues/1584 + // TODO: re-encoding the clips like this every frame, right? https://github.com/phetsims/scenery/issues/1584 encoding.encodeShape( clipArea, true, true, 0.01 ); } else { encoding.encodeRect( 0, 0, this.canvas.width, this.canvas.height ); } - // TODO: filters we can do with this - // TODO: ensure NOT Mix.Clip when alpha < 1 (but we can gain performance if alpha is 1?) + // TODO: filters we can do with this https://github.com/phetsims/scenery/issues/1584 + // TODO: ensure NOT Mix.Clip when alpha < 1 (but we can gain performance if alpha is 1?) https://github.com/phetsims/scenery/issues/1584 encoding.encodeBeginClip( Mix.Normal, Compose.SrcOver, filterMatrix ); } } @@ -347,7 +347,7 @@ class VelloBlock extends FittedBlock { dispose() { sceneryLog && sceneryLog.VelloBlock && sceneryLog.VelloBlock( `dispose #${this.id}` ); - // TODO: many things to dispose!? + // TODO: many things to dispose!? https://github.com/phetsims/scenery/issues/1584 // clear references cleanArray( this.dirtyDrawables ); @@ -366,7 +366,7 @@ class VelloBlock extends FittedBlock { assert && assert( drawable ); assert && assert( !drawable.isDisposed ); - // TODO: instance check to see if it is a canvas cache (usually we don't need to call update on our drawables) + // TODO: instance check to see if it is a canvas cache (usually we don't need to call update on our drawables) https://github.com/phetsims/scenery/issues/1584 this.dirtyDrawables.push( drawable ); this.markDirty(); } diff --git a/js/display/drawables/ImageVelloDrawable.js b/js/display/drawables/ImageVelloDrawable.js index 5b35534d8..81af642df 100644 --- a/js/display/drawables/ImageVelloDrawable.js +++ b/js/display/drawables/ImageVelloDrawable.js @@ -52,8 +52,8 @@ class ImageVelloDrawable extends ImageStatefulDrawable( VelloSelfDrawable ) { return false; } - // TODO: only re-encode the image when IT changes, not when the transform changes!!! - // TODO: This is fairly important for performance it seems + // TODO: only re-encode the image when IT changes, not when the transform changes!!! https://github.com/phetsims/scenery/issues/1584 + // TODO: This is fairly important for performance it seems https://github.com/phetsims/scenery/issues/1584 this.encoding.reset( true ); diff --git a/js/display/drawables/PathVelloDrawable.js b/js/display/drawables/PathVelloDrawable.js index b18cd6423..f6649ffbd 100644 --- a/js/display/drawables/PathVelloDrawable.js +++ b/js/display/drawables/PathVelloDrawable.js @@ -84,16 +84,16 @@ class PathVelloDrawable extends PathStatefulDrawable( VelloSelfDrawable ) { return false; } - // TODO: consider caching the encoded shape, and only re-encoding if the shape changes. We should be able to - // TODO: append out-of-order? - // TODO: If we cache the encoded shape, IF THE SHAPE CHANGES EVERY FRAME then are we getting performance loss? - // TODO: performance gain would only happen if it was MOVING but SELF-STATIC. Fully static won't call this update. + // TODO: consider caching the encoded shape, and only re-encoding if the shape changes. We should be able to https://github.com/phetsims/scenery/issues/1584 + // TODO: append out-of-order? https://github.com/phetsims/scenery/issues/1584 + // TODO: If we cache the encoded shape, IF THE SHAPE CHANGES EVERY FRAME then are we getting performance loss? https://github.com/phetsims/scenery/issues/1584 + // TODO: performance gain would only happen if it was MOVING but SELF-STATIC. Fully static won't call this update. https://github.com/phetsims/scenery/issues/1584 this.encoding.reset( true ); const node = this.node; - // TODO: can we have this included in the computation? + // TODO: can we have this included in the computation? https://github.com/phetsims/scenery/issues/1584 const matrix = scalingMatrix.timesMatrix( this.instance.relativeTransform.matrix ); if ( node.shape ) { @@ -109,8 +109,8 @@ class PathVelloDrawable extends PathStatefulDrawable( VelloSelfDrawable ) { this.encoding.encodeMatrix( matrix ); let shape = node.shape; if ( node.lineDash.length ) { - // TODO: cache dashed shapes? OR AT LEAST DO NOT UPDATE THE IMAGE ENCODNG IF IT IS THE SAME - // TODO: See SVG example + // TODO: cache dashed shapes? OR AT LEAST DO NOT UPDATE THE IMAGE ENCODNG IF IT IS THE SAME https://github.com/phetsims/scenery/issues/1584 + // TODO: See SVG example https://github.com/phetsims/scenery/issues/1584 shape = node.shape.getDashedShape( node.lineDash, node.lineDashOffset ); } this.encoding.encodeLineWidth( node.lineWidth ); diff --git a/js/display/drawables/TextVelloDrawable.js b/js/display/drawables/TextVelloDrawable.js index de6d4c70e..896943bd0 100644 --- a/js/display/drawables/TextVelloDrawable.js +++ b/js/display/drawables/TextVelloDrawable.js @@ -93,7 +93,7 @@ class TextVelloDrawable extends PathStatefulDrawable( VelloSelfDrawable ) { if ( node.hasFill() || node.hasStroke() ) { - // TODO: font fallbacks! + // TODO: font fallbacks! https://github.com/phetsims/scenery/issues/1584 const font = ( node._font.weight === 'bold' ? ArialBoldFont : ArialFont ); let useSwash = window.phet?.chipper?.queryParameters?.swashText; @@ -108,25 +108,25 @@ class TextVelloDrawable extends PathStatefulDrawable( VelloSelfDrawable ) { shapedText = font.shapeText( node.renderedText, true ); // If we don't have all of the glyphs we'll need to render, fall back to the non-swash version - // TODO: don't create closures like this + // TODO: don't create closures like this https://github.com/phetsims/scenery/issues/1584 if ( !shapedText || shapedText.some( glyph => badIDs.includes( glyph.id ) ) ) { useSwash = false; } } if ( !useSwash ) { - // TODO: pooling, but figure out if we need to wait for the device.queue.onSubmittedWorkDone() + // TODO: pooling, but figure out if we need to wait for the device.queue.onSubmittedWorkDone() https://github.com/phetsims/scenery/issues/1584 const selfBounds = node.selfBoundsProperty._value; if ( selfBounds.isValid() && selfBounds.hasNonzeroArea() ) { - // TODO: only use accurate bounds?!!! + // TODO: only use accurate bounds?!!! https://github.com/phetsims/scenery/issues/1584 // NOTE: getting value directly, so we don't set off any bounds validation during rendering - // TODO: is 5px enough? too much? + // TODO: is 5px enough? too much? https://github.com/phetsims/scenery/issues/1584 const bounds = node.selfBoundsProperty._value.transformed( matrix ).dilate( 5 ).roundOut(); - // TODO: clip this to the block's Canvas, so HUGE text won't create a huge texture - // TODO: Check... Ohm's Law? - // TODO: NOTE: If a block resizes, WOULD we be marked as dirty? If not, we'd have to listen to it + // TODO: clip this to the block's Canvas, so HUGE text won't create a huge texture https://github.com/phetsims/scenery/issues/1584 + // TODO: Check... Ohm's Law? https://github.com/phetsims/scenery/issues/1584 + // TODO: NOTE: If a block resizes, WOULD we be marked as dirty? If not, we'd have to listen to it https://github.com/phetsims/scenery/issues/1584 textRenderingCanvas.width = bounds.width; textRenderingCanvas.height = bounds.height; @@ -142,7 +142,7 @@ class TextVelloDrawable extends PathStatefulDrawable( VelloSelfDrawable ) { const context = canvas.getContext( '2d' ); context.drawImage( textRenderingCanvas, 0, 0 ); - // TODO: faster function, don't create an object? + // TODO: faster function, don't create an object? https://github.com/phetsims/scenery/issues/1584 this.encoding.encodeMatrix( Matrix3.translation( bounds.minX, bounds.minY ) ); this.encoding.encodeLineWidth( -1 ); this.encoding.encodeRect( 0, 0, canvas.width, canvas.height ); @@ -175,8 +175,8 @@ class TextVelloDrawable extends PathStatefulDrawable( VelloSelfDrawable ) { const swashTextColor = window.phet?.chipper?.queryParameters?.swashTextColor; - // TODO: support this for text, so we can QUICKLY get the bounds of text - // TODO: support these inside Font!!! + // TODO: support this for text, so we can QUICKLY get the bounds of text https://github.com/phetsims/scenery/issues/1584 + // TODO: support these inside Font!!! https://github.com/phetsims/scenery/issues/1584 let x = 0; shapedText.forEach( glyph => { @@ -206,7 +206,7 @@ class TextVelloDrawable extends PathStatefulDrawable( VelloSelfDrawable ) { let encoding = cache.get( shape ); if ( !encoding ) { encoding = new PhetEncoding(); - encoding.encodeShape( shape, isFill, false, 1 ); // TODO: tolerance + encoding.encodeShape( shape, isFill, false, 1 ); // TODO: tolerance https://github.com/phetsims/scenery/issues/1584 cache.set( shape, encoding ); } diff --git a/js/display/vello/Affine.ts b/js/display/vello/Affine.ts index 978537567..5c731660f 100644 --- a/js/display/vello/Affine.ts +++ b/js/display/vello/Affine.ts @@ -1,9 +1,9 @@ // Copyright 2023, University of Colorado Boulder /** - * An affine matrix - TODO: just replace this with typed arrays + * An affine matrix - TODO: just replace this with typed arrays https://github.com/phetsims/scenery/issues/1584 * - * TODO: pooling? + * TODO: pooling? https://github.com/phetsims/scenery/issues/1584 * * @author Jonathan Olson */ @@ -17,7 +17,7 @@ export default class Affine { ) {} public times( affine: Affine ): Affine { - // TODO: Affine (and this method) are a hot spot IF we are doing client-side matrix stuff + // TODO: Affine (and this method) are a hot spot IF we are doing client-side matrix stuff https://github.com/phetsims/scenery/issues/1584 const a00 = this.a00 * affine.a00 + this.a01 * affine.a10; const a01 = this.a00 * affine.a01 + this.a01 * affine.a11; const a02 = this.a00 * affine.a02 + this.a01 * affine.a12 + this.a02; diff --git a/js/display/vello/Atlas.ts b/js/display/vello/Atlas.ts index 80a357585..f31175a4e 100644 --- a/js/display/vello/Atlas.ts +++ b/js/display/vello/Atlas.ts @@ -38,9 +38,9 @@ export default class Atlas { private unused: AtlasSubImage[] = []; public constructor( public device: GPUDevice ) { - // TODO: Do we have "repeat" on images also? Think repeating patterns! + // TODO: Do we have "repeat" on images also? Think repeating patterns! https://github.com/phetsims/scenery/issues/1584 - // TODO: atlas size (1) when no images? + // TODO: atlas size (1) when no images? https://github.com/phetsims/scenery/issues/1584 this.width = ATLAS_INITIAL_SIZE; this.height = ATLAS_INITIAL_SIZE; @@ -52,7 +52,7 @@ export default class Atlas { public updatePatches( patches: VelloImagePatch[] ): void { - // TODO: actually could we accomplish this with a generation? + // TODO: actually could we accomplish this with a generation? https://github.com/phetsims/scenery/issues/1584 this.unused.push( ...this.used ); this.used.length = 0; @@ -72,9 +72,9 @@ export default class Atlas { } } - // TODO: Add some "unique" identifier on BufferImage so we can check if it's actually representing the same image - // TODO: would it kill performance to hash the image data? If we're changing the image every frame, that would - // TODO: be very excessive. + // TODO: Add some "unique" identifier on BufferImage so we can check if it's actually representing the same image https://github.com/phetsims/scenery/issues/1584 + // TODO: would it kill performance to hash the image data? If we're changing the image every frame, that would https://github.com/phetsims/scenery/issues/1584 + // TODO: be very excessive. https://github.com/phetsims/scenery/issues/1584 private getAtlasSubImage( image: EncodableImage ): AtlasSubImage | null { // Try a "used" one first (e.g. multiple in the same scene) @@ -160,7 +160,7 @@ export default class Atlas { private replaceTexture(): void { this.texture && this.texture.destroy(); - // TODO: Check this on Windows! + // TODO: Check this on Windows! https://github.com/phetsims/scenery/issues/1584 const canvasFormat = navigator.gpu.getPreferredCanvasFormat(); this.texture = this.device.createTexture( { @@ -182,15 +182,15 @@ export default class Atlas { public updateTexture(): void { while ( this.dirtyAtlasSubImages.length ) { - // TODO: copy in gutter! (so it's not fuzzy) + // TODO: copy in gutter! (so it's not fuzzy) https://github.com/phetsims/scenery/issues/1584 const atlasSubImage = this.dirtyAtlasSubImages.pop()!; const image = atlasSubImage.image; - // TODO: note premultiplied. Why we don't want to use BufferImages from canvas data + // TODO: note premultiplied. Why we don't want to use BufferImages from canvas data https://github.com/phetsims/scenery/issues/1584 if ( image instanceof BufferImage ) { - // TODO: we have the ability to do this in a single call, would that be better ever for performance? Maybe a single - // TODO: call if we have to update a bunch of sections at once? + // TODO: we have the ability to do this in a single call, would that be better ever for performance? Maybe a single https://github.com/phetsims/scenery/issues/1584 + // TODO: call if we have to update a bunch of sections at once? https://github.com/phetsims/scenery/issues/1584 this.device.queue.writeTexture( { texture: this.texture!, origin: { @@ -283,7 +283,7 @@ export default class Atlas { scenery.register( 'Atlas', Atlas ); -// TODO: pool these? +// TODO: pool these? https://github.com/phetsims/scenery/issues/1584 export class AtlasSubImage { public constructor( public readonly image: EncodableImage, diff --git a/js/display/vello/BufferImage.ts b/js/display/vello/BufferImage.ts index a6d66abce..f0f3174ca 100644 --- a/js/display/vello/BufferImage.ts +++ b/js/display/vello/BufferImage.ts @@ -10,7 +10,7 @@ import { scenery } from '../../imports.js'; import IntentionalAny from '../../../../phet-core/js/types/IntentionalAny.js'; export default class BufferImage { - // TODO: perhaps reorder parameters + // TODO: perhaps reorder parameters https://github.com/phetsims/scenery/issues/1584 // NOTE: IMPORTANT! If using this, make sure the buffer has premultiplied values. Canvas.getImageData() is NOT // premultiplied. Use SourceImage with Canvases public constructor( diff --git a/js/display/vello/BufferPool.ts b/js/display/vello/BufferPool.ts index 4ffdf2a53..2cf65bbe6 100644 --- a/js/display/vello/BufferPool.ts +++ b/js/display/vello/BufferPool.ts @@ -5,8 +5,8 @@ import { scenery } from '../../imports.js'; /** * A pool of GPU buffers that can be reused. * - * TODO: can we reuse some buffers AFTER we ensure the synchronization is clear? - * TODO: // device.queue.onSubmittedWorkDone().then( () => {} ); and clear? + * TODO: can we reuse some buffers AFTER we ensure the synchronization is clear? https://github.com/phetsims/scenery/issues/1584 + * TODO: // device.queue.onSubmittedWorkDone().then( () => {} ); and clear? https://github.com/phetsims/scenery/issues/1584 * * @author Jonathan Olson */ @@ -81,7 +81,7 @@ export default class BufferPool { scenery.register( 'BufferPool', BufferPool ); class BufferEntry { - // TODO: pool these + // TODO: pool these https://github.com/phetsims/scenery/issues/1584 public constructor( public readonly buffer: GPUBuffer, public readonly size: number, diff --git a/js/display/vello/ByteBuffer.ts b/js/display/vello/ByteBuffer.ts index bb6dbbc1f..52cb5d605 100644 --- a/js/display/vello/ByteBuffer.ts +++ b/js/display/vello/ByteBuffer.ts @@ -19,7 +19,7 @@ export default class ByteBuffer { public constructor( initialSize = 512 ) { this._byteLength = 0; - // TODO: resizable buffers once supported by Firefox, use maxByteLength (no copying!!!) + // TODO: resizable buffers once supported by Firefox, use maxByteLength (no copying!!!) https://github.com/phetsims/scenery/issues/1584 this._arrayBuffer = new ArrayBuffer( initialSize ); this._f32Array = new Float32Array( this._arrayBuffer ); this._u32Array = new Uint32Array( this._arrayBuffer ); @@ -53,7 +53,7 @@ export default class ByteBuffer { } public pushByteBuffer( byteBuffer: ByteBuffer ): void { - // TODO: this is a hot spot, optimize + // TODO: this is a hot spot, optimize https://github.com/phetsims/scenery/issues/1584 this.ensureSpaceFor( byteBuffer._byteLength ); this._u8Array.set( byteBuffer._u8Array.slice( 0, byteBuffer._byteLength ), this._byteLength ); @@ -122,7 +122,7 @@ export default class ByteBuffer { // NOTE: this MAY truncate public resize( byteLength = 0 ): void { - // TODO: This is a hot-spot! + // TODO: This is a hot-spot! https://github.com/phetsims/scenery/issues/1584 byteLength = byteLength || this._arrayBuffer.byteLength * 2; byteLength = Math.ceil( byteLength / 4 ) * 4; // Round up to nearest 4 (for alignment) // Double the size of the _arrayBuffer by default, copying memory diff --git a/js/display/vello/DeviceContext.ts b/js/display/vello/DeviceContext.ts index 61f50b187..c5c4f6883 100644 --- a/js/display/vello/DeviceContext.ts +++ b/js/display/vello/DeviceContext.ts @@ -40,10 +40,10 @@ export default class DeviceContext { ? 'bgra8unorm' : 'rgba8unorm'; - // TODO: handle context losses, reconstruct with the device - // TODO: get setup to manually trigger context losses - // TODO: If the GPU is unavailable, we will return ALREADY LOST contexts. We should try an immediate request for a - // TODO: device once, to see if we get a context back (transient loss), otherwise disable it for a while + // TODO: handle context losses, reconstruct with the device https://github.com/phetsims/scenery/issues/1584 + // TODO: get setup to manually trigger context losses https://github.com/phetsims/scenery/issues/1584 + // TODO: If the GPU is unavailable, we will return ALREADY LOST contexts. We should try an immediate request for a https://github.com/phetsims/scenery/issues/1584 + // TODO: device once, to see if we get a context back (transient loss), otherwise disable it for a while https://github.com/phetsims/scenery/issues/1584 device.lost.then( info => { console.error( `WebGPU device was lost: ${info.message}` ); @@ -53,7 +53,7 @@ export default class DeviceContext { // 'reason' will be 'destroyed' if we intentionally destroy the device. if ( info.reason !== 'destroyed' ) { - // TODO: handle destruction + // TODO: handle destruction https://github.com/phetsims/scenery/issues/1584 } } ).catch( err => { throw new Error( err ); @@ -292,15 +292,15 @@ export default class DeviceContext { const binHeaderBuffer = bufferPool.getBuffer( bufferSizes.bin_headers.getSizeInBytes(), 'binHeader buffer' ); - // TODO: wgpu might not have this implemented? Do I need a manual clear? - // TODO: actually, we're not reusing the buffer, so it might be zero'ed out? Check spec - // TODO: See if this clearBuffer is insufficient (implied by engine.rs docs) + // TODO: wgpu might not have this implemented? Do I need a manual clear? https://github.com/phetsims/scenery/issues/1584 + // TODO: actually, we're not reusing the buffer, so it might be zero'ed out? Check spec https://github.com/phetsims/scenery/issues/1584 + // TODO: See if this clearBuffer is insufficient (implied by engine.rs docs) https://github.com/phetsims/scenery/issues/1584 if ( encoder.clearBuffer ) { // NOTE: Firefox nightly didn't have clearBuffer, so we're feature-detecting it encoder.clearBuffer( bumpBuffer, 0 ); } else { - // TODO: can we avoid this, and just fresh-create the buffer every time? + // TODO: can we avoid this, and just fresh-create the buffer every time? https://github.com/phetsims/scenery/issues/1584 device.queue.writeBuffer( bumpBuffer, 0, new Uint8Array( bumpBuffer.size ) ); } @@ -337,7 +337,7 @@ export default class DeviceContext { configBuffer, sceneBuffer, drawMonoidBuffer, binHeaderBuffer, infoBinDataBuffer, pathBuffer, tileBuffer, bumpBuffer, ptclBuffer ] ); - // TODO: Check frees on all buffers. Note the config buffer (manually destroy that, or can we reuse it?) + // TODO: Check frees on all buffers. Note the config buffer (manually destroy that, or can we reuse it?) https://github.com/phetsims/scenery/issues/1584 bufferPool.freeBuffer( sceneBuffer ); bufferPool.freeBuffer( drawMonoidBuffer ); bufferPool.freeBuffer( binHeaderBuffer ); @@ -348,7 +348,7 @@ export default class DeviceContext { this.ramps.updateTexture(); this.atlas.updateTexture(); - // TODO: TS change so this is always defined + // TODO: TS change so this is always defined https://github.com/phetsims/scenery/issues/1584 const rampTextureView = this.ramps.textureView!; assert && assert( rampTextureView ); @@ -380,7 +380,7 @@ export default class DeviceContext { atlasPainter( context ); - // TODO: This is getting cut off at a certain amount of pixels? + // TODO: This is getting cut off at a certain amount of pixels? https://github.com/phetsims/scenery/issues/1584 console.log( canvas.toDataURL() ); } ); } @@ -446,7 +446,7 @@ export default class DeviceContext { } ); } - // for now TODO: can we reuse? Likely get some from reusing these + // for now TODO: can we reuse? Likely get some from reusing these https://github.com/phetsims/scenery/issues/1584 configBuffer.destroy(); fineOutputTexture && fineOutputTexture.destroy(); diff --git a/js/display/vello/Encoding.ts b/js/display/vello/Encoding.ts index 1856468ca..9c43e4aa3 100644 --- a/js/display/vello/Encoding.ts +++ b/js/display/vello/Encoding.ts @@ -528,7 +528,7 @@ export class Layout { export class SceneBufferSizes { - // TODO: perhaps pooling objects like these? + // TODO: perhaps pooling objects like these? https://github.com/phetsims/scenery/issues/1584 public readonly pathTagPadded: number; public readonly bufferSize: number; @@ -604,7 +604,7 @@ export class ConfigUniform { export class DispatchSizes { - // TODO: pooling + // TODO: pooling https://github.com/phetsims/scenery/issues/1584 public useLargePathScan: boolean; public path_reduce: DispatchSize; public path_reduce2: DispatchSize; @@ -685,7 +685,7 @@ export class BufferSize { } export class BufferSizes { - // TODO: this is a GREAT place to view documentation, go to each thing! + // TODO: this is a GREAT place to view documentation, go to each thing! https://github.com/phetsims/scenery/issues/1584 // // Known size buffers // pub path_reduced: BufferSize, // pub path_reduced2: BufferSize, @@ -731,7 +731,7 @@ export class BufferSizes { // The following buffer sizes have been hand picked to accommodate the vello test scenes as // well as paris-30k. These should instead get derived from the scene layout using // reasonable heuristics. - // TODO: derive from scene layout + // TODO: derive from scene layout https://github.com/phetsims/scenery/issues/1584 public readonly bin_data: BufferSize; public readonly tiles: BufferSize; public readonly segments: BufferSize; @@ -768,7 +768,7 @@ export class BufferSizes { // The following buffer sizes have been hand picked to accommodate the vello test scenes as // well as paris-30k. These should instead get derived from the scene layout using // reasonable heuristics. - // TODO: derive from scene layout + // TODO: derive from scene layout https://github.com/phetsims/scenery/issues/1584 this.bin_data = new BufferSize( ( 1 << 18 ) >>> 0, 4 ); this.tiles = new BufferSize( ( 1 << 21 ) >>> 0, TILE_BYTES ); this.segments = new BufferSize( ( 1 << 21 ) >>> 0, PATH_SEGMENT_BYTES ); @@ -1003,7 +1003,7 @@ export default class Encoding { // Clears the encoding. public reset( isFragment: boolean ): void { // Clears the rustEncoding too, reinitalizing it - // TODO: don't require hardcoding TRUE for isFragment? Almost all of them will be fragments + // TODO: don't require hardcoding TRUE for isFragment? Almost all of them will be fragments https://github.com/phetsims/scenery/issues/1584 sceneryLog && sceneryLog.Encoding && this.rustLock === 0 && ( this.rustEncoding = `let mut encoding${this.id}: Encoding = Encoding::new();\n` ); sceneryLog && sceneryLog.Encoding && this.rustLock === 0 && ( this.rustEncoding += `encoding${this.id}.reset(true);\n` ); this.transforms.length = 0; @@ -1026,7 +1026,7 @@ export default class Encoding { } } - // TODO: pool the encodings! + // TODO: pool the encodings! https://github.com/phetsims/scenery/issues/1584 public dispose(): void { while ( this.patches.length ) { this.patches.pop()!.freeToPool(); @@ -1290,7 +1290,7 @@ export default class Encoding { } // zero: => false, one => color, many => true (icky) - // TODO: better return values! + // TODO: better return values! https://github.com/phetsims/scenery/issues/1584 private addRamp( colorStops: VelloColorStop[], alpha: number, extend: Extend ): null | true | ColorRGBA32 { const offset = this.drawDataBuf.byteLength; const stopsStart = this.colorStops.length; @@ -1380,7 +1380,7 @@ export default class Encoding { u8array = new Uint8Array( image.buffer ); } else { - // TODO: Can we avoid this extra copy for a Canvas? + // TODO: Can we avoid this extra copy for a Canvas? https://github.com/phetsims/scenery/issues/1584 const canvas = document.createElement( 'canvas' ); canvas.width = image.width; canvas.height = image.height; @@ -1459,7 +1459,7 @@ export default class Encoding { } } - // TODO: make this workaround not needed + // TODO: make this workaround not needed https://github.com/phetsims/scenery/issues/1584 public finalizeScene(): void { this.encodePath( true ); this.moveTo( 0, 0 ); @@ -1486,7 +1486,7 @@ export default class Encoding { public encodeShape( shape: Shape, isFill: boolean, insertPathMarker: boolean, tolerance: number ): number { this.encodePath( isFill ); - // TODO: better code that isn't tons of forEach's that will kill our GC and add jank + // TODO: better code that isn't tons of forEach's that will kill our GC and add jank https://github.com/phetsims/scenery/issues/1584 shape.subpaths.forEach( subpath => { if ( subpath.isDrawable() ) { const startPoint = subpath.getFirstSegment().start; @@ -1510,14 +1510,14 @@ export default class Encoding { const scaled_err = maxRadius / tolerance; const n_err = Math.max( Math.pow( 1.1163 * scaled_err, 1 / 6 ), 3.999999 ); - // TODO: hacked with *4 for now, figure out how to better do this + // TODO: hacked with *4 for now, figure out how to better do this https://github.com/phetsims/scenery/issues/1584 const n = Math.ceil( n_err * Math.abs( segment.getAngleDifference() ) * ( 1.0 / ( 2.0 * Math.PI ) ) ) * 4; // For now, evenly subdivide const segments = n > 1 ? segment.subdivisions( _.range( 1, n ).map( t => t / n ) ) : [ segment ]; // Create cubics approximations for each segment - // TODO: performance optimize? + // TODO: performance optimize? https://github.com/phetsims/scenery/issues/1584 segments.forEach( subSegment => { const start = subSegment.start; const middle = subSegment.positionAt( 0.5 ); @@ -1581,7 +1581,7 @@ export default class Encoding { // Path tag stream layout.pathTagBase = sizeToWords( dataBuf.byteLength ); dataBuf.pushByteBuffer( this.pathTagsBuf ); - // TODO: what if we... just error if there are open clips? Why are we padding the streams to make this work? + // TODO: what if we... just error if there are open clips? Why are we padding the streams to make this work? https://github.com/phetsims/scenery/issues/1584 for ( let i = 0; i < this.numOpenClips; i++ ) { dataBuf.pushU8( PathTag.PATH ); } @@ -1616,7 +1616,7 @@ export default class Encoding { else { assert && assert( patch.atlasSubImage ); bytes = u32ToBytes( ( patch.atlasSubImage!.x << 16 ) >>> 0 | patch.atlasSubImage!.y ); - // TODO: assume the image fit (if not, we'll need to do something else) + // TODO: assume the image fit (if not, we'll need to do something else) https://github.com/phetsims/scenery/issues/1584 } // Patch data directly into our full output @@ -1625,7 +1625,7 @@ export default class Encoding { } // Transform stream - // TODO: Float32Array instead of Affine? + // TODO: Float32Array instead of Affine? https://github.com/phetsims/scenery/issues/1584 layout.transformBase = sizeToWords( dataBuf.byteLength ); for ( let i = 0; i < this.transforms.length; i++ ) { const transform = this.transforms[ i ]; diff --git a/js/display/vello/Ramps.ts b/js/display/vello/Ramps.ts index 7c7895a82..690f46148 100644 --- a/js/display/vello/Ramps.ts +++ b/js/display/vello/Ramps.ts @@ -28,7 +28,7 @@ export default class Ramps { public constructor( public readonly device: GPUDevice ) { // NOTE: we can increase the size in the future - // TODO: test the increase in size, and other ramp characteristics + // TODO: test the increase in size, and other ramp characteristics https://github.com/phetsims/scenery/issues/1584 this.arrayBuffer = new ArrayBuffer( NUM_RAMP_SAMPLES * STARTING_RAMPS * 4 ); this.arrayView = new DataView( this.arrayBuffer ); this.width = NUM_RAMP_SAMPLES; @@ -164,7 +164,7 @@ export default class Ramps { scenery.register( 'Ramps', Ramps ); class RampEntry { - // TODO: make poolable? + // TODO: make poolable? https://github.com/phetsims/scenery/issues/1584 public constructor( public readonly colorStops: VelloColorStop[], public readonly index: number, diff --git a/js/display/vello/SourceImage.ts b/js/display/vello/SourceImage.ts index 4dd0f235d..1a6d283ea 100644 --- a/js/display/vello/SourceImage.ts +++ b/js/display/vello/SourceImage.ts @@ -15,7 +15,7 @@ import { scenery } from '../../imports.js'; import IntentionalAny from '../../../../phet-core/js/types/IntentionalAny.js'; export default class SourceImage { - // TODO: perhaps reorder parameters + // TODO: perhaps reorder parameters https://github.com/phetsims/scenery/issues/1584 public constructor( public readonly width: number, public readonly height: number, diff --git a/js/nodes/Text.ts b/js/nodes/Text.ts index 0f267176c..baf0c9e0d 100644 --- a/js/nodes/Text.ts +++ b/js/nodes/Text.ts @@ -320,14 +320,14 @@ export default class Text extends Paintable( Node ) { bitmask |= Renderer.bitmaskCanvas; } if ( !this._isHTML ) { - // TODO: Do we still do _isHTML at all?!? Let's get rid of that + // TODO: Do we still do _isHTML at all?!? Let's get rid of that https://github.com/phetsims/scenery/issues/1584 bitmask |= Renderer.bitmaskSVG; } // fill and stroke will determine whether we have DOM text support bitmask |= Renderer.bitmaskDOM; - // TODO: make this very conditional + // TODO: make this very conditional https://github.com/phetsims/scenery/issues/1584 bitmask |= Renderer.bitmaskVello; return bitmask; diff --git a/js/util/RendererSummary.js b/js/util/RendererSummary.js index 9b068877c..821ffc44a 100644 --- a/js/util/RendererSummary.js +++ b/js/util/RendererSummary.js @@ -327,7 +327,7 @@ class RendererSummary { return false; // sanity check } - // TODO: reduce code duplication + // TODO: reduce code duplication https://github.com/phetsims/scenery/issues/1584 /** * Given a bitmask representing a list of ordered preferred renderers, we check to see if all of our nodes can be * displayed in a single Vello block, AND that given the preferred renderers, that it will actually happen in our