godotengine / godotengine/godot

CanvasItem::draw_ellipse() has mis-named 'major' and 'minor' parameters

Open
#116,387 1 comment 0 reactions 0 assignees View on GitHub
bug documentation topic:gui
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.