search

cadop / dhart

A library for creating Navigation Graphs, Visibility graphs, Raycasting used in design analysis, architecture, robotics, and human factors.
https://cadop.github.io/dhart/
GNU General Public License v3.0
14 stars 9 forks source link

Add overloads accepting float[] for all raytracing methods #51

Closed mariuszhermansdorfer closed 1 year ago

cadop commented 1 year ago

Can you split up PRs to be just one feature?

I thought this was referring to https://github.com/cadop/dhart/pull/46 (i.e. the float[] overload for c#), but this PR seems like there are a lot of additions to a codebase for accepting a float on a few functions.

It seems like part of this is referring to this issue: https://github.com/cadop/dhart/issues/38 which I set as on hold because its not clear to me how the benefits outweigh the increase in managing the codebase.

cadop commented 1 year ago

Regarding the primid, lets discuss on this thread https://github.com/cadop/dhart/discussions/54

mariuszhermansdorfer commented 1 year ago

I reverted the primId part as suggested in #54.

The float[] overloads do come with a significant speed gain (#46) hence my suggestion to expand it to other methods as well.

cadop commented 1 year ago

Thanks for cleaning it up, its much easier to see. I was a bit confused between https://github.com/cadop/dhart/issues/38 which was about changing the c_interface and https://github.com/cadop/dhart/pull/46 about the C# interface.

I'll writeup new performance tests for passing a float vs the new flatten method to make sure that difference is still there (the flatten array method I ended up using as an overload was much faster than the original one). Then I can make pull this

cadop commented 1 year ago

sorry it took so long, i pulled this and made some changes to how its called with the overloads in this commit https://github.com/cadop/dhart/commit/65107be4899bcd4ca32a341733f5bba0aced2429

If you notice it causes any issues please let me know