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 support for drawing images with rounded corners. #845

Merged
merged 2 commits into from
Nov 20, 2017

Conversation

thedmd
Copy link
Contributor

@thedmd thedmd commented Sep 26, 2016

PR was recreated due to lost references after changing fork origin. That was not my intention, sorry.
For comments please look at previous PR.

This PR add rounding and rounding_corners to ImDrawList::AddImage.

With rounding it is easier to draw over already rounded rectangles. This is for ImDrawList only. Widgets does not take advantage of this feature.

-     IMGUI_API void  AddImage(ImTextureID user_texture_id, const ImVec2& a, const ImVec2& b, const ImVec2& uv0 = ImVec2(0,0), const ImVec2& uv1 = ImVec2(1,1), ImU32 col = 0xFFFFFFFF);
+     IMGUI_API void  AddImage(ImTextureID user_texture_id, const ImVec2& a, const ImVec2& b, const ImVec2& uv0 = ImVec2(0,0), const ImVec2& uv1 = ImVec2(1,1), ImU32 col = 0xFFFFFFFF, float rounding = 0.0f, int rounding_corners = 0x0F);

image

@ocornut
Copy link
Owner

ocornut commented Nov 19, 2017

Hello Michał,

Three comments:

  • I'm tempted to suggest a separate entry point AddImageRounded() because it would be rarely used (and it could call AddImage if rounding==0.0).
    Being still a little traumatized by the fiasco of ImGui::Image/ImageButton signatures which are the most complete mess ever (and tricky to fix), if we can keep ImDrawList::AddImage() a little welcoming it'd be nice!

  • Your function PrimDistributeUV() mimics what I've been doing recently with the ShadeVertsXXX functions:

void ShadeVertsLinearColorGradientKeepAlpha(ImDrawVert* vert_start, ImDrawVert* vert_end, ImVec2 gradient_p0, ImVec2 gradient_p1, ImU32 col0, ImU32 col1);
void ShadeVertsLinearAlphaGradientForLeftToRightText(ImDrawVert* vert_start, ImDrawVert* vert_end, float gradient_p0_x, float gradient_p1_x);

I suggest your function could be called e.g. ShadeVertsLinearUV() and be declared in imgui_internal.h and not publicly in ImDrawList?

  • Your ImQuotient() isn't used, and while I'm generally for adding those other functions I think for the present use we may not need to expose all three. Maybe ImMul() (for ImProduct) and handle the rest int the function?

@thedmd thedmd force-pushed the 2016-08-rounded-image branch 4 times, most recently from e32e2e8 to 0dfd324 Compare November 19, 2017 20:56
@thedmd thedmd force-pushed the 2016-08-rounded-image branch from 0dfd324 to 79f07f6 Compare November 19, 2017 20:57
@thedmd
Copy link
Contributor Author

thedmd commented Nov 19, 2017

Hello Omar,

I added AddImageRounded(). I also rearranged arguments to make rounding non-default. Is that ok?
Also I made all changes you suggested.

Let me know if code can be improved more.

@ocornut ocornut merged commit 79f07f6 into ocornut:master Nov 20, 2017
ocornut added a commit that referenced this pull request Nov 20, 2017
…xed coding style, restored argument order from original PR. (#845)
@ocornut
Copy link
Owner

ocornut commented Nov 20, 2017

Michał,
This is now merged (with a few tweaks).

I also rearranged arguments to make rounding non-default. Is that ok?

I reverted that part. I realize it makes the call a little more awkward to make (and it suggests we could have used extra parameters on AddImage) but I feel it is more consistent.

I will also move the ImGuiCorner enum to imgui.h to make those values more clear.

Mildly unrelated, for the number 1 ideal fix I would like to make to the ImDrawList API would be to change:
AddRect(const ImVec2& a, const ImVec2& b, ImU32 col, float rounding = 0.0f, int rounding_corners_flags = ~0, float thickness = 1.0f)
to
AddRect(const ImVec2& a, const ImVec2& b, ImU32 col, float thickness = 1.0f, float rounding = 0.0f, int rounding_corners_flags = ~0)
But that will break so much code I don't now where to start...

However if we decided to break it in two functions:
AddRect(const ImVec2& a, const ImVec2& b, ImU32 col, float thickness = 1.0f)
AddRectRounded(const ImVec2& a, const ImVec2& b, ImU32 col, float thickness = 1.0f, float rounding = 0.0f, int rounding_corners_flags = ~0)
Then the breakage would be more bearable and noticeable (in a good way). Not sure when/if I'll want to go with that change, it's pretty agressive..

@thedmd
Copy link
Contributor Author

thedmd commented Nov 20, 2017

AddRectRounded is about rounded rectangles so I made rounding argument mandatory. Without it description of a shape isn't complete. This is also why I put rounding before thickness. What do you think?

As for remaining default arguments I don't think there is a correct/intuitive order.

@ocornut
Copy link
Owner

ocornut commented Nov 20, 2017

Are we talking about the same thing? You submitted AddImageRounded(). I agree rounding should be mandatory if the function are called XXRounded, and that is the case right now. There's no thickness parameter in there.

The AddRect stuff above are somehow off-topic / tangential stuff I brought it, should have made that more clear. I only discussed them because I'd ideally want to make change to them. There's no AddRectRounded function at the moment and I'm unhappy about the existing signature of AddRect hence my situation to split them but that would break some calls to AddRect silently which is pretty much the worse thing we could do to this API :(

@thedmd
Copy link
Contributor Author

thedmd commented Nov 20, 2017

I misunderstood you when you said you did revert changes to argument order. I thought you put rounding after uv_b with default value, so I tried to be more explicit about my reasons and used 'AddRectRounded' as an example. After looking at the code I now understand what you did you mean. My mistake.

As to AddRectRounded I think signature should look more like that:
void AddRectRounded(const ImVec2& a, const ImVec2& b, float rounding, ImU32 col, float thickness = 1.0f, int rounding_corners_flags = ~0);

@ocornut
Copy link
Owner

ocornut commented Nov 20, 2017

Btw I have exposed the defines ImDrawCornerFlags_TopLeft, ImDrawCornerFlags_All, etc.
ImDrawCornerFlags_All = 0xF // In your function calls you may use ~0 (= all bits set) instead of ImDrawCornerFlags_All, as a convenience

And used the new function in the demo:
image

@thedmd thedmd deleted the 2016-08-rounded-image branch December 19, 2017 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants