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 a resolution argument to CanvasItem.draw_circle() #7309

Open
CarpenterBlue opened this issue Jul 17, 2023 · 5 comments
Open

Add a resolution argument to CanvasItem.draw_circle() #7309

CarpenterBlue opened this issue Jul 17, 2023 · 5 comments

Comments

@CarpenterBlue
Copy link

CarpenterBlue commented Jul 17, 2023

Describe the project you are working on

I am slowly patching up my submission to GMTK23 jam.

Describe the problem or limitation you are having in your project

I want to draw a big single circle but the resolution (it's hardcoded to 64) is too small.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

I would like to replace the hardcoded value with an argument that defaults to 64 for backwards combability.
And possibly add logic to to clamp it to minimum value of 3.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

I would like to give contributing to Godot a shot and this seems like an easy first commit.
I found the code defining the circle here. As you can see on line 1151, the resolution seems to be hardcoded to 64, I would like to move this to an additional argument that defaults to 64 for backwards combability.

If this enhancement will not be used often, can it be worked around with a few lines of script?

For something so simple, and quite frankly essential for custom drawing, the user has to write their own implementation with GDScript which is extremely annoying so yes it can be easily worked around with a script but this is about quality of life.

Is there a reason why this should be core and not an add-on in the asset library?

This is too small of an addition to be an addon in the asset library and simultaneously too big of an issue to not do something about it. You either put up with ugly circle or waste 10 minutes on your own implementation when it could be an argument.

@CarpenterBlue
Copy link
Author

As a side effect, it allows to draw circles at smaller resolutions like hexagons which is very much desirable feature.

@Calinou
Copy link
Member

Calinou commented Jul 17, 2023

See godotengine/godot#14416, which exposed this parameter (and more, such as drawing filled circles and changing the line thickness). Godot now supports MSAA in 2D when the using Forward+ or Mobile rendering methods, so this comment is likely outdated now.

That PR would have to be recreated from scratch at this point though, since a lot of the 2D drawing code has changed.

In the meantime, you can use this add-on in 3.x.

@Calinou Calinou changed the title Add Resolution Argument to the draw_circle() Add a resolution argument to CanvasItem.draw_circle() Jul 17, 2023
@CarpenterBlue
Copy link
Author

I have taken a look at the pull request and that seems very unnecessarily over-complicated to me.
I just want filled circle with points , all of that logic seems like feature creep because for everything else there is draw_arc().

@jsjtxietian
Copy link

So is it worth to add a additional resolution argument for now? Or we just recreate the godotengine/godot#14416 directly ?

@Calinou
Copy link
Member

Calinou commented Nov 16, 2023

So is it worth to add a additional resolution argument for now? Or we just recreate the godotengine/godot#14416 directly ?

There's a separate proposal about adding filled and width options to draw_circle(), so a PR should only concern itself with adding a resolution parameter now.

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

4 participants