TODO fixes, see #1584

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();
 

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;

js/display/VelloBlock.js

@@ -48,15 +48,15 @@ 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;
 
     // // {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();
   }

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 );
 

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 );

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 );
     }
 

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 <jonathan.olson@colorado.edu>
  */
@@ -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;

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,

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(

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 <jonathan.olson@colorado.edu>
  */
@@ -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,