Re: Additional issues to address for the constraints_experiments branch?

[Elijah Newren <newren gmail com>]

On 12/3/05, BJörn Lindqvist <bjourne gmail com> wrote:
> Hi. I know it's a long time since constraints_experiments were
> branched, but I just want to comment thing and ask some questions. In
> constraints.c there are quite a lot of instances of inconsequent
> variable initialisation:
>
> For example, in meta_window_constrain() on line 285:
>
> ConstraintPriority priority = PRIORITY_MINIMUM;
>
> First, the variable declaration appears in the middle of the block and
> is only valid in C99. Second, the variable is initialised on the same
> line as it is declared. "Normal" Metacity-style seem to be more
> something like:
>
> ConstraintPriority priority;
> priority = PRIORITY_MINIMUM;
>
> At the beginning of the block. Don't get me wrong - I really love c99
> because you can declare variables closer to where they are
> initialised. I'm just curious.

Yeah, the "we need to support braindead versions of c" stuff is filed
as bug 322622 with a patch I need to review.  Thanks for the reminder.
 :)

> I would also like to complain on L723 and L725, multiplication by zero ...??

What's wrong with that?  Granted, having it be "= 0" instead of "*= 0"
would be just fine too, I was just probably thinking of it in terms of
percentage that I wanted to keep at the time I wrote it.  *shrug*

> And then the lines /* Yeah, I suck for doing implicit rounding -- sue
> me */. But they aren't roundings, they are truncations aren't they?
> And IMHO, it isn't that hard to add the (int) casts, then you wouldn't
> need to suck and noone would have to sue. :)

Hehe.  ;-)  You're right that they are truncations.  Either the
comments should be fixed to reflect that or the code should be changed
to reflect the comments.  We could probably be more correct by doing
some form of rounding, e.g. rounding the minimum size up and the
maximum size down.  The original version did that, and then I realized
that if the min ratio and max ratio were the same (as is often the
case), then rounding the lower one up and the upper one down would
give an invalid interval to clamp to.  So we need to keep cases like
that in mind.  Maybe we should just look at gtk+ code and determine
how it rounds and do things the same way (otherwise gtk+ apps are just
going to send ConfigureRequest events to "fix" things to the way it
likes the rounding to be done).

Anyway, we probably need some kind of fixup here.  Feel free to file a
bug and a patch.

Cheers,
Elijah