Re: [RFC] Framebuffer rotation patch, or why 'unsigned' fails us

[Michal Suchanek]

On 15 February 2010 18:05, Colin D Bennett <address@hidden> wrote:
> On Sat, 13 Feb 2010 18:29:41 +0100
> Michal Suchanek <address@hidden> wrote:
>
>> 2010/2/11 Vladimir 'φ-coder/phcoder' Serbinenko <address@hidden>:
>> > Michal Suchanek wrote:
>> >> 2010/2/11 Vladimir 'φ-coder/phcoder' Serbinenko
>> >> <address@hidden>:
>> >>
>> >>> Michal Suchanek wrote:
>> >>> -    unsigned int x;
>> >>> -    unsigned int y;
>> >>> -    unsigned int width;
>> >>> -    unsigned int height;
>> >>> +    int x;
>> >>> +    int y;
>> >>> +    int width;
>> >>> +    int height;
>> >>> Why do you need negative values?
>> >>>
>> >>
>> >> I don't need the negative values everywhere but this change was to
>> >> align the typing so that casts are not required.
>> >>
>> >>
>> > Perhaps few casts together with appropiate checks when casting is
>> > better than potentially exposing the whole code to the values it's
>> > not intended for.
>>
>> How is it exposed to more unwanted values than now?
>> It uses the variables only to store the viewport dimensions and at
>> most adds a border around it. The unsigned integer is able to
>> represent values outside of the viewport as much as signed integers
>> are, only signed integer can represent values outside of viewport in
>> both directions. The unwanted values (if erroneously calculated which
>> does not seem to be the case here) are clipped by the viewport
>> clipping in video_fb.
>>
>> On the other hand, there would be more than a few casts as the
>> variables are used multiple times and the interface now uses signed
>> integers.
>
> I have to chime in on this one since I've learned the hard way that
> 'unsigned' doesn't pull its weight in these use cases.  Using unsigned
> integer values in C when a lot of arithmetic is performed (as in
> graphics programming) is really a nightmare. Well, maybe if you are a
> grand master C programmer you would remember when you need to cast and
> never make a mistake, but remember that there are us mere mortals who
> would benefit by more logical and predictable code behavior.  (I use
> 'predictable' not in the sense that expressions involving both signed
> and unsigned values are have an undefined result, but rather that it's
> usually not the obvious result that the average C programmer would
> expect at first glance.
>
> Certainly it would be nice if we could use the 'unsigned' qualifier to
> enforce an invariant requiring only non-negative values be stored in
> it (which it technically does, but it doesn't necessarily make the
> result any more correct).  It almost never really fixes the kind of
> problems we'd like it to.  We could easily try to store the result of a
> subtraction that conceptually should produce a negative value into an
> unsigned variable, and--OOPS!--we now have an enormous value instead of
> a negative value. Not much better.  More significant is code that
> involves multiple operations such that the result of the entire
> expression should be nonnegative, but it involves temporary negative
> values in subexpressions such that the implicit signed-unsigned
> conversion produces the "wrong" result (i.e., not what the programmer
> intended even though the expression looks correct).
>

I started using negative integers because I need that rectangles
overflowing to the top and left are not special and different from
rectangles overflowing to the bottom and right as transforms might map
the latter to the former.

This also enhances the video interface so that drawing at negative
position is representable in the arguments. Iit is then possible to
blit a bitmap without first clipping the topleft part by just blitting
at negative position which should make some operations easier to carry
out.

What is left unsigned is the framebuffer size in mode info and it
indeed requires some casts and causes trouble. It made me change one
of the transform routines to asymmetric types because it is used
(almost) exclusively on the mode info structure. The members of
mode_info should not ever become negative, though and the unsigned
type is probably meant to make that clear. I can't say if making that
clear outweights the possible issues when using the values in
calculations.


Thanks

Michal