[cairo] [PATCH 2/3] Support a different pixel format for HDC

[Bryce Harrington]

On Fri, Apr 03, 2015 at 09:14:49PM +0200, Uli Schlachter wrote:
> Am 26.03.2015 um 20:44 schrieb LRN:
> > On 11.03.2015 0:31, Bryce Harrington wrote:
> >> On Tue, Mar 10, 2015 at 05:52:07PM +0300, LRN wrote:
> >>>>
> >>>> Patch #1 is from cairo bugzilla, see the link you gave me in [1].
> >>>>
> >>>> Patch #2 (this patch) was used by Mozilla[2] back in 2010. They have [had?]
> >>>> internal copy of cairo to patch, so maybe they just haven't bothered with
> >>>> submitting it. Or maybe they did submit it to cairo bugzilla and it got lost
> >>>> there. Who knows? Either way, the patch is trivial enough, so i wouldn't
> >>>> worry about this one too much.
> >>>>
> >>>>
> >>>> [1] http://lists.cairographics.org/archives/cairo/2014-April/025148.html
> >>>> [2] https://bugzilla.mozilla.org/show_bug.cgi?id=577200
> >>>>
> >>>
> >>> Any progress on this?
> >>>
> >>> The fallback surface-related crash was fixed in 0.14, so the patch labeled "Enlarge fallback surface" is no longer necessary. I would only need this patch ("Support a different pixel format for HDC") and at least one bit from "Adjust assertions and checks to handle more pixel formats".
> >>>
> >>> These patches are very small, the only prerequisite for being able to evaluate them is understanding of cairo and W32 API. I'd very much like to stop patching cairo and push Windows RGBA code into GTK.
> >>>
> >>
> >> Mind extracting whatever bits are still needed, into new patches?
> >> Please use git format-patch against cairo trunk for creating them.
> >>
> >
> > Here they are.
> >
> > Do i need to send each one as a separate email (not an as an attachment) as well?
>
> We talked about these patches a bit on IRC. I have no clue about win32, but LRN
> and Google managed to convince me that this might make sense. It would be great
> if this were covered by the test suite, but the cairo_win32_surface_create()
> function isn't covered either (the source code claims that this doesn't make
> sense and the existing *_dib() boilerplate is enough).
>
> So this has my +1, although I would prefer if someone with some actual clue
> about win32 would look over this and say that this API makes sense.

Thinking about the API a bit...

+static cairo_surface_t *
+cairo_win32_surface_create_internal (HDC hdc, cairo_format_t format)
+
+cairo_surface_t *
+cairo_win32_surface_create_with_alpha (HDC hdc)

First, side note that the first routine is internal and should be
prefixed with an underscore, i.e. _cairo_...

But bigger question is why add a whole API for just setting format to a
particular value?  Why not a generalized public API that takes format as
an input?  In other words, merge the above two routines:

+cairo_surface_t *
+cairo_win32_surface_create_with_format (HDC hdc, cairo_format_t format))

And then we still have cairo_win32_surface_create() calling this, with
format=CAIRO_FORMAT_RGB24.


The one concern is that if we expose setting the format in a public API
then it permits sending invalid formats.  But that's easy to fix:
*_create_with_format needs to simply check that the format parameter is
either of the two the supported values, and error otherwise.

Then, if new formats become supported in the future, the changes to
support them would be internal, and not require adding more API.

Bryce

> Cheers,
> Uli
> --
> 99 little bugs in the code
> 99 little bugs in the code
> Take one down, patch it around
> 117 little bugs in the code
>   -- @irqed
> --
> cairo mailing list
> cairo at cairographics.org
> http://lists.cairographics.org/mailman/listinfo/cairo