Issue 4529048: MDef BVH Support
|
Created:
12 years, 11 months ago by joeedh Modified:
11 years, 6 months ago Base URL:
https://svn.blender.org/svnroot/bf-blender/trunk/blender/ Visibility:
Public. |
Patch Set 1 #
Total comments: 2
MessagesTotal messages: 10
Looks good to me, only the use of epsilon in bvhtree made me wonder, but i may be wrong about that one. Other than that i'd say: ready for trunk! http://codereview.appspot.com/4529048/diff/1/source/blender/editors/armature/... File source/blender/editors/armature/meshlaplacian.c (right): http://codereview.appspot.com/4529048/diff/1/source/blender/editors/armature/... source/blender/editors/armature/meshlaplacian.c:1740: mdb->bvhtree = bvhtree_from_mesh_faces(&mdb->bvhdata, mdb->cagedm, FLT_EPSILON*100, 4, 6); Is the epsilon necessary here? Afaik the epsilon value simply "inflates" the bounding volumes, which should not have any effect on the intersection tests, except slightly lower performance? To my understanding, for nearest point search it defines the search radius (see Shrinkwrap), but other callers all have epsilon=0.
Sign in to reply to this message.
The function meshdeform_tri_intersect has a big epsilon to ensure no rays slip through (float precision issues). Actually my main concern with this patch is that the BVH epsilon is possibly too low. It should be tested to give the same results on geometry with mixed small/big faces, faces close to and nearly overlapping the cage boundary.
Sign in to reply to this message.
i'll develop a test suite for comparison based on this then cheers Daniel Salazar 3Developer.com On Sat, Sep 10, 2011 at 2:10 AM, <brechtvanlommel@gmail.com> wrote: > The function meshdeform_tri_intersect has a big epsilon to ensure no > rays slip through (float precision issues). Actually my main concern > with this patch is that the BVH epsilon is possibly too low. It should > be tested to give the same results on geometry with mixed small/big > faces, faces close to and nearly overlapping the cage boundary. > > http://codereview.appspot.com/4529048/ > _______________________________________________ > Bf-codereview mailing list > Bf-codereview@blender.org > http://lists.blender.org/mailman/listinfo/bf-codereview >
Sign in to reply to this message.
Gah, sadly patch is failing in svn patching file source/blender/editors/armature/meshlaplacian.c Hunk #1 succeeded at 1106 (offset 19 lines). Hunk #2 FAILED at 1165. Hunk #3 succeeded at 1249 (offset 19 lines). Hunk #4 succeeded at 1763 (offset 19 lines). Hunk #5 succeeded at 1880 (offset 19 lines). 1 out of 5 hunks FAILED -- saving rejects to file source/blender/editors/armature/meshlaplacian.c.rej Daniel Salazar 3Developer.com On Sun, Sep 11, 2011 at 5:17 PM, Daniel Salazar - 3Developer.com <zanqdo@gmail.com> wrote: > i'll develop a test suite for comparison based on this then > > cheers > > Daniel Salazar > 3Developer.com > > > > On Sat, Sep 10, 2011 at 2:10 AM, <brechtvanlommel@gmail.com> wrote: >> The function meshdeform_tri_intersect has a big epsilon to ensure no >> rays slip through (float precision issues). Actually my main concern >> with this patch is that the BVH epsilon is possibly too low. It should >> be tested to give the same results on geometry with mixed small/big >> faces, faces close to and nearly overlapping the cage boundary. >> >> http://codereview.appspot.com/4529048/ >> _______________________________________________ >> Bf-codereview mailing list >> Bf-codereview@blender.org >> http://lists.blender.org/mailman/listinfo/bf-codereview >> >
Sign in to reply to this message.
Fix for latest trunk (r40555 tested), need explicit conversion to non-const vector argument. http://codereview.appspot.com/4529048/diff/1/source/blender/editors/armature/... File source/blender/editors/armature/meshlaplacian.c (right): http://codereview.appspot.com/4529048/diff/1/source/blender/editors/armature/... source/blender/editors/armature/meshlaplacian.c:1187: if (!meshdeform_tri_intersect(ray->origin, end, face[0], face[1], face[2], co, uvw)) need to convert ray->origin to non-const vector here, meshdeform_tri_intersect requires non-const argument (same next line)
Sign in to reply to this message.
Lukas it seems it's failing again could someone fix the patch? cheers Daniel Salazar 3Developer.com On Mon, Sep 26, 2011 at 2:41 AM, <lukas.toenne@googlemail.com> wrote: > Fix for latest trunk (r40555 tested), need explicit conversion to > non-const vector argument. > > > http://codereview.appspot.com/4529048/diff/1/source/blender/editors/armature/... > File source/blender/editors/armature/meshlaplacian.c (right): > > http://codereview.appspot.com/4529048/diff/1/source/blender/editors/armature/... > source/blender/editors/armature/meshlaplacian.c:1187: if > (!meshdeform_tri_intersect(ray->origin, end, face[0], face[1], face[2], > co, uvw)) > need to convert ray->origin to non-const vector here, > meshdeform_tri_intersect requires non-const argument (same next line) > > http://codereview.appspot.com/4529048/ >
Sign in to reply to this message.
I'm trying this patch http://codereview.appspot.com/4529048/diff/1/source/blender/editors/armature/... do I need anything else? cheers Daniel Salazar 3Developer.com On Thu, Oct 27, 2011 at 2:50 PM, Daniel Salazar - 3Developer.com <zanqdo@gmail.com> wrote: > Lukas it seems it's failing again could someone fix the patch? > > cheers > > Daniel Salazar > 3Developer.com > > > > On Mon, Sep 26, 2011 at 2:41 AM, <lukas.toenne@googlemail.com> wrote: >> Fix for latest trunk (r40555 tested), need explicit conversion to >> non-const vector argument. >> >> >> http://codereview.appspot.com/4529048/diff/1/source/blender/editors/armature/... >> File source/blender/editors/armature/meshlaplacian.c (right): >> >> http://codereview.appspot.com/4529048/diff/1/source/blender/editors/armature/... >> source/blender/editors/armature/meshlaplacian.c:1187: if >> (!meshdeform_tri_intersect(ray->origin, end, face[0], face[1], face[2], >> co, uvw)) >> need to convert ray->origin to non-const vector here, >> meshdeform_tri_intersect requires non-const argument (same next line) >> >> http://codereview.appspot.com/4529048/ >> >
Sign in to reply to this message.
Here's an updated patch for r50858 by dfelinto http://www.pasteall.org/35522/diff Daniel Salazar patazstudio.com On Thu, Oct 27, 2011 at 3:00 PM, Daniel Salazar - 3Developer.com <zanqdo@gmail.com> wrote: > I'm trying this patch > > http://codereview.appspot.com/4529048/diff/1/source/blender/editors/armature/... > > do I need anything else? > > cheers > Daniel Salazar > 3Developer.com > > > > On Thu, Oct 27, 2011 at 2:50 PM, Daniel Salazar - 3Developer.com > <zanqdo@gmail.com> wrote: >> Lukas it seems it's failing again could someone fix the patch? >> >> cheers >> >> Daniel Salazar >> 3Developer.com >> >> >> >> On Mon, Sep 26, 2011 at 2:41 AM, <lukas.toenne@googlemail.com> wrote: >>> Fix for latest trunk (r40555 tested), need explicit conversion to >>> non-const vector argument. >>> >>> >>> http://codereview.appspot.com/4529048/diff/1/source/blender/editors/armature/... >>> File source/blender/editors/armature/meshlaplacian.c (right): >>> >>> http://codereview.appspot.com/4529048/diff/1/source/blender/editors/armature/... >>> source/blender/editors/armature/meshlaplacian.c:1187: if >>> (!meshdeform_tri_intersect(ray->origin, end, face[0], face[1], face[2], >>> co, uvw)) >>> need to convert ray->origin to non-const vector here, >>> meshdeform_tri_intersect requires non-const argument (same next line) >>> >>> http://codereview.appspot.com/4529048/ >>> >>
Sign in to reply to this message.
Committed patch to svn now.
Sign in to reply to this message.
YES! This is big, than you very much! Daniel Salazar patazstudio.com On Thu, Oct 4, 2012 at 2:37 PM, <brechtvanlommel@gmail.com> wrote: > Committed patch to svn now. > > https://codereview.appspot.com/4529048/
Sign in to reply to this message.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
