Hi Chris,
These patches generally make sense to me, but I have a few comments
below.
> - if (_pixman_implementation_lookup_composite (
> - imp->toplevel, info->op,
> - src_image->common.extended_format_code, src_flags,
> - mask_format, mask_flags,
> - dest_image->common.extended_format_code, info->dest_flags,
> - &imp, &func))
> + _pixman_implementation_lookup_composite (
> + imp->toplevel, info->op,
> + src_image->common.extended_format_code, src_flags,
> + mask_format, mask_flags,
> + dest_image->common.extended_format_code, info->dest_flags,
> + &imp, &func);
> {
This looks like it leaves an isolated set of braces in the code.
> diff --git a/pixman/pixman-glyph.c b/pixman/pixman-glyph.c
> index 5451f42..5a271b6 100644
> --- a/pixman/pixman-glyph.c
> +++ b/pixman/pixman-glyph.c
> @@ -464,13 +464,12 @@ pixman_composite_glyphs_no_mask (pixman_op_t
> op,
> glyph_format = glyph_img->common.extended_format_code;
> glyph_flags = glyph_img->common.flags;
>
> - if (! _pixman_implementation_lookup_composite (
> - get_implementation(), op,
> - src->common.extended_format_code, src->common.flags,
> - glyph_format, glyph_flags | extra,
> - dest_format, dest_flags,
> - &implementation, &func))
> - goto out;
Was this patch generated as a diff against the first patch? I don't see
"if (! _pixman_implementation_..." in the current code.
> + _pixman_implementation_lookup_composite (
> + get_implementation(), op,
> + src->common.extended_format_code, src->common.flags,
> + glyph_format, glyph_flags | extra,
> + dest_format, dest_flags,
> + &implementation, &func);
> }
>
> info.src_x = src_x + composite_box.x1 - dest_x;
> @@ -574,13 +573,12 @@ add_glyphs (pixman_glyph_cache_t *cache,
> white_src = TRUE;
> }
>
> - if (! _pixman_implementation_lookup_composite (
> - get_implementation(), PIXMAN_OP_ADD,
> - src_format, info.src_flags,
> - mask_format, info.mask_flags,
> - dest_format, dest_flags,
> - &implementation, &func))
> - goto out;
> + _pixman_implementation_lookup_composite (
> + get_implementation(), PIXMAN_OP_ADD,
> + src_format, info.src_flags,
> + mask_format, info.mask_flags,
> + dest_format, dest_flags,
> + &implementation, &func);
> }
>
> +void
> _pixman_implementation_lookup_composite (pixman_implementation_t *toplevel,
> pixman_op_t op,
> pixman_format_code_t src_format,
> @@ -142,7 +148,11 @@ _pixman_implementation_lookup_composite
> (pixman_implementation_t *toplevel,
> ++info;
> }
> }
> - return FALSE;
> +
> + /* We should never reach this point */
> + _pixman_log_error (FUNC, "No known composite function\n");
I think it would be useful to have some text about TLS most likely being
broken. "No known composite function" isn't particularly informative.
> diff --git a/pixman/pixman.c b/pixman/pixman.c
> index 3fabed1..24c93c5 100644
> --- a/pixman/pixman.c
> +++ b/pixman/pixman.c
> @@ -678,10 +678,10 @@ pixman_image_composite32 (pixman_op_t op,
> */
> op = optimize_operator (op, src_flags, mask_flags, dest_flags);
>
> - if (_pixman_implementation_lookup_composite (
> - get_implementation (), op,
> - src_format, src_flags, mask_format, mask_flags, dest_format,
> dest_flags,
> - &imp, &func))
> + _pixman_implementation_lookup_composite (
> + get_implementation (), op,
> + src_format, src_flags, mask_format, mask_flags, dest_format, dest_flags,
> + &imp, &func);
> {
> pixman_composite_info_t info;
> const pixman_box32_t *pbox;
Another case of braces left in the code.
Søren
_______________________________________________
Pixman mailing list
Pixman@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/pixman
