View Issue Details

IDProjectCategoryView StatusLast Update
0015801ScribusGeneralpublic2019-12-08 21:24
Reporterale Assigned Toale  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Product Version1.5.6.svn 
Target VersionFixed in Version1.5.6.svn 
Summary0015801: multiple duplicate on pages: allow duplicating items from facing pages
Description- create a facing pages document with the first page left and six pages
- create a text frame on page 1
- create a linked text frane on page 2
- select both frames
- item > duplicate > multiple duplicate > by pages > odd following pages + with linked text


this should create a chain of six linked texts from page 1 to page 6.
currently, scribus crashes.

in the first version of the patch it used to create a chain of 5 linked text that excluded the text frame on page 2. (which was also not correct)
TagsNo tags attached.
PatchYes

Activities

ale

2019-09-03 12:08

manager   ~0046619

i can have a look at it...

ale

2019-09-03 13:40

manager   ~0046621

Last edited: 2019-09-04 08:02

View 3 revisions

notes about the implementation:

- odd and even duplication are disabled when items are on multiple pages
- the user has to do something "intelligent", otherwise useless things will happen.
- the main target is to allow duplicating the linked frames from facing pages. with linking.
- i've removed the check for the chain of text frames being on the same page

ale

2019-09-04 08:01

manager   ~0046622

this patch is a partial solution that makes scribus do the right thing when the selection and the duplication settings are sane.

once we know how the feature is used, we can add further checks:

- sanitize the manual range (currently, the range can only be a list of pages. all other value will lead to "useless" results; it might even be better to disable it and add a field with the _maximal_ number of copies)
- when the document has facing pages, the number of pages in the selection must be even and the selection must start on an odd page (otherwise you get "useless" results)
- when linking text frames, the first text frame in the selection will be used: we probably cannot change this (without a big UI and cognitive overhead). how can we explain this to the user (should we issue a warning if the selection does not look "correct")
- if the text being linked is in a chain, we simply link the first duplicated text frame in the selection with the last frame in the preview copy (or the original): how can we explain this to the user? how can we "easily" detect an ill-defined selection?

i can continue working on this patch, if any of the points above should be done NOW...


jean, can i suggest that you review the patch on gitlab?

https://gitlab.com/scribus/scribus/merge_requests/3

multiple-duplicate-facing-pages.diff (4,125 bytes)
diff --git a/scribus/scribusdoc.cpp b/scribus/scribusdoc.cpp
index a7591fbae8d4754bad6496821562793a50065c6c..a04c3615180dd0783bba58903f8d9e196db92fd4 100644
--- a/scribus/scribusdoc.cpp
+++ b/scribus/scribusdoc.cpp
@@ -13609,7 +13609,30 @@ void ScribusDoc::multipleDuplicateByPage(const ItemMultipleDuplicateData& dialog
 	std::vector<int> pages;
 	QString pageRange;
 
-	if (dialogData.pageSelection == 1)
+	if (!m_Selection->itemsAreSamePage())
+	{
+		int firstPage = Pages->count(), lastPage = 0;
+		for (auto item: selection.items())
+		{
+			if (item->OwnPage < firstPage)
+				firstPage = item->OwnPage;
+			if (item->OwnPage > lastPage)
+				lastPage = item->OwnPage;
+		}
+		setCurrentPage(Pages->at(firstPage));
+		int pageSpread = lastPage - firstPage + 1;
+		QStringList pageList;
+		if (dialogData.pageSelection == 1)
+			for (int i = lastPage + 2; i < Pages->count(); i += pageSpread)
+				pageList << QString::number(i);
+		else if (dialogData.pageSelection == 4)
+		{
+			// TODO: what to do with manual selections?
+			pageRange = dialogData.pageRange;
+		}
+		pageRange = pageList.join(',');
+	}
+	else if (dialogData.pageSelection == 1)
 		pageRange = QString("%1-%2").arg(currPageNumber + 2).arg(Pages->count());
 	else if ((dialogData.pageSelection == 2) || dialogData.pageSelection == 3)
 	{
@@ -13639,15 +13662,10 @@ void ScribusDoc::multipleDuplicateByPage(const ItemMultipleDuplicateData& dialog
 			// get the first text frame in the selection, the first frame in the chain
 			if (item->itemType() != PageItem::TextFrame)
 				continue;
-			if (item->isInChain() && item->areNextInChainOnSamePage())
-			{
+			if (item->isInChain())
 				lastInChain = item->lastInChain();
-				assert( !lastInChain->nextInChain() );
-			}
 			else
-			{
 				lastInChain = item;
-			}
 			break;
 		}
 	}
diff --git a/scribus/selection.cpp b/scribus/selection.cpp
index 52206db0551af3b65fed04a2f95d3855519fbd6e..6f750c88c18cfa51a230f1d16b95cdc398435ec2 100644
--- a/scribus/selection.cpp
+++ b/scribus/selection.cpp
@@ -532,6 +532,24 @@ bool Selection::itemsAreSameType() const
 	return true;
 }
 
+bool Selection::itemsAreSamePage() const
+{
+	//CB Putting count=1 before isempty test as its probably the most likely, given our view code.
+	if (m_SelList.count() == 1)
+		return true;
+	if (m_SelList.isEmpty())
+		return false;
+	auto it = m_SelList.begin();
+	auto itend = m_SelList.end();
+	auto ownPage = (*it)->OwnPage;
+	for ( ; it!=itend ; ++it)
+	{
+		if ((*it)->OwnPage != ownPage)
+			return false;
+	}
+	return true;
+}
+
 int Selection::objectsLayer() const
 {
 	if (m_SelList.isEmpty())
diff --git a/scribus/selection.h b/scribus/selection.h
index 1db11e9f949b28e701ee2159621884b85fc4fe7c..f14abe5cef62dacea98a86d6dc1b98a9182cfeeb 100644
--- a/scribus/selection.h
+++ b/scribus/selection.h
@@ -158,6 +158,8 @@ class SCRIBUS_API Selection : public QObject
 		bool containsItemType(PageItem::ItemType type) const;
 		//!\brief Test to see if all items in the selection are the same typedef
 		bool itemsAreSameType() const;
+		//!\brief Test to see if all items in the selection are the same typedef
+		bool itemsAreSamePage() const;
 
 		/**
 		 * \brief get the layer ID of items in the selection
diff --git a/scribus/ui/multipleduplicate.cpp b/scribus/ui/multipleduplicate.cpp
index 44af91d95ae73acefaa6f4c43dc7c6930fd57c84..ad296271c377c40763e13104f26a9402314a9519 100644
--- a/scribus/ui/multipleduplicate.cpp
+++ b/scribus/ui/multipleduplicate.cpp
@@ -15,6 +15,7 @@ for which a new license (GPL+exception) is in place.
 
 #include "iconmanager.h"
 #include "scribusdoc.h"
+#include "selection.h"
 #include "scrspinbox.h"
 #include "units.h"
 #include "ui/createrange.h"
@@ -43,6 +44,12 @@ MultipleDuplicate::MultipleDuplicate(QWidget* parent, ScribusDoc *doc) : QDialog
 	rotationSpinBox->setValues(-180.0, 180.0, 6, 0.0);
 	rotationSpinBox->setDecimals(1);
 	rotationSpinBox->setNewUnit(6);
+
+	if (!m_Doc->m_Selection->itemsAreSamePage())
+	{
+		radioButtonPageOdd->setEnabled(false);
+		radioButtonPageEven->setEnabled(false);
+	}
 	
 	createGapRadioButton->setChecked(true);
 	setCopiesGap();

jghali

2019-09-08 11:42

administrator   ~0046641

I had a look at your patch and found an issue: a crash is triggered when the selection contains an object outside of a page.

ale

2019-09-09 07:01

manager   ~0046642

ok, i'll have a look at that!

thanks for reviewing!

Issue History

Date Modified Username Field Change
2019-09-03 12:08 ale New Issue
2019-09-03 12:08 ale Note Added: 0046619
2019-09-03 13:40 ale Note Added: 0046621
2019-09-03 15:31 jghali Category - => General
2019-09-03 15:39 jghali Summary multiple duplcaite on pages: allow duplicating items from facing pages => multiple duplicate on pages: allow duplicating items from facing pages
2019-09-03 16:04 ale Note Edited: 0046621 View Revisions
2019-09-04 08:01 ale File Added: multiple-duplicate-facing-pages.diff
2019-09-04 08:01 ale Note Added: 0046622
2019-09-04 08:02 ale Note Edited: 0046621 View Revisions
2019-09-04 08:02 ale Patch No => Yes
2019-09-08 11:42 jghali Note Added: 0046641
2019-09-09 07:01 ale Note Added: 0046642
2019-09-09 14:25 jghali Assigned To => ale
2019-09-09 14:25 jghali Status new => resolved
2019-09-09 14:25 jghali Resolution open => fixed
2019-09-09 14:25 jghali Fixed in Version => 1.5.6.svn
2019-12-08 21:24 cbradney Status resolved => closed