Fixed several issues with drawing ellipses

I've replaced the ellipse drawing algorithms with implementations by
Zingl Alois, originally written in 2012 and published under MIT license.

This addresses several issues with the previously used implementations
of pointsOnEllipse and ellipseRegion:

* Ellipses no longer degrade when very large (near 2000 tiles in size).

* No more gaps in the circle for some sizes.

* Shape Fill tool can now actually draw circles of equal width and
  height and can draw both even and odd sized ellipses.

The Shape Fill tool was adjusted such that drawing ellipses work in
bounding-box mode by default. The Alt modifier now toggles to center +
radius mode, which also applies when drawing rectangles.

Closes #3775

NEWS.md

@@ -9,6 +9,7 @@
 * Fixed hover highlight rendering with active parallax factor (#3669)
 * Fixed updating of object selection outlines when changing parallax factor (#3669)
 * Fixed "Offset Map" action to offset all objects when choosing "Whole Map" as bounds
+* Fixed several issues with drawing ellipses (#3776)
 * Godot 4 plugin: Export custom tile properties as Custom Data Layers (with Kevin Harrison, #3653)
 * AppImage: Updated to Sentry 0.6.4
 * Qt 6: Increased the image allocation limit from 1 GB to 4 GB (#3616)

docs/manual/editing-tile-layers.rst

@@ -110,7 +110,16 @@ Shape Fill Tool
 Shortcut: ``P`` |rectangle-fill|
 
 This tool provides a quick way to fill rectangles or ellipses with a certain
-tile or pattern. Hold ``Shift`` to fill an exact square or circle.
+tile or pattern.
+
+- Holding ``Shift`` fills an exact square or circle.
+
+.. raw:: html
+
+   <div class="new">Since Tiled 1.10.2</div>
+
+- Holding ``Alt`` draws the rectangle or ellipse centered around the starting
+  location.
 
 You can also flip and rotate the current stamp as described for the
 :ref:`stamp-tool`.

src/tiled/geometry.cpp

@@ -1,6 +1,9 @@
 /*
  * geometry.cpp
  * Copyright 2010-2011, Stefan Beller <stefanbeller@googlemail.com>
+ * Copyright 2017, Benjamin Trotter <bdtrotte@ucsc.edu>
+ * Copyright 2020, Zingl Alois
+ * Copyright 2017-2023, Thorbjørn Lindeijer <bjorn@lindeijer.nl>
  *
  * This file is part of Tiled.
  *
@@ -26,141 +29,93 @@ namespace Tiled {
 
 /**
  * Returns a lists of points on an ellipse.
- * (x0,y0) is the midpoint
- * (x1,y1) determines the radius.
+ * (xm,ym) is the midpoint
+ * (a,b) determines the radii.
  *
- * It is adapted from http://en.wikipedia.org/wiki/Midpoint_circle_algorithm
- * here is the original: http://homepage.smc.edu/kennedy_john/belipse.pdf
+ * From "Bresenham Curve Rasterizing Algorithms".
+ *
+ * @version V20.15 april 2020
+ * @copyright MIT open-source license software
+ * @url https://github.com/zingl/Bresenham
+ * @author  Zingl Alois
  */
-QVector<QPoint> pointsOnEllipse(int x0, int y0, int x1, int y1)
+QVector<QPoint> pointsOnEllipse(int xm, int ym, int a, int b)
 {
     QVector<QPoint> ret;
-    int x, y;
-    int xChange, yChange;
-    int ellipseError;
-    int twoXSquare, twoYSquare;
-    int stoppingX, stoppingY;
-    int radiusX = x0 > x1 ? x0 - x1 : x1 - x0;
-    int radiusY = y0 > y1 ? y0 - y1 : y1 - y0;
-
-    if (radiusX == 0 && radiusY == 0)
-        return ret;
-
-    twoXSquare = 2 * radiusX * radiusX;
-    twoYSquare = 2 * radiusY * radiusY;
-    x = radiusX;
-    y = 0;
-    xChange = radiusY * radiusY * (1 - 2 * radiusX);
-    yChange = radiusX * radiusX;
-    ellipseError = 0;
-    stoppingX = twoYSquare*radiusX;
-    stoppingY = 0;
-    while (stoppingX >= stoppingY) {
-        ret += QPoint(x0 + x, y0 + y);
-        ret += QPoint(x0 - x, y0 + y);
-        ret += QPoint(x0 + x, y0 - y);
-        ret += QPoint(x0 - x, y0 - y);
-        y++;
-        stoppingY += twoXSquare;
-        ellipseError += yChange;
-        yChange += twoXSquare;
-        if ((2 * ellipseError + xChange) > 0) {
-            x--;
-            stoppingX -= twoYSquare;
-            ellipseError += xChange;
-            xChange += twoYSquare;
-        }
-    }
-    x = 0;
-    y = radiusY;
-    xChange = radiusY * radiusY;
-    yChange = radiusX * radiusX * (1 - 2 * radiusY);
-    ellipseError = 0;
-    stoppingX = 0;
-    stoppingY = twoXSquare * radiusY;
-    while (stoppingX <= stoppingY) {
-        ret += QPoint(x0 + x, y0 + y);
-        ret += QPoint(x0 - x, y0 + y);
-        ret += QPoint(x0 + x, y0 - y);
-        ret += QPoint(x0 - x, y0 - y);
-        x++;
-        stoppingX += twoYSquare;
-        ellipseError += xChange;
-        xChange += twoYSquare;
-        if ((2 * ellipseError + yChange) > 0) {
-            y--;
-            stoppingY -= twoXSquare;
-            ellipseError += yChange;
-            yChange += twoXSquare;
-        }
+
+    long x = -a, y = 0;          /* II. quadrant from bottom left to top right */
+    long e2 = b, dx = (1+2*x)*e2*e2;                       /* error increment  */
+    long dy = x*x, err = dx+dy;                             /* error of 1.step */
+
+    do {
+        ret += QPoint(xm-x, ym+y);                            /*   I. Quadrant */
+        ret += QPoint(xm+x, ym+y);                            /*  II. Quadrant */
+        ret += QPoint(xm+x, ym-y);                            /* III. Quadrant */
+        ret += QPoint(xm-x, ym-y);                            /*  IV. Quadrant */
+        e2 = 2*err;
+        if (e2 >= dx) { x++; err += dx += 2*(long)b*b; }             /* x step */
+        if (e2 <= dy) { y++; err += dy += 2*(long)a*a; }             /* y step */
+    } while (x <= 0);
+
+    while (y++ < b) {            /* too early stop for flat ellipses with a=1, */
+        ret += QPoint(xm, ym+y);                   /* -> finish tip of ellipse */
+        ret += QPoint(xm, ym-y);
     }
 
     return ret;
 }
 
 /**
- * returns an elliptical region centered at x0,y0 with radius determined by x1,y1
+ * Returns an elliptical region based on a rectangle given by x0,y0 (top-left)
+ * and x1,y1 (bottom-right), inclusive.
+ *
+ * From "Bresenham Curve Rasterizing Algorithms", adjusted to output a filled
+ * region instead of an outline.
+ *
+ * @version V20.15 april 2020
+ * @copyright MIT open-source license software
+ * @url https://github.com/zingl/Bresenham
+ * @author  Zingl Alois
  */
 QRegion ellipseRegion(int x0, int y0, int x1, int y1)
 {
     QRegion ret;
-    int x, y;
-    int xChange, yChange;
-    int ellipseError;
-    int twoXSquare, twoYSquare;
-    int stoppingX, stoppingY;
-    int radiusX = x0 > x1 ? x0 - x1 : x1 - x0;
-    int radiusY = y0 > y1 ? y0 - y1 : y1 - y0;
-
-    if (radiusX == 0 && radiusY == 0)
-        return ret;
-
-    twoXSquare = 2 * radiusX * radiusX;
-    twoYSquare = 2 * radiusY * radiusY;
-    x = radiusX;
-    y = 0;
-    xChange = radiusY * radiusY * (1 - 2 * radiusX);
-    yChange = radiusX * radiusX;
-    ellipseError = 0;
-    stoppingX = twoYSquare*radiusX;
-    stoppingY = 0;
-    while (stoppingX >= stoppingY) {
-        ret += QRect(-x, y, x * 2, 1);
-        ret += QRect(-x, -y, x * 2, 1);
-        y++;
-        stoppingY += twoXSquare;
-        ellipseError += yChange;
-        yChange += twoXSquare;
-        if ((2 * ellipseError + xChange) > 0) {
-            x--;
-            stoppingX -= twoYSquare;
-            ellipseError += xChange;
-            xChange += twoYSquare;
-        }
-    }
-    x = 0;
-    y = radiusY;
-    xChange = radiusY * radiusY;
-    yChange = radiusX * radiusX * (1 - 2 * radiusY);
-    ellipseError = 0;
-    stoppingX = 0;
-    stoppingY = twoXSquare * radiusY;
-    while (stoppingX <= stoppingY) {
-        ret += QRect(-x, y, x * 2, 1);
-        ret += QRect(-x, -y, x * 2, 1);
-        x++;
-        stoppingX += twoYSquare;
-        ellipseError += xChange;
-        xChange += twoYSquare;
-        if ((2 * ellipseError + yChange) > 0) {
-            y--;
-            stoppingY -= twoXSquare;
-            ellipseError += yChange;
-            yChange += twoXSquare;
-        }
+
+    auto addRect = [&ret](int x0, int y0, int x1, int y1) {
+        ret += QRect(QPoint(x0, y0), QPoint(x1, y1));
+    };
+
+    long a = abs(x1-x0), b = abs(y1-y0), b1 = b&1;                 /* diameter */
+    double dx = 4*(1.0-a)*b*b, dy = 4*(b1+1)*a*a;           /* error increment */
+    double err = dx+dy+b1*a*a, e2;                          /* error of 1.step */
+
+    if (x0 > x1) { x0 = x1; x1 += a; }        /* if called with swapped points */
+    if (y0 > y1) y0 = y1;                                  /* .. exchange them */
+    y0 += (b+1)/2; y1 = y0-b1;                               /* starting pixel */
+    a = 8*a*a; b1 = 8*b*b;
+
+    do {
+       // (x1, y0)                                            /*   I. Quadrant */
+       // (x0, y0)                                            /*  II. Quadrant */
+       // (x0, y1)                                            /* III. Quadrant */
+       // (x1, y1)                                            /*  IV. Quadrant */
+
+       addRect(x0, y0, x1, y0);                                 /* Bottom half */
+       addRect(x0, y1, x1, y1);                                    /* Top half */
+
+       e2 = 2*err;
+       if (e2 <= dy) { y0++; y1--; err += dy += a; }                 /* y step */
+       if (e2 >= dx || 2*err > dy) { x0++; x1--; err += dx += b1; }  /* x step */
+    } while (x0 <= x1);
+
+    while (y0-y1 <= b) {                /* too early stop of flat ellipses a=1 */
+       addRect(x0-1, y0, x1+1, y0);                /* -> finish tip of ellipse */
+       addRect(x0-1, y1, x1+1, y1);
+       y0++;
+       y1--;
     }
 
-    return ret.translated(x0, y0);
+    return ret;
 }
 
 /**

src/tiled/geometry.h

@@ -1,6 +1,8 @@
 /*
  * geometry.h
  * Copyright 2010-2011, Stefan Beller <stefanbeller@googlemail.com>
+ * Copyright 2017, Benjamin Trotter <bdtrotte@ucsc.edu>
+ * Copyright 2017-2023, Thorbjørn Lindeijer <bjorn@lindeijer.nl>
  *
  * This file is part of Tiled.
  *
@@ -27,12 +29,15 @@
 
 namespace Tiled {
 
-QVector<QPoint> pointsOnEllipse(int x0, int y0, int x1, int y1);
+QVector<QPoint> pointsOnEllipse(int xm, int ym, int a, int b);
 QRegion ellipseRegion(int x0, int y0, int x1, int y1);
 QVector<QPoint> pointsOnLine(int x0, int y0, int x1, int y1, bool manhattan = false);
 
-inline QVector<QPoint> pointsOnEllipse(QPoint a, QPoint b)
-{ return pointsOnEllipse(a.x(), a.y(), b.x(), b.y()); }
+inline QVector<QPoint> pointsOnEllipse(QPoint center, int radiusX, int radiusY)
+{ return pointsOnEllipse(center.x(), center.y(), radiusX, radiusY); }
+
+inline QRegion ellipseRegion(QRect rect)
+{ return ellipseRegion(rect.left(), rect.top(), rect.right(), rect.bottom()); }
 
 inline QVector<QPoint> pointsOnLine(QPoint a, QPoint b, bool manhattan = false)
 { return pointsOnLine(a.x(), a.y(), b.x(), b.y(), manhattan); }

src/tiled/shapefilltool.cpp

@@ -21,16 +21,15 @@
 #include "shapefilltool.h"
 
 #include "actionmanager.h"
-#include "addremovetileset.h"
 #include "brushitem.h"
 #include "geometry.h"
 #include "mapdocument.h"
-#include "painttilelayer.h"
 #include "stampactions.h"
 
-#include <QApplication>
 #include <QActionGroup>
+#include <QApplication>
 #include <QToolBar>
+#include <QUndoStack>
 
 #include <memory>
 
@@ -62,9 +61,9 @@ ShapeFillTool::ShapeFillTool(QObject *parent)
     ActionManager::registerAction(mRectFill, "ShapeFillTool.RectangleFill");
     ActionManager::registerAction(mCircleFill, "ShapeFillTool.CircleFill");
 
-    connect(mRectFill, &QAction::triggered,
+    connect(mRectFill, &QAction::triggered, this,
             [this] { setCurrentShape(Rect); });
-    connect(mCircleFill, &QAction::triggered,
+    connect(mCircleFill, &QAction::triggered, this,
             [this] { setCurrentShape(Circle); });
 
     setActionsEnabled(false);
@@ -207,27 +206,27 @@ void ShapeFillTool::updateFillOverlay()
         dy = ((dy > 0) - (dy < 0)) * min;
     }
 
-    const QRect boundingRect(mStartCorner, mStartCorner + QPoint(dx, dy));
+    const bool alt = mModifiers & Qt::AltModifier;
+    const QPoint p1 = alt ? mStartCorner - QPoint(dx, dy)
+                          : mStartCorner;
+    const QPoint p2 = mStartCorner + QPoint(dx, dy);
 
-    switch (mCurrentShape) {
-    case Rect: {
 #if QT_VERSION < QT_VERSION_CHECK(6, 0, 0)
-        QRect area = boundingRect.normalized();
-        if (area.width() == 0)
-            area.adjust(-1, 0, 1, 0);
-        if (area.height() == 0)
-            area.adjust(0, -1, 0, 1);
+    QRect area = QRect(p1, p2).normalized();
+    if (area.width() == 0)
+        area.adjust(-1, 0, 1, 0);
+    if (area.height() == 0)
+        area.adjust(0, -1, 0, 1);
 #else
-        QRect area = QRect::span(mStartCorner, mStartCorner + QPoint(dx, dy));
+    QRect area = QRect::span(p1, p2);
 #endif
+
+    switch (mCurrentShape) {
+    case Rect:
         updatePreview(area);
         break;
-    }
     case Circle:
-        updatePreview(ellipseRegion(boundingRect.left(),
-                                    boundingRect.top(),
-                                    boundingRect.right(),
-                                    boundingRect.bottom()));
+        updatePreview(ellipseRegion(area));
         break;
     }
 }

src/tiled/stampbrush.cpp

@@ -617,7 +617,9 @@ void StampBrush::updatePreview(QPoint tilePos)
             drawPreviewLayer(pointsOnLine(mStampReference, tilePos));
             break;
         case CircleMidSet:
-            drawPreviewLayer(pointsOnEllipse(mStampReference, tilePos));
+            drawPreviewLayer(pointsOnEllipse(mStampReference,
+                                             qAbs(mStampReference.x() - tilePos.x()),
+                                             qAbs(mStampReference.y() - tilePos.y())));
             break;
         case Capture:
             // already handled above