adrianinsaval
commented
1 month ago probably should do a proper bisect to find out if it really was introduced in #16540
Closed adrianinsaval closed 1 month ago
adrianinsaval
commented
1 month ago probably should do a proper bisect to find out if it really was introduced in #16540
luzpaz
commented
1 month ago cc @bgbsww
adrianinsaval
commented
1 month ago Also, like I said in RC1 the model has errors on a recompute (I haven't looked deeper into it or tried to fix them), should that be considered a regression? if so, is that also a blocker or an acceptable side effect of toponaming?
bgbsww
commented
1 month ago We are getting a Sigabrt from this assertion:
void SketchObject::rebuildExternalGeometry(bool defining, bool addIntersection)
{
Base::StateLocker lock(managedoperation, true); // no need to check input data validity as this is an sketchobject managed operation.
// get the actual lists of the externals
auto Objects = ExternalGeometry.getValues();
auto SubElements = ExternalGeometry.getSubValues();
assert(externalGeoRef.size() == Objects.size());with 0 externalGeoRef and 1 Object during the recompute of the "/home/brad/x5/tourniquet-master/injection_molding/Glia Tourniquet Inj-Mold Design (FreeCAD)/sliderAssy.FCStd" Document.
Opening just that document and attempting an on load recompute also crashes. However, opening it without the recompute and then recomputing succeeds.
Opening the clip and parameters documents first and the opening the sliderAssy still crashes.
The quick facile way to solve this would likely to be to modify this code:
To only act on the single document loaded, instead of trying to do all the others. That may have been a mistake to bring in.
bgbsww
commented
1 month ago Also, like I said in RC1 the model has errors on a recompute (I haven't looked deeper into it or tried to fix them), should that be considered a regression? if so, is that also a blocker or an acceptable side effect of toponaming?
Without investigation, it's tough to tell. When I look, I see 2 RuledSurface errors, each of which involves two binders. both inside the A and B plates document. Those are execute() giving up, and it isn't immediately clear that that is toponaming, although it may be.
adrianinsaval
commented
1 month ago To only act on the single document loaded, instead of trying to do all the others. That may have been a mistake to bring in.
if you do this, what should be the workflow for an user that wants to port his files to 1.0? Asking as user, this is way above my head so I won't pretend to know what's the best course of action
bgbsww
commented
1 month ago Yeah, that's exactly why I'm hesitating, investigating and hoping for maybe input from others. I'll try to answer that question in a bit, I'm still looking at pinning down the RuledSurface thing so we know where to send it.
bgbsww
commented
1 month ago RuledSurface recompute fail is https://github.com/bgbsww/FreeCAD/blob/914a7616f7490973434bb086b76d3f730b6a905f/src/Mod/Part/App/TopoShapeExpansion.cpp#L2164-L2167 and that restriction did not exist in the original non-element map code; maybe https://github.com/FreeCAD/FreeCAD/pull/12431 is the moment we went south?
So I think that recompute issue is a separate, regression bug specific to ruled surfaces ( there may be others ). That should get filed.
I'm on to the overall recompute on load question now ...
bgbsww
commented
1 month ago ... And the answer is that you will get the recompute dialog multiple times. Specifically, once for every sub document that is loaded, which could be a lot and very annoying to the user. A better solution may be required here.
bgbsww
commented
1 month ago Another recompute issue: empty Pads are now showing errors, and they used to be allowed apparently. Do we need to match this behavior (regression)? See tourniquet-master/injection_molding/Inj. Molding Part Models (FreeCAD)/buckle.FCStd for an example.
adrianinsaval
commented
1 month ago ... And the answer is that you will get the recompute dialog multiple times. Specifically, once for every sub document that is loaded, which could be a lot and very annoying to the user. A better solution may be required here.
"yes to all" button?
bgbsww
commented
1 month ago Implementation of same is turning out to be non trivial. You can't do it in the scope of just the signal from the top level document load, you need to wait until all the documents are in and then do it, or else you risk cross document references breaking, like in this case. I'm working on it though.
I was just going to assume that if you answer Yes once to the already plural description, we should do all subdocuments. That's what LS3 does, and if you only want to recompute the top level file, you can 'No' and then do that yourself.
bgbsww
commented
1 month ago Ruled Surface issue is found. An issue with Slices has surfaced that I'm looking for, and then Pads.
Good test document set this is.
bgbsww
commented
1 month ago Okay, the Pad issue seems to have gone away after fixing the Ruled Surface one.
There is a remaining error showing up in Slice_child3 and Slice_child4 - Compound Filters of a Slice that I can't understand enough to try to address. I'm not sure if there's a modeling problem here or a a code one.
@adrianinsaval any clues you can give?
adrianinsaval
commented
1 month ago @bgbsww sorry I'm just now getting back into this, from an initial look, it seems the slices aren't the actual problem, the problem is that Fusion AKA shot + slide in file A and B Plates isn't actually fusing it's two components. Some of the slice compound filters are then expected to fail since some slices will be missing
this is shot + slide in 0.21.2:
and in 1.1.0.38834:
all of the solid representing the plastic shot is missing. There might be other regressions lurking but that's the most most obvious one that jumped up to me.
bgbsww
commented
1 month ago Thanks! It all starts to make more sense. Turning off Refine on Shot appears to make the fusion work. Still an issue with the slice of the buckle that remains though.
We have strayed pretty far from the original bug now - we're trying to track down which changes are causing regressions and not the recompute dialog. The Buckle has got a DatumPlane and Pad, Fillet, Mirrored, Binder, Boolean ... lots of potential places to be wrong.
I'm starting to wonder if this is the justification for #15741 that's been missing all this time. Need to prove that though.
Got Check Geometry errors in the Cone inside the sprue & runners of the Shot - you can just go to this subfile. That's the cause of the first slice issue, I think.
~Hasher Mismatch errors coming from MultiFuse::execute - pointer to something wrong there.~
The buckle is still confounding me, I can't find what's wrong with it enough to know where to look. It seems fine, but the slice is out of range.
Cone is pretty simple, hard to see a problem there. But as the last step in a Body? Maybe it's the AdditivePipes preceding it or the binders.
Okay, I'm fairly sure the cone problem is coming from the combination of a Part object inside a Part Design body triggering some subtlety. The AdditivePipe code is substantially different after the TNP merge ( and has diverged from LS3 ) so I strongly suspect something there is setting up a Shape that then causes issues later, including somehow messing up the very simple Cone. Alternatively, it's about the different ShapeBinder code.
I note that the first AdditivePipe is recomputing too long and interfering with Master Gate001 and the Windlass I don't know why. It is driven by a horizontal constraint using the WindlassPostition in an expression.
A few hacks to try to throw in some shape fixes didn't resolve the issues.
I could use some help in extracting smaller test cases and understanding which operations are misbehaving here.
bgbsww
commented
1 month ago A test case with crossed AdditivePipes with Cones on the ends of them works perfectly.
bgbsww
commented
1 month ago Oh! It is recompute order dependent. If you don't accept the overall recompute on opening shot, and then ~recompute Runner system first, everything is fine.~ If you recompute Binder or Shot first, there are problems.
And the naming fooled me - that's a PD Cone and not a Part Cone.
No that happened once. So not repeatable. But that was using revert to disk. And the sub files were still open.
Fresh session. Turn off visibility of Shot and Runner System. Make just "sprue and runners" visible. turn off visibility, Recompute "sprue and runners", turn on visibility of Runner System and off the sprue and runners. Recompute each of the MasterGates. then "Runner System". Now everything is good. But open Sketch and close it and everything is broken again.
Recompute of Runner system continues to mess up the length and end of sprue going to Windlass. That seems to be in Sketch, where the construction geo 9-Line is controlled by Constraint27 which is an expression driven by <
But ultimately this comes down to the calculation of the Cone into sprue & runners - Body011.
If you change the attachment of the Cone from Deactivated to XY on plane using XY_Plane012 to allow an attachment offset, and then change the attachment offset z to 0.1 then everything starts working, so this is really about the computation of the AdditiveShape AdditiveShape001 and Cone coming together.
Let me see if I can successfully add this to the test... Well, the recompute of the cone fails if the radius if it interferes with the ends of the pipes, so that's a start.
bgbsww
commented
1 month ago But fundamentally no, that test won't break. However, on thinking, the Sprue and runners do recalculate just fine, it's the Runner System where it starts to go wrong, and the runner that goes wrong uses Master Gate, which is the only gate that is attached to the end of the additive pipes. So does the combination of the two cause the rendering failure on the segment of additive pipe?
bgbsww
commented
1 month ago Okay, I duplicated it. Not yet sure how to fix it. The key here is that both pipes must have their "seam" from the closure point of the circle on the "side" of the pipe and then the cone must stop exactly at that seam. And you must be refining the cone. Disable the refine, or Move the cone a um, move the rotation of the pipe a fraction of a degree, and everything fixes. That's the replication part. Anyone ideas on the fix part?
adrianinsaval
commented
1 month ago is the pre toponaming code still around and buildable in main? it could be worth checking if this isn't a regression in occt rather than freecad
Is there an existing issue for this?
Problem description
Files are open source available at https://github.com/GliaX/tourniquet download and open
injection_molding/Glia Tourniquet Inj-Mold Design (FreeCAD)/Assembly.FCStdin freecad, click yes when prompted to recompute after a few seconds recomputing freecad crashes:same with latest appimage, on the other hand opening the file and clicking recompute in rc1 results in a broken model but no crash so it would seem this has something to do with the recently added dialog @bgbsww
Full version info
Subproject(s) affected?
None
Anything else?
No response
Code of Conduct