Michal Suchanek wrote:
> 2010/2/11 Vladimir 'φ-coder/phcoder' Serbinenko <phco...@gmail.com>:
>
>> Michal Suchanek wrote:
>>
>>> Hello
>>>
>>> Sending a preliminary framebuffer rotation patch.
>>>
>>> You can use videotest to see 4 tiles rotated from the same bitmap data.
>>>
>>>
>>>
>> +char leaf_data[] = { 0x00, 0x0f, 0xe0, 0x00,
>> + 0x00, 0x7f, 0xfc, 0x00,
>> + 0x01, 0xff, 0xff, 0x00,
>> + 0x03, 0xff, 0xff, 0x80,
>> This is a blob. Could it be generated automatically at build time?
>>
>
> How less of a blob it would be if it was included as a bitmap?
>
>
It's not easily editable in current form.
> This is just a shape used for the video tests.
>
> There are some X tools for working with bitmaps/pixmaps which could
> generate this from a viewable file but they are usually not available
> on Windows and other non-X platforms AFAIK.
>
>
Using XBM is fine (it's easily editable in either gimp or emacs) and you
should be able to
#include "leaf1.xbm"
>> +
>> + sans = grub_font_get ("Helvetica Bold 14");
>> Please use free font rather than Helvetica
>>
>
> I am pretty sure I did not choose the fonts, they must have been there
> before I started with modifications to the tests.
>
>
Ok, it has to be fixed in mainstream then.
>> Could you split addition of videotests from the rest of the patch?
>> - 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.
>> +/* Supported operations are simple and easy to understand.
>> + * MIRROR | swap image across (around) the vertical axis
>> + * FLIP - swap image across the horizontal axis - upside down
>> + * SWAP / swap image across the x=y axis - swap the x and y coordinates
>> It's just a D_8 group. Could you add a comment to functions what they do
>> in group theoretical sense? It would make the code easier to follow and
>>
>
> Obviously it is a group,
> any set of transforms closed on composition is.
>
>
Not true. Consider an example of empty set: it has no identity element.
Less trivial counter-example: set of transformations: (x,y)->(kx,ky),
0<k<1. It's non-empty but has no identity element. If you replace < 1
with <=1 you will have an identity element but no other element is
invertible.
> And according to Wikipedia it's actually called D4.
>
>
There are 2 conventions for naming dihedral groups:
1) Geometrical: since dihedral group is a symetry group of a regular
n-gone it's called D_n
2) Algebraical: since dihedral group has 2n elements it's called D_{2n}
Sometimes these conventions bear to confusion.
>> compute transformations for mathematicians. Perhaps another
>> representation of D_8 would result in more efficient code?
>>
>
> If you have clearer explanation or more efficient code please share.
>
>
I was thinking of using 2 generators instead of 3:
1) standard generators: s=rot90 or rot270, t=any reflection. Then all
elements are s^k or s^k*t. It makes computation of composition easier.
Rules on generators are:
s^n=t^2=1
tst=s^{-1}
2) or using 2 reflections. E.g. u=| and v=/. You already figured how to
generate with u=|, v=/, w=-
But w=uvuv
Then rules of computation are:
u^2=v^2=(uv)^4=1
>> +static inline long
>> +grub_min (long x, long y)
>> +{
>> + if (x > y)
>> + return y;
>> + else
>> + return x;
>> +}
>> +
>> Same here
>> +
>> + int transform;
>> I would prefer an enum
>>
>
> This is a bit field. How does it transform into an enum?
>
enum my_bitfield
{
MY_BITFIELD_NONE=0,
MY_BITFIELD_BIT0 = 1 << 0,
MY_BITFIELD_BIT1 = 1 << 1,
MY_BITFIELD_BIT2 = 1 << 2
};
> I would rather get a patch with minimal changes first so that it's
> obvious what it does.
>
> I can look into separating this later.
>
>
ok
--
Regards
Vladimir 'φ-coder/phcoder' Serbinenko
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
