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

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

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