RECT class additions. Are they are useful?

[Ulf]

NOTE:
This thread has been moved from the thread RECT (rectangle) class additions. Are they are useful?
I made the mistake of mixing 2 threads.
So I'm splitting them and moving both.

I am working on making a game engine. 2D Raster type graphics to start with. In doing so, I am taking my time and immersing myself in each aspect of the engine development rather than just getting sprites drawing and moving, then trying to make a game.

RECT class Patch
Straight away I needed some extra functions in the RECT class. I didn't see them in the code.
I know this is simple, but I wanted to know if it's useful for everyone in general. I found them necessary, or at least practical.

Here's the rect code

Code: Select all

//! (Re)Sets the rectangle to the parameters
void set(T x, T y, T x2, T y2)
{//! Modified by Ulf
   UpperLeftCorner.X = x;
   UpperLeftCorner.Y = y;
   LowerRightCorner.X = x2;
   LowerRightCorner.Y = y2;
}

//! (Re)Sets the rectangle to the parameters
void set(const position2d<T>& upperLeft, const position2d<T>& lowerRight)
{//! Modified by Ulf
   UpperLeftCorner.X = upperLeft.X;
   UpperLeftCorner.Y = upperLeft.Y;
   LowerRightCorner.X = lowerRight.X;
   LowerRightCorner.Y = lowerRight.Y;
}

//! (Re)Sets the rectangle so that it keeps the same dimensions, but anchors its Upper left corner at the position given.
void setUpperLeft(const position2d<T>& upperLeft)
{//! Modified by Ulf
   T w = LowerRightCorner.X - UpperLeftCorner.X;
   T h = LowerRightCorner.Y - UpperLeftCorner.Y;

   UpperLeftCorner.X = upperLeft.X;
   UpperLeftCorner.Y = upperLeft.Y;
   LowerRightCorner.X = upperLeft.X + w;
   LowerRightCorner.Y = upperLeft.Y + h;
}

//! (Re)Sets the rectangle so that it keeps the same dimensions, but anchors its left side at the position given.
void setLeft(T left)
{//! Modified by Ulf
   T w = LowerRightCorner.X - UpperLeftCorner.X;

   UpperLeftCorner.X = left;
   LowerRightCorner.X = left + w;
}

//! (Re)Sets the rectangle so that it keeps the same dimensions, but anchors its top side at the position given.
void setTop(T top)
{//! Modified by Ulf
   T h = LowerRightCorner.Y - UpperLeftCorner.Y;

   UpperLeftCorner.Y = top;
   LowerRightCorner.Y = top + h;
}

//! (Re)Sets the rectangle so that it keeps the same dimensions, but anchors its Lower right corner at the position given.
void setLowerRight(const position2d<T>& lowerRight)
{//! Modified by Ulf
   T w = LowerRightCorner.X - UpperLeftCorner.X;
   T h = LowerRightCorner.Y - UpperLeftCorner.Y;

   LowerRightCorner.X = lowerRight.X;
   LowerRightCorner.Y = lowerRight.Y;
   UpperLeftCorner.X = lowerRight.X - w;
   UpperLeftCorner.Y = lowerRight.Y - h;
}

//! (Re)Sets the rectangle so that it keeps the same dimensions, but anchors its right side at the position given.
void setRight(T right)
{//! Modified by Ulf
   T w = LowerRightCorner.X - UpperLeftCorner.X;

   LowerRightCorner.X = right;
   UpperLeftCorner.X = right - w;
}

//! (Re)Sets the rectangle so that it keeps the same dimensions, but anchors its bottom side at the position given.
void setBottom(T bottom)
{//! Modified by Ulf
   T h = LowerRightCorner.Y - UpperLeftCorner.Y;

   LowerRightCorner.Y = bottom;
   UpperLeftCorner.Y = bottom - h;
}

//! (Re)Sets the rectangle so that it keeps the same dimensions, but offsets its Upper left and Lower right X positions.
void offsetX(T x)
{//! Modified by Ulf
   UpperLeftCorner.X += x;
   LowerRightCorner.X += x;
}

//! (Re)Sets the rectangle so that it keeps the same dimensions, but offsets its Upper left and Lower right Y positions.
void offsetY(T y)
{//! Modified by Ulf
   UpperLeftCorner.Y += y;
   LowerRightCorner.Y += y;
}

//! (Re)Sets the rectangle so that it maintains its Upper left position, but changes its dimensions.
void setDimensions(const position2d<T>& size)
{//! Modified by Ulf
   LowerRightCorner.X = UpperLeftCorner.X + size.X;
   LowerRightCorner.Y = UpperLeftCorner.Y + size.Y;
}

//! (Re)Sets the rectangle so that it maintains its Upper left position, but changes its dimensions.
void setDimensions(const dimension2d<T>& size)
{//! Modified by Ulf
   LowerRightCorner.X = UpperLeftCorner.X + size.Width;
   LowerRightCorner.Y = UpperLeftCorner.Y + size.Height;
}

//! increases or decreases the width and height of the specified rectangle.
/** The InflateRect function adds xInflate units to the left and right ends of the rectangle and yInflate units to the top and bottom. */
void inflate(const T xInflate, const T yInflate)
{//! Modified by Ulf
   UpperLeftCorner.X -= xInflate;
   UpperLeftCorner.Y -= yInflate;
   LowerRightCorner.X += iXShrink;
   LowerRightCorner.Y += iYShrink;
}

void inflate(const position2d<T>& ptInflate)
{//! Modified by Ulf
   UpperLeftCorner.X -= ptInflate.X;
   UpperLeftCorner.Y -= ptInflate.Y;
   LowerRightCorner.X += ptInflate.X;
   LowerRightCorner.Y += ptInflate.Y;
}

//! Returns the Lower Left position.
position2d<T> getLowerLeft() const
{//! Modified by Ulf
   return position2d<T>(UpperLeftCorner.X, LowerRightCorner.Y);
}

Also, there should be typedef's for int and float rect as follows.

Code: Select all

//! Typedef for an f32 rect.
typedef rect<f32> rectf;   //! Modified by Ulf

//! Typedef for an s32 rect.
typedef rect<s32> recti;   //! Modified by Ulf 

Is any of it useful?

[Ulf]

CuteAlien replied with this:

rectf and recti have already been added in svn and will be available in 1.6.

For others:
set would be fine, but I'm not sure if set or setRect should be used as other irrlicht classes mix that (plane uses "setPlane", vector uses just "set" for example). As other classes have similar set functions it probably should belong in rect.

setUpperLeft is imho the wrong name as it sounds like setting UpperLeftCorner which it doesn't. moveTo or setPosition would probably be better.

For the rest - I never missed them as they are all easy to code in 1-2 lines on need and it would inflate the interface a lot. So, not sure...

Also I think inflate wouldn't compile so far, did you test?

[Ulf]

For others:
set would be fine, but I'm not sure if set or setRect should be used as other irrlicht classes mix that (plane uses "setPlane", vector uses just "set" for example). As other classes have similar set functions it probably should belong in rect.

"set" functions should definitely be in rect. It makes it easier for all other classes that use its functionality. As you said, several classes implement it!

setUpperLeft is imho the wrong name as it sounds like setting UpperLeftCorner which it doesn't. moveTo or setPosition would probably be better.

Agreed. Sort of.
It is actually setting the Upper Left Corner, and keeping the same dimensions. I don't know, anyone, opinions?

For the rest - I never missed them as they are all easy to code in 1-2 lines on need and it would inflate the interface a lot. So, not sure...

I think all the set functions are useful. SetLeft, SetTop etc. though possibly they should be named something like MoveLeftTo, MoveTopTo.

I also think the setDimensions might be often used, so the user doesn't have to set the members individually.

The offset functions also make life easier for adjustments.

The way I did it, you can basically adjust and move the rect in every practical way in just one line of code. That is useful!

I don't see it as bloat. It's not much code, and it makes the rect class very powerful from a user perspective. It makes all the code using rect more elegant.
If it is easy to code but often used then it's not actually a waste of time to have is it?

Also I think inflate wouldn't compile so far, did you test?

Yes inflate compiles and works. Why wouldn't it?

By the way, CuteAlien, thanks for the great feedback.

[Ulf]

I had also said,

PS. I think inflate is useful. Also, getLowerLeft is useful for game programming when you want to order 2D sprites for drawing, based on y position. You need the lower left, not upper left.
I don't know if getLowerLeft is useful in other applications.

[Ulf]

hybrid then replied with this:

Ulf wrote:

Also I think inflate wouldn't compile so far, did you test? Wink

Yes inflate compiles and works. Why wouldn't it?

Because you have used a wrong parameter name.

A general question: Where's set to be favored over assignment of a newly generated object? so writing r.set(x,y,z,a) should be largely equivalent in run-time to r=recti(x,y,z,a). So maybe we should instead deprecate the set methods instead?

[Ulf]

CuteAlien then replied with this:

About rect - I'm not really convinced about adding that many functions to the interface. Personally I prefer keeping library interfaces minimal. I agree that a set would fit - simply because similar classes have such a function. offset on the other hand was a function I only understood after reading it's source - I guess an operator+ would work better for me. And all the others, well - I certainly needed similar functionality already a few times, I just never missed it in rect itself. So not sure - what do others think? Lean interface or big interface prefered?

hybrid wrote:

A general question: Where's set to be favored over assignment of a newly generated object? so writing r.set(x,y,z,a) should be largely equivalent in run-time to r=recti(x,y,z,a). So maybe we should instead deprecate the set methods instead?

Ah - posting took me so long that you answered in the meantime ^^. I guess I prefer "set" functions (but not for example "setPlane" or "setLine" in those classes) simply because it's less to type. Also maybe easier for beginners. But not really something I cared about so far.

[Ulf]

To Hybrid

I said:

Yes inflate compiles and works. Why wouldn't it?

Hybrid wrote:

Because you have used a wrong parameter name.

haha. True. The first inflate function had typo. I copied and pasted it while I was in the middle of changing the variable names. Sorry. It does all compile though when that typo is fixed.

Here is the corrected inflate:

Code: Select all

void inflate(const T xInflate, const T yInflate)
{//! Modified by Ulf
   UpperLeftCorner.X -= xInflate;
   UpperLeftCorner.Y -= yInflate;
   LowerRightCorner.X += xInflate;
   LowerRightCorner.Y += yInflate;
}

A general question: Where's set to be favored over assignment of a newly generated object? so writing r.set(x,y,z,a) should be largely equivalent in run-time to r=recti(x,y,z,a). So maybe we should instead deprecate the set methods instead?

Can that be done if it's a pointer? I don't believe so.
Anyway, if we want to change the member values, why should we create a new object? That's my perspective.

[Ulf]

offset on the other hand was a function I only understood after reading it's source - I guess an operator+ would work better for me.

Hmmm... No it wouldn't work at all to do what I'm trying to do. With a += operator, you can't offset the Width or Height value. You can only write a function to do one or the other. I've already thought about that.

I wanted access to setting any single, or combination of values in one line of code and without creating extra objects.

and all the others, well - I certainly needed similar functionality already a few times, I just never missed it in rect itself. So not sure - what do others think? Lean interface or big interface prefered?

Well, if you look at some other libraries, wxWidgets for example, my RECT class would still be tiny, and it works much more naturally than the wxWidgets one for example.

[Ulf]

My quote:

Well, if you look at some other libraries, wxWidgets for example, my RECT class would still be tiny, and it works much more naturally than the wxWidgets one for example.

Maybe I exaggerated, but I still think the rect class is not so large.

Apparently, my SetLeft, SetUpperLeft etc are not clear as to what they do.
As far as function naming goes, I have some suggestions:

For setLeft(), we could use setLeftTo(), or moveLeftTo().
Isn't that CLEAR?
I get it intrinsically!

Note: I believe 100% that the rect class should have inflate().

Note(also): When I make functions I prefer having the option of passing the parameters separately rather than just in a point or rect. It provides more flexibility in useage. I prefer that rather than having to create an object to call the function. Often we are dealing with individual parameters, not just the whole rect or the x & y of a point. We might change just the x.

Any opinions on that? It makes the class larger, but it's just the same operations. You can't say that will slow down a program or make it too large! It's just a rect class after all, it's not large at all!

So I generally make 2 versions of every function.

[bitplane]

I certainly needed similar functionality already a few times, I just never missed it in rect itself. So not sure - what do others think? Lean interface or big interface prefered?

I'd say lean apart from complex or useful things.. I added constrainTo a while back, setPosition(position2d) and move(position2d relative) would be useful IMO, I think most of the others are overkill though.

[Ulf]

I have thought about it for hours now while working on patch ideas for mouse handling, and the google video on api development sort of helped.

I've come to the conclusion for now anyway, that several of my proposed RECT functions may be unnecessary.

I have a question.
Are immutable operations preferred?
For example, rect class has an equals operator that returns a const
But it doesn't have a set function.

Is it faster or safer to create an object, copy it's values by passing reference, and then return const . ?
Because I don't understand completely how that works and what is better, safer, faster. And what is most important between those things.

Code: Select all

const rect<T>& operator=(const rect<T>& other)

[bitplane]

Code: Select all

const rect<T>& operator=(const rect<T>& other)

In an assignment operator you have to change the value, creating a temp and returning it can't work, making the return type a const reference doesn't achieve anything as they are rarely used anyway (who writes a=b=c?). If you return a reference to a temporary object on the local stack then it is deleted and accessing it will cause a violation. If you create a new one on the heap then the user must delete it. Also, memory allocation is always slower than changing a value.

[Ulf]

Ok, that sort of answered what I wanted to know. I think I wasn't totally precise in my question though. Need to learn more about referencing, and revise my academic studies on the stack and heap, I'm sort of just getting into programming having finished my studies years ago and done other things since. I've done some hobby programs over the years to keep me in touch.

But, going with what you said:
Bitplane said:

Also, memory allocation is always slower than changing a value.

so should there definitely be a some set functions in rect class? Because the = operator needs a new object created when it is used. Doesn't it?

Code: Select all

rect = irr::core::rect<irr::s32>(0, 0, 640, 480);

Damn! How do you put someone's name in the quote title??

[hybrid]

Also, memory allocation is always slower than changing a value.

so should there definitely be a some set functions in rect class? Because the = operator needs a new object created when it is used. Doesn't it?

Code: Select all

rect = irr::core::rect<irr::s32>(0, 0, 640, 480);

No, not necessarily. The C++ standard allows to avoid object creation in some places. Moreover, the compiler can also generate such objects on the stack, as they are statically determined and only of temporary nature. Again, rely on your compiler to avoid such situations and don't give too much on manually performed peephole optimizations.

Damn! How do you put someone's name in the quote title??

Just press the "quote button" and see yourself, or just add quote="name" instead of quote in the beginning block

[Ulf]

Again, rely on your compiler to avoid such situations and don't give too much on manually performed peephole optimizations.

Ok.
But..

No, not necessarily. The C++ standard allows to avoid object creation in some places. Moreover, the compiler can also generate such objects on the stack, as they are statically determined and only of temporary nature.

Maybe not necessarily, but if the rect constructor involved dynamic variables, then it cannot be static.
By mistake the example I gave was static.

I can't see why rect doesn't have more functions in it because even the dimension2d class has a set function! I'd say that's less useful than for a rect class. There's only 2 values.