[OpenSmalltalk/opensmalltalk-vm] Fix to Squeak3D b3dMain.c::b3dMailLoop is only a partial fix (#463)

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

[OpenSmalltalk/opensmalltalk-vm] Fix to Squeak3D b3dMain.c::b3dMailLoop is only a partial fix (#463)

David T Lewis
 

(Line numbers correct at time of writing)
There is a major loop in b3dMainLoop with condition while(aetStart < aetSize) { at line 1352 which is in the MAIN LOOP at 1227. At line 1372 there is a sub loop with condition else while(aetStart < aetSize) { which ends with the code

                        /* If the edge is not on top toggle its (back) fills */

                        b3dToggleBackFills(fillList, rightEdge, yValue, nextIntersection);

                        rightEdge = NULL;

                    }

                    /*-- end of search for next top edge --*/

This code used to create crashes because following the if-then-else containing the loop in the else branch there was nothing to handle a NULL rightEdge. I extended the assert that was there from assert(leftEdge); to assert(leftEdge && rightEdge); and that allowed us to identify the assignment as the cause of crashes in games written by @stéphane Rollandin.

Recently Stéphane encouraged me to take another look at the code and I was able to create a patch that stopped the plugin from crashing. That added the following to the code to break out of the main loop

                        /* If the edge is not on top toggle its (back) fills */

                        b3dToggleBackFills(fillList, rightEdge, yValue, nextIntersection);

                        rightEdge = NULL;

                    }

                    /*-- end of search for next top edge --*/



                    /* If the "edge is not on top toggle its (back) fills" guard

                     * above nilled rightEdge then presumably we're done.  So

                     * arrange that we quit the main loop (see BEGIN MAINLOOP at

                     * about line 1227 above). eem 2019/12/29

                     */

                    if (!rightEdge) {

                        aet->size = 0;

                        b3dCleanupFill(fillList);

                        break;

                    }

                    assert(leftEdge && rightEdge);



                    /*-- Now do the drawing from leftEdge to rightEdge --*/

                    if(fillList->firstFace) {

...

Stéphane does confirm that this fixes the crash but it is not a good fix; he experiences glitches:

I have now played a fair bit with my game and I have not had any crash 

so far.



But, instead, there are rendering glitches at times. Of course I suppose 

that these glitches occurs whenever the plugin would have crashed 

previously.



It's no big deal, but it shows that indeed, as you stated, some 

continuation is needed when "the edge is not on top" to "toggle its 

(back) fills", whatever that means.



Best, Stef

So the challenge is to determine what should happen at this point. There are several failure macros which resume in various ways, see lines 1558 forward:

/* Failure adding objects */

#define FAIL_ADDING(reason) { obj->start = objStart; FAIL(reason, B3D_RESUME_ADDING) }

#define PROCEED_ADDING { objStart = obj->start; PROCEED }



/* Failure merging objects */

#define FAIL_MERGING(reason) { FAIL(reason, B3D_RESUME_MERGING); }

#define PROCEED_MERGING { PROCEED }



/* Failure during paint */

#define FAIL_PAINTING(reason) { aet->start = aetStart; aet->leftEdge = leftEdge; aet->rightEdge = rightEdge; FAIL(reason, B3D_RESUME_PAINTING) }

#define PROCEED_PAINTING(reason) { aetStart = aet->start; leftEdge = aet->leftEdge; rightEdge = aet->rightEdge; PROCEED }

So what kind of failure is described by Andreas' comment "If the edge is not on top toggle its (back) fills"?


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.

<script type="application/ld+json">[ { "@context": "http://schema.org", "@type": "EmailMessage", "potentialAction": { "@type": "ViewAction", "target": "https://github.com/OpenSmalltalk/opensmalltalk-vm/issues/463?email_source=notifications\u0026email_token=AIJPEW42KRN4DDWEF3N4JCLQ3UDSJA5CNFSM4KB424E2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4IDS4XYQ", "url": "https://github.com/OpenSmalltalk/opensmalltalk-vm/issues/463?email_source=notifications\u0026email_token=AIJPEW42KRN4DDWEF3N4JCLQ3UDSJA5CNFSM4KB424E2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4IDS4XYQ", "name": "View Issue" }, "description": "View this Issue on GitHub", "publisher": { "@type": "Organization", "name": "GitHub", "url": "https://github.com" } } ]</script>