Skip to content

Commit 416bd6c

Browse files
glpugafacontidavide
glpuga
authored and
facontidavide
committed
Stop using QDomDocument::toString to avoid QHash random reordering (#46)
1 parent 1a90233 commit 416bd6c

File tree

6 files changed

+259
-3
lines changed

6 files changed

+259
-3
lines changed

‎bt_editor/mainwindow.cpp‎

Lines changed: 101 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#include <QSettings>
66
#include <QTextStream>
77
#include <QList>
8+
#include <QMap>
89
#include <QMessageBox>
910
#include <QFileDialog>
1011
#include <QMenu>
@@ -14,6 +15,7 @@
1415
#include <QSvgWidget>
1516
#include <QShortcut>
1617
#include <QTabBar>
18+
#include <QXmlStreamWriter>
1719
#include <QDesktopServices>
1820
#include <QInputDialog>
1921
#include <nodes/Node>
@@ -466,7 +468,6 @@ QString MainWindow::saveToXML() const
466468
continue;
467469
}
468470

469-
470471
QDomElement node = doc.createElement( QString::fromStdString(toStr(model.type)) );
471472

472473
if( !node.isNull() )
@@ -487,7 +488,105 @@ QString MainWindow::saveToXML() const
487488
root.appendChild(root_models);
488489
root.appendChild( doc.createComment(COMMENT_SEPARATOR) );
489490

490-
return doc.toString(4);
491+
return xmlDocumentToString(doc);
492+
}
493+
494+
QString MainWindow::xmlDocumentToString(const QDomDocument &document) const
495+
{
496+
QString output_string;
497+
498+
QXmlStreamWriter stream(&output_string);
499+
500+
stream.setAutoFormatting(true);
501+
stream.setAutoFormattingIndent(4);
502+
503+
stream.writeStartDocument();
504+
505+
auto root_element = document.documentElement();
506+
507+
stream.writeStartElement(root_element.tagName());
508+
509+
streamElementAttributes(stream, root_element);
510+
511+
auto next_node = root_element.firstChild();
512+
513+
while ( !next_node.isNull() )
514+
{
515+
recursivelySaveNodeCanonically(stream, next_node);
516+
517+
if ( stream.hasError() )
518+
{
519+
break;
520+
}
521+
next_node = next_node.nextSibling();
522+
}
523+
524+
stream.writeEndElement();
525+
stream.writeEndDocument();
526+
527+
return output_string;
528+
}
529+
530+
void MainWindow::streamElementAttributes(QXmlStreamWriter &stream, const QDomElement &element) const
531+
{
532+
if (element.hasAttributes())
533+
{
534+
QMap<QString, QString> attributes;
535+
const QDomNamedNodeMap attributes_map = element.attributes();
536+
537+
for (int i = 0; i < attributes_map.count(); ++i)
538+
{
539+
auto attribute = attributes_map.item(i);
540+
attributes.insert(attribute.nodeName(), attribute.nodeValue());
541+
}
542+
543+
auto i = attributes.constBegin();
544+
while (i != attributes.constEnd())
545+
{
546+
stream.writeAttribute(i.key(), i.value());
547+
++i;
548+
}
549+
}
550+
}
551+
552+
void MainWindow::recursivelySaveNodeCanonically(QXmlStreamWriter &stream, const QDomNode &parent_node) const
553+
{
554+
if ( stream.hasError() )
555+
{
556+
return;
557+
}
558+
559+
if ( parent_node.isElement() )
560+
{
561+
const QDomElement parent_element = parent_node.toElement();
562+
563+
if ( !parent_element.isNull() )
564+
{
565+
stream.writeStartElement(parent_element.tagName());
566+
567+
streamElementAttributes(stream, parent_element);
568+
569+
if (parent_element.hasChildNodes())
570+
{
571+
auto child = parent_element.firstChild();
572+
while ( !child.isNull() )
573+
{
574+
recursivelySaveNodeCanonically(stream, child);
575+
child = child.nextSibling();
576+
}
577+
}
578+
579+
stream.writeEndElement();
580+
}
581+
}
582+
else if (parent_node.isComment())
583+
{
584+
stream.writeComment(parent_node.nodeValue());
585+
}
586+
else if (parent_node.isText())
587+
{
588+
stream.writeCharacters(parent_node.nodeValue());
589+
}
491590
}
492591

493592
void MainWindow::on_actionSave_triggered()

‎bt_editor/mainwindow.h‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,13 @@ private slots:
141141
void refreshNodesLayout(QtNodes::PortLayout new_layout);
142142

143143
void refreshExpandedSubtrees();
144-
144+
145+
void streamElementAttributes(QXmlStreamWriter &stream, const QDomElement &element) const;
146+
147+
QString xmlDocumentToString(const QDomDocument &document) const;
148+
149+
void recursivelySaveNodeCanonically(QXmlStreamWriter &stream, const QDomNode &parent_node) const;
150+
145151
struct SavedState
146152
{
147153
QString main_tree;

‎test/editor_test.cpp‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ private slots:
1717
void renameTabs();
1818
void loadFile();
1919
void loadFailed();
20+
void savedFileSameAsOriginal();
2021
void undoRedo();
2122
void testSubtree();
2223
void modifyCustomModel();
@@ -130,6 +131,68 @@ void EditorTest::loadFile()
130131
sleepAndRefresh( 500 );
131132
}
132133

134+
void EditorTest::savedFileSameAsOriginal()
135+
{
136+
QString file_xml = readFile(":/test_xml_key_reordering_issue.xml");
137+
main_win->on_actionClear_triggered();
138+
main_win->loadFromXML( file_xml );
139+
140+
QString saved_xml = main_win->saveToXML();
141+
142+
QFile qFile("crossdoor_EditorTest_savedFileSameAsOriginal.xml");
143+
if (qFile.open(QIODevice::WriteOnly))
144+
{
145+
QTextStream out(&qFile);
146+
out << saved_xml;
147+
qFile.close();
148+
}
149+
150+
sleepAndRefresh( 500 );
151+
//-------------------------------
152+
// Compare AbsBehaviorTree
153+
154+
main_win->on_actionClear_triggered();
155+
main_win->loadFromXML( file_xml );
156+
auto tree_A1 = getAbstractTree("BehaviorTree");
157+
auto tree_A2 = getAbstractTree("RunPlannerSubtree");
158+
auto tree_A3 = getAbstractTree("ExecutePath");
159+
160+
main_win->loadFromXML( saved_xml );
161+
auto tree_B1 = getAbstractTree("BehaviorTree");
162+
auto tree_B2 = getAbstractTree("RunPlannerSubtree");
163+
auto tree_B3 = getAbstractTree("ExecutePath");
164+
165+
bool same_maintree = (tree_A1 == tree_B1);
166+
if( !same_maintree )
167+
{
168+
tree_A1.debugPrint();
169+
tree_B1.debugPrint();
170+
}
171+
QVERIFY2( same_maintree, "AbsBehaviorTree comparison fails" );
172+
173+
bool same_runplanner = tree_A2 == tree_B2;
174+
if( !same_runplanner )
175+
{
176+
tree_A2.debugPrint();
177+
tree_B2.debugPrint();
178+
}
179+
QVERIFY2( same_runplanner, "AbsBehaviorTree comparison fails" );
180+
181+
bool same_executepath = tree_A2 == tree_B2;
182+
if( !same_executepath )
183+
{
184+
tree_A3.debugPrint();
185+
tree_B3.debugPrint();
186+
}
187+
QVERIFY2( same_executepath, "AbsBehaviorTree comparison fails" );
188+
189+
//-------------------------------
190+
// Compare original and save file contents
191+
192+
QVERIFY2( file_xml.simplified() == saved_xml.simplified(),
193+
"Loaded and saved XML are not the same" );
194+
}
195+
133196
void EditorTest::loadFailed()
134197
{
135198
QString file_xml = readFile(":/crossdoor_with_subtree.xml");

‎test_data/crossdoor_with_subtree.xml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
<?xml version="1.0"?>
12
<root main_tree_to_execute="MainTree">
23
<!-- ////////// -->
34
<BehaviorTree ID="DoorClosed">

‎test_data/test_files.qrc‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
<RCC>
22
<qresource prefix="/">
33
<file>crossdoor_with_subtree.xml</file>
4+
<file>test_xml_key_reordering_issue.xml</file>
45
<file>crossdoor_palette.xml</file>
56
<file>show_all.xml</file>
67
<file>crossdoor_trace.fbl</file>

‎test_data/test_xml_key_reordering_issue.xml‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
<?xml version="1.0"?>
2+
<root main_tree_to_execute="BehaviorTree">
3+
<!-- ////////// -->
4+
<BehaviorTree ID="BehaviorTree">
5+
<Sequence>
6+
<Action ID="ReceiveTargetPose" target_pose="{target_pose}"/>
7+
<Action ID="LoadConstraints" constrain_a="{constraint_a}" constraint_b="{constraint_b}" constraint_c="{constraint_c}" file="&quot;configuration.yaml&quot;"/>
8+
<SubTree ID="RunPlannerSubtree" constraint_a="constraint_a" constraint_b="constraint_b" constraint_c="constraint_c" path="path" target_pose="target_pose"/>
9+
<SubTree ID="ExecutePath" path="path"/>
10+
</Sequence>
11+
</BehaviorTree>
12+
<!-- ////////// -->
13+
<BehaviorTree ID="ExecutePath">
14+
<Fallback>
15+
<Sequence>
16+
<Condition ID="OnAir"/>
17+
<Action ID="Fly" path="{path}"/>
18+
</Sequence>
19+
<Sequence>
20+
<Condition ID="OnGround"/>
21+
<Action ID="Roll" path="{path}"/>
22+
</Sequence>
23+
<Sequence>
24+
<Condition ID="OnWater"/>
25+
<Action ID="Swim" path="{path}"/>
26+
</Sequence>
27+
</Fallback>
28+
</BehaviorTree>
29+
<!-- ////////// -->
30+
<BehaviorTree ID="RunPlannerSubtree">
31+
<Fallback>
32+
<Action ID="RunPlannerA" constraint_a="{constraint_a}" constraint_b="{constraint_b}" contratint_c="{constraint_c}" path="{path}" target_pose="{target_pose}"/>
33+
<Action ID="RunPlannerB" constraint_a="{constraint_a}" constraint_b="{constraint_b}" constraint_c="{constraint_c}" path="{path}" target_pose="{target_pose}"/>
34+
</Fallback>
35+
</BehaviorTree>
36+
<!-- ////////// -->
37+
<TreeNodesModel>
38+
<SubTree ID="ExecutePath">
39+
<input_port name="path"/>
40+
</SubTree>
41+
<Action ID="Fly">
42+
<input_port name="path"/>
43+
</Action>
44+
<Action ID="LoadConstraints">
45+
<output_port name="constrain_a"/>
46+
<output_port name="constraint_b"/>
47+
<output_port name="constraint_c"/>
48+
<input_port name="file"/>
49+
</Action>
50+
<Condition ID="OnAir"/>
51+
<Condition ID="OnGround"/>
52+
<Condition ID="OnWater"/>
53+
<Action ID="ReceiveTargetPose">
54+
<output_port name="target_pose"/>
55+
</Action>
56+
<Action ID="Roll">
57+
<input_port name="path"/>
58+
</Action>
59+
<Action ID="RunPlannerA">
60+
<input_port name="constraint_a"/>
61+
<input_port name="constraint_b"/>
62+
<input_port name="contratint_c"/>
63+
<output_port name="path"/>
64+
<input_port name="target_pose"/>
65+
</Action>
66+
<Action ID="RunPlannerB">
67+
<input_port name="constraint_a"/>
68+
<input_port name="constraint_b"/>
69+
<input_port name="constraint_c"/>
70+
<output_port name="path"/>
71+
<input_port name="target_pose"/>
72+
</Action>
73+
<SubTree ID="RunPlannerSubtree">
74+
<input_port name="constraint_a"/>
75+
<input_port name="constraint_b"/>
76+
<input_port name="constraint_c"/>
77+
<output_port name="path"/>
78+
<input_port name="target_pose"/>
79+
</SubTree>
80+
<Action ID="Swim">
81+
<input_port name="path"/>
82+
</Action>
83+
</TreeNodesModel>
84+
<!-- ////////// -->
85+
</root>
86+

0 commit comments

Comments
 (0)