Refactored and fixed saving scene states between modes in photo editor.

Telegram/SourceFiles/editor/editor_paint.cpp

@@ -153,86 +153,30 @@ void Paint::applyTransform(QRect geometry, int angle, bool flipped) {
 }
 
 std::shared_ptr<Scene> Paint::saveScene() const {
-	_scene->saveItemsState(SaveState::Save);
+	_scene->save(SaveState::Save);
-	_scene->clearSelection();
 	return _scene->items().empty()
-		? nullptr
-		: ranges::none_of(_scene->items(), &QGraphicsItem::isVisible)
 		? nullptr
 		: _scene;
 }
 
 void Paint::restoreScene() {
-	_scene->restoreItemsState(SaveState::Save);
+	_scene->restore(SaveState::Save);
 }
 
 void Paint::cancel() {
-	_scene->restoreItemsState(SaveState::Keep);
+	_scene->restore(SaveState::Keep);
-	_scene->clearSelection();
-	_scene->cancelDrawing();
-
-	const auto filtered = _scene->items(Qt::AscendingOrder);
-	if (filtered.empty()) {
-		return;
-	}
-
-	for (const auto &item : filtered) {
-		const auto it = ranges::find(
-			_previousItems,
-			item,
-			&SavedItem::item);
-		if (it == end(_previousItems)) {
-			_scene->removeItem(item);
-		} else {
-			it->item->setVisible(!it->undid);
-		}
-	}
-
-	_itemsToRemove.clear();
 }
 
 void Paint::keepResult() {
-	_scene->saveItemsState(SaveState::Keep);
+	_scene->save(SaveState::Keep);
-	_scene->clearSelection();
-	_scene->cancelDrawing();
-
-	for (const auto &item : _itemsToRemove) {
-		_scene->removeItem(item);
-	}
-	_itemsToRemove.clear();
-
-	const auto items = _scene->items();
-	_previousItems = ranges::views::all(
-		items
-	) | ranges::views::transform([=](ItemPtr i) -> SavedItem {
-		return { i, !i->isVisible() };
-	}) | ranges::to_vector;
 }
 
 void Paint::clearRedoList() {
-	const auto items = _scene->items(Qt::AscendingOrder);
+	_scene->clearRedoList();
-	auto &&filtered = ranges::views::all(
-		items
-	) | ranges::views::filter(
-		[=](const ItemPtr &i) { return isItemHidden(i); }
-	);
-
-	ranges::for_each(std::move(filtered), [&](ItemPtr item) {
-		item->hide();
-		_itemsToRemove.push_back(item);
-	});
 
 	_hasRedo = false;
 }
 
-bool Paint::isItemHidden(const ItemPtr &item) const {
-	return !item->isVisible() && !isItemToRemove(item);
-}
-
-bool Paint::isItemToRemove(const ItemPtr &item) const {
-	return ranges::contains(_itemsToRemove, item);
-}
-
 void Paint::updateUndoState() {
 	_hasUndo = _scene->hasUndo();
 	_hasRedo = _scene->hasRedo();

Telegram/SourceFiles/editor/editor_paint.h

@@ -51,9 +51,6 @@ private:
 
 	void clearRedoList();
 
-	bool isItemToRemove(const std::shared_ptr<QGraphicsItem> &item) const;
-	bool isItemHidden(const std::shared_ptr<QGraphicsItem> &item) const;
-
 	const std::shared_ptr<Controllers> _controllers;
 	const std::shared_ptr<Scene> _scene;
 	const base::unique_qptr<QGraphicsView> _view;
@@ -65,9 +62,6 @@ private:
 		float64 zoom = 0.;
 	} _transform;
 
-	std::vector<SavedItem> _previousItems;
-	std::vector<std::shared_ptr<QGraphicsItem>> _itemsToRemove;
-
 	rpl::variable<bool> _hasUndo = true;
 	rpl::variable<bool> _hasRedo = true;
 

Telegram/SourceFiles/editor/photo_editor_content.cpp

@@ -107,8 +107,10 @@ void PhotoEditorContent::applyModifications(
 void PhotoEditorContent::save(PhotoModifications &modifications) {
 	modifications.crop = _crop->saveCropRect();
 	_paint->keepResult();
+
+	const auto savedScene = _paint->saveScene();
 	if (!modifications.paint) {
-		modifications.paint = _paint->saveScene();
+		modifications.paint = savedScene;
 	}
 }
 

Telegram/SourceFiles/editor/scene/scene.cpp

@@ -66,11 +66,8 @@ void Scene::removeItem(not_null<QGraphicsItem*> item) {
 	removeItem(*it);
 }
 
-void Scene::removeItem(const std::shared_ptr<QGraphicsItem> &item) {
+void Scene::removeItem(const ItemPtr &item) {
-	// Scene loses ownership of an item.
+	item->setStatus(NumberedItem::Status::Removed);
-	QGraphicsScene::removeItem(item.get());
-
-	_items.erase(ranges::remove(_items, item), end(_items));
 	_removesItem.fire({});
 }
 
@@ -147,22 +144,6 @@ void Scene::updateZoom(float64 zoom) {
 	}
 }
 
-void Scene::saveItemsState(SaveState state) {
-	for (const auto &item : items()) {
-		if (item->type() >= ItemBase::Type) {
-			static_cast<ItemBase*>(item.get())->save(state);
-		}
-	}
-}
-
-void Scene::restoreItemsState(SaveState state) {
-	for (const auto &item : items()) {
-		if (item->type() >= ItemBase::Type) {
-			static_cast<ItemBase*>(item.get())->restore(state);
-		}
-	}
-}
-
 bool Scene::hasUndo() const {
 	return ranges::any_of(_items, &NumberedItem::isNormalStatus);
 }
@@ -189,6 +170,55 @@ void Scene::performRedo() {
 	}
 }
 
+void Scene::removeIf(Fn<bool(const ItemPtr &)> proj) {
+	auto copy = std::vector<ItemPtr>();
+	for (const auto &item : _items) {
+		const auto toRemove = proj(item);
+		if (toRemove) {
+			// Scene loses ownership of an item.
+			// It seems for some reason this line causes a crash. =(
+			// QGraphicsScene::removeItem(item.get());
+		} else {
+			copy.push_back(item);
+		}
+	}
+	_items = std::move(copy);
+}
+
+void Scene::clearRedoList() {
+	for (const auto &item : _items) {
+		if (item->isUndidStatus()) {
+			item->setStatus(NumberedItem::Status::Removed);
+		}
+	}
+}
+
+void Scene::save(SaveState state) {
+	removeIf([](const ItemPtr &item) {
+		return item->isRemovedStatus()
+			&& !item->hasState(SaveState::Keep)
+			&& !item->hasState(SaveState::Save);
+	});
+
+	for (const auto &item : _items) {
+		item->save(state);
+	}
+	clearSelection();
+	cancelDrawing();
+}
+
+void Scene::restore(SaveState state) {
+	removeIf([=](const ItemPtr &item) {
+		return !item->hasState(state);
+	});
+
+	for (const auto &item : _items) {
+		item->restore(state);
+	}
+	clearSelection();
+	cancelDrawing();
+}
+
 Scene::~Scene() {
 	// Prevent destroying by scene of all items.
 	QGraphicsScene::removeItem(_canvas.get());

Telegram/SourceFiles/editor/scene/scene.h

@@ -35,7 +35,7 @@ public:
 		Qt::SortOrder order = Qt::DescendingOrder) const;
 	void addItem(ItemPtr item);
 	void removeItem(not_null<QGraphicsItem*> item);
-	void removeItem(const std::shared_ptr<QGraphicsItem> &item);
+	void removeItem(const ItemPtr &item);
 	[[nodiscard]] rpl::producer<> addsItem() const;
 	[[nodiscard]] rpl::producer<> removesItem() const;
 
@@ -47,19 +47,22 @@ public:
 
 	void cancelDrawing();
 
-	void saveItemsState(SaveState state);
-	void restoreItemsState(SaveState state);
-
 	[[nodiscard]] bool hasUndo() const;
 	[[nodiscard]] bool hasRedo() const;
 
 	void performUndo();
 	void performRedo();
+
+	void save(SaveState state);
+	void restore(SaveState state);
+
+	void clearRedoList();
 protected:
 	void mousePressEvent(QGraphicsSceneMouseEvent *event) override;
 	void mouseReleaseEvent(QGraphicsSceneMouseEvent *event) override;
 	void mouseMoveEvent(QGraphicsSceneMouseEvent *event) override;
 private:
+	void removeIf(Fn<bool(const ItemPtr &)> proj);
 	const std::shared_ptr<ItemCanvas> _canvas;
 	const std::shared_ptr<float64> _lastZ;
 

Telegram/SourceFiles/editor/scene/scene_item_base.cpp

@@ -60,6 +60,10 @@ bool NumberedItem::isUndidStatus() const {
 	return _status == Status::Undid;
 }
 
+bool NumberedItem::isRemovedStatus() const {
+	return _status == Status::Removed;
+}
+
 void NumberedItem::save(SaveState state) {
 }
 
@@ -413,16 +417,13 @@ void ItemBase::applyData(const Data &data) {
 }
 
 void ItemBase::save(SaveState state) {
-	if (state == SaveState::Keep) {
+	const auto z = zValue();
-		const auto z = zValue();
+	auto &saved = (state == SaveState::Keep) ? _keeped : _saved;
-		_keeped = {
+	saved = {
-			.data = generateData(),
+		.data = generateData(),
-			.zValue = z,
+		.zValue = z,
-			.visible = isVisible(),
+		.status = status(),
-		};
+	};
-	} else if (state == SaveState::Save) {
-		_saved = _keeped;
-	}
 }
 
 void ItemBase::restore(SaveState state) {
@@ -432,7 +433,7 @@ void ItemBase::restore(SaveState state) {
 	const auto &saved = (state == SaveState::Keep) ? _keeped : _saved;
 	applyData(saved.data);
 	setZValue(saved.zValue);
-	setVisible(saved.visible);
+	setStatus(saved.status);
 }
 
 bool ItemBase::hasState(SaveState state) const {

Telegram/SourceFiles/editor/scene/scene_item_base.h

@@ -27,6 +27,7 @@ public:
 	enum class Status {
 		Normal,
 		Undid,
+		Removed,
 	};
 
 	enum { Type = UserType + 1 };
@@ -40,6 +41,7 @@ public:
 	void setStatus(Status status);
 	[[nodiscard]] bool isNormalStatus() const;
 	[[nodiscard]] bool isUndidStatus() const;
+	[[nodiscard]] bool isRemovedStatus() const;
 
 	virtual void save(SaveState state);
 	virtual void restore(SaveState state);
@@ -135,7 +137,7 @@ private:
 	struct {
 		Data data;
 		float64 zValue = 0.;
-		bool visible = true;
+		NumberedItem::Status status;
 	} _saved, _keeped;
 
 	struct {

Telegram/SourceFiles/editor/scene/scene_item_line.cpp

@@ -39,23 +39,24 @@ bool ItemLine::collidesWithPath(
 }
 
 void ItemLine::save(SaveState state) {
-	if (state == SaveState::Keep) {
+	auto &saved = (state == SaveState::Keep) ? _keeped : _saved;
-		_keeped = true;
+	saved = {
-	} else if (state == SaveState::Save) {
+		.saved = true,
-		_saved = true;
+		.status = status(),
-	}
+	};
 }
 
 void ItemLine::restore(SaveState state) {
+	if (!hasState(state)) {
+		return;
+	}
+	const auto &saved = (state == SaveState::Keep) ? _keeped : _saved;
+	setStatus(saved.status);
 }
 
 bool ItemLine::hasState(SaveState state) const {
-	if (state == SaveState::Keep) {
+	const auto &saved = (state == SaveState::Keep) ? _keeped : _saved;
-		return _keeped;
+	return saved.saved;
-	} else if (state == SaveState::Save) {
-		return _saved;
-	}
-	return false;
 }
 
 } // namespace Editor

Telegram/SourceFiles/editor/scene/scene_item_line.h

@@ -34,7 +34,10 @@ private:
 	const QPixmap _pixmap;
 	const QRectF _rect;
 
-	bool _saved, _keeped = false;
+	struct {
+		bool saved = false;
+		NumberedItem::Status status;
+	} _saved, _keeped;
 
 };