video::IImage::getDimention returns signed dimension2d...why

DtD · Post by **DtD** » Sat Jan 17, 2009 8:23 pm

This is mostly because I am curious, but is there any particular reason video::IImage::getDimension returns core::dimension2d<s32> instead of core::dimension2d<u32>? It seems uneccesary to use signed values since the image can't have a negitive size. (Also, considering video::IImage::getPixel uses u32 for its x & y arguments.)

Maybe I'm wrong, but wouldn't this break support for images larger than 2,147,483,647 * 2,147,483,647 pixels? Of course, nobody would ever use that size

(That's about a 1844 TB RAW ARGB image if I did my math right XD)

~DtD
PS> Remember, this is just curiosity

Sylence · Post by **Sylence** » Sat Jan 17, 2009 9:47 pm

So who cares?
If we ever come near to this 1844 TB it is just a small change. But until then (I guess at least 10 years) this is no problem.

If it would be u32 one could ask why it isn't u64 because u32 only allows 4,300,000,000 * 4,300,000,000 pixels...

DtD · Post by **DtD** » Sun Jan 18, 2009 10:08 pm

I was just curious if there was any particular reason, I don't care at all

~DtD

Dorth · Post by **Dorth** » Sun Jan 18, 2009 10:15 pm

It is an excellent point. A variable type does not only contain what it can and cannot do, it also contains information about how it SHOULD be used, what SHOULD be expected. There is no gain for the s32 but a small gain for the u32 (clarity) thus I think it should be changed. While minor, I see this as a valid concern. Maybe add it to the tracker since you're the one who found it? Getting the credit and all

hybrid · Post by **hybrid** » Mon Jan 19, 2009 8:36 am

We'll also need an abbreviation for u32 based types, because right now we only have them for s32 and f32 (such as dimension2di and dimension2df). Is dimension2du an appropriate solution?

Sylence · Post by **Sylence** » Mon Jan 19, 2009 8:49 am

Yes sounds good

rogerborg · Post by **rogerborg** » Mon Jan 19, 2009 12:44 pm

I will killlllllllllllllll you.

I mean: *Wow, thanks!* Once I got to thinking about it, there are relatively few cases where a negative dimension actually makes much sense.

I'm working on this now. For those who want to follow along, I started by adding dimension2du and then did a global find/replace on dimension2di / dimension2d<s32>, then restored s32 as necessary.

There are still some signed dimensions used in GUI sizes, and in 2D clip methods, that require some (explicit) casting of dimension2du to dimension2di. We'll have to have a think about whether those can be unsigned as well.

I'm now looking into some issues in the GUI. Actually, now I'm going to the bathroom, then I'll get a can of Irn Bru on the way back. Can I get you anything while I'm up?

rogerborg · Post by **rogerborg** » Mon Jan 19, 2009 1:49 pm

OK, I've done a first tranche on the trunk in SVN 2092.

There's been considerable knock-on effects through the code base. We should consider this to be Work In Progress; in fact, we may even end up reverting some of it.

Please do sing out if this creates any problems.

Dorth · Post by **Dorth** » Mon Jan 19, 2009 5:45 pm

Laughing just a bit too much right now, will maybe make a check myself later, but that I will .... comment of yours rogerborg...