Re: [PATCH] GSoC #09 Bitmap scaling

[Vesa Jääskeläinen]

Colin D Bennett wrote:
> On Fri, 03 Oct 2008 23:16:51 +0300
> Vesa Jääskeläinen <address@hidden> wrote:
>> I have been thinking this a bit and I think best solution to bitmaps
>> is something like following.
>>
>> Two types of bitmap: easily accessible and optimized bitmaps for
>> hardware.
>>
>> Easily accessible are meant to be modified by user and provides slow
>> blitting performance. Basically we should only support two formats.
>> RGB888 and ARGB8888 (order can be different). This way we can make
>> easy code to modify bitmaps.
>>
>> When there is no need to modify bitmap anymore, one calls
>> grub_video_bitmap_optimize(bitmap, rendering_target) and then bitmap
>> is optimized to match reder_targets format.
>>
>> there would be two new helpers that gives access to bitmap data.
>> grub_video_bitmap_lock() and to release it grub_video_bitmap_unlock().
>> Lock will fail if bitmap is optimized.
> 
> What is the effect of lock/unlock on an unoptimized bitmap?  Is it like
> a reference counting memory management scheme?

Idea is that if bitmap is still locked you cannot optimize it or you
cannot lock it again. And if bitmap is optimized you cannot get pointer
to modify it. Eg. Function call returns memory pointer.

>> I am wondering should we have grub_video_bitmap_unoptimize() to map
>> back to editable mode. But that might be more pain and promote bad
>> ways to do conversion.
> 
> If some code needed to modify an optimized bitmap, we could just have
> that code keep the unoptimized version of the bitmap around for
> modification, and just re-optimize the bitmap after modifying it.  Of
> course it would only make sense to optimize bitmaps that will be
> blitted more than once.

And if bitmap is animated, there is no good reason to optimize it either.

>> Now bitmap loaders would be modified to return data in easily
>> accessible format so bitmaps can be modified and so on.
>>
>> Now to bitmap scaling. This can only be done for easily accessible
>> formats and could be something like this:
>>
>> +grub_err_t
>> +grub_video_bitmap_create_scaled (struct grub_video_bitmap **dst,
>> +                                 int dst_width, int dst_height,
>> +                                 struct grub_video_bitmap *src,
>> +                                 enum
>> +                                 grub_video_bitmap_scale_method
>> scale_method);
>>
>> Now in example static bitmaps would be optimized right away in order
>> to make their blitting fast. I do not know if we need special
>> handling for transparent bitmaps. May need some experimenting.
> 
> I have been scaling bitmaps with alpha transparency and it works fine
> with the scaling code in the patch.  It's used for the menu frame, the
> selected item highlight, and entry icons, to name a few uses.

Well... For our use it should be sufficient. So lets use that.

>> Actual scalers would be hidden from API and can only be specified by
>> enum type.
> 
> Ok; sounds good.
> 
>> It could be a good idea to implement all scalers in same file. Unless
>> there is some weird scaler that needs thousands of lines of code.
> 
> I'm fine with that.  (Though in general my preference is for short
> source files:  900+ line files become like a labyrinth of code, a
> maze of twisty passages all alike.)

True. But sometimes it is good idea to keep it in same place. I didn't
look at the files but I think it should not be too long. In example you
can hide those other methods with static specifier.

Thanks,
Vesa Jääskeläinen