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

Offscreen primitive shapes throws error #3243

Open
eyaler opened this issue Sep 26, 2024 · 3 comments
Open

Offscreen primitive shapes throws error #3243

eyaler opened this issue Sep 26, 2024 · 3 comments
Labels
Area:Preview For features and bugs relating to the embedded preview sketch Bug Priority:Medium

Comments

@eyaler
Copy link

eyaler commented Sep 26, 2024

p5.js version

1.9.2, 1.9.3, 1.9.4, 1.10.0

What is your operating system?

Windows

Web browser and version

firefox 130

Actual Behavior

I am hitting something very similar to processing/p5.js#7259
but it is only happening for me in p5js editor on firefox 130, for the case ellipse(400, -1, 400,400); of the mentioned issue, as well as several of my own sketches (linked below).

TypeError: cells[ingredients[x][y].loc.locY] is undefined
    at undefined:53173:93

🌸 p5.js says: 
[p5.js, line 53173] 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.10.0/p5.js:53173:93] 
	 Error at line 53173 in _gridMap()
└[https://cdnjs.cloudflare.com/ajax/libs/p5.js/1.10.0/p5.js:53138:34] 
	 Called from line 53138 in [286]</_main.default.prototype._updateGridOutput()
└[https://cdnjs.cloudflare.com/ajax/libs/p5.js/1.10.0/p5.js:53690:20] 
	 Called from line 53690 in [287]</_main.default.prototype._updateAccsOutput()
└[https://cdnjs.cloudflare.com/ajax/libs/p5.js/1.10.0/p5.js:78952:22] 
	 Called from line 78952 in [318]</_main.default.prototype.redraw()
└[https://cdnjs.cloudflare.com/ajax/libs/p5.js/1.10.0/p5.js:66079:23] 
	 Called from line 66079 in [306]</p5/this._draw()

Expected Behavior

it used to work fine just a couple of weeks ago. it doesn't crash when run locally or in openprocessing, using Firefox, and it's also fine in p5js editor when using chromium 129. so it seems like a new interaction issue between the p5.js editor and firefox

Steps to reproduce

minimal code to recreate based on processing/p5.js#7259:
https://editor.p5js.org/eyaler/sketches/qk9CHYwA8

these notebooks also break:
https://editor.p5js.org/eyaler/sketches/H37vvAls6
https://editor.p5js.org/eyaler/sketches/z0zZfHohS
https://editor.p5js.org/eyaler/sketches/uXxfMdLcF
https://editor.p5js.org/eyaler/sketches/mH2EXhjDm

@eyaler eyaler added the Bug label Sep 26, 2024
@alexodden
Copy link

alexodden commented Oct 14, 2024

I am experiencing exactly the same, in Chrome, Brave and Edge, on two separate computers, running Windows 11. Sometimes it will run fine though, and I have not yet been able to determine what's causing it to start behaving like that.

EDIT:
I've done some testing now and it seems to be related to whether or not I am logged in. If I start with a fresh browser, not logged in to p5.js, my own sketch runs fine, as does yours. As soon as I log in, objects can no longer be placed in negative y. Furthermore, if I log out the error persists. I have to log out, quit the browser and restart it to get it back to a working state when it comes to negative y.

Here's a bit of a bloated test of my own:
https://editor.p5js.org/alexanderodden/sketches/-pfKFwZer

I got carried away trying to make it pretty, haha.

I am not a very experienced programmer so please excuse any messy or incorrect code :)

EDIT 2:

Here's a little tweaked version of your test:
https://editor.p5js.org/alexanderodden/sketches/vt3ZE75Gn

@michaelo
Copy link

michaelo commented Oct 16, 2024

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

Verified that that triggers the error both in processing/p5.js#7259 as well as a selection of the linked examples in original post.

This check in p5.js (v1.10.0) at line 53173 ...:

if (ingredients[x][y].loc.locY < cells.length && ingredients[x][y].loc.locX < cells[ingredients[x][y].loc.locY].length) {

... only checks if .loc.locY is < cells.length before using it to index into the cells-array.

It should probably also check if it's >= 0. Or depending on the desired accessibility behaviour once an object is even slightly above upper treshold of canvas a different strategy could be considered altogether.

I can make a pull request for the simple fix, but I do not have enough insight into p5 in general, and the accessibility-features in particular to address the larger one.

Minor note: using 'x' and 'y' as variable names throw me off for a bit until I inspected and saw that 'x' represented shape type, and 'y' simply was an index into this, and not coordinates as first assumed.

@michaelo
Copy link

Another suggestion is for the UI to somehow communicate the fact that the accessibility mode is enabled or not. As it appears now there's no visual clue indicating this.

@raclim raclim added Priority:Medium Area:Preview For features and bugs relating to the embedded preview sketch labels Nov 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area:Preview For features and bugs relating to the embedded preview sketch Bug Priority:Medium
Projects
None yet
Development

No branches or pull requests

4 participants