elliotf / heekscad

Automatically exported from code.google.com/p/heekscad
0 stars 0 forks source link

Heekscad not loading sketch solver contstraints #304

Open GoogleCodeExporter opened 8 years ago

GoogleCodeExporter commented 8 years ago
What steps will reproduce the problem?
1.Draw a rectangle using the sketcher
2.Set Length, Vertical and Horizontal Contrainst
3. Save,Exit Heekscad and reopen file. Length Verical contrainst are not loaded.

What is the expected output? 
Same thing as before save
What do you see instead?
Rectangle without constraints.

Please use labels and text to provide additional information.

Original issue reported on code.google.com by jonastho...@gmail.com on 10 Sep 2010 at 12:10

GoogleCodeExporter commented 8 years ago
Tests:
1) Rectangle with with 4 CoincidentPointConstraints  and save as 
“test_1_4CoincidentPointConstraints.heeks”

2) With the same file open add a vertical and horizontal point constraint and 
save as “test_2_4CoincidentPointConstraints_2AbsoluteAngleConstraints.heeks”

3) Exit heekscad and restart. Open 
“test_2_4CoincidentPointConstraints_2AbsoluteAngleConstraints.heeks”  and 
save into 
“test_3_4CoincidentPointConstraints_2AbsoluteAngleConstraints_resaved.heeks”

4)Exit heekscad and restart. Open 
“test_1_4CoincidentPointConstraints.heeks” and save into 
“test_4_4CoincidentPointConstraints_resaved.heeks”

Observations:

(Test 1) It appears that test_1_4CoincidentPointConstraints.heeks has saved 
constraints correctly
(Test 2) Normal function occurs when “AbsoluteAngleConstraint” is added to 
a open file and saved.
(Test 3) The file with 4 AbsoluteAngleConstraint and 4 
CoincidantPointConstraint are not read in properly. It appears that the 
AbsoluteAngleConstraint have not been read in properly.
I’m observing what appears to be a read file and a write file error.  When a 
file is opened, observed on the screen it appears that the 
AbsoluteAngleConstraint have been lost but the CoincidantPointConstraint have 
been read. What is exceeding curious when this file is save 3 of the 4 
CoincidantPointConstraint are lost. My theory that on a fresh drawing 
everything everything saves correctly but things get weird with and existing 
drawing. It appears that there is a read bug and a write bug saving constraints.

(Test 4) In this test a open 4 lines constrained by the endpoints only. On the 
screen it appears that everything reads in properly. The endpoints appear 
constrained properly.  When this datafile is saved 3 of the 4 point constraints 
are lost.

Hypothesis:
It seems as if normal function occurs on new data drawing.  It appears that 
data is saving correctly at this point.
It appears that there are read and write errrors related to constraints on an 
existing drawing.

I could be off on this but I looks as there are two error paths:
AbsoluteAngleConstraints are not being read
A counter error? is occuring when data comes from a existing file not all is 
being read back.

Original comment by jonastho...@gmail.com on 11 Sep 2010 at 2:04

Attachments:

GoogleCodeExporter commented 8 years ago
Thanks for looking at this. I don't think I will be much help with it, though.
It was written by Jon Pry and he doesn't seem to have been around for quite a 
few months. He was going to live in Grenada, last I heard. His gmail account 
was hacked into, so don't trust emails that seem to be from him asking for 
money!
I was getting an error myself, today, with saving constraints, after I did 
"Combine Sketches" on a couple of sketches and tried to save the drawing.
Dan Heeks.

Original comment by danhe...@gmail.com on 11 Sep 2010 at 4:22

GoogleCodeExporter commented 8 years ago
How expensive is it to live in Grenada. Perhaps it's Jon impersonating 
himself... 
Anyway... I started to single stepping through the file trying to make sense of 
what's going on here...
It struck me as weird that a coincident point would have a angle and a length 
defined.
Also I manually stripped out my test 2 file with what I perceived as info that 
wasn't required just to see what would happen.  It seems to make sense to me 
after all, For AbsoluteAngleConstraint you only have one object defined in the 
constraint.
I got a general segmentation fault.   

Original comment by jonastho...@gmail.com on 11 Sep 2010 at 9:44

Attachments:

GoogleCodeExporter commented 8 years ago
Dan,  
Since the sketcher works setting the constraints, I tried making the file read 
look like the sketcher.
My experiments failed, but I thought perhaps this might spur some ideas.
Anyway... Hear's what I tried to do.
http://www.metalshaperman.com/?p=332

Original comment by jonastho...@gmail.com on 12 Sep 2010 at 12:47

GoogleCodeExporter commented 8 years ago
Jon Pry on the IRC had mentioned that id's where not correct in the file.
I manually fixed that (test_6).. It didn't seem to make a difference loading 
that up.  

He also said that ">iirc absoluteangleconstraints are children of the line" (I 
edited the file (test_7) based on what he said to see if that would load. I get 
a segmentation fault when the file is opened up.

Original comment by jonastho...@gmail.com on 12 Sep 2010 at 7:11

Attachments:

GoogleCodeExporter commented 8 years ago
Both of these generated segmentation faults

Original comment by jonastho...@gmail.com on 12 Sep 2010 at 7:54

Attachments:

GoogleCodeExporter commented 8 years ago
Dan,
 Jon suggested that I // the following lines (919-922 in HeeksCAD.cpp) the problems goes away:
// where operations are pointing to the same sketch, for example, make sure 
that they are not duplicated sketches
for (std::list::iterator itObject = objects.begin(); itObject != objects.end(); 
itObject++)
{
*itObject = MergeCommonObjects( unique_set, *itObject );
} 

Well... it gotten better. It seems that I can set/save horizontal/vertical 
constraints now within heekscad but the thing still crashes if I try save a 
CoincidantPointConstraint that I have created within heekscad.  It seems if I 
hardcode the .heeks file from gedit I can get it to load.  So the bugs appear 
to be in the saving process.

At this point, I'm pursuing my problem by hard-coding my constraints in.  Jon 
had created a utility function to display the Id's of the individual objects 
(http://code.google.com/p/heekscad/source/detail?r=1286 ) but he didn't connect 
it up to anything. (I'm trying to figure out how to do that, any suggestions 
would be appreciated)

I not sure why Jon didn't commit this fix.  He sent out an email:
"I've traced the problem back to mergecommonobjects(). I'm not sure what this 
function is supposed to do. Repair bad files? Well, for whatever reason it is 
toasting the object layout. Which eventually leads to constraints not being 
able to be applied to the tree. Commenting it out solves the problem. "

Apparently, one one ever got back to him on this. I did a little googling and 
it seems this bug may have been induced at this point. 
http://code.google.com/p/heekscad/source/detail?r=1218 and 
http://code.google.com/p/heekscad/source/detail?r=1220

I'm cross posting this issue in R1218 and R1220

Original comment by jonastho...@gmail.com on 18 Sep 2010 at 2:08

GoogleCodeExporter commented 8 years ago
Most problems with constraints saving seems to have stopped with R1289.  I"m 
still having a problem with Toggle Fixed constraint.  A sketch with this 
contraint crashes when your trying to save.  Still trying to isolate what's 
causing that.

Original comment by jonastho...@gmail.com on 21 Sep 2010 at 2:12

GoogleCodeExporter commented 8 years ago
The MergeCommonObjects routine looks for duplicate child objects in the tree of 
objects being read in. It was added when we added the ability to have sketch 
objects become children of machine operation objects. i.e. A single sketch 
could be a child of two or more machine operation objects. When we read these 
machine operation objects back in we want all the machine operation objects to 
point to the same (shared) sketch objects. The decision as to whether an object 
is merged with a pre-existing object is made based on the type/Id pair

Original comment by David.Ni...@gmail.com on 22 Sep 2010 at 12:34

GoogleCodeExporter commented 8 years ago
Can you have another look at this one.  I'm hoping that my change to the 
Constraints.cpp file fixed the original problem and that my re-introduction of 
the MergeCommonObjects() call has not reverted other problems.

Original comment by David.Ni...@gmail.com on 29 Sep 2010 at 12:34

GoogleCodeExporter commented 8 years ago
R1293 re-introduces problems with constraints.  I tested on a previously 
functioning file.  It appears that coincident points constraints loaded, but it 
appears that all the horizontal/vertical constraints did not (expect for one)  
I was going to post the file, but some it got saved (not sure how that 
happened) and I think the constraints are damaged.

Original comment by jonastho...@gmail.com on 30 Sep 2010 at 12:33

GoogleCodeExporter commented 8 years ago
I tried it with a new file and it worked alright for me.  I used horozontal, 
vertical, perpendicular and coincident constraints.  Of the two shapes in the 
file, the top one is what I drew manually.  The bottom one has had the 
constraints added.  If it reads in correctly this will be obvious but if it 
doesn't I thought it might need some explanation.

Can you post your damaged file anyway?  I am curious to look at the data.

Original comment by David.Ni...@gmail.com on 30 Sep 2010 at 3:45

Attachments:

GoogleCodeExporter commented 8 years ago
Attached it the damaged file per your request.
With R1289 I was getting consistent results with what you see posted here. 
http://www.metalshaperman.com/?p=343

As a control, I loaded up your file and took a screen shot here.
http://www.metalshaperman.com/?p=396

As you can tell, I'm slowly trying to figure this stuff out.
(I haven't test this yet, prior to R1293 I was getting a crash when I was 
trying to save a fixed point constraint.  Are you getting that also... My 
suspicion is that it's a different critter at work.

If it would be a help, I suppose I can figure out how to roll back to R1289 
re-create the contraints, save and repost.

Let me know,
 JT.

Original comment by jonastho...@gmail.com on 1 Oct 2010 at 1:57

Attachments:

GoogleCodeExporter commented 8 years ago
I am starting to agree that the MergeCommonObjects() routine is the flaw in the 
constraints stuff right now but I am eager to resolve it without removing the 
call altogether.  We need the MergeCommonObjects() routine if we're going to 
share common sketch objects with more than one machining operation object.

When an object is read into memory via the ReadFromXMLElement() methods, it 
creates a new copy of the object in memory.  eg: 'CFixture *new_object = new 
CFixture(.....)'.  This is fine if the XML in the data file only refers to this 
object (type/id pair) once.  If this object is a child of two machine objects 
then the ReadFromXMLElement() routine will be called once for each child 
resulting in two separate copies of the object in memory.  This means that, 
where the XML data had two parents both referring to a single child, we now 
have separate children for each parent.  If we modify one child, the other one 
doesn't change.  It also means that we have two HeeksObj pointers assigned to 
the same ID.

The MergeCommonObjects() routine takes a tree of HeeksObj pointers and keeps a 
list of pointers to object it has seen before.  If a duplicate child is found 
(by type/id pair) then the previously seen pointer is used and the duplicate 
pointer is removed.

The constraints are held within a std::vector<HeeksObj *>.  The 
MergeCommonObjects() routine does not do this pointer removal process for these 
lists.  Further, as a constraint object is being read into memory, it is 
looking up object pointer references from the global pointer cache 
(GetIDObject()).  This pointer is not being repaired if a duplicate pointer is 
returned form the global cache.

I think the fix is to update the various constraint pointer lists when a 
duplicate child is detected.  I'm just not sure how to do this yet.

The only other option is to revert the way we store graphical objects as 
children of machine operation objects.  We used to store references to 
graphical objects by retaining the type/id pairs but I changed this to pointers 
to children instead (i.e. I changed the COp class to be based on ObjList rather 
than on HeeksObj).

Sorry to muddy the waters but I think this is where the problem lays.  I just 
haven't come up with a solution to it yet.

Original comment by David.Ni...@gmail.com on 2 Oct 2010 at 3:21

GoogleCodeExporter commented 8 years ago
I should also have added;

In my ideal world, we would have very clear 'roots' of all HeeksObj pointer 
trees.  At the moment we keep a global cache of type/id pairs and their 
corresponding HeeksObj pointers.  i.e. we have one global tree of elements.  
This works well if we're just drawing but, if we are importing 'extra' elements 
from another HeeksCAD data file then there is confusion between the type/id 
pairs already in memory and those that are currently being imported.  This is 
where the MergeCommonObjects() routine was born.  I was trying to detect and 
remove duplicate HeeksObj pointers from within the tree of files being read as 
opposed to those that are already in memory.

All this would have been easier if we added a static pointer to the root of a 
list of HeeksObj pointers.  By default they would inherit from whatever parent 
created them.  It would also, however, allow a separate set of objects to be 
held aside from the main set during an import.  When they were merged with the 
main list, their id values could be adjusted accordingly.  It also means that 
references to other objects (as is done for both constraints and children of 
machine operations), we could look in the 'local' tree for HeeksObj pointer 
references rather than a single global tree.

Again, I'm just 'typing out loud' here.

Original comment by David.Ni...@gmail.com on 2 Oct 2010 at 3:28

GoogleCodeExporter commented 8 years ago
Ok... I disabled the call to mergecommonobjects so I was able to save my 
ultimate contraints file.  This basically would be a pop-up box mounted 
diagonally on the fold line of a card.
I have 3 horizontal lines that are meant to be streched to resize the box.
the 4'th is fixed length to have a consistent.  
I needed a 45 degree angle, that's what the triangle is all about.

If mergecommonobjects is enabled, it totally hoses up the constraints.

If setfixedpoint is set.. It works as long as I don't save.  If I save it 
crashes heeks.

@ David on the fixed points ???
@ JonPry  and you look at that setfixedpoint constraint.

Original comment by jonastho...@gmail.com on 2 Oct 2010 at 4:26

Attachments:

GoogleCodeExporter commented 8 years ago
The prior file may have been damaged. I had inadvertently dragged a line to 
point and then deleted it. The suspicion is that undo/redo is causing some 
problems.

The following file was generated with: 
mergecommonobjects disabled 
//#define USE_UNDO_ENGINE commented out

So far as I can tell it save,loads and performs as intended.   This test a lot 
of constraints but not all of them. This project didn't utilize any arcs or arc 
constraints.

Original comment by jonastho...@gmail.com on 3 Oct 2010 at 1:16

Attachments:

GoogleCodeExporter commented 8 years ago
Ok... I should also mention to document there was a secondary issue with 
setfixedpointcontraint.

Jonpry snuffed that critter out with R1303 AngleBoxAllContraints was the first 
time that I was able to save/load a fixed point constraint.

Thanks, 
Jon

Original comment by jonastho...@gmail.com on 3 Oct 2010 at 1:26

GoogleCodeExporter commented 8 years ago
Jonas,
  I forgot to add a comment to this issue.  Can you fetch the source and give this a go now?  If you're reading a file into Heeks it should work alright now.  If you're importing a file into an existing set of data then we're still out of luck.
  Ta
  David Nicholls

Original comment by David.Ni...@gmail.com on 5 Oct 2010 at 12:59