[poppler] Toward to JBIG2 support in CairoOutputDev
Albert Astals Cid
aacid at kde.org
Mon Jan 12 11:10:43 PST 2015
El Dilluns, 12 de gener de 2015, a les 21:44:22, Adrian Johnson va escriure:
> The latest patch looks good. I'll wait a week before committing to see
> if anyone else has any comments. This will give me time to do some
> testing of the patch.
Can you guys explain what the patch is for? Export? Or so that it's cairo the
one rendering JBig2 and not poppler? Or?
Cheers,
Albert
> On 11/01/15 23:05, suzuki toshiya wrote:
> > Dear Adrian,
> >
> > OK - considering that there is a lag to the day that Linux
> > distribution ship the poppler stable branch including
> > CairoOutputDev with JBIG2 support even if I support some
> > snapshot of Cairo in current development branch of poppler
> > (and there would be no need to support Cairo snapshot in
> > the day), I decided to follow original version checking.
> >
> > Also I removed the default value (NULL) for Ref object to
> > JBIG2Stream constructor and removed NULL pointer checking
> > in JBIG2Stream constructor (as same as the argument to pass
> > Globals object).
> >
> > I replaced "st" in CairoOutputDev, by "status".
> >
> > Here I attached revised patch - oh, I'm quite sorry, my
> > previous posts were misunderstanding this year as 2014.
> >
> > Regards,
> > mpsuzuki
> >
> > Adrian Johnson wrote:
> >> On 08/01/15 13:35, suzuki toshiya wrote:
> >>> Dear Adrian,
> >>>
> >>> Great Thank you for prompt review! I attached the revised patch,
> >>> but please let me discuss a few points.
> >>>
> >>> Adrian Johnson wrote:
> >>>> On 07/01/15 02:58, suzuki toshiya wrote:
> >>>>> Hi all,
> >>>>>
> >>>>> Here I submit a preliminary patch for git head,
> >>>>> which enables JBIG2 embedding to PDF surface via Cairo.
> >>>>
> >>>> +static cairo_status_t setJBIG2GlobalsIdByRefNums(int refNum, int
> >>>> refGen,
> >>>> + cairo_surface_t
> >>>> *image)
> >>>> +{
> >>>> + GooString *globalsStrId;
> >>>> + char *idBuffer;
> >>>> + cairo_status_t st;
> >>>> +
> >>>> +
> >>>> + globalsStrId = new GooString;
> >>>> + globalsStrId->appendf("{0:d}-{1:d}", refNum, refGen);
> >>>> + idBuffer = copyString(globalsStrId->getCString());
> >>>> + st = cairo_surface_set_mime_data (image,
> >>>> CAIRO_MIME_TYPE_JBIG2_GLOBAL_ID,
> >>>>
> >>>> The cairo JBIG2 mime types are new to 1.14.0 so will need to be
> >>>> inside a
> >>>> #if CAIRO_VERSION >= CAIRO_VERSION_ENCODE(1, 14, 0).
> >>>
> >>> If it's acceptable, I want to use the conditional like
> >>>
> >>> #ifdef CAIRO_MIME_TYPE_JBIG2_GLOBAL_ID
> >>>
> >>> The official release of cairo including JBIG2 support is 1.14.0,
> >>> however, some distributions use a snapshot before 1.14 which
> >>> supports JBIG2. For example, Ubuntu 14.04 (long time support)
> >>> ships with 1.13.0~20140204.
> >>>
> >>> http://packages.ubuntu.com/search?suite=trusty§ion=all&arch=any&keyw
> >>> ords=libcairo2&searchon=names
> >>>
> >>>
> >>> It includes JBIG2 support. Thus I prefer checking by the JBIG2
> >>> macros, instead of the version checking.
> >>
> >> I'm not keen on making an exception for this case. The target audience
> >> for this is very small. It would only help users that compile a new
> >> version of poppler on an old distribution without also recompiling
> >> cairo. JBIG2 support for pdftocairo is a very minor feature that not
> >> many people would be using (so far no one else has requested this
> >> feature).
> >>
> >> If you really want to support distributions using a cairo 1.13 snapshot
> >> I suggest adding a comment before the first occurrence of
> >> #ifdef CAIRO_MIME_TYPE_JBIG2_GLOBAL_ID to explain why the version macro
> >> is not used. Also include
> >> "#if CAIRO_VERSION >= CAIRO_VERSION_ENCODE(1, 14, 0)" in the comment so
> >> that:
> >> a) searching for CAIRO_VERSION will find it, and
> >> b) to document that JBIG2 support is in 1.14 or later.
> >>
> >>> BTW, when the string for UNIQUE_ID is constructed, its syntax
> >>> is RefGen-RefNum. In PDF data, RefNum then RefGen is popular
> >>> syntax. Is there any reason why the order is reverted?
> >>
> >> No reason. Feel free to change it to num-gen order.
> >>
> >>>> +GBool CairoOutputDev::setMimeDataForJBIG2Globals(Stream *str,
> >>>> + cairo_surface_t
> >>>> *image)
> >>>> +{
> >>>> + JBIG2Stream *jbig2Str = static_cast<JBIG2Stream *>(str);
> >>>> + Object* globalsStr = jbig2Str->getGlobalsStream();
> >>>> + char *globalsBuffer;
> >>>> + int globalsLength;
> >>>> +
> >>>> + // According to PDF spec, Globals should be stream (or nothing)
> >>>> +
> >>>> + if (globalsStr->isNull())
> >>>> + return gTrue;
> >>>> +
> >>>> + if (!globalsStr->isStream())
> >>>> + return gFalse;
> >>>> +
> >>>> + // See JBIG2Stream constructor for valid reference number
> >>>> + if (!jbig2Str->getGlobalsStreamRefNum())
> >>>> + return gFalse;
> >>>>
> >>>> The globalsStr->isStream() check should be sufficient. The reference
> >>>> number should always be valid of the global stream is valid.
> >>>
> >>> Umm. Requesting "both of JBIG2Stream::globalsStream and Ref to
> >>> it must be always valid as far as GlobalsStream is used" is
> >>> intuitive solution, but public JBIG2Stream constructor has not
> >>> requested the Ref to GlobalsStream before in earlier version.
> >>>
> >>> If some developers use JBIG2Stream constructor with valid
> >>> GlobalsStream but without Ref to it (as the syntax before my
> >>> patch), there is the situation that JBIG2Stream::globalsStream
> >>> is valid but Ref to it is invalid (if I supply the default
> >>> value for the argument to pass Ref). How do we detect or prevent
> >>> such case?
> >>>
> >>> A) Supplying the default value (NULL pointer) in the argument
> >>> to pass the Ref, and JBIG2Stream returns an error when
> >>> globalsStream is valid stream but its Ref is not passed - and
> >>> prevent the case that globalsStream is valid but its Ref is
> >>> invalid.
> >>>
> >>> B) Not supplying the default value, and prevent the construction
> >>> without the Ref to the globalsStream.
> >>>
> >>> C) Add new boolean value in JBIG2Stream class, to indicate if
> >>> Ref to globalsStream is explicitly set.
> >>>
> >>> D) Initialize Ref with irregular value (e.g. 0,0 or 0,65535)
> >>> internally when Ref is not passed, and deal the case as Ref to
> >>> the globalsStream is not initialized (my last patch design)
> >>>
> >>> Which is the best? Or, no need to case such case?
> >>
> >> JBIG2Stream is not part of the public API so there is no need to
> >> preserve compatibility.
> >>
> >> Something else I just noticed - could you rename "st" to "status".
> >> CairoOutputdev.cc always uses the name "status" for cairo status so we
> >> should be consistent with this convention.
>
> _______________________________________________
> poppler mailing list
> poppler at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/poppler
More information about the poppler
mailing list