Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[p5.js 2.0 Beta Bug Report]: vertexProperty with single number arguments throws error #7518

Open
1 of 17 tasks
davepagurek opened this issue Feb 1, 2025 · 8 comments
Open
1 of 17 tasks

Comments

@davepagurek
Copy link
Contributor

Most appropriate sub-area of p5.js?

  • Accessibility
  • Color
  • Core/Environment/Rendering
  • Data
  • DOM
  • Events
  • Image
  • IO
  • Math
  • Typography
  • Utilities
  • WebGL
  • Build process
  • Unit testing
  • Internationalization
  • Friendly errors
  • Other (specify if possible)

p5.js version

2.0 beta 1

Web browser and version

Firefox

Operating system

MacOS

Steps to reproduce this

Steps:

  1. Draw a shape with vertexProperty between each vertex
  2. Make sure you pass a single value as the property (e.g. 0) as opposed to an array (e.g. [0])

This throws the error Error: Can't convert 0 to array!. If you use an array, there is no error. It should be converting to an array under the hood, but I must have messed that up in the shapes refactor.

Snippet:

function setup() {
  createCanvas(400, 400, WEBGL);
  
  const pts = []
  for (let i = 0; i < 50; i++) {
      pts.push(createVector(random(width), random(height)))
  }
  
  const snake = buildGeometry(() => {
    noFill()
    beginShape()
    for (const [i, { x, y }] of pts.entries()) {
        vertexProperty('length', i/(pts.length-1))
        splineVertex(x, y)
    }
    endShape()
  })
}

Live: https://editor.p5js.org/davepagurek/sketches/YPu6ixNV5

@dhruvinjs
Copy link

See I think there was a issue with your vertexProperty Because when i looked it up in codebase it does not have vertexProperty function.

Fixing the Issue

Since vertexProperty isn't a recognized function in p5.js, it seems like you're trying to use a custom or experimental feature that isn't natively available. If you were expecting it to work similarly to vertex(), you'll need to remove it or replace it with a valid alternative.

Updated Working Code

function setup() {
  createCanvas(400, 400, WEBGL);
  
  const pts = [];
  for (let i = 0; i < 50; i++) {
      pts.push(createVector(random(-width / 2, width / 2), random(-height / 2, height / 2)));
  }
  
  noFill();
  beginShape();
  for (let i = 0; i < pts.length; i++) {
      let lengthValue = i / (pts.length - 1); // This is what `vertexProperty` was trying to set
      // Instead of `vertexProperty`, just pass the values correctly to vertex functions
      vertex(pts[i].x, pts[i].y, lengthValue); 
  }
  endShape();
}

What I Fixed:

  1. Removed vertexProperty()

    • This function doesn't exist in p5.js, so I replaced it with a direct approach.
  2. Modified vertex() to include extra data

    • In some shader-based setups, you might need to pass extra attributes like lengthValue, but standard p5.js doesn't use it this way.

@dhruvinjs
Copy link

hey @GregStanton I just came up with the fix but the problem is I dont know how to test it? Can u help me

@GregStanton
Copy link
Collaborator

GregStanton commented Feb 5, 2025

Hi @dhruvinjs! Thanks so much for taking a look at this one. The issue is that vertexProperty() isn't defined in the main branch of p5.js, but p5.js is in the process of transitioning to version 2.0, which is in its own branch. If you try to search the codebase through the GitHub interface, it'll only return results from the main branch. That may be why you didn't find it.

Here's the source code for vertexProperty(). I'm not sure what @davepagurek thinks (he's very knowledgeable about p5 and is the one who noticed this particular bug), but if you're just getting started with contributing to p5, this might be a bit tricky for you.

If you check out the link I provided, you'll see that vertexProperty() calls a method of the same name on the renderer, so you'll need to know how to track that down. Ordinarily I'd point you in the right direction, but there may be some other obstacles for you: p5.js 2.0 has a number of changes in it, and they're not all documented publicly yet, since it's still in beta testing. We still need to do some cleanup in the code as well.

After the 2.0 release (scheduled by the end of March), contributing should get easier, but you're welcome to keep looking around for ways to help! In case you're still interested in the audit issue, I'll ping you there when I publish some specific tasks that need to be done.

@dhruvinjs
Copy link

Hey @GregStanton first of all sorry for late message because this days I have been working for my personal project plus the I was completing but yeah now I will be tracking and if possible I will be sending a pr and plus I am still interested in the audit issue

@dhruvinjs
Copy link

**Hey @GregStanton ** eslint node --require @babel/register ./utils/sample-linter.js × eslint found some errors. Please fix them and try committing again
This is the main error I am facing while making commit where to vertex in rendere gl where I have updated that function so it can now also accept number as an argument so can u help me fix it

@dhruvinjs
Copy link

Possible Fix:

You need to call vertexProperty from the correct context. Try using this.vertexProperty inside the buildGeometry function, ensuring that this refers to the correct p5.js instance.

Alternative Fix: Using p5.prototype.vertexProperty

If vertexProperty is a custom function, you may need to explicitly attach it to the p5 instance:

p5.prototype.vertexProperty = function(attributeName, data) {
  this._renderer.vertexProperty(attributeName, data);
};

Then, make sure to call it like:

vertexProperty('length', i / (pts.length - 1));

inside buildGeometry().

I think this might help you to solve this issue sir

@davepagurek
Copy link
Contributor Author

The spot where vertex properties are added to a shape happens here, after going from p5.prototype to p5.RendererGL to p5.Shape:

vertexProperty(name, data) {
this.userVertexProperties = this.userVertexProperties || {};
const key = this.vertexPropertyKey(name);
if (!this.userVertexProperties[key]) {
this.userVertexProperties[key] = data.length ? data.length : 1;
}
this.#vertexProperties[key] = data;
}

So we already have some checking in place for whether or not the data is an array when we pick the size of the data. data.length ? data.length : 1 is like saying "is this an array? if so, use its length as the data size; otherwise, the data size is 1."

However, later on, we just set this.#vertexProperties[key] = data. I think that part needs to be updated to be like, "is this an array? if so, set the data directly; otherwise, set it to a new array containing the data as its single item."

@Vaivaswat2244
Copy link

Hello @davepagurek ,
Something like this should work...
I have tested this against your test case making random curves. Its working there.
Should I open a pr for this

vertexProperty(name, data) {
    this.userVertexProperties = this.userVertexProperties || {};
    const key = this.vertexPropertyKey(name);
    
    // Convert single value to array if it's not already an array
    const dataArray = Array.isArray(data) ? data : [data];
    
    if (!this.userVertexProperties[key]) {
      this.userVertexProperties[key] = dataArray.length;
    }
    this.#vertexProperties[key] = dataArray;
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants