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

[Adam Reichold]

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

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.

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
> 
> 
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQEcBAEBCAAGBQJUqwIuAAoJEPSSjE3STU34jEEH/3vsrWvHpVqwB1cEhke7QKw9
YqwarcjtfEdUW/wpn0kiOSKtIDBTx7nqcH+wwyZ1p3J64eWULP/IG59UmLiB3Im9
RoSBgK5QL03BbaD6flHcb99Q9oBvw1Mp8T6DFOO+joQsRbwFLcM8JY00LFaFitdP
QH82Ed8n9NAZJJ6Tis1Q2ZWvyN9ceEZTCsohUKKcb4GyBkWlw/ddRIFITw9xtH0y
ipMlZrhFXfz2jNN+pe51Zhn/yO/9cWfwf0daLPX5B/L1B4+MjbQMcymX9c7cCMb1
yuadAuEaMhe0kmPzOHjp6Zg3ZvatamN8WrkOW2PyBReg2vBdi6IMBHNwfR7s8R4=
=4rQj
-----END PGP SIGNATURE-----
-------------- next part --------------
A non-text attachment was scrubbed...
Name: static-inline-dogetchar.patch
Type: text/x-patch
Size: 4885 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/poppler/attachments/20150105/d0db4e0c/attachment.bin>