Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[For v0.17.1] Fix #3401 Warning on multiple solids in PD #1398

Closed
wants to merge 3 commits into from

Conversation

Projects
None yet
5 participants
@WandererFan
Copy link
Contributor

WandererFan commented Apr 8, 2018

This PR displays a message when the result of an operation in Part Design would result in multiple solids within the Body. Please merge when convenient. Thanks,

  • PartDesign only uses the first result shape
    of an operation and discards the rest without
    warning.

  • this also fixes #1707

Thank you for creating a pull request to contribute to FreeCAD! To ease integration, please confirm the following:

  • Branch rebased on latest master git pull --rebase upstream master
  • Unit tests confirmed to pass by running ./bin/FreeCAD --run-test 0
  • Commit message is well-written
  • Commit message includes issue #<id> or fixes #<id> where <id> is the associated MantisBT issue id if one exists

And please remember to update the Wiki with the features added or changed once this PR is merged.


@WandererFan

This comment has been minimized.

Copy link
Contributor Author

WandererFan commented Apr 9, 2018

Unit test for Pocket Through All generated a multi-solid result. Changed to produce single solid.

@yorikvanhavre

This comment has been minimized.

Copy link
Member

yorikvanhavre commented Apr 12, 2018

So this should go to both master and the 0.17 branch, right?

@WandererFan

This comment has been minimized.

Copy link
Contributor Author

WandererFan commented Apr 13, 2018

@wwmayer

This comment has been minimized.

Copy link
Contributor

wwmayer commented Apr 14, 2018

It has been discussed in the forum that we want to drop the one-solid rule. So, at least adding it to 0.18 makes almost no sense to me.

@WandererFan

This comment has been minimized.

Copy link
Contributor Author

WandererFan commented Apr 14, 2018

@normandc

This comment has been minimized.

Copy link

normandc commented Apr 14, 2018

What we've seen on the forum is end users complaining about the one-solid rule, which has existed since the very beginning of the PartDesign workbench. I have not seen a decision to retract this rule either. Did ickby specifically said so?

As for the PR as I understand it, it is only informing the end user about this rule, who are confused about it. One great reason for the confusion is that because of a bug in v0.16 that was never taken care of, the first Pad wrongfully allowed creation of multiple solids. But any further step (pad, pocket) enforced the rule again.

@WandererFan

This comment has been minimized.

Copy link
Contributor Author

WandererFan commented Apr 20, 2018

reopen if/when direction decided.

@wwmayer

This comment has been minimized.

Copy link
Contributor

wwmayer commented Apr 22, 2018

@normandc It once has been discussed in the German sub-forum together with freecad-heini-1 and end of last year there was also a telephone conference where this issue was brought up.

@normandc

This comment has been minimized.

Copy link

normandc commented Apr 22, 2018

Has ickby been part of this discussion? Where has this decision been mentioned? Who will implement it?

In the meanwhile, end users continue to be confused.

@wwmayer

This comment has been minimized.

Copy link
Contributor

wwmayer commented Apr 22, 2018

Has ickby been part of this discussion?

Yes. freecad-heini-1 was complaining about the strict one-solid rule that it makes things unnecessarily complicated. And it would be fine if only the very last step is a solid but not the intermediate steps (but I don't know how would you know what is the last step). ickby agreed but it would take a long time to implement it.

Where has this decision been mentioned?

https://forum.freecadweb.org/viewtopic.php?f=13&t=21342&hilit=meeting&start=40#p198094
The term one-solid rule was not explicitly mentioned but the sentence "Dann gab es eine Diskussion über die Designfreiheit beim Konstruieren ..." is about it.

Who will implement it?

No idea.

@normandc

This comment has been minimized.

Copy link

normandc commented Jun 10, 2018

Thanks for fishing out the info and sorry for not replying earlier, kind of forgot about it.

So basically because of a hypothetical removal of the single-solid restriction that may or may not happen eventually, users are left in confusion in the here and now because the rule is not clearly defined in the GUI, which this PR addresses. This is continually brought up on the help forum.

And this makes sense how?

Couldn't it be merged now, then removed once (or rather if) the single-solid rule is removed?

Thanks.

@normandc

This comment has been minimized.

Copy link

normandc commented Jun 24, 2018

Here is another topic (in German) that shows this confusion (first post). Every week people post questions on the forum about this.

https://forum.freecadweb.org/viewtopic.php?f=13&t=28297

Again, it makes no sense at all not to merge this because hypothetically, possibly maybe pretty please the Body could accept multiple solids in the future, but nobody is making it happen.

The error message could simply be changed to "Result has multiple solids. This is not supported at this time." That way, it indicates that it may change in the future but clearly indicates that it is not supported in the here and now.

PLEASE open this PR again, and merge it.

@normandc

This comment has been minimized.

Copy link

normandc commented Jun 28, 2018

Since I'm not getting any answer here I will post a comment each time I see a topic where a user is confused by the one-solid rule.

Today in the French forum: https://forum.freecadweb.org/viewtopic.php?f=12&t=29493

@WandererFan

This comment has been minimized.

Copy link
Contributor Author

WandererFan commented Jun 28, 2018

@WandererFan

This comment has been minimized.

Copy link
Contributor Author

WandererFan commented Jun 28, 2018

Reopened at request of @normandc .

@WandererFan WandererFan reopened this Jun 28, 2018

@yorikvanhavre

This comment has been minimized.

Copy link
Member

yorikvanhavre commented Jul 2, 2018

I am seeing this as well, but I don't have a strong opinion on this. It's a PartDesign-only subject, which I rarely use... But I'd vote for @normandc's suggestion ( replacing "Result has multiple solids. Check parameters." in this PR with "Result has multiple solids. This is not supported at this time.")

WandererFan added some commits Apr 4, 2018

Fix #3401 warning on multiple solid
- PartDesign only uses the first result shape
  of an operation and discards the rest without
  warning.

- this also fixes #1707
Fix PD Pocket Through All Unit Test
- test for Pocket Through All case
  generated a multi-solid solution.
  Now returns single solid.

@WandererFan WandererFan force-pushed the WandererFan:PDMultiWarn branch from f0cb78b to d695321 Jul 4, 2018

@WandererFan

This comment has been minimized.

Copy link
Contributor Author

WandererFan commented Jul 4, 2018

Message text revised as requested.

@sgrogan

This comment has been minimized.

Copy link
Contributor

sgrogan commented Jul 5, 2018

I vote for NormandC and a merge. If/until the restriction is removed the user should be informed.
Thanks @WandererFan, nice that you do the work, just in case.

@normandc

This comment has been minimized.

Copy link

normandc commented Jul 8, 2018

Today in the Help forum:
https://forum.freecadweb.org/viewtopic.php?f=3&t=29664

Once again a topic where we have to spend time explaining the single-solid rule because FreeCAD does not explicitly informs the end user. How much time we forum regulars would have saved if this had been merged and backported to 0.17 like it should have been?

I'm going to continue posting links here as long as this PR is not merged.

P.S. Thanks for reopening and changing this PR Wandererfan.

@yorikvanhavre

This comment has been minimized.

Copy link
Member

yorikvanhavre commented Jul 13, 2018

It's merged now, in both master and 0.17

@normandc

This comment has been minimized.

Copy link

normandc commented Jul 14, 2018

Huge and heartfelt thanks Wandererfan and Yorik. Now we need to update the Win-Mac 0.17 installers and AppImage...

@WandererFan WandererFan deleted the WandererFan:PDMultiWarn branch Jul 22, 2018

@dulouie dulouie referenced this pull request Feb 3, 2019

Closed

part design one-solid rule #192

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.