Mostly offscreen primitive shapes throws error

[mimiyin]

Most appropriate sub-area of p5.js?

• [ ] Accessibility
• [ ] Color
• [X] 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

1.8.0+

Web browser and version

128.0.6613.86 (Official Build) (x86_64)

Operating system

MacOSX

Steps to reproduce this

Steps:

1. Draw ellipse(400, 400, 400,400);

2. Change the y-coordinate to 440

3. Error

4. Draw ellipse(400, 0, 400,400);

5. Change the y-coordinate to -1

6. Error

I was not able to repro with changes to the x-coordinate I was able to repro with line() and rect(). Errors go away with pre 1.8 versions.

Error: TypeError: Cannot read properties of undefined (reading '9') at undefined:52576:53

🌸 p5.js says: [p5.js, line 52576] Cannot read property of undefined. Check the line number in error and make sure the variable which is being operated is not undefined.

• More info: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/Cant_access_property#what_went_wrong ┌[https://cdnjs.cloudflare.com/ajax/libs/p5.js/1.8.0/p5.js:52576:53] Error at line 52576 in _gridMap() └[https://cdnjs.cloudflare.com/ajax/libs/p5.js/1.8.0/p5.js:52541:26] Called from line 52541 in _main.default._updateGridOutput() └[https://cdnjs.cloudflare.com/ajax/libs/p5.js/1.8.0/p5.js:52977:20] Called from line 52977 in _main.default._updateAccsOutput() └[https://cdnjs.cloudflare.com/ajax/libs/p5.js/1.8.0/p5.js:71385:22] Called from line 71385 in _main.default.redraw() └[https://cdnjs.cloudflare.com/ajax/libs/p5.js/1.8.0/p5.js:62766:23] Called from line 62766 in _draw()

Snippet:

function setup() {
  createCanvas(400, 400);
}

function draw() {
  background(220);
  
  // No-Error
  ellipse(400, 0, 400,400);

//   // Error
//   ellipse(400, -1, 400,400);

//   // No-Error
//   ellipse(400, 400, 400,400);

//   // Error
//   ellipse(400, 440, 400,400);
}

[mimiyin]

I can no longer reproduce this issue.

[michaelo]

Just a FYI that I may have identified the step required to reproduce the issue:

It seems to me that a key factor to trigger the error is to enable accessible output (ctrl+shift+1 to enable, ctrl+shit+2 to disable on Windows, replace "ctrl" with "cmd" for macOS).

https://github.com/processing/p5.js-web-editor/issues/3243#issuecomment-2416112003

[eyaler]

i get consistent crashes with v1.11.2 on firefox and chrome without enabling anything and with a single negative y pixel:

line(0, 0, 0, -1)

run

also quick workaround hack i found here: https://github.com/processing/p5.js/issues/7130#issuecomment-2499403258 is to add gridOutput = function(){} to setup()

[davepagurek]

Looks like in https://github.com/processing/p5.js/pull/7135 a check was added for shapes that go off of the max end of the canvas. Probably a similar PR is needed to add an additional check for the min end of the canvas (checking that all the indices are >= 0) if anyone is interested in contributing to this!

[eyaler]

@davepagurek if gridOutput is not part of the main functionally maybe in addition to the check, we should put it in a try/catch?

[davepagurek]

Sorry for the delay @eyaler, sounds good!

[digitaldina]

Hi! New contributor to the p5.js repo, attempting this issue @eyaler - could you clarify what you mean by adding a try/catch?

My proposed change is build off of #7135, added a check for negatives indices in the if statement so that it skips shapes with negative indices. Let me know if that's sufficient or if I'm missing something here! thanks

[davepagurek]

@digitaldina I think that works too! Wrapping the whole thing in a try/catch could prevent any error from escaping, but also runs the risk of us missing important bugs, so adding checks for known cases causing the issue sounds good to me.

[eyaler]

i previously closed the bug i opened in editor https://github.com/processing/p5.js-web-editor/issues/3243, in favor of this one, but i actually think now that there are indeed two issues: the first is the underlying p5.js bug discussed here. but this will only fix future versions. i think the web editor should actually add the try-catch around the accessible output stuff so it doesn't fail on previous versions of p5.js - other wise it will continue to break on many existing sketches. @davepagurek - what do you think?

[davepagurek]

@eyaler that makes sense to me!

[southwest-git]

I think gridOutput.js has this function _gridMap that doesn't expect locX and locY to be negative when checking if a shape is in canvas. I have a simple fix for this but I can't publish a new branch and hence submit a pull request. Let me know how to proceed. I can also simply share the fix here. Thanks!

[davepagurek]

@southwest-git to submit a patch, you can fork this repo, make the change in your fork, and then create a PR from your fork to this upstream repository. There are some instructions here: https://p5js.org/contribute/contributor_guidelines/#forking-p5js-and-working-from-your-fork

Also as an additional heads up, we are close to releasing version 2.0 of p5! In addition to fixing this on the main branch, it would be great to make another branch off of dev-2.0 afterwards and making a PR into dev-2.0 to fix it there too. Those can be done one at a time if you prefer.

[himanshuukholiya]

I think grid is a table where shapes are placed into cells based on their calculated locX and locY positions, determined by the _canvasLocator() function. The bug arises when shapes are offscreen, as their center coordinates can result in locX or locY values that are negative or greater, leading to errors when trying to access the grid table cells.

So I declared two variables x & y which takes the values of args[0] & arg[1], then check, 0 <= x < canvasWidth and 0 <= y < canvasHeight. If not, return null.