Change in GeometryUtility.implode

[steffen]

I just changed the implode code so that edges will only be created when the given IndexedFaceSet had already Edges. In my opinion, the implode method should not calculate edges at all... What about other methods that implicitly do expensive calculations?

[gunn]

I agree that static methods that return geometry shouldn't invoke expensive options on the underlying geometry factory.

However, I think we should discuss whether the right solution is just to omit such calls. Suppose a user wants to generate edges from faces. Then after getting the geometry from the implode() call, he must access "low-level" static methods, in this case, IndexedFaceSetUtility.generateEdgesFromFaces(). We created the factories to avoid just such situations. There are 8 or 10 such boolean flags in the geometry factories which users might want to activate. If these options are not turned on by default (and I think we all agree they should not be), then how should the user activate them? Without having a geometry factory to access them, he has to learn 8 or 10 oddly named methods spread around in several different utility classes.

Would it be better, perhaps, to change the static methods to return geometry factories instead of geometry? Then the user could make any "customizing" calls to the factory (in this case, setGenerateEdgesFromFaces()), call update(), and then get the resulting geometry.

Instead of:

Code: Select all

IndexedFaceSet imploded = IndexedFaceSetUtility.implode(null, .5); 
IndexedFaceSetUtility.generateEdgesFromFaces(imploded);

you'd use:

Code: Select all

IndexedFaceSetFactory impFactory = IndexedFaceSetUtility.implode(null, .5);
impFactory.setGenerateEdgesFromFaces(true);
impFactory.update();
IndexedFaceSet imploded = impFactory.getIndexedFaceSet()

[steffen]

Sounds reasonable, another way would be to pass in an IndexedFaceSetFactory which gets imploded. Maybe we should have both...