Re: [Linux-fbdev-devel] fb_imageblit()

[James S. <jsi...@in...>]

> I've thought of that also, packing the data according to the pixel depth.  

Actually I was think more along the image depth. The largest the penguin 
logo gets is 224 colors. Meaning the largest image.depth is 8 bits per 
pixel. I do agree it is messy to pack the logo data. So for say the 16 
color logo (4 bpp) putting two pixels worth of data into each byte and 
then seperating it out would be time consuming. The only reason I was 
thinking this way is because of userland using the cursor means imageblit 
is visible to userland. 

> a. Prepare logo data so each pixel of data is in directcolor format (if
> we will use the cmap), so depth corresponds to framebuffer depth, and
> data is packed. 

The depth doesn't have to be the same as the framebuffer depth. The logo 
data is in terms of color map indices. I did think in terms of packed data 
tho.

> d. Get each color component from the cmap data. 
> 
> e. Recreate pixel data based on var.[color].offset and
> var.[color].length. 

Yuck no. Doing a fb_set_cmap should handle that before we call 
xxxfb_imageblit. Then xxxfb_imageblit only has to take the data in 
the logo data as indices of the color map. Of 
 
> f. Write the pixel data in packed form to the framebuffer. 

> Whereas, with the current approach: 
> 
> a. Prepare logo data such that each pixel corresponds to one byte. 
> 
> b. Pass the structure to cfb_imageblit. 
> 
> c. Read color information from pseudopalette if directcolor/truecolor. 
> 
> d. Write the pixel data in packed form to the framebuffer. 

I was think the same way except for the idea of more than one color 
indices per byte for less than 8 bpp modes.
 
> Secondly, indexing the cmap instead of the pseudo_palette means that
> cfb_imageblit has to know the native framebuffer format.  

No indexing the cmap. Instead call fb_set_cmap before we call 
xxxfb_imageblit. Personally I think we should call fb_set_cmap always.

> I would rather have everything refer to
> the pseudopalette, regardless of the visual format.  This will be better
> especially for some of the corner cases, like monochrome cards with
> bits_per_pixel = 8. 

It works pretty good for for any pack pixel type modes. Now for planar 
cards this isn't the case. Ideally struct fb_cmap should have contained 
a 

unsigned long pixel;

field. Like X does. It does not and changing that would break things :-(