zturtleman / mm3d

Maverick Model 3D is a 3D model editor and animator for games.
https://clover.moe/mm3d
GNU General Public License v2.0
116 stars 24 forks source link

MU_Select (unselectTriangle) is pathological #93

Open m-7761 opened 5 years ago

m-7761 commented 5 years ago

unselectTriangle is only used by the undo/redo object. Every time it's used (for every triangle) it unselects all vertices and iterates over all triangles selecting their vertices... via selectVerticesFromTriangles.

I tried to fix this by deferring selectVerticesFromTriangles until after all triangles, however the undos are not combined, so they end up doing one triangle at a time anyway. The combining code is disabled without comment:


    /*
    MU_Select *undo = dynamic_cast<MU_Select *>(u);
    if(undo&&getSelectionMode()==undo->getSelectionMode())
    {
        SelectionDifferenceList::iterator it;
        for(it = undo->m_diff.begin(); it!=undo->m_diff.end(); it++)
        {
            setSelectionDifference((*it).number,(*it).selected,(*it).oldSelected);
        }
        return true;
    }
    else
    */

m_diff is "sorted_list" for some reason (I can't think of a legit reason) that was probably std::list ... but I changed it to std::vector if so. I need to look into where it's used. Why is it important to sort a selection buffer? It probably isn't.

m-7761 commented 5 years ago

Note, this might not be the worst of it... triangle undo https://github.com/zturtleman/mm3d/issues/92 is even worse.

I gave my brain a few days to gestate these problems. I think my plan is to add an "m_group" and "m_index" field to triangles, and have it so that they are not automatically ordered by index. Instead when they need to be group or index ordered they can be sorted on the fly. In this way the undo system can use indices to recall state, but shouldn't have to keep the list ordered. If it's necessary to write group sets out, the triangles can be group sorted.

m-7761 commented 4 years ago

ALSO: MU_Select doesn't implement select/unselectProjection.

m-7761 commented 4 years ago

When combining void MU_Select::undo(Model *model) should be changed to iterate backward (through time) over its list.

More undo issues I just worked on:

m-7761 commented 4 years ago

Yesterday I wrapped these select/unselect APIs with a test for m_selecting so they are compatible with beginSelectionDifference ... although now I think that beginSelectionDifference is insane and needs to go.

For unselectTriangle which is crazy for calling selectVerticesFromTriangles I was able to just leverage the new connectivity data I've added for calculating vertex normals to make it less bad.

That connectivity data is often helpful for converting quadratic time algorithms into linear time algorithms.

As an exercise the other day I implemented the Edge Turn command using this data. But the data is awkward because it has to use pointers, so the resulting code is more of a hybrid of a new organization and the old organization since it still needs to use indices for undoable APIs. To work around this problem I added another marker like value to Triangle so it can be set to its current index for later retrieval from a pointer.

The library really needs to abandon the indexed based APIs in favor of a C++ data structure, but this shows that a middle ground is possible in the meantime.