Some minor bug fixes and some cleaning #96
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
schustermartin
commented
Nov 12, 2020
schustermartin
commented
- Fixed some edge cases where in(...) function was returning false results due to forth and back conversion between Cylindrical and Cartesian Points. This is done by skipping trigonometric conversions where unnecessary.
- Fixed in(...) function for the Cone Primitive. So far translate vector was not taken into account.
- Switched to the use of RotationsPackage for VolumePrimitives (not used so far, just how general structure could look like).
- Got rid of some unnecessary lines of code (expressing the same in less lines).
…tion matrices; Note: they still don't really do anything yet, but this is the structure we envision. Fixed the in() function to respect the translation vector for Cones.
… also Conversions that occasionally led to numerical issues.(Even false returns of the in() function.) Summarized some lines into a single line where applicable.
…uld only work if that float is XY.0 .
fhagemann
approved these changes
Nov 12, 2020
| @@ -56,20 +56,14 @@ function Geometry(T::DataType, t::Val{:tube}, dict::Dict{Union{Any,String}, Any} | |||
| end | |||
|
|
|||
| function in(point::CartesianPoint{T}, tube::Tube{T}) where T | |||
| ismissing(tube.translate) ? nothing : point -= tube.translate | |||
| (ismissing(tube.translate) || tube.translate == CartesianVector{T}(0.0,0.0,0.0)) ? nothing : point -= tube.translate | |||
There was a problem hiding this comment.
tube.translate should never be zero
There was a problem hiding this comment.
Well, we have to change the general struct of our primitives anyhow when we add Rotations.
This if ismissing should never appear. I guess we will use parametric types to handle this on compilation level.
There was a problem hiding this comment.
For both, translation and rotation.
lmh91
approved these changes
Nov 12, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.