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

Handle constraints from infinite faces #797

Merged
merged 4 commits into from
Jul 6, 2021

Conversation

marcus7070
Copy link
Member

@marcus7070 marcus7070 commented Jun 25, 2021

Will close #758, where I wanted to specify a plane for a constraint (defined by an origin and a normal), so the obvious Shape to use was from Face.makePlane(width=None, length=None), but this shape has a center at 1e99.

To handle it I had to use gp_Pln a lot because it turns out two offset infinite faces will still have the same Center:

>>> a = cq.Face.makePlane(width=None, length=None, basePnt=(1, 2, 3), dir=(1, 1, 1))
>>> a.Center()
Vector: (5e+99, -5e+99, 5e+99)
>>> b = cq.Face.makePlane(width=None, length=None, basePnt=(1000, 0, 0), dir=(1, 1, 1))
>>> b.Center()
Vector: (5e+99, -5e+99, 5e+99)
>>> a.Center() == b.Center()
True

But now by converting them to gp_Pln you can get the origin that the user intends:

>>> cq.Vector(a.toPln().Location())
Vector: (1.0, 2.0, 3.0)

@codecov
Copy link

codecov bot commented Jun 25, 2021

Codecov Report

Merging #797 (c98df58) into master (3cb7237) will increase coverage by 0.04%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #797      +/-   ##
==========================================
+ Coverage   95.17%   95.21%   +0.04%     
==========================================
  Files          32       32              
  Lines        7436     7489      +53     
  Branches      797      798       +1     
==========================================
+ Hits         7077     7131      +54     
  Misses        221      221              
+ Partials      138      137       -1     
Impacted Files Coverage Δ
cadquery/assembly.py 93.13% <100.00%> (+0.34%) ⬆️
cadquery/occ_impl/shapes.py 92.46% <100.00%> (+0.02%) ⬆️
cadquery/occ_impl/solver.py 94.32% <100.00%> (+0.70%) ⬆️
tests/test_assembly.py 100.00% <100.00%> (ø)
tests/test_cadquery.py 99.20% <100.00%> (+<0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3cb7237...c98df58. Read the comment docs.

@marcus7070 marcus7070 marked this pull request as ready for review June 25, 2021 12:30
@marcus7070
Copy link
Member Author

I should mention, as an alternative to this we could just put a warning in the Face.makePlane docstring that if the user specifies None as the width then the center will be at 1e99.

@jmwright
Copy link
Member

Is the docs build hung?

@marcus7070
Copy link
Member Author

Just GitHub not updating, the docs build is done: https://readthedocs.org/projects/cadquery/builds/14095186/

@jmwright
Copy link
Member

Just GitHub not updating, the docs build is done

Interesting, I haven't seen that glitch before.

@adam-urbanczyk
Copy link
Member

Hm, same result on a rerun.

@jmwright
Copy link
Member

At the bottom of the logs it says this without ever appending done as the other steps do.

The HTML pages are in _build/html.
Updating searchtools for Read the Docs search...

Could that be related to the search box now working?

@marcus7070 marcus7070 force-pushed the marcus7070/infinite-face-constraint branch from caf83f1 to cb1dfcc Compare June 27, 2021 03:16
@marcus7070
Copy link
Member Author

I've rebased to include #801, incase that fixes RTD build.

@jmwright
Copy link
Member

Looks like it did fix it.

cadquery/assembly.py Outdated Show resolved Hide resolved
cadquery/assembly.py Outdated Show resolved Hide resolved
@marcus7070
Copy link
Member Author

Actually, if/when #806 goes through I'll have to rewrite these tests. We might as well wait for that before merging this.

@marcus7070 marcus7070 marked this pull request as draft June 30, 2021 11:39
Infinite faces have their center at 1e99, which was causing overflows in
the solver and also was not what the user intended when creating the
Shape. They are now converted to the expected values.
@marcus7070 marcus7070 force-pushed the marcus7070/infinite-face-constraint branch from a489a44 to 4a03d7b Compare July 3, 2021 10:51
@marcus7070 marcus7070 marked this pull request as ready for review July 3, 2021 11:50
@marcus7070 marcus7070 requested a review from jmwright July 3, 2021 11:51
Copy link
Member

@jmwright jmwright left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @marcus7070 !

cadquery/assembly.py Outdated Show resolved Hide resolved
@adam-urbanczyk
Copy link
Member

Thanks @marcus7070 , shall I merge?

@marcus7070 marcus7070 merged commit 6a440b6 into master Jul 6, 2021
@marcus7070 marcus7070 deleted the marcus7070/infinite-face-constraint branch July 6, 2021 07:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants