-
-
Notifications
You must be signed in to change notification settings - Fork 10.2k
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.
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
Add new methods to ImDrawList that support drawing ellipses #2743
Conversation
Thank you @Doohl ! |
Hello, Thank you very much for this new method ! Just in time with my need :-) I tested your code (see screenshot). For the record, it works well, but I didn't investigate much. The only issue I have, is that I was not able to make rotation work. The code is big and not public yet, since I've created a class named Canvas since, and something similar will be in miniDart 0.9.7 . See https://framagit.org/ericb/miniDart. Below, the essential to understand:
The preview:
What is drawn:
Thanks ! |
Hello @ebachard I'd say your problem is here: static float rotation = (ImGui::IsKeyPressed(ImGui::GetIO().KeyCtrl) == true) ? radius_y / radius_x : 0.0f; Specifically, Instead, do: static float rotation = ImGui::GetIO().KeyCtrl ? radius_y / radius_x : 0.0f; Which will evaluate as true when the CTRL key is down. Hope you find this helpful! |
@Doohl : uff, you are right, I was mistaken with KeyCtrl use. Shame on me ... In fact, in meantime, I identified another visibility issue, and the final working solution is:
Thanks a lot for your help, and for your nice feature ! :-) |
@Doohl Question about the API.. while I can see the use for |
You are probably right, @ocornut there is not an outstanding reason for why I've added rotation to ellipses. It was a combination of my own personal need and the fact that Canvas2D's ellipse() has that argument. I can remove that parameter for now and open an issue or PR for standardizing rotation for all primitives. |
The rotation is justified because most of the other primitives can be rotated by the user. AddRect can be replaced by AddQuad with rotated points. Whereas if AddEllipse doesn’t offer the option, there’s no way to achieve a rotated output. Only exception is AddRectFilledMultiColor() but that’s a rarely used and very simple function. I guess what I am most curious about is how one may be using ellipses in a codebase. How about you? |
For background, I'm using ImGui's primitives for a quick program to render a 2D model of the solar system. Trying to avoid either writing up my own methods for primitives or using something like SDL_gfx. Ellipses in my case require rotation, because in the context of a topdown 2D perspective of a Keplerian two-body solar system, the longitude of the periapsis defines the rotation of the ellipse that represents an orbit. So we could:
I speculate most people who would draw ellipses probably won't need them rotated like I do. But you do bring up a good point in that other primitives can already be rotated! |
You just did :) My gut feeling right now is that we should go for your initial plan. |
Haha, I meant I was TRYING to avoid writing my own methods. Actually, I think a more performant solution for me would be to draw my ellipses w/ GLSL especially since their x/y radii can go up to like 5000 pixels... but it's always good to have a quick software solution. I'll leave that last commit in the branch for posterity; if you decide to keep rotation or not. I figure you probably cherry pick commits anyway! Thank you for all the input. |
Hi, is there some update on this? |
Hello Matheus,
No update per se,
But you can freely include this function in your codebase (as a loose function taking a ImDrawList* parameter) so its mostly a matter of copying that code here.
|
Hi, @ocornut thanks. I had some issues using those functions manually so I decided to simply add a WebView (CEF) to ImGui. A bit overkill but it looks awesome if combining WebView on top of ImGui. Hopefully, I can provide an ImGui WebView Addon component when I have the time. |
0c1e5bd
to
bb6a60b
Compare
8b83e0a
to
d735066
Compare
b3b85d8
to
0755767
Compare
c817acb
to
8d39063
Compare
Hi guys, what a pity this never got merged. Do you have plans to work on this again? |
It's about 15 lines of code that you can yourself add anywhere in your own code/application. I don't think there's any pressure from adding this in the codebase. In principle I am for it. I think it was a mistake on my part to suggest a version without rotation would be more standard. For reference, here's the code (in its version before removing the angle parameter) made external: #include "imgui_internal.h"
void ImDrawList_PathEllipticalArcTo(ImDrawList* draw_list, const ImVec2& center, float radius_x, float radius_y, float rot, float a_min, float a_max, int num_segments)
{
draw_list->_Path.reserve(draw_list->_Path.Size + (num_segments + 1));
const float cos_rot = ImCos(rot);
const float sin_rot = ImSin(rot);
for (int i = 0; i <= num_segments; i++)
{
const float a = a_min + ((float)i / (float)num_segments) * (a_max - a_min);
ImVec2 point(ImCos(a) * radius_x, ImSin(a) * radius_y);
const float rel_x = (point.x * cos_rot) - (point.y * sin_rot);
const float rel_y = (point.x * sin_rot) + (point.y * cos_rot);
point.x = rel_x + center.x;
point.y = rel_y + center.y;
draw_list->_Path.push_back(point);
}
}
void ImDrawList_AddEllipse(ImDrawList* draw_list, const ImVec2& center, float radius_x, float radius_y, ImU32 col, float rot, int num_segments, float thickness)
{
if ((col & IM_COL32_A_MASK) == 0 || num_segments <= 2)
return;
// Because we are filling a closed shape we remove 1 from the count of segments/points
const float a_max = IM_PI * 2.0f * ((float)num_segments - 1.0f) / (float)num_segments;
ImDrawList_PathEllipticalArcTo(draw_list, center, radius_x, radius_y, rot, 0.0f, a_max, num_segments - 1);
draw_list->PathStroke(col, true, thickness);
}
void ImDrawList_AddEllipseFilled(ImDrawList* draw_list, const ImVec2& center, float radius_x, float radius_y, ImU32 col, float rot, int num_segments)
{
if ((col & IM_COL32_A_MASK) == 0 || num_segments <= 2)
return;
// Because we are filling a closed shape we remove 1 from the count of segments/points
const float a_max = IM_PI * 2.0f * ((float)num_segments - 1.0f) / (float)num_segments;
ImDrawList_PathEllipticalArcTo(draw_list, center, radius_x, radius_y, rot, 0.0f, a_max, num_segments - 1);
draw_list->PathFillConvex(col);
} I have rebased this locally and added support for auto-tesselation but I'm unsure of the right API. @Doohl themselves link to https://developer.mozilla.org/en-US/docs/Web/API/CanvasRenderingContext2D/ellipse which has a start_angle and end_angle exposed in the high-level Ellipse() function. |
Survey of APIs CanvasRenderingContext2D.ellipse()
Old GDI BOOL Ellipse(
[in] HDC hdc,
[in] int left,
[in] int top,
[in] int right,
[in] int bottom
); .Net Graphics DrawEllipse(Pen, Single, Single, Single, Single) Processing:
Matplotlib xy : (float, float): xy coordinates of ellipse centre.
width : float : Total length (diameter) of horizontal axis.
height : float: Total length (diameter) of vertical axis.
angle : scalar, optional: Rotation in degrees anti-clockwise. Allegro 5
Raylib
SDL2gfx
What's evident to me is that most API have limitations, the only one that doesn't is JS one, mathlib, and low-level version proposed by this PR. It's also apparent that all API offering an angle/rotation are expressing the ellipsis as center + radiuses instead of bounding box. I believe bounding box is simpler to use in some situations but it doesn't map to rotated ellipsis. This low-level API I have no issue: PathEllipticalArcTo(const ImVec2& center, float radius_x, float radius_y, float rot, float a_min, float a_max, int num_segments = 0)` The high-level IHMO have limitations: void AddEllipse(const ImVec2& center, float radius_x, float radius_y, ImU32 col, float rot = 0.0f, int num_segments = 0, float thickness = 1.0f);
void AddEllipseFilled(const ImVec2& center, float radius_x, float radius_y, ImU32 col, float rot = 0.0f, int num_segments = 0); But they can be lifted using the low-level version, and that's exactly analogous to using AddCircle() vs PathArcTo() so I am fine with them. I'll merge this now. |
…ed(). (#2743) Rebased with mods by ocornut: defaults to num_segments==0, supports for auto-tesselation, tweak demo.
I have pushed the modified version as e3d9b87 For auto-tesseletation I used:
Which is pessimistic and not ideal (maybe counter-productive in some extreme case). I would appreciate if someone can investigate proper auto-tesselation for ellipsis, based on ellipsis perimeter. |
Why is |
As per 868facf, please note that the prototype of the ellipse functions have been changed to use a single ImVec2 instead of two floats for the radius. |
…ImVec2 radius in PathEllipticalArcTo(), AddEllipse(), AddEllipseFilled(). (ocornut#2743, ocornut#7417)
Contained are three new ImDrawList functions that closely mimic the existing AddEllipse/PathArcTo functions:
Parameters for drawing an ellipse, more or less, were taken from Javascript's Canvas2D ellipse() method. Willing to make any modifications that will improve performance and/or clarity.
Though ellipses can currently be drawn with the existing bezier curve feature set, it does not seem proper - ellipses, I would argue, are primitive shapes that should be included.
[Example use: drawing 2D kepler orbits]