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

Templatize E57Simple to support double coordinates #63

Merged
merged 1 commit into from
Jan 7, 2021

Conversation

gjacquem
Copy link
Contributor

@gjacquem gjacquem commented Jan 5, 2021

Hi,

The E57Format lib is overloaded to work with several coordinate types, yet the current E57Simple API only supports float coordinates.
Making the E57Simple Data3DPointsData struct a template enables the use of different types, such as double.

In order to keep the current behavior of the API, the default float template was typedefined using the previous Data3DPointsData name.
A new double version Data3DPointsData_d was also typedefined so that it can be used directly as well.

The intent behind this change is that the last official release of the E57Simple API was using double coordinates, and there are still some use-cases where this level of precision is needed.
This is especially the case when working on large scale scenes - such as we do here at Trimble - and this feature should provide more retro-compatibility to the API.

Regards,
Grégoire

trimble
Grégoire Jacquemin | Trimble
Software Development Engineer

The E57Simple API only supports float coordinates. Added templatable
type to work with either float or double.
Copy link
Owner

@asmaloney asmaloney left a comment

Choose a reason for hiding this comment

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

Thanks Grégoire!

This looks reasonable to me, but I would like to give @ptc-jhoerner a chance to take a look since he brought the simple API over from the original lib.

@ptc-jhoerner
Copy link
Contributor

Thanks! Looks good to me as well. Very nice way to support both float precisions.

@asmaloney asmaloney merged commit aef4fd7 into asmaloney:master Jan 7, 2021
@asmaloney
Copy link
Owner

Thanks @ptc-jhoerner.

And thank you @gjacquem for your first contribution! Much appreciated.

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.

None yet

3 participants