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

On 07.04.2015 22:42, Bryce Harrington wrote:
> 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_...

That is reasonable.

> 
> 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.

That is also reasonable.

> 
> 
> 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.

Okay.


-- 
O< ascii ribbon - stop html email! - www.asciiribbon.org

-- 
cairo mailing list
[email protected]
http://lists.cairographics.org/mailman/listinfo/cairo

-----BEGIN PGP PUBLIC KEY BLOCK-----
Version: GnuPG v1.4.11 (MingW32)

mQENBE48DHkBCADjAv/EeFMN+i5XDN2WjSBU/yHbJlIG93/Hpj7Hee65qr82O9us
n3t4W1bk2+YwGBrFdfVlesHF4DObckXveayC+IulVvTJwZhR8igVENvWjIo6oF1N
1B3GV/c8zVCnHdkF0+vYJ9akX6DZf8KvBqKZapK1kc3tll0o+kS9lwNfpWRarUpQ
SBTw2uq+FUEsO3pwVAyvwom4b7AB6fz1tpyl28dOaNDKr2W55ZDPC8aM5PPe/kiH
n3ylOwpgXYqZLhIyJStqL/KcZ76y8o/gDXnilFRLwXu9CbXjapo7MARXByuSVMfb
PHa3XNj5JCzlv4GhEF4HOp4qoVRk8YASTqf/ABEBAAG0F0xSTiA8bHJuMTk4NkBn
bWFpbC5jb20+iQE9BBMBAgAnAhsjBQkJZgGAAh4BAheABQJT5qxpBQsJCAcDBRUK
CQgLBRYCAwEAAAoJEOs4Jb6SI2CwBNMIAKu3L9CGoTlJHZ/0G5AbbcP9zeiIkKkN
yvi1XszHiDz1kaESQYjpDsEWeHQutEhmnK5DNRyA6VV7Tcc49+1luq5Uf1kb8gGr
2mbF+eLG0Rv/WGoi/Aa4eH7K9dH2BSOUn+eX3Ft/HbBp9ZWCBRnABuO4oKmS+zfd
O9wMaQJkRRnwo151TNVMpWIO/kE7t4YeJyV/fUdYsMzlH53hzXnCM5DQ3ovje7NI
0prrSaqrthS0W5ELKZnb7PDY7pII0xh5/+u+f9Vs0mdMjvYZT42DiJnHvtfLEo1h
7KwLkC1lltZ/eaQOKFAszDzi2KoilSY/29mK7wLCd1TFJjklJNQqyxu5AQ0ETjwM
eQEIAKpvqHog2hpurRnsF6IKQtqR7JXYie9mvNoPW7XWmN3TtkHp5zrUG4SkR4dU
CTXxO82kMlwC94s79YJNTr30fahW6BVe72aZ+1D5qJcHk0CeHj56lri2kPOxyZXo
19Rhw43YtGZNvkNOg5xmMIzxUbMnxRTSppEFi0YQ+cCjnQksHiQPcCsb5bow53Ii
M9pICELT0d3nB5iDFCQb3oiXdRitDJJtZ97vOUE+xpUeYTcHXZyLmXYyMA7cFa/b
wRwj/5xofXYE0WlnHDn+0QOTf1BOGaWH1eZqYYBVKegKXW65Y7UOU4f0pZQUQ+FQ
wbwnSDlFuJPKA5MDgDczqtAVtNsAEQEAAYkBJQQYAQIADwUCTjwMeQIbDAUJCWYB
gAAKCRDrOCW+kiNgsJ2FCACaycPmmvwar2FwTbT+/OFM9rt9KQ6JCldNSePGNzHE
k0WBu5HyzgGcuOqoQSIwPHGnu2zkZl0PMW/WW9648a4OBuK08zNmypHXQw3fG0nG
KNsO6j86tOLINS6p/P4vMcF8Fz6ZdIwSj8oXnuku0C0nkLB/ja/VZk1wcI30ulJF
A2EhgLWIXZaTBlWR4KitGiGL7yzpUzBarPE8YFZLF+rfqu7NlxeG3NbBA/5UmH6r
FL5Cva/ajOJW2CtGyuROTYhqBfolxZtfSHHj1wOAlc+dA0utIW4NDbIyZxCoxDTF
/lUIjgj/Xu0IRIsX5m6TbBl40LH81JI0g1750FJfP4de
=aWCf
-----END PGP PUBLIC KEY BLOCK-----

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (MingW32)

iQEcBAEBAgAGBQJVJDXTAAoJEOs4Jb6SI2CwRKkIAJjgvwfKnBA4AD+zk6Knzcfv
qoaDHLHJiA3CzVKuuPY/pUjd57JMIVVCkwmUT5se66MwxHfMVxAFrrfnUmPP/XTS
z2n8/oQtHP503Pxyrp7aU9yl1rySHvovZNkNlbzebPKXEyGAIA1Wq5cfXgw1CXuv
UmQMof1Z62bmHawIeGaRbfJVLuMOjxcRQfxqY79yp92VcpAKQlZoMIvPmkbL5SGg
DUFue1KeRRixCOzU9SLjoeTvwFUWkMUs9z36Tj8GkrkEwZTlL4/GO7S7RB6+x+s3
nqfL/t/gSGKzHV7SK5yHbNG8uXxJodzJsBc4REppk81HYJFTCedwt9RrSUqDLgE=
=2xaa
-----END PGP SIGNATURE-----