|
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
|