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

Fix working array overflow #29

Merged
merged 1 commit into from
Feb 8, 2021
Merged

Fix working array overflow #29

merged 1 commit into from
Feb 8, 2021

Conversation

ChrisLoer
Copy link
Contributor

When I increased the size of the grid to accommodate halos that could extend outside of the default canvas size, I should have increased the size of the three working arrays to match. Instead, I made it possible for edt1d to index past the end of the working arrays, at which point it does a bunch of nonsense math with NaN and undefined and then assigns the results to non-existent array positions. At the end it doesn't actually copy any undefined values into the output grid, so the very "end" of the grid (the lower right corner of the output) is left at the "alpha 0" state it's initialized to, which happens to be correct for almost any glyph (thus we didn't notice the problem).

However, on Firefox, running this code is dramatically slower. As far as I can tell, the logic is all being evaluated the same way, but since this is the innermost loop of an expensive operation, it's not surprising that small differences in handling out-of-bounds array access could have a big impact.

@mourner

@ChrisLoer ChrisLoer requested a review from mourner February 8, 2021 01:26
@mourner mourner merged commit 5f64848 into master Feb 8, 2021
@mourner mourner deleted the fix_array_overflow branch February 8, 2021 05:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants