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 GeometryTypes.Point. #19

Merged
merged 1 commit into from
Jul 2, 2019
Merged

Handle GeometryTypes.Point. #19

merged 1 commit into from
Jul 2, 2019

Conversation

tkoolen
Copy link
Collaborator

@tkoolen tkoolen commented Jun 26, 2019

I was surprised that GeometryTypes.Vec is used for points instead of GeometryTypes.Point (also in the GJK implementation in GeometryTypes). I'd like to at least also support Point, since Vec is meant to be used for free vectors (e.g. displacements) instead of bound vectors, as far as I understand.

I was doubting whether to make this change in GeometryTypes or here, but I feel like here is more appropriate, as GeometryTypes.support_vector_max also returns a score, which is unused in this package and is just wasted computation (unless the compiler is able to optimize that away). In general, I feel like we should maybe stop relying on GeometryTypes.support_vector_max for this reason.

@codecov-io
Copy link

codecov-io commented Jun 26, 2019

Codecov Report

Merging #19 into master will increase coverage by 0.05%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #19      +/-   ##
=========================================
+ Coverage   93.54%   93.6%   +0.05%     
=========================================
  Files           8       8              
  Lines         217     219       +2     
=========================================
+ Hits          203     205       +2     
  Misses         14      14
Impacted Files Coverage Δ
src/gjk.jl 95.77% <100%> (+0.06%) ⬆️
src/tagged_points.jl 100% <100%> (ø) ⬆️

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 7d354c2...e005c78. Read the comment docs.

@tkoolen tkoolen requested a review from rdeits June 26, 2019 21:39
@tkoolen tkoolen merged commit 10ce89b into master Jul 2, 2019
@tkoolen tkoolen deleted the tk/point branch July 2, 2019 02:54
@tkoolen tkoolen restored the tk/point branch July 2, 2019 02:54
@tkoolen tkoolen deleted the tk/point branch July 3, 2019 02:48
@tkoolen tkoolen restored the tk/point branch July 3, 2019 02:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants