Path from ShapeString fails : bug in master c. sept 2019

Here's the place for discussion related to CAM/CNC and the development of the Path module.
chrisb
Posts: 20632
Joined: Tue Mar 17, 2015 9:14 am

Re: Path from ShapeString fails : bug in master c. sept 2019

Postby chrisb » Tue Nov 19, 2019 6:34 pm

Is it really the DisplayMode? I thought the filling was controlled by by the preferences setting Draft->GeneralSettings->Fill Objects with faces whenever possible. This still worked in 0.19.17681 before the Big Merge.
chrisb
Posts: 20632
Joined: Tue Mar 17, 2015 9:14 am

Re: Path from ShapeString fails : bug in master c. sept 2019

Postby chrisb » Tue Nov 19, 2019 6:36 pm

It even worked after the Big Merge in 0.19.17798 :? .
Syres
Posts: 416
Joined: Thu Aug 09, 2018 11:14 am

Re: Path from ShapeString fails : bug in master c. sept 2019

Postby Syres » Tue Nov 19, 2019 8:05 pm

chrisb wrote:
Tue Nov 19, 2019 6:34 pm
Is it really the DisplayMode? I thought the filling was controlled by by the preferences setting Draft->GeneralSettings->Fill Objects with faces whenever possible. This still worked in 0.19.17681 before the Big Merge.
Another hack I'm afraid by changing src\Mod\Draft\Draft.py approx line 544 from:

Code: Select all

            if "ShapeColor" in obrep.PropertiesList: obrep.ShapeColor = fcol
        else:
to:

Code: Select all

            if "ShapeColor" in obrep.PropertiesList: obrep.ShapeColor = fcol
            for p in obrep.listDisplayModes():
                if p == "Flat Lines":
                    obrep.DisplayMode = "Flat Lines"
                if p == "3D":
                    obrep.DisplayMode = "3D"
        else:
This fixes Shapestrings and I've tested creating circles, lines and rectangles to make sure there's no side effects on them and it appears to be stable on Windows, it would nice for any Linux and/or Mac users to try it and give feedback.

Edit: Original hack broke Dimensions
mlampert
Posts: 1441
Joined: Fri Sep 16, 2016 9:28 pm

Re: Path from ShapeString fails : bug in master c. sept 2019

Postby mlampert » Tue Nov 19, 2019 10:19 pm

The problem is that I added "orientWire" which simplifies the rest of the engraving. It seems to not support Bezier curves, so I need to add that. Definitely worth a ticket.

BTW: if you revert to the old version the feed rate will be the horizontal feed rate - if you use multiple step downs that might be a problem (IIRC that was the reason for the change).
vocx
Posts: 2378
Joined: Thu Oct 18, 2018 9:18 pm

Re: Path from ShapeString fails : bug in master c. sept 2019

Postby vocx » Tue Nov 19, 2019 10:43 pm

chrisb wrote:
Tue Nov 19, 2019 6:34 pm
Is it really the DisplayMode? I thought the filling was controlled by by the preferences setting Draft->GeneralSettings->Fill Objects with faces whenever possible. This still worked in 0.19.17681 before the Big Merge.
Draft objects have two relevant properties, one is the Data property "MakeFace", which by default is True. The other is the View property "DisplayMode", which should be "FlatLines". See Part_Feature for the properties that all basic scripted objects have.
Draft_DisplayMode.png
Draft_DisplayMode.png (47.67 KiB) Viewed 195 times
"DisplayMode" is inherited from the C++ view provider classes, while "MakeFace" is defined in each individual Draft object (rectangle, circle, wire, etc.). The default value of "MakeFace" can be set in the Parameter Editor.

The big LinkMerge wasn't responsible for changing the "DisplayMode". The changes came a bit after, in a subsequent collection of commits by realthunder, around 0.19.18351, git commit fb7a03a04.
User avatar
freman
Posts: 867
Joined: Tue Nov 27, 2018 10:30 pm

Re: Path from ShapeString fails : bug in master c. sept 2019

Postby freman » Tue Nov 19, 2019 11:54 pm

mlampert wrote:
Tue Nov 19, 2019 10:19 pm
The problem is that I added "orientWire" which simplifies the rest of the engraving. It seems to not support Bezier curves, so I need to add that. Definitely worth a ticket.

BTW: if you revert to the old version the feed rate will be the horizontal feed rate - if you use multiple step downs that might be a problem (IIRC that was the reason for the change).
Hi mlampert, could you explain what the feedrate you refer to is.

I did the following having unblocked problem with the file Syres posted here:
https://forum.freecadweb.org/viewtopic. ... 10#p348472

Code: Select all

G1 X117.473 Y-77.141 Z-0.500 F240.000
G1 X116.727 Y-77.364 Z-0.500 F240.000
G0 Z3.000
G0 X130.845 Y-68.536 Z3.000
G0 X130.845 Y-68.536 Z3.000
G1 X130.845 Y-68.536 Z-0.500 F60.000
G1 X131.309 Y-67.902 Z-0.500 F240.000
I seem to get the 1 mm/sec and 4mm/sec feedrates defined for the tool.
mlampert
Posts: 1441
Joined: Fri Sep 16, 2016 9:28 pm

Re: Path from ShapeString fails : bug in master c. sept 2019

Postby mlampert » Wed Nov 20, 2019 12:13 am

If you have a z-depth deeper than the step down the engraving happens in multiple passes. However the engraving op always applies the horizontal feed rate, so the step down between passes also happens at horizontal feed rate.

Note that the posted fix is not valid and I recommend checking the resulting g-code carefully. The algorithm relies on the edges being properly oriented with is what syres has removed.
User avatar
freman
Posts: 867
Joined: Tue Nov 27, 2018 10:30 pm

Re: Path from ShapeString fails : bug in master c. sept 2019

Postby freman » Wed Nov 20, 2019 6:57 am

thanks, that's what I thought you meant but I wanted to make sure I had not misunderstood since it did not seem to be the case.

I reworked the same job to 2.5mm in 0.5 steps and the vertical cuts seem to have the vertical speed, as is the case for the initial depth above.

Code: Select all

G1 X62.411 Y-48.000 Z-0.500 F240.000
G1 X47.608 Y-48.000 Z-0.500 F240.000
G1 X47.608 Y-48.000 Z-1.000 F60.000
G1 X47.608 Y-47.257 Z-1.000 F240.000
G1 X48.612 Y-47.162 Z-1.000 F240.000
Syres did say it was a hack but it did get me flying and got the job done. Having identified the problem hopefully a more rigorous fix can be applied.

With FreeCAD being such a complex program with many interacting parts, of which most people only use a small subset, I wonder if there is not a need for a more formal overall strategy for testing.

Many contributions seem to just affect one particular facet ( one or two workbenches ) which someone is interested in but have broader reach in other areas which go untested against the changes before they get committed. Can some sort of test strategy be defined ( if it does not already exist ) to reduce this kind of breakage?

It may be the old cat herding problem but it seems overall testing structure maybe needed with such a complex software suite. Otherwise a fix to one bug is likely to cause another oddity elsewhere and the software will degrade over time into a constant flow of unintended consequences which drain resources and make FC of decreasing not increasing quality and reliability.
vocx
Posts: 2378
Joined: Thu Oct 18, 2018 9:18 pm

Re: Path from ShapeString fails : bug in master c. sept 2019

Postby vocx » Wed Nov 20, 2019 7:38 am

freman wrote:
Wed Nov 20, 2019 6:57 am
... Can some sort of test strategy be defined ( if it does not already exist ) to reduce this kind of breakage? ...
FreeCAD includes unit tests for each workbench. See Testing. This page needs to be seriously rewritten, by the way.

Run all unit tests

Code: Select all

freecad --run-test 0
freecad -t 0
So, before committing changes to the repository, every developer should run the entire set of tests to make sure nothing is broken in another workbench. Some changes are pretty small and it's evident that they don't affect anything but a small component of the software, but others, particularly those done to the C++ code, may have wide implications everywhere.

However, the quality of this testing depends on the way the unit tests themselves are written. It's like in a manufacturing line; if you are testing cubes, you may need to test three dimensions, length, width, and height. If you only test one, you are ignoring potential problems with the other two qualities. Same in FreeCAD; unit tests need to be very exhaustive in order to catch a lot of different potential problems; however, this is difficult to do, and relatively boring. Just like writing documentation, it's a tedious task, so developers usually prefer to do something more enjoyable like writing new functions, rather than go back and describe or test older code.

I haven't taken an exhaustive look at the unit tests of the Path workbench, but these tests take a lot of time to complete; they also throw a ton of messages about a "Helix" operation or whatever. However, they always pass, so there doesn't seem to be a major problem. Definitely improving testing is something that could be done.
User avatar
freman
Posts: 867
Joined: Tue Nov 27, 2018 10:30 pm

Re: Path from ShapeString fails : bug in master c. sept 2019

Postby freman » Wed Nov 20, 2019 8:22 am

Thanks for the explanations vocx.
So to take the current breakage as convenient example, is there a mechanism in place which should have picked this up and it slipped through the net, or is it a case of there being no test included for creating a path from a Bezier ( which to my understanding is the root cause of this failure ) so the Path tests may need one or more extra tests to be included?

I've seen a list of automatic tasks and tickboxes against PRs but I can't find out how to get to this right now. can you link an example of where that can be seen.