Introduced object group drawing order attribute

This attribute allows specifying per object group whether its objects
should be drawn top down (from top to bottom) or in index order
(which enables manual ordering).

This change also fixes an issue where new objects could be invisible
during placement due to being drawn below other layers.

src/libtiled/mapreader.cpp

@@ -681,6 +681,17 @@ ObjectGroup *MapReaderPrivate::readObjectGroup()
     if (!color.isEmpty())
         objectGroup->setColor(color);
 
+    if (atts.hasAttribute(QLatin1String("draworder"))) {
+        QString value = atts.value(QLatin1String("draworder")).toString();
+        ObjectGroup::DrawOrder drawOrder = drawOrderFromString(value);
+        if (drawOrder == ObjectGroup::UnknownOrder) {
+            delete objectGroup;
+            xml.raiseError(tr("Invalid draw order: %1").arg(value));
+            return 0;
+        }
+        objectGroup->setDrawOrder(drawOrder);
+    }
+
     while (xml.readNextStartElement()) {
         if (xml.name() == QLatin1String("object"))
             objectGroup->addObject(readObject());

src/libtiled/mapwriter.cpp

@@ -448,6 +448,9 @@ void MapWriterPrivate::writeObjectGroup(QXmlStreamWriter &w,
         w.writeAttribute(QLatin1String("color"),
                          objectGroup->color().name());
 
+    w.writeAttribute(QLatin1String("draworder"),
+                     drawOrderToString(objectGroup->drawOrder()));
+
     writeLayerAttributes(w, objectGroup);
     writeProperties(w, objectGroup->properties());
 

src/libtiled/objectgroup.cpp

@@ -40,12 +40,14 @@ using namespace Tiled;
 
 ObjectGroup::ObjectGroup()
     : Layer(ObjectGroupType, QString(), 0, 0, 0, 0)
+    , mDrawOrder(TopDownOrder)
 {
 }
 
 ObjectGroup::ObjectGroup(const QString &name,
                          int x, int y, int width, int height)
     : Layer(ObjectGroupType, name, x, y, width, height)
+    , mDrawOrder(TopDownOrder)
 {
 }
 
@@ -88,9 +90,9 @@ void ObjectGroup::moveObjects(int from, int to, int count)
     Q_ASSERT(count >= 0);
     Q_ASSERT(to <= from || to >= from + count);
 
-    // Nothing to be done when 'to' is the start of the end of the range, or
+    // Nothing to be done when 'to' is the start or the end of the range, or
     // when the number of objects to be moved is 0.
-    if (count == 0 || to == from || to == from + count)
+    if (to == from || to == from + count || count == 0)
         return;
 
     const QList<MapObject*> movingObjects = mObjects.mid(from, count);
@@ -233,3 +235,32 @@ ObjectGroup *ObjectGroup::initializeClone(ObjectGroup *clone) const
     clone->setColor(mColor);
     return clone;
 }
+
+
+QString Tiled::drawOrderToString(ObjectGroup::DrawOrder drawOrder)
+{
+    switch (drawOrder) {
+    default:
+    case ObjectGroup::UnknownOrder:
+        return QLatin1String("unknown");
+        break;
+    case ObjectGroup::TopDownOrder:
+        return QLatin1String("topdown");
+        break;
+    case ObjectGroup::IndexOrder:
+        return QLatin1String("index");
+        break;
+    }
+}
+
+ObjectGroup::DrawOrder Tiled::drawOrderFromString(const QString &string)
+{
+    ObjectGroup::DrawOrder drawOrder = ObjectGroup::UnknownOrder;
+
+    if (string == QLatin1String("topdown"))
+        drawOrder = ObjectGroup::TopDownOrder;
+    else if (string == QLatin1String("index"))
+        drawOrder = ObjectGroup::IndexOrder;
+
+    return drawOrder;
+}

src/libtiled/objectgroup.h

@@ -49,6 +49,18 @@ class MapObject;
 class TILEDSHARED_EXPORT ObjectGroup : public Layer
 {
 public:
+    /**
+     * Objects within an object group can either be drawn top down (sorted
+     * by their y-coordinate) or by index (manual stacking order).
+     *
+     * The default is top down.
+     */
+    enum DrawOrder {
+        UnknownOrder = -1,
+        TopDownOrder,
+        IndexOrder
+    };
+
     /**
      * Default constructor.
      */
@@ -165,16 +177,11 @@ class TILEDSHARED_EXPORT ObjectGroup : public Layer
     bool canMergeWith(Layer *other) const;
     Layer *mergedWith(Layer *other) const;
 
-    /**
-     * Returns the color of the object group, or an invalid color if no color
-     * is set.
-     */
-    const QColor &color() const { return mColor; }
+    const QColor &color() const;
+    void setColor(const QColor &color);
 
-    /**
-     * Sets the display color of the object group.
-     */
-    void setColor(const QColor &color) {  mColor = color; }
+    DrawOrder drawOrder() const;
+    void setDrawOrder(DrawOrder drawOrder);
 
     Layer *clone() const;
 
@@ -184,8 +191,57 @@ class TILEDSHARED_EXPORT ObjectGroup : public Layer
 private:
     QList<MapObject*> mObjects;
     QColor mColor;
+    DrawOrder mDrawOrder;
 };
 
+
+/**
+ * Returns the color of the object group, or an invalid color if no color
+ * is set.
+ */
+inline const QColor &ObjectGroup::color() const
+{ return mColor; }
+
+/**
+ * Sets the display color of the object group.
+ */
+inline void ObjectGroup::setColor(const QColor &color)
+{ mColor = color; }
+
+/**
+ * Returns the draw order for the objects in this group.
+ *
+ * \sa ObjectGroup::DrawOrder
+ */
+inline ObjectGroup::DrawOrder ObjectGroup::drawOrder() const
+{ return mDrawOrder; }
+
+/**
+ * Sets the draw order for the objects in this group.
+ *
+ * \sa ObjectGroup::DrawOrder
+ */
+inline void ObjectGroup::setDrawOrder(DrawOrder drawOrder)
+{ mDrawOrder = drawOrder; }
+
+
+/**
+ * Helper function that converts a drawing order to its string value. Useful
+ * for map writers.
+ *
+ * @return The draw order as a lowercase string.
+ */
+TILEDSHARED_EXPORT QString drawOrderToString(ObjectGroup::DrawOrder);
+
+/**
+ * Helper function that converts a string to a drawing order enumerator.
+ * Useful for map readers.
+ *
+ * @return The draw order matching the given string, or
+ *         ObjectGroup::UnknownOrder if the string is unrecognized.
+ */
+TILEDSHARED_EXPORT ObjectGroup::DrawOrder drawOrderFromString(const QString &);
+
 } // namespace Tiled
 
 Q_DECLARE_METATYPE(Tiled::ObjectGroup*)

src/tiled/abstractobjecttool.cpp

@@ -162,12 +162,7 @@ void AbstractObjectTool::showContextMenu(MapObjectItem *clickedObjectItem,
         return;
 
     const QList<MapObject*> &selectedObjects = mapDocument()->selectedObjects();
-
-    QList<ObjectGroup*> objectGroups;
-    foreach (Layer *layer, mapDocument()->map()->layers()) {
-        if (ObjectGroup *objectGroup = layer->asObjectGroup())
-            objectGroups.append(objectGroup);
-    }
+    const QList<ObjectGroup*> objectGroups = mapDocument()->map()->objectGroups();
 
     MapDocumentActionHandler *handler = MapDocumentActionHandler::instance();
 
@@ -176,20 +171,17 @@ void AbstractObjectTool::showContextMenu(MapObjectItem *clickedObjectItem,
     menu.addAction(handler->actionRemoveObjects());
 
     menu.addSeparator();
-    QAction *horizontalAction = menu.addAction(tr("Flip Horizontally"));
-    QAction *verticalAction = menu.addAction(tr("Flip Vertically"));
-    connect(horizontalAction, SIGNAL(triggered()), SLOT(flipHorizontally()));
-    connect(verticalAction, SIGNAL(triggered()), SLOT(flipVertically()));
+    menu.addAction(tr("Flip Horizontally"), this, SLOT(flipHorizontally()), QKeySequence(tr("X")));
+    menu.addAction(tr("Flip Vertically"), this, SLOT(flipVertically()), QKeySequence(tr("Y")));
 
-    menu.addSeparator();
-    QAction *raiseAction = menu.addAction(tr("Raise Object"));
-    QAction *lowerAction = menu.addAction(tr("Lower Object"));
-    QAction *raiseToTopAction = menu.addAction(tr("Raise Object to Top"));
-    QAction *lowerToBottomAction = menu.addAction(tr("Lower Object to Bottom"));
-    connect(raiseAction, SIGNAL(triggered()), SLOT(raise()));
-    connect(lowerAction, SIGNAL(triggered()), SLOT(lower()));
-    connect(raiseToTopAction, SIGNAL(triggered()), SLOT(raiseToTop()));
-    connect(lowerToBottomAction, SIGNAL(triggered()), SLOT(lowerToBottom()));
+    ObjectGroup *objectGroup = RaiseLowerHelper::sameObjectGroup(selection);
+    if (objectGroup && objectGroup->drawOrder() == ObjectGroup::IndexOrder) {
+        menu.addSeparator();
+        menu.addAction(tr("Raise Object"), this, SLOT(raise()), QKeySequence(tr("PgUp")));
+        menu.addAction(tr("Lower Object"), this, SLOT(lower()), QKeySequence(tr("PgDown")));
+        menu.addAction(tr("Raise Object to Top"), this, SLOT(raiseToTop()), QKeySequence(tr("Home")));
+        menu.addAction(tr("Lower Object to Bottom"), this, SLOT(lowerToBottom()), QKeySequence(tr("End")));
+    }
 
     if (objectGroups.size() > 1) {
         menu.addSeparator();

src/tiled/changeobjectgroupproperties.cpp

@@ -31,29 +31,32 @@ using namespace Tiled;
 using namespace Tiled::Internal;
 
 ChangeObjectGroupProperties::ChangeObjectGroupProperties(
-    MapDocument *mapDocument,
-    ObjectGroup *objectGroup,
-    const QColor &color)
+        MapDocument *mapDocument,
+        ObjectGroup *objectGroup,
+        const QColor &newColor,
+        ObjectGroup::DrawOrder newDrawOrder)
     : QUndoCommand(
         QCoreApplication::translate(
             "Undo Commands", "Change Object Layer Properties"))
     , mMapDocument(mapDocument)
     , mObjectGroup(objectGroup)
     , mUndoColor(objectGroup->color())
-    , mRedoColor(color)
+    , mRedoColor(newColor)
+    , mUndoDrawOrder(objectGroup->drawOrder())
+    , mRedoDrawOrder(newDrawOrder)
 {
 }
 
 void ChangeObjectGroupProperties::redo()
 {
     mObjectGroup->setColor(mRedoColor);
-    mMapDocument->mapObjectModel()->emitObjectsChanged(mObjectGroup->objects());
+    mObjectGroup->setDrawOrder(mRedoDrawOrder);
     mMapDocument->emitObjectGroupChanged(mObjectGroup);
 }
 
 void ChangeObjectGroupProperties::undo()
 {
     mObjectGroup->setColor(mUndoColor);
-    mMapDocument->mapObjectModel()->emitObjectsChanged(mObjectGroup->objects());
+    mObjectGroup->setDrawOrder(mUndoDrawOrder);
     mMapDocument->emitObjectGroupChanged(mObjectGroup);
 }

src/tiled/changeobjectgroupproperties.h

@@ -22,13 +22,12 @@
 #ifndef CHANGEOBJECTGROUPPROPERTIES_H
 #define CHANGEOBJECTGROUPPROPERTIES_H
 
+#include "objectgroup.h"
+
 #include <QColor>
 #include <QUndoCommand>
 
 namespace Tiled {
-
-class ObjectGroup;
-
 namespace Internal {
 
 class MapDocument;
@@ -45,7 +44,8 @@ class ChangeObjectGroupProperties : public QUndoCommand
      */
     ChangeObjectGroupProperties(MapDocument *mapDocument,
                                 ObjectGroup *objectGroup,
-                                const QColor &newColor);
+                                const QColor &newColor,
+                                ObjectGroup::DrawOrder newDrawOrder);
 
     void undo();
     void redo();
@@ -55,6 +55,8 @@ class ChangeObjectGroupProperties : public QUndoCommand
     ObjectGroup *mObjectGroup;
     const QColor mUndoColor;
     const QColor mRedoColor;
+    ObjectGroup::DrawOrder mUndoDrawOrder;
+    ObjectGroup::DrawOrder mRedoDrawOrder;
 };
 
 } // namespace Internal

src/tiled/createobjecttool.cpp

@@ -162,6 +162,7 @@ void CreateObjectTool::mouseMoved(const QPointF &pos,
 
         mNewMapObjectItem->mapObject()->setPosition(tileCoords);
         mNewMapObjectItem->syncWithMapObject();
+        mNewMapObjectItem->setZValue(10000); // sync may change it
         break;
     }
     case CreatePolygon:
@@ -337,6 +338,7 @@ void CreateObjectTool::startNewMapObject(const QPointF &pos,
     objectGroup->addObject(newMapObject);
 
     mNewMapObjectItem = new MapObjectItem(newMapObject, mapDocument());
+    mNewMapObjectItem->setZValue(10000); // same as the BrushItem
     mapScene()->addItem(mNewMapObjectItem);
 }
 

src/tiled/mapdocument.h

@@ -410,7 +410,7 @@ inline void MapDocument::emitTileTerrainChanged(const QList<Tile *> &tiles)
 
 /**
  * Emits the objectGroupChanged signal, should be called when changing the
- * color of an object group.
+ * color or drawing order of an object group.
  */
 inline void MapDocument::emitObjectGroupChanged(ObjectGroup *objectGroup)
 {

src/tiled/mapobjectitem.cpp

@@ -225,9 +225,12 @@ void MapObjectItem::syncWithMapObject()
     bounds.translate(-pixelPos);
 
     setPos(pixelPos);
-//    setZValue(pixelPos.y());
     setRotation(mObject->rotation());
 
+    if (ObjectGroup *objectGroup = mObject->objectGroup())
+        if (objectGroup->drawOrder() == ObjectGroup::TopDownOrder)
+            setZValue(pixelPos.y());
+
     mSyncing = true;
 
     if (mBoundingRect != bounds) {