CoACD icon indicating copy to clipboard operation
CoACD copied to clipboard

Another crash during decomposing in debug mode

Open Pindrought opened this issue 2 years ago • 1 comments

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 avatar Oct 20 '23 18:10 Pindrought

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.

Pindrought avatar Dec 27 '23 23:12 Pindrought

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: https://github.com/SarahWeiii/CoACD/pull/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! :)

ChuangTseu avatar Jul 19 '24 17:07 ChuangTseu

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

SarahWeiii avatar Jul 20 '24 05:07 SarahWeiii