[cairo] Problem with text rendering on Win32+ClearType, and possible fix

[Owen Taylor]

On Wed, 2006-10-04 at 11:04 -0700, Carl Worth wrote:
> I don't know anything about the win32 text API being discussed here,
> but hopefully I can make some useful comments about using git to
> explore the code history.
> 
> On Wed, 04 Oct 2006 11:35:38 -0400, Owen Taylor wrote:
> > 
> > I'm not really sure how to extract the diff and commit message
> > for the 1.0.2 => 1.0.4 change.
> 
> You can see all commit messages between 1.0.2 and 1.0.4 with:

[ Darn I got taken literally. I meant more "I'm too lazy..."
  Thanks for the tutorial and the diffs, though :-) ]

It's pretty hard for me to comment on these diffs, since they
overlap, and things were changed multiple times; things were
broken and then later reverted back to a fixed state.

It does look that the current trunk code does the right thing
again for BITSPIXEL==32 -- uses CAIRO_FORMAT_RGB24; I'm not sure how
that corresponds to what Tor was testing with.


I can't say I'm that perfectly happy with the current state of 
cairo-win32-surface.c. There's lots of confused comments, 
#if 0'ed out code, etc. :-(

Instead of commenting on the diffs, which is hard, I'll comment
on the current trunk code instead.

===
    /* We can't create real RGB24 bitmaps because something seems to
     * break if we do, especially if we don't set up an image
     * fallback.  It could be a bug with using a 24bpp pixman image
     * (and creating one with masks).  So treat them like 32bpp.
     * NOTE: This causes problems when using BitBlt/AlphaBlend/etc!
     * see end of file.
     */
    case CAIRO_FORMAT_RGB24:
    case CAIRO_FORMAT_ARGB32:
        bitmap_info->bmiHeader.biBitCount = 32;
        bitmap_info->bmiHeader.biCompression = BI_RGB;
        bitmap_info->bmiHeader.biClrUsed = 0;   /* unused */
        bitmap_info->bmiHeader.biC
===

"Something seems to break if we do" ... well, yes. biBitCount == 24
is a packed 24-bit image, and doesn't correspond to
CAIRO_FORMAT_RGB24. 

"NOTE: This causes problems when using BitBlt/AlphaBlend/etc!" I don't
think so. I think rather when drawing to a biBitCount == 32/xRGB with
GDI, what ends up in the 'x' is *completely undefined*. Now, it may
be that in some cases drawing a packed-24 bit image onto and xRGB
image you end up with 0xff in the 'x', but that's not something that's
defined by GDI. If we have a CAIRO_FORMAT_ARGB32 surface 
we simply can't draw onto it with GDI. Period. Well, except  
ARGB => ARGB bitblt, which is pretty safe.

....

In _cairo_win32_surface_create_similar_internal:
===
    /* if the parent is a DIB or if we need alpha, then
     * we have to create a dib */
    if (force_dib || src->is_dib || (content & CAIRO_CONTENT_ALPHA)) {
        new_surf = (cairo_win32_surface_t*)
            _cairo_win32_surface_create_for_dc (src->dc, format, width, height);
    } else {
        /* otherwise, create a ddb */
===

The tradeoff should at least be commented:
 
  DIB: Software for everything. Nothing really fast, nothing really slow.
  DDB: Solid colors, text, some images really fast. Gradients, vectors,
       dead slow.

Using a DDB is definitely a gamble. A complex vector image might be 
hundreds of times slower on a DDB Since we don't have any "smarts" to
know when we are losing and take corrective action, I'm not sure of 
the wisdom of preferring DDB's.

In _composite_alpha_blend:

===
    if (src->format == CAIRO_FORMAT_RGB24 && dst->format == CAIRO_FORMAT_ARGB32)
    {
        /* Both of these are represented as 32bpp internally, and AlphaBlend
         * DOES NOT throw away source alpha is AC_SRC_ALPHA is not specified,
         * it just multiplies it by the SourceConstantAlpha, along with the
         * R G B components.
         * XXX there has to be a way to do this!
         */
        return CAIRO_INT_STATUS_UNSUPPORTED;
    }
====

First, this check doesn't belong here. _composite_alpha_blend is only
supposed to be called when we've already decided that using AlphaBlend
is the appropriate code path.

The comment is just explained by what I said above, you can't draw onto
a CAIRO_FORMAT_ARGB32 drawable with GDI, period.

I see no reason why "there has to be a way to do this!". 

....

In _cairo_win32_surface_composite:

===
    /* Disable this for now */
#if 0
    if (src->base.type == CAIRO_SURFACE_TYPE_IMAGE) {
        src_image = (cairo_image_surface_t*) src;

        if (src_image->format != CAIRO_FORMAT_RGB24)
            return CAIRO_INT_STATUS_UNSUPPORTED;
    } else
#endif
[...]
if (src_image) {
        BITMAPINFO bi;
        bi.bmiHeader.biSize = sizeof(BITMAPINFOHEADER);
        bi.bmiHeader.biWidth = src_image->width;
        bi.bmiHeader.biHeight = src_image->height;
        bi.bmiHeader.biSizeImage = 0;
        bi.bmiHeader.biXPelsPerMeter = PELS_72DPI;
        bi.bmiHeader.biYPelsPerMeter = PELS_72DPI;
        bi.bmiHeader.biPlanes = 1;
        bi.bmiHeader.biBitCount = 32;
        bi.bmiHeader.biCompression = BI_RGB;
        bi.bmiHeader.biClrUsed = 0;
        bi.bmiHeader.biClrImportant = 0;

        if (!StretchDIBits (dst->dc,
                            dst_x, dst_y, dst_width, dst_height,
                            src_x, src_y, src_width, src_height,
                            src_image->data,
                            &bi,
                            DIB_RGB_COLORS,
                            SRCCOPY))
            return _cairo_win32_print_gdi_error ("_cairo_win32_surface_composite(StretchDIBits)");

        return CAIRO_STATUS_SUCCESS;
    } 
===

Not clear to me why this code has been "disabled for now", or how anyone would
figure that out in the future. There is clearly a missing check on the type
of the destination surface, but maybe the result is ugly, maybe the code 
otherwise broken. Who knows...


>From _cairo_win32_surface_fill_rectangles:
===
    /* XXXperf If it's not RGB24, we need to do a little more checking
     * to figure out when we can use GDI.  We don't have that checking
     * anywhere at the moment, so just bail and use the fallback
     * paths. */
    if (surface->format != CAIRO_FORMAT_RGB24)
        return CAIRO_INT_STATUS_UNSUPPORTED;
===

A1, A8 would take considerably more work all over the code. RGB32 will
never work. As compared to the original version of the code, I don't
see any reason to prefer GDI over pixman when we could use either ...
Maybe if for win32 pixman code still isn't using MMX, but well, decent
performance of cairo does pretty depend on the MMX code paths. Anyways,
if GDI works, GDI vs. pixman doesn't matter much.

At the end of the file:
===
/* Notes:
 *
 * Win32 alpha-understanding functions
 *
 * BitBlt - will copy full 32 bits from a 32bpp DIB to result
 *          (so it's safe to use for ARGB32->ARGB32 SOURCE blits)
 *          (but not safe going RGB24->ARGB32, if RGB24 is also represented
 *           as a 32bpp DIB, since the alpha isn't discarded!)
 *
 * AlphaBlend - if both the source and dest have alpha, even if AC_SRC_ALPHA isn't set,
 *              it will still copy over the src alpha, because the SCA value (255) will be
 *              multiplied by all the src components.
 */
===

For AlphaBlend, GDI simply doesn't have destination alpha support. Period. So, 
this is just a comment about undefined side-effects that happened on one 
system with one set of video drivers.
						- Owen