Closed GoogleCodeExporter closed 8 years ago
Great to hear you like Poly2Tri. Poly2Tri is active in that sense that any
found bugs should be fixed, but it seems the first release was pretty stable :)
Did you want me to setup a repository for the Ruby bindings?
Original comment by thahlen@gmail.com
on 13 Oct 2010 at 12:32
I'm not amazingly familiar with hg, so for the bindings that's not required.
There are some changes I need to make to poly2tri for the bindings, but before
implementing them, I wanted to see if they could be done in a way that would be
acceptable upstream. I'm willing to work in hg if it makes it easier to get
things pulled. Perhaps you can advise me on the process that'd be preferred
for these types of changes to the C++ implementation:
1) Getting the bail cases off of assert(false); When poly2tri is compiled
without assertions (-DNDEBUG), my bindings just crash on invalid input like
coincident points in the polyline. I assume the exceptions were commented out
and replaced with asserts for a reason (overhead?), but I'd like to know if
/any/ solution would be acceptable in poly2tri proper (error callback, errno,
bring back exceptions?) I'm pretty much forced to do this to have usable
bindings.
2) There are quite a few areas which return pretty large structures by value,
both internally and in the user-visible API. Profiling showed a fair amount of
time in the vector<Point*> copy-constructor. A lot of these could be changed
to const-refs to avoid the copy without changing source compatibility. Unlike
#1, this is just a wishlist type of thing. Tt struck me because for the
bindings, AFTER that copy, I unavoidably must copy/convert them yet again to
get them into Ruby values, so I was trying to lighten up this bit.
Original comment by mike%fil...@gtempaccount.com
on 13 Oct 2010 at 4:22
I am maintaining the Java version and have forwarded this to Mason Green who
has done the C++ port.
1. In the case where you send in multiple points with the exact same coordinate
the algorithm will fail. The only way around that for now is to validate the
input before the triangulation.
If this happens with a polyline then the input wouldn't be a simple polygon?
Original comment by thahlen@gmail.com
on 13 Oct 2010 at 7:44
I guess the case you have might encounter is when a closed polyline start and
end at the same point and both points are included in the point list. In the
java implementation I do a simple check and fix for these cases since they seem
common.
All other cases with multiple point with same coordinates fail and will require
some pre validation.
Original comment by thahlen@gmail.com
on 13 Oct 2010 at 7:56
Thanks thahlen.
I've found a (hacky) solution to problem #1 by basically unconditionally
enabling asserts, and redefining "assert" in the build to a function that
throws[1]. Not ideal, but doesn't require me to have a diverging copy of
poly2tri's C++ implementation, or pre-filter every polygon that comes in.
Regardless, when Mason has a chance to look at this, the assert behavior does
pose issues for bindings (vs. ports). I don't know if this is a use-case
anyone is interested in, though.
The issue is, as rbpoly2tri is a wrapper over the C++ implementation, poly2tri
is compiled once (when the extension is built), so the users of the bindings
can't readily leave it in "debug mode" while developing and "release mode" once
all their bugs have been worked out. As the maintainer of the bindings, I have
to choose between:
1. Enable asserts: When poly2tri is given an invalid polygon, it aborts the program. This kills the entire VM, as I have no way to step in.
2. Disable asserts: rbpoly2tri passes along the invalid polygon, but poly2tri's asserts have been compiled down to NOPs, so it runs into uncharted territory and ends up with a segv.
I can live with the solution I'm using, but I have a bit of flexibility being
an extension written in native code. FFI, ctypes, or dlopen-style bindings in
any language wouldn't be able to deal with this without choosing between the
behaviors above. This is also the reason I cannot just point users at poly2tri
and say "install that, then install this".
Not a big deal, just something to consider.
[1] http://github.com/mieko/rbpoly2tri/blob/master/throw_assert/assert.h
Original comment by mike%fil...@gtempaccount.com
on 13 Oct 2010 at 8:33
Mike,
To address your second post above:
1) I have no problem replacing the asserts with error callbacks or a better
method of catching the bail cases.
2) I would definitely appreciate any memory optimization contributions you
would be able to make :)
3) Currently I'm a little busy with other aspects of my life and would not mind
giving you access to the C++ trunk if you're interested in carrying the torch
forward. I'm definitely not abandoning the project, although I'm time
constrained at this point.
4) hg is fairly easy and *not* that much different than Git. The learning curve
should be fairly low.
/Mason
Original comment by mason.gr...@gmail.com
on 15 Oct 2010 at 2:52
I've created a branch with some of the changes I'm working on here:
http://code.google.com/r/mike-poly2tri-staging/
> 1) I have no problem replacing the asserts with error callbacks or a better
method of catching the bail cases.
Trying to be as un-invasive to the structure as possible, what my branch is
does is bail with a new p2t_fail(), which depending on the configuration-time
option --no-sanity-checks, expands to either an assert(0) or an inline function
that throws.
Figured it was a good middle ground: doesn't penalize users that have debugged
their code and can guarantee all input is valid, but allows builds for cases
like rbpoly2tri to catch input errors.
> 2) I would definitely appreciate any memory optimization contributions you
would be able to make :)
I've backed out the changes I was doing here to make sure it was a totally
thorough approach. I created p2tdump (in the repo) so I can make sure the
less-copying/const fixes have no effect at all on the output when I get back
around to it. My branch just has the ultra-low-hanging fruit, like avoiding a
copy on the way out of GetTriangles.
> 3) not mind giving you access to the C++ trunk if you're interested in
carrying the torch forward.
For now, I'd prefer to simmer in the branch: I've only been using poly2tri for
a few weeks, and the changes I'm making, while I believe are universal, may be
biased toward making rbpoly2tri more viable. For example, don't think I'd
honestly care about asserts vs. throws in this context if I were using poly2tri
as a native C++ consumer. In a few weeks when I'm done with my current
project, the changes and bindings have had more stress on them, I'll get up
with you.
Until then, I am trying to do my best to make changes independently pullable
(though some intermingle: To had add p2tdump, I had to bring waf up to
something more recent just to be working with something that the current waf
documentation makes sense with.)
Thanks again guys for working on poly2tri.
-M
(BTW please feel free to mark this defect closed/fixed as I've already pull it
off-topic.)
Original comment by mike%fil...@gtempaccount.com
on 16 Oct 2010 at 3:53
Oki I marked this as fixed.
Nice to see you are using the lib for a game :-)
Original comment by thahlen@gmail.com
on 18 Oct 2010 at 10:26
Hope you don't mind but I added a link to your Ruby bindings on the front page
;)
Original comment by thahlen@gmail.com
on 18 Oct 2010 at 11:52
Original issue reported on code.google.com by
mike%fil...@gtempaccount.com
on 12 Oct 2010 at 7:50