Re: [PATCH] GSoC #09 Bitmap scaling

[Colin D Bennett]

On Fri, 03 Oct 2008 23:16:51 +0300
Vesa Jääskeläinen <address@hidden> wrote:

> Vesa Jääskeläinen wrote:
> > Vesa Jääskeläinen wrote:
> >> It is really necessary that all generic graphical menu code like
> >> bitmap scaler works always. Not just for some optimized formats as
> >> otherwise it would render graphical menu unusable in some cases.
> >> It can be not so pretty on low end systems, but it should at least
> >> work. That is the reason there is map/unmap functions in video api.
> >>
> >> So there has to be first generic implementation and then you can
> >> use optimized one if there exist one.
> > 
> > Just thinking as this is operating in a bitmap, we could just make
> > sure we use only couple of formats and let video driver dot the
> > rest. That would simplify it a bit.
> 
> Hi,
> 
> Now that Colin is back in action I am reviving this thread :).
> 
> 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?

> 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.

> 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.

> 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.)

> Any opinions?
> 
> Thanks,
> Vesa Jääskeläinen

Regards,
Colin

signature.asc
Description: PGP signature