zturtleman / mm3d

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

Mode::draw does glBegin/glEnd on every triangle with GL_TRIANGLES #98

Closed m-7761 closed 2 years ago

m-7761 commented 4 years ago

EDITED: Actually the first branch in the excerpt link got it right, so I've now omitted that part. There are a few different blocks like this.

This procedure is difficult to follow, seems to have a lot of duplicated code, but you can see the logic is wrong here:

https://github.com/zturtleman/mm3d/blob/dcb15f3440a62376ed2e09d01960b58426c13ef6/src/libmm3d/model_draw.cc#L1049-L1063

Basically the lines below should be inside the conditional branches like so:

                    else
                    {
                        if(colorSelected==true)
                        {
                            if(!(drawOptions &DO_TEXTURE))
                            {
                                glColor3f(0.9f,0.9f,0.9f);
                            }
                            glEnd();
                            glDisable(GL_LIGHT1);
                            glEnable(GL_LIGHT0);
                            glBegin(GL_TRIANGLES);
                            colorSelected = false;
                        }                       
                    }
m-7761 commented 4 years ago

Update: It's more complicated than it seems...

There are a couple places where "colorSelected" isn't initialized, and the defaults aren't set either from what I can tell.

So what I did was change colorSelected to an int type and set it to -1 at the top of the loops, and invert the comparison, so that colorSelected==true becomes colorSelected!=false.

That should avoid the insane behavior that is the bug, and I've doubts the glEnd calls are always correctly matched, but I don't want to study this sloppy code unnecessarily.

zturtleman commented 2 years ago

Thanks. Fixed in 3247cea3a3ad4a70b38e730599adbe2c70417372.

Default light0/light1 enable state and color are set way at the top of Model::Draw(). https://github.com/zturtleman/mm3d/blob/3247cea3a3ad4a70b38e730599adbe2c70417372/src/libmm3d/model_draw.cc#L533-L536