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

Another crash during decomposing in debug mode #36

Closed
Pindrought opened this issue Oct 20, 2023 · 3 comments
Closed

Another crash during decomposing in debug mode #36

Pindrought opened this issue Oct 20, 2023 · 3 comments

Comments

@Pindrought
Copy link

Pindrought commented Oct 20, 2023

For some reason, the top row value is outside of the size of the costMatrix std::vector when I am decomposing this model in debug mode.
The size of the vector is 4005, and the top_row value is 4005.
https://github.com/SarahWeiii/CoACD/blob/main/src/process.cpp#L181

image

Model: https://drive.google.com/file/d/1Z1TG8CQqk_eUBzhLKfzvWFQh8ACBlSRN/view?usp=share_link

Code example: (Too long for pastebin)
https://drive.google.com/file/d/1eG44C6UPMUqGZjLxwxXyjIVD6zIzuabh/view?usp=share_link

@Pindrought
Copy link
Author

Just to add - since this is accessing bad data, you can run coacd on the same mesh many times and get different results as far as # of shapes, points, etc.

For now, i've replaced the following...
https://github.com/SarahWeiii/CoACD/blob/main/src/process.cpp#L162-L186

with

if (p1 < costSize)
{
    rowIdx = (addrI * p1) >> 1;
    size_t top_row = erase_idx;
    for (size_t i = 0; i < p1; ++i)
    {
        //I ADDED THIS CHECK BECAUSE OUT OF BOUNDS STUFF WAS HAPPENING UNTIL I CAN FIGURE OUT WHAT IS GOING ON HERE
        if (rowIdx < costMatrix.size() && rowIdx < precostMatrix.size() &&
            top_row < costMatrix.size() && top_row < precostMatrix.size())
        {
            if (i != p2)
            {
                costMatrix[rowIdx] = costMatrix[top_row];
                precostMatrix[rowIdx] = precostMatrix[top_row];
            }
        }
        ++rowIdx;
        ++top_row;
    }

    ++top_row;
    rowIdx += p1;
    for (size_t i = p1 + 1; i < (costSize + 1); ++i)
    {
        //I ADDED THIS CHECK BECAUSE OUT OF BOUNDS STUFF WAS HAPPENING UNTIL I CAN FIGURE OUT WHAT IS GOING ON HERE
        if (rowIdx < costMatrix.size() && rowIdx < precostMatrix.size() &&
            top_row < costMatrix.size() && top_row < precostMatrix.size())
        {
            costMatrix[rowIdx] = costMatrix[top_row];
            precostMatrix[rowIdx] = precostMatrix[top_row];
        }
        top_row++;
        rowIdx += i;
        assert(rowIdx >= 0);
    }
}

Really hoping someone smarter than me can figure out what the proper fix is here though.
I don't know what this is even supposed to be doing or what a cost matrix is so it's hard to fix.

@ChuangTseu
Copy link
Contributor

Reviving this one as I've been encountering this same crash in a non-optimized build.

I spent way too much time figuring out the whole matrix indexing thing but in the end I'm fairly confident I've found the issue!
I've sent this PR: #51
Hopefully I managed to describe the issue well enough in the PR, let me know!

The good news is: behaviour will be strictly the same, it was not a "logical" error because it was writing to a location that was immediately discarded after, so it was just an issue due to crashing in some configurations.

Thanks! :)

@SarahWeiii
Copy link
Owner

Hi thank you for the PR. I have tested it and merged it with the main branch.

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

No branches or pull requests

3 participants