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

Replace assert ()'s with error handling #4

Open
patrickmeehan opened this issue Feb 27, 2014 · 6 comments
Open

Replace assert ()'s with error handling #4

patrickmeehan opened this issue Feb 27, 2014 · 6 comments

Comments

@patrickmeehan
Copy link

Hi, Mikko! Thanks for the awesome library!

We'd like to see libtess2 fail gracefully on malformed geometry, etc. instead of assert. Ideally we'd get back an error code indicating what happened.

Any plans for a modification like this?

@memononen
Copy link
Owner

There's no plans for that at this point. Some asserts are there to capture a crash (i.e. mesh topology checks), some are more a validation (i.e. the assert in normalize).

The lib uses longjmp as a way to fail, so I think most asserts-as-errors could be implemented as:
if (fail) {
tess->errorCode = x;
longjmp(tess->env,1);
}
Which asserts you are getting currently?

@patrickmeehan
Copy link
Author

Had thought about hooking assert that way, but not sure if libtess will be left in a state where we can clean it up and continue. We're working on an SVG viewer. Desired behavior is to ignore malformed geometry (zero area rectangles, etc.). If I hook it, can I just free the buffers and continue on to the next shape?

Thanks!!

@memononen
Copy link
Owner

Sorry for the slow reply, I was on vacation.
Looking at the code it looks like the tess->mesh is not properly cleaned up when the longjump is used. This should fix it:

    if (setjmp(tess->env) != 0) { 
        /* come back here if out of memory */
        if (tess->mesh != NULL) {
            tessMeshDeleteMesh( &tess->alloc, tess->mesh );
            tess->mesh = NULL;
        }
        return 0;
    }

With that change it /should/ be safe to use longjmp instead of assert.

@patrickmeehan
Copy link
Author

No worries, Mikko. Will give that a shot.

Just out of curiosity, do you have any bandwidth for funded work-for-hire on libtess2? Email me if so. My email is just my first name at ziplinegames dot com.

@memononen
Copy link
Owner

Let me know if you run into any problems.

My free time is quite random, so I don't dare to make any commitments and I have to say no :)

@patrickmeehan
Copy link
Author

I know how that goes! Let me know if you change your mind. I am working on a client project and they do have a budget. So far they've been happy to fund open source. If you wind up having a few evenings free, email me and I'll let you know what they can pay.

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

2 participants