0.20 bug/regression Array Pattern issue #6641

About the development of the Part Design module/workbench. PLEASE DO NOT POST HELP REQUESTS HERE!
wmayer
Founder
Posts: 18651
Joined: Thu Feb 19, 2009 10:32 am

Re: 0.20 bug/regression Array Pattern issue #6641

Post by wmayer »

So would you say the if-branch is equivalent to the following?
No, it's not for the simple reason that the if-branch in current master still fails while in my branch it works. When comparing the code there are a few differences and I haven't investigated what exactly is the reason of the failure:
  • In current master two boolean operations are performed. For the first boolean operation TopoShape::fuse() is used that internally also uses the new API but the output is converted into a shell.
  • For the first boolean operation a tolerance value of Precision::Confusion() is used for fuzzy boolean operations.
Forgot to mention fillet/dressup has issues with simply using comp-solids for pattern/transform, and that is a regression from 0.19. We should either choose the if-branch, or also make sure dressup gives compsolids for add/sub shapes.
Yes, I would just use the if-branch from my branch. I have tested the code with David's initial example and the execution is as fast as in current master. In fact my code is even a bit faster because it avoids creating the compsolid if it's rejected afterwards.
jnxd
Posts: 516
Joined: Mon Mar 30, 2015 2:30 pm

Re: 0.20 bug/regression Array Pattern issue #6641

Post by jnxd »

wmayer wrote: Fri Apr 08, 2022 8:16 am
So would you say the if-branch is equivalent to the following?
No, it's not for the simple reason that the if-branch in current master still fails while in my branch it works. When comparing the code there are a few differences and I haven't investigated what exactly is the reason of the failure:
I was talking about replacing some lines for testing, but that was more of an experiment than anything else. As for the failure, the issue is that fusion of compounds and solids is not supported properly.
wmayer wrote: Fri Apr 08, 2022 8:16 am
  • In current master two boolean operations are performed. For the first boolean operation TopoShape::fuse() is used that internally also uses the new API but the output is converted into a shell.
  • For the first boolean operation a tolerance value of Precision::Confusion() is used for fuzzy boolean operations.
If you are talking of TopoShape::makeShell note this comment :

Code: Select all

    // we need a compound that consists of only faces
That method only weaves the faces together if possible, but otherwise leaves the input as is. This might have applications in certain cases which are not relevant here.

As such I am a little skeptical of Precision::Confusion() since that is assumed to be used anyway because of rounding errors. Precision::Approximation() or such might be more appropriate. This can be discussed later---I just brought it up since you mentioned it.
user1234
Veteran
Posts: 1546
Joined: Mon Jul 11, 2016 5:08 pm

Re: 0.20 bug/regression Array Pattern issue #6641

Post by user1234 »

jnxd wrote: Fri Apr 08, 2022 5:01 am I still doubt all the examples in this thread work with 0.19 though, but the only thing I have to check it is reverting the changes.
Yes and no. Fillet patterns work in 0.19 only, when they are all only additive, or all subtractive.

For example in this thread: the example with only additive fillets works (the fillet along the length comes from the sweep)
0.19_1.png
0.19_1.png (222.39 KiB) Viewed 758 times



while this with additive and subtractive fails.

0.19_2.png
0.19_2.png (249.34 KiB) Viewed 758 times


You also can see this in this thread: https://forum.freecadweb.org/viewtopic.php?t=59260



Should we make a separate thread for this?

Greetings
user1234
wmayer
Founder
Posts: 18651
Joined: Thu Feb 19, 2009 10:32 am

Re: 0.20 bug/regression Array Pattern issue #6641

Post by wmayer »

jnxd wrote: Fri Apr 08, 2022 6:28 pm
If you are talking of TopoShape::makeShell note this comment :

Code: Select all

    // we need a compound that consists of only faces
That method only weaves the faces together if possible, but otherwise leaves the input as is. This might have applications in certain cases which are not relevant here.
Even if the original shape is returned there is still the problem that it's still a compound. Somehow the kernel (often) makes a difference if an input shape is a solid or a compound of a single solid.
As such I am a little skeptical of Precision::Confusion() since that is assumed to be used anyway because of rounding errors. Precision::Approximation() or such might be more appropriate. This can be discussed later---I just brought it up since you mentioned it.
This doesn't seem to be a problem. I have checked the source code of BOPAlgo_Options (that's the base class that stores the fuzzy value). The default value is set to Precision::Confusion() and the SetFuzzyValue method doesn't allow to set a smaller value.
user1234
Veteran
Posts: 1546
Joined: Mon Jul 11, 2016 5:08 pm

Re: 0.20 bug/regression Array Pattern issue #6641

Post by user1234 »

I tested this

https://github.com/AjinkyaDahale/FreeCA ... -issue6641

branch. The behavior is indent with this

https://forum.freecad.org/viewtopic.php ... 30#p583637

post.


Greetings
user1234
jnxd
Posts: 516
Joined: Mon Mar 30, 2015 2:30 pm

Re: 0.20 bug/regression Array Pattern issue #6641

Post by jnxd »

user1234 wrote: Sat Apr 09, 2022 9:37 pm The behavior is indent with this

https://forum.freecad.org/viewtopic.php ... 30#p583637

post.
I do not expect this to change. The changes in that commit only address the regression from 0.19, but any issues in 0.19 are unfortunately here to stay. Lots of it appears an OCC issue.
jnxd
Posts: 516
Joined: Mon Mar 30, 2015 2:30 pm

Re: 0.20 bug/regression Array Pattern issue #6641

Post by jnxd »

user1234 wrote: Sat Apr 09, 2022 5:18 pm
jnxd wrote: Fri Apr 08, 2022 5:01 am I still doubt all the examples in this thread work with 0.19 though, but the only thing I have to check it is reverting the changes.
Yes and no. Fillet patterns work in 0.19 only, when they are all only additive, or all subtractive.
So are you saying the additive+subtractive fillets also make bad patterns in 0.19? From a code point of view that sounds counterintuitive, but I only glanced at the before times code.
jnxd
Posts: 516
Joined: Mon Mar 30, 2015 2:30 pm

Re: 0.20 bug/regression Array Pattern issue #6641

Post by jnxd »

wmayer wrote: Sat Apr 09, 2022 6:24 pm Even if the original shape is returned there is still the problem that it's still a compound. Somehow the kernel (often) makes a difference if an input shape is a solid or a compound of a single solid.
As per the documentation (https://dev.opencascade.org/doc/refman/ ... ml#details),
For Boolean operation FUSE all arguments should have equal dimensions;
So this just sounds like implementation details. I am unsure what they would consider to be the "dimension" of a compound, since it can contain absolutely any shape.
This doesn't seem to be a problem. I have checked the source code of BOPAlgo_Options (that's the base class that stores the fuzzy value). The default value is set to Precision::Confusion() and the SetFuzzyValue method doesn't allow to set a smaller value.
Indeed there isn't any problem, at least for now. As I said Precision::Confusion() is just for the rounding errors, so by definition a smaller value should not be allowed. For the "fuzzy value" let me give my understanding through an example of an air conditioner. We set the AC to give us around a temperature of 24, so the unit will do something only if it detects the ambient temperature to be too different. Precision::Confusion() is equivalent to the least count of the thermometer used to check the temperature, say 0.5 degrees C. The "fuzzy value" is basically saying: we are fine if the temperature is, say 26, but if it gets to 27 or so the unit should be activated to suck heat out. It could the stay active till we reach a temperature of 22 so it doesn't have to start again too soon.
User avatar
uwestoehr
Veteran
Posts: 4097
Joined: Sun Jan 27, 2019 3:21 am
Location: Germany

Re: 0.20 bug/regression Array Pattern issue #6641

Post by uwestoehr »

The polar pattern was fixed 4 days ago by Werner.

There is however still a regression in the new pipe handling. Other objects like Pads work fine.

- take this example file which a pipe which does not work:
Kappe50-jeno-mirror-fail.FCStd
(58.85 KiB) Downloaded 14 times
So it's not the polar pattern, but the pipe that files. Because any transformation with a pipe fails. This works with however with FC 0.19 -> regression

- take this example file which works:
Kappe50-jeno-pattern-works.FCStd
(98.74 KiB) Downloaded 10 times
Any transformation with pads work.

So affected are any pipes and also lofts, here an example:
Kappe50-jeno-pattern-fail-loft.FCStd
(54.69 KiB) Downloaded 11 times
Ajinkya could you please have a look?
jnxd
Posts: 516
Joined: Mon Mar 30, 2015 2:30 pm

Re: 0.20 bug/regression Array Pattern issue #6641

Post by jnxd »

uwestoehr wrote: Mon Apr 11, 2022 7:56 pm The polar pattern was fixed 4 days ago by Werner.
Note that this is still in his branch instead of in master as he usually does. Is the bug reproducible in that branch?
wmayer wrote: Sat Apr 09, 2022 6:24 pm Hi
@wmayer are you waiting for something to merge this branch?

[EDIT] Conveniently I just checked 15 seconds after posting this and the first file works with the following after a recompute:

Code: Select all

OS: Manjaro Linux (GNOME/gnome)
Word size of FreeCAD: 64-bit
Version: 0.20.28650 +1 (Git)
Build type: Release
Branch: issue_6641
Hash: 6662a0afdb6af02678b89d64ebf01602799e3ed8
Python 3.10.2, Qt 5.15.3, Coin 4.0.1, OCC 7.5.3
Locale: English/United States (en_US)
Installed mods: 
  * Curves 0.3.0
Post Reply