Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

ImDrawList: merge float radius_{x,y} parameters into ImVec2 radius in ellipse functions #7417

Closed
wants to merge 1 commit into from

Conversation

cfillion
Copy link
Contributor

For API consistency: the center x/y parameter is already a ImVec2. Other functions like Circle and Ngon also have a single radius parameter. #2743 (comment)

@ocornut
Copy link
Owner

ocornut commented Mar 18, 2024

Thank you, this looks good. I am currently on the move so hard to merge as is, but if you wish to add the minor stuff:

  • Entry in changelog.txt
  • Entry in breaking section of imgui.cpp
  • Increase IMGUI_VERSION_NUM
  • Add link to # related to affected functions in Changelog and commit message.
  • Add Comment in the obsoleted block with current version name for quick reference.
  • Temporary set DISABLE_OBSOLETE_FUNCTIONS and fix affected code (eg demo).

Since Ellipse are relatively new and supposedly not massively used I may be tempted to remove the obsolete version sooner (eg 3-6 months rather than 2+ years), since it’ll affect frequent updaters+adopters only.

Great job, thanks for attention to details!

…Vec2 radius` in PathEllipticalArcTo(), AddEllipse(), AddEllipseFilled()
@cfillion
Copy link
Contributor Author

Done!

  • Temporary set DISABLE_OBSOLETE_FUNCTIONS and fix affected code (eg demo).

Already did. 😄

ocornut pushed a commit that referenced this pull request Mar 19, 2024
…ImVec2 radius in PathEllipticalArcTo(), AddEllipse(), AddEllipseFilled(). (#2743, #7417)
@ocornut
Copy link
Owner

ocornut commented Mar 19, 2024

Thank you Christian for your help! this is now merged.
As I realized that the original functions were added in 1.90 quite recently, and likely have few users, I've changed my mind on the obsolete function, and pushed them commented. It should hopefully be an obvious fix for affected users.

@ocornut ocornut closed this Mar 19, 2024
pull bot pushed a commit to TeamREPENTOGON/imgui that referenced this pull request Mar 19, 2024
…ImVec2 radius in PathEllipticalArcTo(), AddEllipse(), AddEllipseFilled(). (ocornut#2743, ocornut#7417)
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants