Uvs ready to be merge #26
Conversation
Conflicts: core/src/style/style.cpp
| }else{ | ||
| _pts = sV; | ||
| } | ||
| } |
There was a problem hiding this comment.
Patricio could you check through your files and make sure they all have newlines at the end? It's kind of a dumb rule but unix tools like 'cat' expect them to be there and can behave a little weirdly if they're not.
-The polyline tesselation algorithm was duplicated in several functions, now all polyline building functions call a single generalized building function -Renamed buildDynamicPolyline and related variables to buildScalablePolyline, to better reflect usage
|
Almost ready to merge this one, few things:
|
|
Yes we use the 'builder' terminology in JS. Not really married to that either, though "geometry handler" seems pretty generic. |
|
+1 to only include geometry functions we currently need, we can add them later as we have a use (convex hull etc) |
| float lerpValue(const float &_start, const float &_stop, float const &_amt); | ||
|
|
||
| // Scale a vector by a pct (0.0-1.0) of it lenght | ||
| void scale(glm::vec3 &_vec, float _length); |
There was a problem hiding this comment.
I think scale is a misleading name here, if I understand it correctly this function actually just sets the length of the input vector to _length. How about renaming it setLength ?
| public: | ||
|
|
||
| Rectangle(); | ||
| Rectangle(const glm::vec4 &_vec4); |
There was a problem hiding this comment.
Our style in this repo has been to declare references like glm::vec4& _vec4 instead of glm::vec4 &_vec4, please use this rule from now on
|
@patriciogonzalezvivo I've left a number of comments here about style and documentation and also revised geom.h to give you an example of the documentation style. When you've addressed all the comments here let me know and I'll see about merging this. |
No description provided.