godotengine / godotengine/godot
CanvasItem::draw_ellipse() has mis-named 'major' and 'minor' parameters
- Dominant language
- C++
- Stars
- 117k
- Forks
- 26.8k
- PR merge metrics
- PR metrics pending
Description
### Tested versions
- Present in: 4.6.stable
- Not Applicable to versions prior to 4.6 (method(s) in question introduced in 4.6)
### System information
Godot v4.6.stable - macOS Sonoma (14.0.0) - Multi-window, 1 monitor - Metal (Forward+) - integrated Apple M1 (Apple7) - Apple M1 (8 threads) - 16.00 GiB memory
### Issue description
The `CanvasItem::draw_ellipse()` 2nd and 3rd parameters are named in the source code as `major` and `minor`, and described in the documentation as the "semi-major axis" and "semi-minor axis". However, how they are coded and how they behave are as the X-direction axis and the Y-direction axis.
In geometry, the major axis of an ellipse is its longest diameter. Correspondingly, the semi-major axis is the longest semidiameter; and, the minor is the shortest. e.g., both `draw_ellipse(..., 20, 10, ...)` and `draw_ellipse(..., 10, 20, ...)` draw an ellipse with a semi-major axis of 20. The first has its major axis in the X-direction ("squashed") and the second has its major axis in the Y-direction ("tall").
Thus, the parameter names would be better as something like "x_axis" and "y_axis", or similar.
Throughout its API, Godot (naturally) relies on knowledge, references, concepts, and metaphors from geometry. People approaching `draw_ellipse()` with a background in geometry will be misled by both the parameter names and how they are described in the API documentation. People approaching geometry with a background in working in Godot will be mistaken about what a 'major' and 'minor' axis of an ellipse are.
(This same observation applies to `draw_ellipse_arc()`, and to corresponding methods in `RenderServer`, and anywhere else that is tightly associated with these `CanvasItem` methods.)
### Steps to reproduce
On a 500x500 `Control`, issue the drawing commands:
```
draw_ellipse(Vector2(150, 250), 50, 100, Color.FIREBRICK, true)
draw_ellipse(Vector2(350, 250), 100, 50, Color.FIREBRICK, true)
```
Observe that both ellipses have the same major- and minor-axis sizes. To check the definition of major and minor axis in this context, see [Wikipedia](https://en.wikipedia.org/wiki/Semi-major_and_semi-minor_axes)
### Minimal reproduction project (MRP)
N/A - This issue is not project-dependent. This is an issue with the parameter names as written into the source code, and how they are therefore described in the API documentation.
Contributor guide
Assessment
This issue has not been assessed yet.