<html>
<head>
<base href="https://bugs.freedesktop.org/" />
</head>
<body>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW --- - [TAGGEDPDF] Parse the Tagged-PDF document structure tree when present"
href="https://bugs.freedesktop.org/show_bug.cgi?id=64815#c36">Comment # 36</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW --- - [TAGGEDPDF] Parse the Tagged-PDF document structure tree when present"
href="https://bugs.freedesktop.org/show_bug.cgi?id=64815">bug 64815</a>
from <span class="vcard"><a class="email" href="mailto:carlosgc@gnome.org" title="Carlos Garcia Campos <carlosgc@gnome.org>"> <span class="fn">Carlos Garcia Campos</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=82287" name="attach_82287" title="[PATCH v6 02/10] Tagged-PDF: Implement parsing of StructTreeRoot">attachment 82287</a> <a href="attachment.cgi?id=82287&action=edit" title="[PATCH v6 02/10] Tagged-PDF: Implement parsing of StructTreeRoot">[details]</a></span> <a href='page.cgi?id=splinter.html&bug=64815&attachment=82287'>[review]</a>
[PATCH v6 02/10] Tagged-PDF: Implement parsing of StructTreeRoot
Review of <span class=""><a href="attachment.cgi?id=82287" name="attach_82287" title="[PATCH v6 02/10] Tagged-PDF: Implement parsing of StructTreeRoot">attachment 82287</a> <a href="attachment.cgi?id=82287&action=edit" title="[PATCH v6 02/10] Tagged-PDF: Implement parsing of StructTreeRoot">[details]</a></span> <a href='page.cgi?id=splinter.html&bug=64815&attachment=82287'>[review]</a>:
-----------------------------------------------------------------
If we simplify this patch removing the tagged pdf bits and role/class maps that
are currently unused maybe you can merge this with the following one avoiding
the dummy class.
::: poppler/StructTreeRoot.cc
@@ +43,5 @@
<span class="quote">> + // The RoleMap/ClassMap dictionaries are needed by all the parsing
> + // functions, which will resolve the custom names to canonical
> + // standard names.
> + root->lookup("RoleMap", &roleMap);
> + root->lookup("ClassMap", &classMap);</span >
Since these are actually unused in this patch, we can probably add them in the
first patch that actually uses them, since they are optional entries in the
end.
@@ +103,5 @@
<span class="quote">> + }
> + obj.free();
> +
> + // Parse the children StructElements
> + const GBool marked = doc->getCatalog()->getMarkInfo() & Catalog::markInfoMarked;</span >
As I commented previously I would focus first on logical structure (Section
14.7) only and then add the tagged pdf support (Section 14.8) on top of that.
That would leave this patch even simpler and maybe you can even merge this and
the following one instead of using a dummy class here just to make it build.
@@ +111,5 @@
<span class="quote">> + error(errSyntaxWarning, -1, "K in StructTreeRoot has more than one children in a tagged PDF");
> + }
> + for (int i = 0; i < kids.arrayGetLength(); i++) {
> + Object obj, ref;
> + kids.arrayGetNF(i, &ref);</span >
This reference is only used in the if below.
@@ +122,5 @@
<span class="quote">> + child->getType() == StructElement::Div)) {
> + error(errSyntaxWarning, -1, "StructTreeRoot element of tagged PDF is wrong type ({0:s})", child->getTypeName());
> + }
> + appendElement(child);
> + if (ref.isRef()) {</span >
Here you could directly do:
if (kids.arrayGetNF(i, &ref)->isRef())</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are on the CC list for the bug.</li>
</ul>
</body>
</html>