[poppler] Openjpeg2 patches: Why de-inline doGetChar?

[Adam Reichold]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Hello,

Am 05.01.2015 um 22:49 schrieb Albert Astals Cid:
> El Dilluns, 5 de gener de 2015, a les 22:29:22, Adam Reichold va
> escriure:
>> Hello again,
>> 
>> Am 05.01.2015 um 21:59 schrieb Albert Astals Cid:
>>> El Dilluns, 5 de gener de 2015, a les 21:46:32, Adam Reichold
>>> va
>>> 
>>> escriure:
>>>> Hello again,
>>>> 
>>>> suggested changes attached as patch.
>>> 
>>> Doesn't compile, init() is not accessible from that static 
>>> function.
>> 
>> Sorry about that, it seems CMake does not find OpenJPEG (1 or 2)
>> at all on my system with the current master, the library is found
>> alright but the includes are "LIBOPENJPEG_INCLUDE_DIR-NOTFOUND"
>> so the file was not compiled at all. Autoconf on the other hand
>> not problems finding at least the preferred OpenJPEG version 1.
> 
> HAve you tried cleaning your cmake build dir? It works fine to me.

It seems for some reason Arch Linux installs the OpenJPEG header only
into "/usr/include/openjpeg-1.5/openjpeg.h" and "find_path" does not
find that, symlinking "/usr/include/openjpeg.h" there works fine.
(Possibly nobody noticed as the package is built using Autoconf?)
(Also I have to correct myself, OpenJPEG version 2 is found correctly.)

Best regards, Adam.

> Cheers, Albert
> 
>> I also attached a fixed patch that moves the lazy-initialization
>> into the entry points which also has the benefit of making the
>> check only once for each call of 'getChars' but is admittedly
>> uglier. (One solution would be to move the 'inited' flag into the
>> class proper instead of the pimpl or even use "priv != NULL" as
>> the initialization condition, but that felt unnecessarily
>> intrusive.)
>> 
>> Best regards, Adam.
>> 
>>> Cheers, Albert
>>> 
>>>> Best regards, Adam.
>>>> 
>>>> Am 05.01.2015 um 21:24 schrieb Adam Reichold:
>>>>> Hello,
>>>>> 
>>>>> Am 04.01.2015 um 23:44 schrieb Albert Astals Cid:
>>>>>> El Dilluns, 5 de gener de 2015, a les 08:53:26, Adrian 
>>>>>> Johnson
>>>>>> 
>>>>>> va escriure:
>>>>>>> On 05/01/15 08:39, Albert Astals Cid wrote:
>>>>>>>> Adrian, any reason you de-inlined doGetChar in the 
>>>>>>>> patches for openjpeg2?
>>>>>>>> 
>>>>>>>> As far as i remember when i made this function inline
>>>>>>>> it got us a not so bad free speed increase.
>>>>>>> 
>>>>>>> When I wrote the patch I moved all openjpeg specific
>>>>>>> types and code into the .cc file. I could not figure
>>>>>>> out how to make an inline function in the .h access
>>>>>>> the JPXStreamPrivate struct that is defined in the .cc
>>>>>>> file.
>>>>>> 
>>>>>> I see :/
>>>>>> 
>>>>>> Cheers, Albert
>>>>> 
>>>>> Looking at the implemenation, both "doGetChar" and 
>>>>> "doLookChar" will always be called inside
>>>>> "JPEG2000Stream.cc" via the virtual methods "getChar(s)"
>>>>> and "lookChar", so I'd say we could just mark the
>>>>> implemenations as "inline" as the compiler can still inline
>>>>> within the compilation unit.
>>>>> 
>>>>> Actually, I think there is no compelling reason to have
>>>>> those present in the header at all, as they always seem to
>>>>> access the data via the "priv" pointer anyway, so I'd say
>>>>> the declarations like
>>>>> 
>>>>> static inline int doGetChar(JPXStreamPrivate* priv);
>>>>> static inline int doLookChar(JPXStreamPrivate* priv);
>>>>> 
>>>>> only within the source file could open even more
>>>>> possibilities for compiler optimizations. (You could
>>>>> probably also make them inline methods of
>>>>> "JPXStreamPrivate" as well, but then the compiler would
>>>>> have to infer the effective linkage. Maybe we could put the
>>>>> whole definition of "JPXStreamPrivate" within the anonymous
>>>>> namespace anyway?)
>>>>> 
>>>>> Best regards, Adam.
>>>>> 
>>>>>>>> Cheers,
>>>>>>>> 
>>>>>>>> Albert
>>>>>>>> 
>>>>>>>> _______________________________________________
>>>>>>>> poppler mailing list poppler at lists.freedesktop.org 
>>>>>>>> http://lists.freedesktop.org/mailman/listinfo/poppler
>>>>>>>
>>>>>>>
>>>>>>>> 
_______________________________________________ poppler
>>>>>>> mailing list poppler at lists.freedesktop.org 
>>>>>>> http://lists.freedesktop.org/mailman/listinfo/poppler
>>>>>> 
>>>>>> _______________________________________________ poppler 
>>>>>> mailing list poppler at lists.freedesktop.org 
>>>>>> http://lists.freedesktop.org/mailman/listinfo/poppler
>>>>> 
>>>>> _______________________________________________ poppler 
>>>>> mailing list poppler at lists.freedesktop.org 
>>>>> http://lists.freedesktop.org/mailman/listinfo/poppler
> 
> _______________________________________________ poppler mailing
> list poppler at lists.freedesktop.org 
> http://lists.freedesktop.org/mailman/listinfo/poppler
> 
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQEcBAEBCAAGBQJUqwoGAAoJEPSSjE3STU340SgIAMBUIC/PpPpND/m5MQlVhKac
PVZqwtkXWcKF2xEzwl3bv+xzKQbHYIkuCkUDjvrIzyHCVQjoIzoFDiHyB6PaHLyc
Yy9WNCwx62LreZDWOiwBkEs2YQsjcSPjMVOrB5uDco4D2Evls+RXyoUOKgwXIwLD
lvKnzC2O/WO3N1k0IjJUKzAW5bzSEjUaEzO7C8kJOvSnum8exDTdYvQKmG8RTM69
+qFJwY1T+qqlox53rPa2dmy8utLmeUjaLWt4aGVxjrjDyfsUKjkDgIDl2AKS2pjH
H/lcsMb/QuhHy4eViJBAB79AF+fVjiYIJkPes5hqRwXG3fC0AGQAJiSbqlO9+NE=
=/HOF
-----END PGP SIGNATURE-----