Address review feedback for instances API

Author: aashu2006

src/core/p5.Renderer3D.js

@@ -549,12 +549,8 @@ export class Renderer3D extends Renderer {
     this.updateShapeVertexProperties();
   }
 
-  model(model, count = 1) {
-    // Use _instanceCount only when count was NOT explicitly passed.
-    // arguments.length distinguishes model(geom) from model(geom, 1).
-    if (arguments.length < 2 && this._instanceCount !== undefined) {
-      count = this._instanceCount;
-    }
+  model(model, count) {
+    count = count ?? this._instanceCount ?? 1;
     if (model.vertices.length > 0) {
       if (this.geometryBuilder) {
         this.geometryBuilder.addRetained(model);

src/webgl/3d_primitives.js

@@ -2596,6 +2596,18 @@ function primitives3D(p5, fn){
     return this._renderer.curveDetail(d);
   };
 
+  /**
+   * @typedef {Object} InstancesWrapper
+   * @property {function(radius: Number=, detailX: Number=, detailY: Number=): undefined} sphere
+   * @property {function(width: Number=, height: Number=, depth: Number=, detailX: Number=, detailY: Number=): undefined} box
+   * @property {function(width: Number=, height: Number=, detailX: Number=, detailY: Number=): undefined} plane
+   * @property {function(radiusX: Number=, radiusY: Number=, radiusZ: Number=, detailX: Number=, detailY: Number=): undefined} ellipsoid
+   * @property {function(radius: Number=, height: Number=, detailX: Number=, detailY: Number=, bottomCap: Boolean=, topCap: Boolean=): undefined} cylinder
+   * @property {function(radius: Number=, height: Number=, detailX: Number=, detailY: Number=, cap: Boolean=): undefined} cone
+   * @property {function(radius: Number=, tubeRadius: Number=, detailX: Number=, detailY: Number=): undefined} torus
+   * @property {function(model: p5.Geometry, count: Number=): undefined} model
+   */
+
   /**
    * Draws `count` instances of the next 3D primitive or model using WebGL
    * instanced rendering.
@@ -2608,21 +2620,38 @@ function primitives3D(p5, fn){
    * @method instances
    * @param  {Number} count number of instances to draw. Must be a positive
    *   integer.
-   * @returns {Object} an object with methods `sphere`, `box`, `plane`,
+   * @returns {p5.InstancesWrapper} an object with methods `sphere`, `box`, `plane`,
    *   `ellipsoid`, `cylinder`, `cone`, `torus`, and `model`. Call one of
    *   these to draw `count` instances of that primitive.
    *
    * @example
    * <div>
    * <code>
-   * // Draw 10 spheres in a single instanced draw call.
-   * // A custom shader reads gl_InstanceID to offset each sphere.
+   * let myShader;
+   * let count = 5;
+   *
    * function setup() {
-   *   createCanvas(100, 100, WEBGL);
+   *   createCanvas(200, 200, WEBGL);
+   *   myShader = buildMaterialShader(drawSpaced);
+   *   describe('Five red spheres arranged in a horizontal line.');
    * }
+   *
+   * function drawSpaced() {
+   *   worldInputs.begin();
+   *   // Spread spheres evenly across the canvas based on their index
+   *   let spacing = width / count;
+   *   worldInputs.position.x +=
+   *     (instanceIndex - (count - 1) / 2) * spacing;
+   *   worldInputs.end();
+   * }
+   *
    * function draw() {
-   *   background(200);
-   *   instances(10).sphere(20);
+   *   background(220);
+   *   lights();
+   *   noStroke();
+   *   fill('red');
+   *   shader(myShader);
+   *   instances(count).sphere(15);
    * }
    * </code>
    * </div>
@@ -2631,17 +2660,16 @@ function primitives3D(p5, fn){
     this._assert3d('instances');
 
     if (typeof count !== 'number' || !isFinite(count) || count < 1) {
-      console.log(
-        '🌸 p5.js says: instances() requires a positive integer count.' +
-        ' Clamping to 1.'
+      p5._friendlyError(
+        'instances() requires a positive integer count. Clamping to 1.',
+        'instances'
       );
-      count = Math.max(1, Math.round(count) || 1);
+      count = 1;
     } else {
       count = Math.round(count);
     }
 
     const r = this._renderer;
-    const p = this;
 
     // Each wrapped method: set _instanceCount, call the renderer method with
     // the correct `this`, clear _instanceCount in finally so it never leaks.
@@ -2652,7 +2680,6 @@ function primitives3D(p5, fn){
       } finally {
         r._instanceCount = undefined;
       }
-      return p;
     };
 
     return {

test/unit/visual/cases/webgl.js

@@ -1101,6 +1101,25 @@ visualSuite('WebGL', function() {
       p5.model(obj, numInstances);
       screenshot();
     });
+
+    visualTest('instances() API draws multiple spaced primitives', (p5, screenshot) => {
+      p5.createCanvas(50, 50, p5.WEBGL);
+      const count = 5;
+      const shader = p5.baseMaterialShader().modify(() => {
+        p5.getWorldInputs((inputs) => {
+          let spacing = p5.width / count;
+          inputs.position.x += (p5.instanceIndex - (count - 1) / 2.0) * spacing;
+          return inputs;
+        });
+      }, { p5 });
+      p5.background(220);
+      p5.lights();
+      p5.noStroke();
+      p5.fill('red');
+      p5.shader(shader);
+      p5.instances(count).sphere(7);
+      screenshot();
+    });
   });
 
   visualSuite('p5.strands', () => {

test/unit/visual/cases/webgpu.js

@@ -326,6 +326,25 @@ visualSuite("WebGPU", function () {
       await screenshot();
     });
 
+    visualTest('instances() API draws multiple spaced primitives (WebGPU)', async function(p5, screenshot) {
+      await p5.createCanvas(50, 50, p5.WEBGPU);
+      const count = 5;
+      const shader = p5.baseMaterialShader().modify(() => {
+        p5.getWorldInputs((inputs) => {
+          let spacing = p5.width / count;
+          inputs.position.x += (p5.instanceIndex - (count - 1) / 2.0) * spacing;
+          return inputs;
+        });
+      }, { p5 });
+      p5.background(220);
+      p5.lights();
+      p5.noStroke();
+      p5.fill('red');
+      p5.shader(shader);
+      p5.instances(count).sphere(7);
+      await screenshot();
+    });
+
     visualTest('random() colors a basic shader (WebGPU)', async function(p5, screenshot) {
       await p5.createCanvas(50, 50, p5.WEBGPU);
       const shader = p5.baseColorShader().modify(() => {

utils/typescript.mjs

@@ -366,19 +366,27 @@ function convertFunctionTypeForInterface(typeNode, options) {
     .map((param, i) => {
       let typeObj;
       let paramName;
-      if (param.type === 'ParameterType') {
-        typeObj = param.expression;
-        paramName = param.name ?? `p${i}`;
-      } else if (typeof param.type === 'object' && param.type !== null) {
-        typeObj = param.type;
-        paramName = param.name ?? `p${i}`;
+      let isOptional = false;
+
+      let currentParam = param;
+      if (currentParam && currentParam.type === 'OptionalType') {
+        isOptional = true;
+        currentParam = currentParam.expression;
+      }
+
+      if (currentParam && currentParam.type === 'ParameterType') {
+        typeObj = currentParam.expression;
+        paramName = currentParam.name ?? `p${i}`;
+      } else if (currentParam && typeof currentParam.type === 'object' && currentParam.type !== null) {
+        typeObj = currentParam.type;
+        paramName = currentParam.name ?? `p${i}`;
       } else {
         // param itself is a plain type node
-        typeObj = param;
+        typeObj = currentParam;
         paramName = `p${i}`;
       }
       const paramType = convertTypeToTypeScript(typeObj, options);
-      return `${paramName}: ${paramType}`;
+      return `${paramName}${isOptional ? '?' : ''}: ${paramType}`;
     })
     .join(', ');