DecompressBitmap / Decompress4bpp / Decompress8bpp

[Love Nystrom]

Hi guys,

Sorry for barging in on a discussion about code that's not on my desk,
but looking at your code there is something I want to point out.

Graphics code need to be *fast*, that's a primary consideration,
so I'm taken aback when I look at your inner bit expansion loop.
This is horrible from a standpoint of performance.
>    length = (*bits++) >> shift;
>    if (length)
>    {
>        c = *bits++;
>        while (length--)
>        {
>            if (x >= width) break;
>            temp = UncompressedBits + (((height - y) * Delta) + x);
>            x++;
>            *temp = c;
>        }
>    }

You're recomputing the start of the bit-run for every pixel you emit,
when you should move that calculation out of the inner loop to get
high performance. Graphics code is not the arena where you can be lazy
and hope that the optimizer will make your inefficient code faster.
At the very least You ought to write it something like this:
>    length = (*bits++) >> shift;
>    if (length)
>    {
>        c = *bits++;
>        if (x + length > width) {
>            // RLE encoding error - Bit-run exceeds width
>            length = width - x;
>        }
>        temp = UncompressedBits + ((height - y) * Delta);
>        x += length; // precompute finishing x
>        while (length--)
>        {
>            *temp++ = c;
>        }
>    }

As a sideline note I'd like to mention that it's standard practice
in graphics libraries to use one unified bitmap format internally
to make internal processing like alpha-blend or text rendering or
whatever straight forward. The 32bit DIB format is quite suitable,
even if it uses the most memory - Graphics never was cheap on memory.

Just my penny to the pot
Best Regards
// Love

[Gregor Schneider]

Hi Love,

your concern is correct, we're wasting cycles here. On the other hand, as
pointed out in the other RLE discussion, this code is a temporary solution.
We incorrectly decode RLE bitmaps to be able to process them with our
standard blitting routines. Low level RLE support for DIBs does not exist at
the moment. Same for JPEG and PNG formats.
Surprisingly everbody seems to be quite hot about RLE bitmaps recently,
considering we use exactly _ONE_ RLE bitmap (I know of) in ReactOS.

Concerning using an internal format: let's assume we got three 8bpp surfaces
(source, destination, pattern), which are quite big pending for a raster
operation. You would convert these three to another format like 32bpp,
process them and convert back? For these three surfaces the memory need for
processing is increased by a factor of 4 and performance is obviously bad
too. That's not an option considering that Starcraft for example spends most
of the time in DIB routines, highlighting that our current approach is not
even fast enough. Why decrease performance even further? Standard DIB access
functions to access all DIB formats exist, why not use them? The same
applies for our alpha blending approach. I have no clue if it would be
beneficial for fonts, but I doubt it.

Best regards,
Gregor

2010/5/9 Love Nystrom [e-mail]
> Hi guys,
>
> Sorry for barging in on a discussion about code that's not on my desk,
> but looking at your code there is something I want to point out.
>
> Graphics code need to be *fast*, that's a primary consideration,
> so I'm taken aback when I look at your inner bit expansion loop.
> This is horrible from a standpoint of performance.
>    length = (*bits++) >> shift;
> >   if (length)
> >   {
> >       c = *bits++;
> >       while (length--)
> >       {
> >           if (x >= width) break;
> >           temp = UncompressedBits + (((height - y) * Delta) + x);
> >           x++;
> >           *temp = c;
> >       }
> >   }
>
> You're recomputing the start of the bit-run for every pixel you emit,
> when you should move that calculation out of the inner loop to get
> high performance. Graphics code is not the arena where you can be lazy
> and hope that the optimizer will make your inefficient code faster.
> At the very least You ought to write it something like this:
>    length = (*bits++) >> shift;
> >   if (length)
> >   {
> >       c = *bits++;
> >       if (x + length > width) {
> >           // RLE encoding error - Bit-run exceeds width
> >           length = width - x;
> >       }
> >       temp = UncompressedBits + ((height - y) * Delta);
> >       x += length; // precompute finishing x
> >       while (length--)
> >       {
> >           *temp++ = c;
> >       }
> >   }
>
> As a sideline note I'd like to mention that it's standard practice
> in graphics libraries to use one unified bitmap format internally
> to make internal processing like alpha-blend or text rendering or
> whatever straight forward. The 32bit DIB format is quite suitable,
> even if it uses the most memory - Graphics never was cheap on memory.
>
> Just my penny to the pot
> Best Regards
> // Love
>
> Ros-dev mailing list
> [e-mail]
> http://www.reactos.org/mailman/listinfo/ros-dev