From: Paul M. <pm...@sk...> - 2012-10-25 16:23:06
|
(AABox::isEmpty(), yes, as per Sebastian's email.) Suggestions to make this change work for everyone: 1. Change mEmpty to mUninitialized in Box, and add mUninitialized to Sphere. Add an isInitialized() function to both Sphere and Box, but also keep isEmpty(). This give us a better name and also maintains backwards compatibility. 2. For the extendVolume() variants that extend a Sphere by a single Point or a single other volume, add the initialization checks to make Sphere work like box. 3. For performance, add extendVolume() functions that take a std::vector of Points. This would also have the initialization check, but would amortize the cost of the check over all the data. Similar code already exists in the makeVolume() functions in Containment.h. Does this sound good? -Paul On 10/25/2012 9:27 AM, Kevin Meinert wrote: > If no gmtl uses mEmpty, > Then, I question why include it in gmtl. > > If for bookkeeping for higher level subsystems... Then you could easily > extend that class yourself to include that data member, through > inheritance. > >>> there are no checks inside the gmtl functions that use the mEmpty > property directly > > Didn't Paul say below that extendVolume does use mEmpty? > > Sent from my Phone > From: Sebastian Messerschmidt > Sent: 10/25/2012 1:30 AM > To: ggt...@li... > Subject: Re: [ggt-devel] Sphere emptiness > Kevin, > >> Got it. >> >> Perhaps a better term would be mExists, or mPositionless, or mUndefined. >> >> Empty implies volume, which 0 could be mistaken for emptiness. > mEmpty was the term that was used in the box, so I'd vote for sticking > to it for the sake of readability. > Of course it doesn't make much sense, as there is no such thing as an > empty volume. >> >> I also would look at the higher level sub system expanding volumes and >> ask why uninitialized ones are even in the list. Leads to the question >> whether it might be more appropriate to do that bookkeeping outside of >> the math system > That is right. But in order to do so, It really helps to have the member ;-) >> >> Doing conditional checks tends to slow down processing, I always prefer >> keeping a list of what needs work, and blasting through that list (or >> memory coherent array preferably) without any conditionals. My >> preference would be for math code that blazes (avoiding branches) and >> keep conditionals to the higher level... > Looking at the code, there are no checks inside the gmtl functions that > use the mEmpty property directly. So no harm done just adding the member. > I totally agree that runtime checking should be done outside the core > functionality. The only arguable option might be an ASSERT, since > merging an "empty" sphere is not valid operation. > > > cheers > Sebastian >> >> Sent from my Phone >> From: Paul Martz >> Sent: 10/24/2012 1:31 PM >> To: ggt...@li... >> Subject: Re: [ggt-devel] Sphere emptiness >> >> >> On 10/24/2012 10:46 AM, Sebastian Messerschmidt wrote: >>> Hello Kevin, >>>> What does it mean to have an empty sphere? Or aabox for that matter. Is >>>> it same as sphere volume == 0. Or is it a flag to signify something >>>> else (I.e. No objects contained in). >>>> >>>> if vol == 0 then why not set the size to 0... If it is so you can >>>> remember previous size, why distinguish 0 volume, why not and an N >>>> volume switch... Why a switch at all... >>>> >>>> Excuse me for not knowing/remembering the purpose of mEmpty... >>> Imagine you have BoundingVolume (Doesn't matter if it is a box or >>> sphere) that is default initialized. >>> Now if you use this object to extend an existing volume it will expand >>> it in a wrong way. >>> Since an empty sphere (i.e. radius == 0.0) still has an position it will >>> expand the volume to be extended incorrectly. >>> (For example you have sphere1 with pos(10,0,0) and r = 1 and an empty >>> sphere (pos(0,0,0), radius=0). The extendVolume will produce a sphere of >>> pos(5.5,0,0) radius = 6, which is wrong) >>> The main purposes of the empty flag is to ignore those primitives and to >>> check if it was initialized. >> Good summary. Also, an mEmpty member variable will differentiate between a >> sphere that contains nothing (uninitialized), and a sphere that >> contains exactly >> one point. Radius == 0 alone doesn't provide this functionality. >> >> As I mentioned previously, I could use a negative radius to indicate an empty >> sphere. But, we could also have AABox use mMax< mMin to indicate an empty box, >> and we don't -- instead we have AABox::mEmpty, and code to respect the setting >> of that variable in Containment.h. I think, for consistency, Sphere should be >> the same way. >> >> I'll proceed with the code change, if no one objects. >> -Paul >> >>> Your approach testing the volume might work in this case, but it is >>> surely not a general solution. >>> So I vote for Paul's approach. >>> >>> cheers >>> Sebastian >>> >>> P.S. Hi Paul, nice to see you outside the OSG mailing list ;-) >>>> Thanks, >>>> >>>> Sent from my Phone >>>> From: Paul Martz >>>> Sent: 10/23/2012 5:15 PM >>>> To: ggt...@li... >>>> Subject: [ggt-devel] Sphere emptiness >>>> Hi all -- My project uses both gmtl::Sphere and gmtl::AABox as >>>> bounding volumes. >>>> But I've noticed that Sphere lacks the 'bool mEmpty' member variable that is >>>> present in AABox. Is there a reason for this? If not, I'd like to add it. >>>> >>>> I would also modify Containment.h's extendVolume(Sphere&,...) to respect that >>>> variable (as is already done for extendVolume(AABox&,...) ). Finally, I'd also >>>> add new extendVolume() functions to extend Sphere around an AABox and >>>> vice versa. >>>> >>>> Just wanted to see if there was a logical reason for the current >>>> design before I >>>> embarked on changing the code. I know that I can emulate the desired behavior >>>> with a negative radius, but it seems to me it would be desirable to have this >>>> functionality in gmtl rather than only in my project. >>>> >>>> Thanks, >>>> -Paul >>>> >>>> >>>> >>>> >>>> ------------------------------------------------------------------------------ >>>> Everyone hates slow websites. So do we. >>>> Make your web apps faster with AppDynamics >>>> Download AppDynamics Lite for free today: >>>> http://p.sf.net/sfu/appdyn_sfd2d_oct >>>> _______________________________________________ >>>> ggt-devel mailing list >>>> ggt...@li... >>>> https://lists.sourceforge.net/lists/listinfo/ggt-devel >>>> >>>> ------------------------------------------------------------------------------ >>>> Everyone hates slow websites. So do we. >>>> Make your web apps faster with AppDynamics >>>> Download AppDynamics Lite for free today: >>>> http://p.sf.net/sfu/appdyn_sfd2d_oct >>>> _______________________________________________ >>>> ggt-devel mailing list >>>> ggt...@li... >>>> https://lists.sourceforge.net/lists/listinfo/ggt-devel >>> >>> ------------------------------------------------------------------------------ >>> Everyone hates slow websites. So do we. >>> Make your web apps faster with AppDynamics >>> Download AppDynamics Lite for free today: >>> http://p.sf.net/sfu/appdyn_sfd2d_oct >>> _______________________________________________ >>> ggt-devel mailing list >>> ggt...@li... >>> https://lists.sourceforge.net/lists/listinfo/ggt-devel >>> >>> >> ------------------------------------------------------------------------------ >> Everyone hates slow websites. So do we. >> Make your web apps faster with AppDynamics >> Download AppDynamics Lite for free today: >> http://p.sf.net/sfu/appdyn_sfd2d_oct >> _______________________________________________ >> ggt-devel mailing list >> ggt...@li... >> https://lists.sourceforge.net/lists/listinfo/ggt-devel >> >> ------------------------------------------------------------------------------ >> Everyone hates slow websites. So do we. >> Make your web apps faster with AppDynamics >> Download AppDynamics Lite for free today: >> http://p.sf.net/sfu/appdyn_sfd2d_oct >> _______________________________________________ >> ggt-devel mailing list >> ggt...@li... >> https://lists.sourceforge.net/lists/listinfo/ggt-devel > > > ------------------------------------------------------------------------------ > Everyone hates slow websites. So do we. > Make your web apps faster with AppDynamics > Download AppDynamics Lite for free today: > http://p.sf.net/sfu/appdyn_sfd2d_oct > _______________________________________________ > ggt-devel mailing list > ggt...@li... > https://lists.sourceforge.net/lists/listinfo/ggt-devel > > ------------------------------------------------------------------------------ > Everyone hates slow websites. So do we. > Make your web apps faster with AppDynamics > Download AppDynamics Lite for free today: > http://p.sf.net/sfu/appdyn_sfd2d_oct > _______________________________________________ > ggt-devel mailing list > ggt...@li... > https://lists.sourceforge.net/lists/listinfo/ggt-devel > > |