par_shapes__weld_points error

[sonoro1234]

Hi,

I am investigating some SIGSEGV error in par_shapes.
Crash happens with par_shapes.create.cylinder(626,100) (perhaps slightly higher numbers depending on the compiler-architecture)

I have find this is due to par_shapes__weld_points generating meshes in which triangles contain points not present in the welded mesh.

If in https://github.com/prideout/par/blob/master/par_shapes.h#L1726 we add

		assert(a < mesh->npoints);
		assert(b < mesh->npoints);
		assert(c < mesh->npoints);

It will assert sometimes when nremoved > 0

Still looking for the bug but seems related to #20

[sonoro1234]

I was specifically getting the crash with this helicoidal generation

local function mix(a,b,f)
	return a*(1-f)+b*f
end
local cb = ffi.cast("par_shapes_fn",function(inp,out,user)
		local u,v=inp[0],inp[1]*NM.vueltas*2*math.pi
		out[0]	= mix(NM.i_r,1,u)*math.cos(v)
		out[2]	= mix(NM.i_r,1,u)*math.sin(v)
		out[1] = inp[1]
	end)

local helicoidal= par_shapes.create.parametric(cb,math.floor(NM.slices*NM.vueltas), math.floor(NM.stacks*NM.vueltas),nil);

with vueltas=6, stacks=6,slices=32,i i_r = 0.625 or higher

[sonoro1234]

@prideout
Am I wrong or after https://github.com/prideout/par/blob/master/par_shapes.h#L1672
assert(p < nindex); should not assert? (as points are sorted, if p > nindex then instead of weld nindex point we should have welded p point before)

In any case, some kind of assertion should be added to happen instead of getting a crash.

[sonoro1234]

Thanks for the asserts!!
I have also checked that a necessary condition for assert(nindex < mesh->npoints); asserting is
(p >= nindex) after https://github.com/prideout/par/blob/master/par_shapes.h#L1697
which should not happen because points where sorted.

[kieroooo]

I know this is old, but i have also stumbled on this problem trying to generate par_shapes_create_subdivided_sphere(). I did some digging trying to figure out what the code is doing, and the whole loop you mention assumes that there is no remapping happening 'backwards'. This means that when the code creates weldmap entry:

if (dist2 < epsilon) {
    weldmap[nindex] = p;
    nremoved++;
}

It assumes that the weldmap[p] was not yet remapped.

if (dist2 < epsilon) {
    assert(nindex < p);
    weldmap[nindex] = p;
    nremoved++;
}

This is a correct assumption coming from the fact the when you go through the vtx list the nindex<->p remapping should happen in the opposite direction earlier. But it is actually not the case.

Correct me if i'm wrong, but you are collecting bins in a bounding box formed by +/-epsilon. But then when actually comparing distances you use distance squared. I think with 'low poly' meshes it will not manifest but with something denser ones it messes up the order. Or am i missing something?