Skip to content
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 Quadratic bezier in ImDrawList #3127

Closed
aiekick opened this issue Apr 14, 2020 · 26 comments
Closed

Add Quadratic bezier in ImDrawList #3127

aiekick opened this issue Apr 14, 2020 · 26 comments

Comments

@aiekick
Copy link
Contributor

aiekick commented Apr 14, 2020

Hello,

i was using Quadratic bezier for display ttf glyph in ImGuiFontStudio
so i added in DrawList two func :

  • ImQuadBezierCalc (like ImBezierCalc but for 3 point bezier)
  • PathQuadCurveTo (like PathBezierCurveTo but for 3 point bezier)

by the way not found a ,casteljau self discretization algo for quadraitc like you do for cubic

is it possibe to add theses functions in Drawlist ?

the names are not very good, the ideal could be to rename existing bezier by :
PathCubicBezierCurveTo
and mine by PathQuadBezierCurveTo

but its like you want :)

ImVec2 ImQuadBezierCalc(const ImVec2& p1, const ImVec2& p2, const ImVec2& p3, float t)
{
    float u = 1.0f - t;
    float w1 = u * u;
    float w2 = 2 * u * t;
    float w3 = t * t;
    return ImVec2(w1 * p1.x + w2 * p2.x + w3 * p3.x, w1 * p1.y + w2 * p2.y + w3 * p3.y);
}

void ImDrawList::PathQuadCurveTo(const ImVec2& p2, const ImVec2& p3, int num_segments)
{
    ImVec2 p1 = _Path.back();
    float t_step = 1.0f / (float)num_segments;
    for (int i_step = 1; i_step <= num_segments; i_step++)
        _Path.push_back(ImQuadBezierCalc(p1, p2, p3, t_step * i_step));
}

i can propose a pull request if needed.

Thanks.

@ocornut ocornut changed the title Add Quabratic bezier in ImDrawList Add Quadratic bezier in ImDrawList Apr 15, 2020
@ocornut
Copy link
Owner

ocornut commented Apr 15, 2020

Hello,

As you pointed there are two issues:

  • A naming issue, for now I don't have a satisfying answer for it (or perhaps using the same name).
  • Missing the auto-tesselated version and other functions such as ImBezierClosestPointCasteljau for consistency.

I suggest you make your PathQuadCurveTo a loose function in your source code instead of modifying ImDrawList+ImGui for now. If you don't need more there's no reason for modifying imgui sources.

@aiekick
Copy link
Contributor Author

aiekick commented Apr 15, 2020

ok

@ocornut
Copy link
Owner

ocornut commented Apr 15, 2020

Closing this for now but I'm adding this in my todo list along with this issue #
If you come up with the full features (auto-tesselation) you can post code here too, it'll be useful for reference! Thank you.

@aiekick
Copy link
Contributor Author

aiekick commented Apr 15, 2020

ok i will search. thanks

@ocornut ocornut closed this as completed Apr 15, 2020
@aiekick
Copy link
Contributor Author

aiekick commented Apr 24, 2020

Done :

i not changed the name of the func "PathBezierToCasteljau" for quadratic since the stamp is not the same, use 3 point instead four. (Its like you want :) )

In fact, it was easier than I thought the first time haha,
I defined a constraint in Cubic version : "point 2 is equal to point 3" and just reduced your function

see the complete code :

ImVec2 ImQuadBezierCalc(const ImVec2& p1, const ImVec2& p2, const ImVec2& p3, float t)
{
    float u = 1.0f - t;
    float w1 = u * u;
    float w2 = 2 * u * t;
    float w3 = t * t;
    return ImVec2(w1 * p1.x + w2 * p2.x + w3 * p3.x, w1 * p1.y + w2 * p2.y + w3 * p3.y);
}

static void PathQuadBezierToCasteljau(ImVector<ImVec2>* path, float x1, float y1, float x2, float y2, float x3, float y3, float tess_tol, int level)
{
    float dx = x3 - x1, dy = y3 - y1;
    float det = (x2 - x3) * dy - (y2 - y3) * dx;
    if (det * det * 4.0f < tess_tol * (dx * dx + dy * dy))
    {
        path->push_back(ImVec2(x3, y3));
    }
    else if (level < 10)
    {
        float x12 = (x1 + x2) * 0.5f, y12 = (y1 + y2) * 0.5f;
        float x23 = (x2 + x3) * 0.5f, y23 = (y2 + y3) * 0.5f;
        float x123 = (x12 + x23) * 0.5f, y123 = (y12 + y23) * 0.5f;
        PathQuadBezierToCasteljau(path, x1, y1, x12, y12, x123, y123, tess_tol, level + 1);
        PathQuadBezierToCasteljau(path, x123, y123, x23, y23, x3, y3, tess_tol, level + 1);
    }
}

void ImDrawList::PathQuadBezierCurveTo(const ImVec2& p2, const ImVec2& p3, int num_segments)
{
    ImVec2 p1 = _Path.back();
    if (num_segments == 0)
    {
        PathQuadBezierToCasteljau(&_Path, p1.x, p1.y, p2.x, p2.y, p3.x, p3.y, _Data->CurveTessellationTol, 0); // Auto-tessellated
    }
    else
    {
        float t_step = 1.0f / (float)num_segments;
        for (int i_step = 1; i_step <= num_segments; i_step++)
            _Path.push_back(ImQuadBezierCalc(p1, p2, p3, t_step * i_step));
    }
}

It work for me with default tolerance: (the blue lines are control points)

ImGuiFontStudio_Mingw32_Win32_Idw2i8yXY5

@aiekick
Copy link
Contributor Author

aiekick commented Dec 19, 2020

is there any chances to add these quads versions in imgui soon ? :)

@ocornut ocornut reopened this Dec 19, 2020
@ocornut
Copy link
Owner

ocornut commented Dec 19, 2020

A proper PR would be appreciated for it, thank you.

@aiekick
Copy link
Contributor Author

aiekick commented Dec 19, 2020

Ha, no problem. i can.
just a question, i can add the new quad func.
but i think we need to rename the normal bezier version with cubic name.
but it can break change for some users who are using them.

I rename theses versions or not ?

@ocornut
Copy link
Owner

ocornut commented Dec 19, 2020

I’ll do the renaming separately.
What I don’t understand is why you didn’t just add those functions to your code and why you need them in core ImGui.

@aiekick
Copy link
Contributor Author

aiekick commented Dec 19, 2020

because it was usefull to me and i think it will be usefull for other guys,
since these functions are designed for ImDraw.
it was not easy to found / create the casteljau version.
The Imdraw is a toolkit for draw no ? its logic to have that inside imgui for the community.
the extra code add around 10 lines of code

@ocornut
Copy link
Owner

ocornut commented Dec 19, 2020

Thanks. I’ll look forward to the PR and will try to merge it for 1.81

@aiekick
Copy link
Contributor Author

aiekick commented Dec 19, 2020

i can add a tab "Curves" to the window "custom rendering" in demo code, to show the cubic and quad bezier if you want ? (with control point manipulation)

with a switch for have self discretized / segment predefined
show bezier quad or cubic
habilty to move control points

@aiekick
Copy link
Contributor Author

aiekick commented Dec 20, 2020

Oups my casteljau code for quadratic is wrong haha. i will found the solution. :)

Btw, I propose this demo for curves :
1yunJ8MBhT

@ocornut
Copy link
Owner

ocornut commented Dec 20, 2020

I am not technically against a demo but it'll give much surface area for code to rework/fix so I'd suggest putting it in a separate commit.

@aiekick
Copy link
Contributor Author

aiekick commented Dec 20, 2020

i have created the PR for the helpers. #3664
you want another PR for the demo ?

@ocornut
Copy link
Owner

ocornut commented Dec 20, 2020

Both in same PR, separate commits, sould be simpler/neater but if you already done it this way it should be ok.

@aiekick
Copy link
Contributor Author

aiekick commented Dec 20, 2020

yep sorry just done that :), PR is not really mastered on my side haha
the PR for the demo is here : #3665
i will manage that better for the next time :)
maybe you can delete the PR #3664, since the #3665 contain all commits :)

@ocornut
Copy link
Owner

ocornut commented Dec 21, 2020

Thanks!

I merged the functions with various tweaks. Semi reluctantly I have to admit. Note that you didn't add the missing functions (e.g. ClosestPoint etc.) I requested in my first answer, but I am not sure we would even want to have them (too much extra code) since I am not firmly convinced this is al very useful the first place, as it could perfectly be done in user code and we have to encourage that instead of encouraging adding more code to core. Using C++ style member function for ImDrawList is quite a burden there :(

I didn't merge the full demo from #3665, it's too much code unrelated to bezier, at first glance usage of IsMouseHoveringRect() is wrong as it doesn't respect the requirements for hovering tests. I think down the line we'll want to have a demo like this but 140 lines is too much noise and I don't have the energy now to rework it. But we should.

In git 1 PR = 1 branch. You can push multiple commits in a branch if necessary, we can choose to merge all of them or just selected ones. It's better to rebase the branch on master and force-push rather than merge master into it, so the branch doesn't need to have unrelated merge commits appearing. You can also use interactive git rebase to further sculpt/improve your commits so the PR branch can look as neat as possible. Typically when I rebase my own long-running small branches (e.g. unfinished features) I frequently also give them a bit of cleaning and prepare a lean history ahead of the merging.

@ocornut ocornut closed this as completed Dec 21, 2020
@aiekick
Copy link
Contributor Author

aiekick commented Dec 21, 2020

Thanks too :)

i forgot theses funcs ClosestPoint , but indeed i can add too, as you want :)

yes for the demo, i seen the code was too mush complex and noisy.
i can try to rework it, make it more clear and more light. with more clear comments too :)
i can create too a helper func in other part of the demo code juste for control point manipulation, let us have light/clear code in demo tab. maybe can be better for learning purpose

thanks for the PR help. not familiar with rebase, i will check that :)
Since now its merged, if i delete my fork, delete some branch and/or other things.
he will affect something here ?

ocornut added a commit that referenced this issue Dec 21, 2020
… ImDrawList::PathBezierCurveTo() to ImDrawList::PathBezierCubicCurveTo(). (#3127, #3664, #3665)

Renamed corresponding internal functions as well.
ocornut added a commit that referenced this issue Dec 21, 2020
… ImDrawList::PathBezierCurveTo() to ImDrawList::PathBezierCubicCurveTo(). (#3127, #3664, #3665)

Renamed corresponding internal functions as well.
@ocornut
Copy link
Owner

ocornut commented Dec 21, 2020

I also renamed the functions with a subsequent commit: 4d8e839
(the new functions are not "documented" as renamed by this commit since they were just added in the commit before)

Overall I think it's nicer to do those renaming, and the old version are so unfrequently used and "obvious" to fix that we can ditch the redirecting inline function sooner than typically.

@ocornut
Copy link
Owner

ocornut commented Dec 21, 2020

You can delete your branches indeed. The trace will be left here (so we will have access e.g. to the demo code here).

@aiekick
Copy link
Contributor Author

aiekick commented Dec 21, 2020

what can i use instead of IsMouseHoveringRect ?
at end i need to know when hovered or/and active, but without add an item

@ocornut
Copy link
Owner

ocornut commented Dec 21, 2020

You would need to use a combination of IsItemHovered(), IsItemActive() first as a filter, if you simply use IsMouseHoveringRect() you are not going to properly handle windows over this one, or popup windows inhibiting windows over this one.

@aiekick
Copy link
Contributor Author

aiekick commented Dec 21, 2020

ok i see my mistake. thanks :)

@aiekick
Copy link
Contributor Author

aiekick commented Dec 21, 2020

i removed grid and scrolling ability from canvas (too bad for me, but the code is smaller)
and made a specifi helper function for manipulate / craw control point. cnaa be reused for other similar cases

// Helper to interact with a control point
// mouse left click/move for move the control point
static ImVec2 HelpManipulateControlPoint(const int point_index, ImVec2& point_coords, const float point_radius, const ImVec4 canvas)
{
    static int selected_control_point = -1;

    // current point control
    ImVec2 point = ImVec2(canvas.x + point_coords.x * canvas.z, canvas.y + point_coords.y * canvas.w);

    bool hovered = false;
    if (ImGui::IsItemHovered())
    {
        if (ImGui::IsMouseHoveringRect(
            ImVec2(point.x - point_radius * 0.5f, point.y - point_radius * 0.5f),
            ImVec2(point.x + point_radius * 0.5f, point.y + point_radius * 0.5f)))
        {
            hovered = true;

            // select if active and no point was selected before
            // for have only one point movable at same time
            if (selected_control_point < 0 && ImGui::IsItemActive())
                selected_control_point = point_index;
        }
    }

    ImDrawList* draw_list = ImGui::GetWindowDrawList();
    // draw point background color
    draw_list->AddCircleFilled(point, point_radius,
        (selected_control_point == point_index) ? IM_COL32(0, 0, 255, 255) :
        (hovered) ? IM_COL32(0, 127, 127, 255) : IM_COL32(255, 0, 0, 255));
    // draw point edge
    draw_list->AddCircle(point, point_radius, IM_COL32(255, 255, 255, 255), 0, 2.0f);

    // unselect point
    if (ImGui::IsMouseReleased(ImGuiMouseButton_Left))
        selected_control_point = -1;

    // move point, will be updated on next frame
    if (selected_control_point == point_index &&
        ImGui::IsMouseDragging(ImGuiMouseButton_Left, 0.0f))
    {
        ImGuiIO& io = ImGui::GetIO();
        point_coords.x += io.MouseDelta.x / (canvas.z + 1e-5f);
        point_coords.y += io.MouseDelta.y / (canvas.w + 1e-5f);
    }

    return point;
}

the demo code look like :

if (ImGui::BeginTabItem("Curves"))
        {
            static ImVec2 bezier_control_points[4] = { ImVec2(0.12, 0.82), ImVec2(0.34, 0.21), ImVec2(0.90, 0.41), ImVec2(0.56, 0.82) }; // ratio of canvas size
            static float control_point_radius = 10.0f;
            static int curve_type = 0; // 0 quadratic, 1 cubic
            static bool curve_segments_override = false;
            static int curve_segments_override_v = 8;

            ImGui::RadioButton("Quad Bezier", &curve_type, 0);
            ImGui::SameLine(); HelpMarker("a Quad bezier have 3 control points;");
            ImGui::SameLine(); ImGui::RadioButton("Cubic Bezier", &curve_type, 1);
            ImGui::SameLine(); HelpMarker("a Cubic bezier have 4 control points;");
            ImGui::Checkbox("##curvessegmentoverride", &curve_segments_override);
            ImGui::SameLine(0.0f, ImGui::GetStyle().ItemInnerSpacing.x);
            if (ImGui::SliderInt("Curves segments", &curve_segments_override_v, 3, 40))
                curve_segments_override = true;

            ImGui::Text("Mouse Left : move control points");

            // canvas from last tab
            // Using InvisibleButton() as a convenience 1) it will advance the layout cursor and 2) allows us to use IsItemHovered()/IsItemActive()
            ImVec2 canvas_p0 = ImGui::GetCursorScreenPos();      // ImDrawList API uses screen coordinates!
            ImVec2 canvas_sz = ImGui::GetContentRegionAvail();   // Resize canvas to what's available
            if (canvas_sz.x < 50.0f) canvas_sz.x = 50.0f;
            if (canvas_sz.y < 50.0f) canvas_sz.y = 50.0f;
            ImVec2 canvas_p1 = ImVec2(canvas_p0.x + canvas_sz.x, canvas_p0.y + canvas_sz.y);

            // Draw border and background color
            ImGuiIO& io = ImGui::GetIO();
            ImDrawList* draw_list = ImGui::GetWindowDrawList();
            draw_list->AddRectFilled(canvas_p0, canvas_p1, IM_COL32(50, 50, 50, 255));
            draw_list->AddRect(canvas_p0, canvas_p1, IM_COL32(255, 255, 255, 255));
            
            // This will catch our interactions, only left button, no scrolling on this one
            ImGui::InvisibleButton("canvas", canvas_sz, ImGuiButtonFlags_MouseButtonLeft);
            ImVec4 canvas_rc = ImVec4(canvas_p0.x, canvas_p0.y, canvas_p1.x - canvas_p0.x, canvas_p1.y - canvas_p0.y);

            draw_list->PushClipRect(canvas_p0, canvas_p1, true); // canvas clipping
            {
                draw_list->ChannelsSplit(2); // split channels

                 // calc and render control point in top layer
                draw_list->ChannelsSetCurrent(1);
                ImVec2 cp[4]; // control point
                for (int i = 0; i < 3 + curve_type; i++) // 3 when curve is quadratic, 4 when curve is cubic
                {
                    cp[i] = HelpManipulateControlPoint(i, bezier_control_points[i], control_point_radius, canvas_rc);
                }

                // render curve and edges in bottom layer
                draw_list->ChannelsSetCurrent(0);
                const ImU32 curve_color = IM_COL32(255, 255, 0, 255);
                const int curve_segments = curve_segments_override ? curve_segments_override_v : 0;
                if (curve_type) // bezier cubic
                {
                    draw_list->AddBezierCurve(cp[0], cp[1], cp[2], cp[3], curve_color, 2.0f, curve_segments); // curve
                    draw_list->AddLine(cp[0], cp[1], IM_COL32(255, 255, 255, 255), 1.5f); // edge from control point 0 to 1
                    draw_list->AddLine(cp[2], cp[3], IM_COL32(255, 255, 255, 255), 1.5f); // edge from control point 2 to 3
                }
                else // bezier quadratic
                {
                    draw_list->AddQuadBezierCurve(cp[0], cp[1], cp[2], curve_color, 2.0f, curve_segments); // curve
                    draw_list->AddLine(cp[0], cp[1], IM_COL32(255, 255, 255, 255), 1.5f); // edge from control point 0 to 1
                    draw_list->AddLine(cp[1], cp[2], IM_COL32(255, 255, 255, 255), 1.5f); // edge from control point 2 to 3
                }

                draw_list->ChannelsMerge(); // merge channels
            }
            draw_list->PopClipRect();

            ImGui::EndTabItem();
        }

i can reduce again the code by removing border and background drawing of canvas.

is it better for you ?

to note, for the moment i have the old names :) i will commit a good one or propose a new PR if ok for you

available here : aiekick@14dacf2

@aiekick
Copy link
Contributor Author

aiekick commented Jan 2, 2021

do you think this new demo code is better now ?
I can adapt it again in your way if needed :)

And by the way, I wish you all the best for this new year :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants