From: Sebastian M. <seb...@gm...> - 2012-10-25 16:05:45
|
Am 25.10.2012 17:27, schrieb Kevin Meinert: > If no gmtl uses mEmpty, > Then, I question why include it in gmtl. Sorry, I grep'd for mEmpty only, not for the isEmpty function which is indeed used in the containment. Mea culpa Anyways it is a valid property. In the AABox it is set to true for the standard constructor. Functions modifying the box set it to true. I think there are two things: 1. I really advocate for adding the member for two reasons: 1.1. The box has it, so the sphere should have it too. Simply for reasons of same interface 1.2. The "empty"/"uninitialized" property is not derivable from the position/radius in a definite way (A sphere with 0,{0,0,0} might still be considered valid), which makes it impossible to check the return/reference value from a function. 2. Adding the check to the extend volume. There is branching to check the isEmpty() for the box. I would add it for the sphere as well, so the functions behave the same. I guess this is not a show-stopper performance-wise. Branch prediction will simply kick in for well formed data sets. > > If for bookkeeping for higher level subsystems... Then you could easily > extend that class yourself to include that data member, through > inheritance. I disagree that deriving is a good idea. Might be personal opinion, but I simple consider deriving basic data structures an overhead. > >>> there are no checks inside the gmtl functions that use the mEmpty > property directly > > Didn't Paul say below that extendVolume does use mEmpty? See above. > > 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 |