sdext/source/pdfimport/tree/writertreevisiting.cxx | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
New commits:
commit 9ea9d3ccc0f8e4833e745d9655b61d42d6d8fe83
Author: Kevin Suo <suokunl...@126.com>
AuthorDate: Sun Oct 23 19:10:29 2022 +0800
Commit: Noel Grandin <noel.gran...@collabora.co.uk>
CommitDate: Sun Nov 6 12:43:07 2022 +0100
sdext.pdfimport Writer: Do not visit DrawElement twice in WriterXmlEmitter
Discovered when working on commit f6004e1c457ddab5e0c91e6159875d25130b108a.
To reproduce, you can print the content of aOutput2 above the line:
https://opengrok.libreoffice.org/xref/core/sdext/source/pdfimport/test/tests.cxx?r=f6004e1c#836,
and then you get the xml content as shown in:
https://bugs.documentfoundation.org/attachment.cgi?id=183217,
in which you can see that there are duplicated draw:frame(s) with identical
draw:z-index.
To dig into the problem, you may take a look at the following code block:
1 for( const auto& rxChild : elem.Children )
2 {
3 PageElement* pPage = dynamic_cast<PageElement*>(rxChild.get());
4 if( pPage )
5 {
6 for( auto child_it = pPage->Children.begin(); child_it !=
pPage->Children.end(); ++child_it )
7 {
8 if( dynamic_cast<DrawElement*>(child_it->get()) != nullptr
)
9 (*child_it)->visitedBy( *this, child_it );
10 }
11 }
12 }
13
14 for( auto it = elem.Children.begin(); it != elem.Children.end(); ++it )
15 {
16 if( dynamic_cast<DrawElement*>(it->get()) != nullptr )
17 (*it)->visitedBy( *this, it );
18 }
In the for loop in lines 1:12:
* "elem" is a "DocumentElement" which is derived from "Element". It's
childen are PageElement(s).
* "pPage" is a "PageElement" which is also derived from "Element". It's
childen are DrawElement(s).
* "child_it", as in the for loop in lines 6:10, is a "DrawElement" which is
derived from
"GraphicalElement", whereas "GraphicalElement" is derived from "Element".
In this block, the code goes through each of the pages and visit the
DrawElements there.
The code in lines 14:18 seems to assume that, a DrawElement, in addition to
be a child of
PageElement, may also be a child of DocumentElement. See the comment in
souce code.
Yes, it may be. For such DrawElement(s), the visiting is done in lines
14:18 separately.
Note that The above code uses dynamic_cast to determine whether a node is a
DrawElement or not.
The problem is that, in determining whether the node during the 2nd
visiting is a DrawElement,
it accidently used "== nullptr". This may be an inadvertent error. If the
dynamic_cast is a nullprt,
then it is not a DrawElement and the visiting should not be done there.
Resolve this by using "!= nullptr" in the second dynamic_cast checking.
Change-Id: I066100e98039d505d8b10c390954e014b78cff4f
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/141680
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.gran...@collabora.co.uk>
diff --git a/sdext/source/pdfimport/tree/writertreevisiting.cxx
b/sdext/source/pdfimport/tree/writertreevisiting.cxx
index 2ece5307bd53..deabf365088b 100644
--- a/sdext/source/pdfimport/tree/writertreevisiting.cxx
+++ b/sdext/source/pdfimport/tree/writertreevisiting.cxx
@@ -382,7 +382,7 @@ void WriterXmlEmitter::visit( DocumentElement& elem, const
std::list< std::uniqu
// only DrawElement types
for( auto it = elem.Children.begin(); it != elem.Children.end(); ++it )
{
- if( dynamic_cast<DrawElement*>(it->get()) == nullptr )
+ if( dynamic_cast<DrawElement*>(it->get()) != nullptr )
(*it)->visitedBy( *this, it );
}
