[bug+fix] Select3D_Box2d does not handle "void" correctly

here is a fixed version :

struct Select3D_Box2d
{
Standard_ShortReal xmin, ymin, xmax, ymax;
Standard_Boolean isVoid;

Select3D_Box2d()
{
xmin = ymin = xmax = ymax = 0.;
isVoid = Standard_True;
}

Select3D_Box2d(const Bnd_Box2d& theBox)
{
SetField(theBox);
}

inline operator Bnd_Box2d() const
{
Bnd_Box2d aBox;
aBox.SetVoid();

if( !isVoid )
aBox.Update(xmin, ymin, xmax, ymax);
return aBox;
}

inline Select3D_Box2d operator = ( const Bnd_Box2d& theBox)
{
SetField(theBox);
return *this;
}
inline void Update(const gp_Pnt2d& thePnt)
{
Bnd_Box2d aBox;
aBox.Set(thePnt);
if ( !isVoid )
aBox.Update(xmin, ymin, xmax, ymax);
SetField(aBox);
}

inline void SetVoid()
{
isVoid = Standard_True;
xmin = ymin = xmax = ymax = 0.;
}
private:
inline void SetField(const Bnd_Box2d& theBox)
{
Standard_Real x, y, x1, y1;
theBox.Get(x, y, x1, y1);

xmin = DToF(x);
ymin = DToF(y);
xmax = DToF(x1);
ymax = DToF(y1);

isVoid = theBox.IsVoid();
}

};

Forum supervisor's picture

Hello Stephane,

When Select3D_Box2d structure was introduced, the idea was to avoid adding any unnecessary fields to it and to make it as memory-efficient as possible. Adding a Standard_Boolean field to it increases its size by sizeof(int), that is, by 4 bytes or 20%.

In fact, "void" property of a 2d bounding box is analyzed only in two places in Select3D package:
- Select3D_SensitiveCircle.cxx line 186
- Select3D_SensitiveFace.cxx line 80
In both cases, Bnd_Box2d instance is created on-the-fly using Select3D_Box2d::operator Bnd_Box2d() method. The latter creates a correct void box in case if all four coordinate values are zero. Thus probably it is enough to correct private Select3D_Box2d::SetField() method and treat a void box in it properly by nullifying the four coordinates. It can look like this:

inline void SetField(const Bnd_Box2d& theBox)
{
if ( theBox.IsVoid() )
SetVoid();
else {
Standard_Real x, y, x1, y1;
theBox.Get(x, y, x1, y1);

xmin = DToF(x);
ymin = DToF(y);
xmax = DToF(x1);
ymax = DToF(y1);
}
}

It should be noted here that Select3D_Box2d is a simplified implementation of a 2D bounding box used by interactive detection algorithm only.

Certainly, if we knew more details about the case that resulted in the proposed patch we would be able to take a more well-grounded decision based not only on the memory efficiency reasons but also on some custom algorithmic requirements, etc.

We are looking forward to your feedback.
On behalf of the team,
Forum Supervisor

Stephane Routelous's picture

I encounter the problem with Select3D_SensitiveCurve after the first rotation.
This means the second time the detection areas are computed, I end up with a Select3d_Box2d "using" the point (0,0) as maximum. This cause the detection areas to be messed up.
The original code will not work correctly if you add only positive of negative points to your box
the problem is not in the IsVoid test, but in the Update method :
ex : if you have
Select3D_Box2d b;
b.Update(gp_Pnt2d(-10,10));
b.Update(gp_Pnt2d(-20,-20));

you will endup with xmin=ymin=-20 xmax=ymax=0 instead of xmax=ymax=-10
because of the "aBox.Update(xmin, ymin, xmax, ymax);"

inline void Update(const gp_Pnt2d& thePnt)
{
Bnd_Box2d aBox;
aBox.Set(thePnt);
if ( !isVoid ) //if you don't add that, the 2d box will be wrong
aBox.Update(xmin, ymin, xmax, ymax);
SetField(aBox);
}

and btw, why are you not defining Standard_Boolean to be a bool or at least a char ? It will use 1 byte instead of unsigned int using 4 ..

Stephane

P Dolbey's picture

This is an interesting discovery from Stephane. I would have thought that in any standard min-max picker you would have something like: -

xmax = ymax = -Precision::Infinity;
xmin = ymin = Precision::Infinity;

then possibly use the IsInfinite test on the values to determine if the box is void. The original algorithm seems to assume that geometries are centred around the origin (unless I'm missing something).

Pete

Stephane Routelous's picture

yes, it is exactly the problem i got. all my objects were in negative quadrant. so I end up with 2d boxes ending (xmax and ymax) at 0. The detection was weird : moving the mouse way outside the model was hilighting some entities (I was in a local context)

and I displayed the detection areas (using AIS_InteractiveContext::DisplayActiveAreas(view3d) ) and the "dashed" selection box where going to the corner of my screen
applying my fix solved the problem (the areas are enclosing correctly my objects)

Stephane

P Dolbey's picture

I've been wondering about the strange behaviour of rubberbanding selection of multiple entities in OCC for some time - it seems that often entities get selected when they are not completely contained in a selection window - It looks like this is the cause. We might be able to come up with a modified algorithm that provides your functionality without the extra boolean. How much do think we should sell it for ?

Pete