Skip to content

geom::Torus radii docs improvement and radiusCentered method #2072

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.

Sign up for GitHub

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

geom::Torus radii docs improvement and radiusCentered method #2072

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6a8e359
Improving geom::Torus major, minor radius documentation
gaborpapp Dec 18, 2018
83522d8
Fixing geom::Torus::ratio()
gaborpapp Dec 18, 2018
146d2ba
Adding geom::Torus radiusCentered method
gaborpapp Dec 18, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions include/cinder/GeomIo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -528,9 +528,11 @@ class CI_API Torus : public Source {
//! Allows you to twist the torus along the ring. The \a offset is in radians.
Torus& twist( unsigned twist, float offset ) { mTwist = twist; mTwistOffset = offset; return *this; }
//! Specifies the major and minor radius as a ratio (minor : major). Resulting torus will fit unit cube.
Torus& ratio( float ratio ) { ratio = math<float>::clamp( ratio ); mRadiusMajor = 1; mRadiusMinor = 1 - ratio; return *this; }
//! Specifies the major and minor radius separately.
Torus& ratio( float ratio ) { ratio = math<float>::clamp( ratio ); mRadiusMajor = 1; mRadiusMinor = ratio; return *this; }
Copy link
Collaborator

@richardeakin richardeakin Dec 21, 2018

Choose a reason for hiding this comment

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

The docs are a nice improvement IMO. though isn't it a breaking change? Though I'm not against it in this case, I don't think the ratio() method is used as much. @paulhoux what do you think?

Copy link
Contributor Author

@gaborpapp gaborpapp Dec 21, 2018

Choose a reason for hiding this comment

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

It is a breaking change indeed, but I think ratio() was not working according to its documentation. if ratio = minor / major and major = 1 then minor should be equal to ratio and not 1 - ratio.

//! Specifies the major and minor radius separately. \a major is the distance from the axis to the farthest point of the torus. \a minor is the distance between the axis and the center of the rotated circle.
Torus& radius( float major, float minor ) { mRadiusMajor = math<float>::max(0, major); mRadiusMinor = math<float>::max(0, minor); return *this; }
//! Sets the radii for a torus that is generated by revolving a small circle of radius \a r along the axis made by a bigger circle of radius \a R.
Torus& radiusCentered( float R, float r ) { mRadiusMajor = math<float>::max(0, R + r); mRadiusMinor = math<float>::max(0, R); return *this; }

size_t getNumVertices() const override;
size_t getNumIndices() const override;
Expand Down Expand Up @@ -1150,9 +1152,11 @@ class CI_API WireTorus : public WireSource {
//! Specifies the number of segments that make up each circle. Defaults to \c 72.
WireTorus& subdivisionsCircle( int subdiv ) { mNumSegments = math<int>::max( 3, subdiv ); return *this; }
//! Specifies the major and minor radius as a ratio (minor : major). Resulting torus will fit unit cube.
WireTorus& ratio( float ratio ) { ratio = math<float>::clamp( ratio ); mRadiusMajor = 1; mRadiusMinor = 1 - ratio; return *this; }
//! Specifies the major and minor radius separately.
WireTorus& ratio( float ratio ) { ratio = math<float>::clamp( ratio ); mRadiusMajor = 1; mRadiusMinor = ratio; return *this; }
//! Specifies the major and minor radius separately. \a major is the distance from the axis to the farthest point of the torus. \a minor is the distance between the axis and the center of the rotated circle.
WireTorus& radius( float major, float minor ) { mRadiusMajor = math<float>::max( 0, major ); mRadiusMinor = math<float>::max( 0, minor ); return *this; }
//! Sets the radii for a torus that is generated by revolving a small circle of radius \a r along the axis made by a bigger circle of radius \a R.
WireTorus& radiusCentered( float R, float r ) { mRadiusMajor = math<float>::max(0, R + r); mRadiusMinor = math<float>::max(0, R); return *this; }

size_t getNumVertices() const override;
void loadInto( Target *target, const AttribSet &requestedAttribs ) const override;
Expand Down