sdext/source/pdfimport/tree/writertreevisiting.cxx | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
New commits:
commit 588e59cc36475ded243ce4fd9062473cddd2c016
Author: Kevin Suo <suokunl...@126.com>
AuthorDate: Sun Oct 23 19:10:29 2022 +0800
Commit: Kevin Suo <suokunl...@126.com>
CommitDate: Thu Nov 24 17:04:06 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>
(cherry picked from commit fc2fb95fdb4262792e94afe61b784c8ae71d171e)
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/142313
Reviewed-by: Kevin Suo <suokunl...@126.com>
diff --git a/sdext/source/pdfimport/tree/writertreevisiting.cxx
b/sdext/source/pdfimport/tree/writertreevisiting.cxx
index af16cde5a599..d3ea6cc05320 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 );
}
