search

TommyKaneko / Sketchup-API-C-Wrapper

A complete set of C++ Wrapper classes for SketchUp C API objects
MIT License
28 stars 8 forks source link

Making use of SUTransformation functions #26

Closed thomthom closed 6 years ago

thomthom commented 6 years ago

The current version of Transformation is doing a lot of direct manipulation of the matrix. The SU2018 version of the SDK added a lot new functions to the geometry types. Best to migrate as much as possible to use the new functions.

For example:

https://github.com/TommyKaneko/Sketchup-API-C-Wrapper/blob/498c19b0c55af2296821df024267940f3246c413/SUAPI-CppWrapper/Transformation.cpp#L75-L80

That's better replaced with SUTransformationTranslation

http://extensions.sketchup.com/developer_center/sketchup_c_api/sketchup/geometry_2transformation_8h.html

TommyKaneko commented 6 years ago

Good to know. I know SU2018 added a lot of vector maths functions - not just in Transformation, but Point3D, Vector3D and others too. I would like to support SU2017 for the wrapper too. How we deal with different versions of SU with the CppWrapper is still a discussion to be had.

thomthom commented 6 years ago

I would like to support SU2017 for the wrapper too.

Why? I see no reason to support multiple versions of the SDK - as you can easily save out .skp files with older file versions. The SU2018 SDK is however required to handle files created with SU2018.

TommyKaneko commented 6 years ago

In this CppWrapper, there are geometric manipulations (particularly in Geometry.hpp) which don't use the C API functions. Switching to the SU2018 SDK would break this for older version users. That said, I should probably ask if the users will mind.

thomthom commented 6 years ago

If there is a C API variant I'd recommend making use of that to remain consistent with SketchUp. The manipulations are done on SketchUp models and to ensure best compliance with how SU operate it's best to utilitlize the API to the full extent.

Do you know if there are many people actively using this project in production already?

One way to address any concern for compatibility with existing users of the lib could be to leave an SU2017 branch for compatibility. (Though don't think it's worth keeping it up to date for new features.) Trying to mix SDK version support into a single branch would quickly get messy. Even more so as newer SDK versions are released.

TommyKaneko commented 6 years ago

Updated with https://github.com/TommyKaneko/Sketchup-API-C-Wrapper/commit/2b7e168804367babcd0d57d5010088a8214c549a

There are a couple more functions I need to get my head around before implementing

thomthom commented 6 years ago

Let me know if I can provide any clarifications. If the docs needs improving I'd like to get that addressed as well.