Re: [RFC] Framebuffer rotation patch

[Vladimir 'φ-coder/phcoder' Serbinenko]

Michal Suchanek wrote:
> On 11 February 2010 11:19, Michal Suchanek <address@hidden> wrote:
>   
>> 2010/2/11 Vladimir 'φ-coder/phcoder' Serbinenko <address@hidden>:
>>     
>>> 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?
>>
>> 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.
>>
>>     
>>> +
>>> +  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.
>>
>>     
>>> 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.
>>
>>     
>>> +/* 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.
>>
>> And according to Wikipedia it's actually called D4.
>>
>>     
>>> 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.
>>
>>     
>
> Let me elaborate a bit more on this one.
>
> The provided explanation should make it possible for anybody who
> understands plain English to also understand the operations used and
> how they interact should they know group theory in English, any other
> language, or not at all.
>   
Perhaps one could add an example of how it moves vertices of a square?
E.g for vertical flip:

1---2            2----1
|    |     -->    |     |
4---3            3----4
> I guess mentioning the English mathematical terms where applicable and
> where I can figure them out should not that much of a problem, as well
> as checking that the transform functions are sufficiently described.
>
>   
Feel free to ask any questions.


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko

signature.asc
Description: OpenPGP digital signature