Mir

Code review comment for lp:~alan-griffiths/mir/more-surface-resize

Revision history for this message
Alan Griffiths (alan-griffiths) wrote :

> Some questions (Aw=aspect W, Ah=aspect H):
>
> 1. In this case, since W/H < Aw/Ah, we can either increase W or decrease H to
> increase the ratio, as is done in the two branches of the if statement. What's
> the rationale behind using the relation between W and H to decide whether to
> change W or H?

I admit it is arbitrary. And now I examine it, probably wrong.

My (misguided) thought was that changing the smaller one leads to a bigger effect on the ratio for a smaller change. But suppose we want a ratio of 10:1, but have 10:20. Then my approach leads to 200:20 when 10:2 would probably be better.

> 2. What's the logic behind the formula in DeltaX (and DeltaY respectively)? If
> we want W/H to reach Aw/Ah we could increase W by error/Ah, so it's not clear
> why we need the other terms (the proposed formula leads to an increase of
> error/Ah + 1 + 1/Ah).

It's to ensure rounding up. (I didn't think common incantation in integer arithmetic needed explaining - I was wrong.)

> It would be good to add a comment documenting how the proposed computations
> and decisions work.

And even better to get it right. ;)

review: Needs Fixing

« Back to merge proposal