-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Mostly offscreen primitive shapes throws error #7259
Comments
I can no longer reproduce this issue. |
Just a FYI that I may have identified the step required to reproduce the issue:
|
i get consistent crashes with v1.11.2 on firefox and chrome without enabling anything and with a single negative y pixel:
also quick workaround hack i found here: #7130 (comment) |
Looks like in #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! |
@davepagurek if gridOutput is not part of the main functionally maybe in addition to the check, we should put it in a try/catch? |
Sorry for the delay @eyaler, sounds good! |
Check for negative indices and skip shape drawing if index is negative
Hi! New contributor to the p5.js repo, attempting this issue 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 |
@digitaldina I think that works too! Wrapping the whole thing in a |
i previously closed the bug i opened in editor processing/p5.js-web-editor#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? |
@eyaler that makes sense to me! |
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! |
@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 |
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. |
Fix #7259 TypeError for Offscreen Shapes
Most appropriate sub-area of p5.js?
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:
Draw ellipse(400, 400, 400,400);
Change the y-coordinate to 440
Error
Draw ellipse(400, 0, 400,400);
Change the y-coordinate to -1
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.
┌[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:
The text was updated successfully, but these errors were encountered: