Skip to content

Conversation

Aarkham
Copy link

@Aarkham Aarkham commented Nov 20, 2022

Added two new flags:

  • ImGuiKnobFlags_RotateRelative
  • ImGuiKnobFlags_RotateAbsolute

With them you have to move around the knob in a circular way. Keyboard input is not currently implemented for these modes.

I attach a video of the relative mode.

RotateKnob

Added two new flags ImGuiKnobFlags_RotateRelative and ImGuiKnobFlags_RotateAbsolute.  With them you have to move around the knob in a circular way. Keyboard input is not currently implemented for these modes.
Copy link
Owner

@altschuler altschuler left a comment

Choose a reason for hiding this comment

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

Thank you very much for this PR, it's a good idea. I've left some comments. In general the code style needs to be in line with the other code, eg. brace placements, use kebab case, spacing around operators and control flow, etc.


angle_min = IMGUIKNOBS_PI * 0.75f;
angle_max = IMGUIKNOBS_PI * 2.25f;
center = {screen_pos[0] + radius, screen_pos[1] + radius};
Copy link
Owner

Choose a reason for hiding this comment

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

Why was this moved?

Copy link
Author

Choose a reason for hiding this comment

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

The center value is needed in RotateBehavior.

imgui-knobs.cpp Outdated
{
ImVec2 dir(aMousePos.x-aPos.x,aMousePos.y-aPos.y);
float angle=atan2(-dir.y,-dir.x);
angle+=(3.141592f*2.5f);
Copy link
Owner

Choose a reason for hiding this comment

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

Can you use the Pi constant instead? Here and other places

Copy link
Author

Choose a reason for hiding this comment

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

Yes, sorry. I was using the previous version with IMGUIKNOBS_PI using double and forgot to fix it when I updated.

imgui-knobs.cpp Outdated
//Re-maps a number from one range to another.
//Numbers outside the range are not clamped to 0 and 1.
template<typename T>
inline T Map(const T& aValue,const T& aIStart,const T& aIStop,const T& aOStart,const T& aOStop)
Copy link
Owner

Choose a reason for hiding this comment

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

Map is only used inside MapC, can you inline it there?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, I copied it from code I was using. As I did not know if you wanted to merge I did not modified it.

imgui-knobs.cpp Outdated

// Re-Maps clamped
template<typename T>
inline T MapC(const T& aValue,const T& aIStart,const T& aIStop,const T& aOStart,const T& aOStop)
Copy link
Owner

Choose a reason for hiding this comment

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

Please use kebab case, and maybe use a more descriptive name, like map_range_clamped. Also don't add the a for paramteres, just name them value, in_start, out_stop, etc.

Copy link
Author

Choose a reason for hiding this comment

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

Sure, I'll try to do this week end.

@Aarkham
Copy link
Author

Aarkham commented Nov 26, 2022

Thanks for taking the time to review the commit.

Added a new commit. Let me know what you think about it.

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.

2 participants