View Issue Details

IDProjectCategoryView StatusLast Update
0012794ScribusShape Drawingpublic2015-11-14 13:08
ReporterFirasH Assigned Tojghali  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Platformx86_64OSopenSUSEOS Version13.1
Product Version1.5.0svn 
Target Version1.5.1Fixed in Version1.5.1svn 
Summary0012794: It is possible to create Bezier Curves with only 1 node (2)
DescriptionIt is possible to create Bezier Curves with only 1 node.
Steps To Reproduce0) Create a new document
1) Go to: Insert > Bezier Curve
2) Click on canvas to start drawing a Bezier Curve
3) Click on "Select Item (C)"

Now there is a polyline with a single node (should not be possible).
Additional Information1.4.5.svn (19594)
1.5.0.svn (19611)
TagsHOST-Oman, patch_tested_ok
PatchYes

Activities

FirasH

2014-11-13 14:44

developer   ~0034202

"Esc" button while in "modeDrawBezierCurve" is managed in scribus.cpp, line 1652.

Enabling "toolsSelect" while in "modeDrawBezierCurve" is managed in appmodehelper.cpp.
Mode is set to "modeNormal" but in the function "setApplicationMode" a part of the code is missing.

Maybe porting that part of "scribus.cpp" to "appmodehelper.cpp" would make mode management easier?

Kunda

2015-01-26 15:04

updater   ~0034388

Confirmed OSX 10.9.5 1.5svn r19768 Qt5.4

Mazoon

2015-10-28 08:29

reporter   ~0037021

Last edited: 2015-10-28 08:35

Patch was uploaded to fix the possibility of creating only 1 node (which is not supposed to be valid).

p.s appmodehelper.cpp file now manage all cases.

Mazoon

2015-10-28 08:30

reporter  

Issue12794.patch (2,182 bytes)   
Index: scribus/appmodehelper.cpp
===================================================================
--- scribus/appmodehelper.cpp	(revision 20498)
+++ scribus/appmodehelper.cpp	(working copy)
@@ -127,9 +127,32 @@
 
 	switch (oldMode)
 	{
-		case modeDrawBezierLine:
+		case modeDrawBezierLine: {
+			//if No nodes were created
+			if (currItem == NULL) {
+				setSpecialEditMode(false);
+				break;
+			}
+
+			//When only one node(size=2) was created; it's not a valid line(min valid PoLine size is 6), delete it
+			if ((currItem->PoLine.size() < 6) && (currItem->PoLine.size() >= 2) ) {
+				doc->view()->Deselect(false);
+				doc->Items->removeOne(currItem);
+			}
+
+			else {
+				doc->SizeItem(currItem->PoLine.WidthHeight().x(), currItem->PoLine.WidthHeight().y(), currItem, false, false);
+				currItem->setPolyClip(qRound(qMax(currItem->lineWidth() / 2.0, 1.0)));
+				doc->AdjustItemSize(currItem);
+ 				currItem->ContourLine = currItem->PoLine.copy();
+				currItem->ClipEdited = true;
+				currItem->FrameType = 3;
+				scmw->slotDocCh();
+			}
+			doc->view()->FirstPoly = true;
 			setSpecialEditMode(false);
 			break;
+		}
 		case modeEdit:
 			{
 				if (newMode != modeEdit)
Index: scribus/scribus.cpp
===================================================================
--- scribus/scribus.cpp	(revision 20498)
+++ scribus/scribus.cpp	(working copy)
@@ -1715,24 +1715,6 @@
 					view->requestMode(modeNormal);
 					break;
 				case modeDrawBezierLine:
-					currItem->PoLine.resize(qMax(0, static_cast<int>(currItem->PoLine.size())-2));
-					if (currItem->PoLine.size() < 4)
-					{
-						view->Deselect(false);
-						doc->Items->removeOne(currItem);
-					}
-					else
-					{
-						doc->SizeItem(currItem->PoLine.WidthHeight().x(), currItem->PoLine.WidthHeight().y(), currItem, false, false);
-						currItem->setPolyClip(qRound(qMax(currItem->lineWidth() / 2.0, 1.0)));
-						doc->AdjustItemSize(currItem);
-						currItem->ContourLine = currItem->PoLine.copy();
-						currItem->ClipEdited = true;
-						currItem->FrameType = 3;
-						slotDocCh();
-					}
-					view->FirstPoly = true;
-					break;
 				default:
 					if (currItem->Sizing)
 					{
Issue12794.patch (2,182 bytes)   

Kunda

2015-10-28 15:44

updater   ~0037048

Mazoon, because of influx of so many patches, you may want to start this thread on the scribus-dev list to stimulate conversation. Thanks!

FirasH

2015-10-30 22:23

developer   ~0037130

Applying the patch fixes the issue here reported! Thanks Mazoon!

cbradney

2015-11-03 15:47

administrator   ~0037192

The use of appmodehelper is not really the right thing to do. Its supposed to be there to help manage action enablement mainly. What was the technical reason that you had to move to using it instead of scribus.cpp?

Mazoon

2015-11-03 18:31

reporter   ~0037196

To be honest, I didn't see (at that time) the point of using scribus.cpp, since appmodehelper deals directly with oldMode/newMode. appmodehelper was straight forward, if Beizer Curve became an oldMode, just do this.
Plus, getting rid of the remnants of an old mode seemed to fit appmodehelper purpose.

(sorry! I'm new to scribus world)

cbradney

2015-11-03 20:29

administrator   ~0037199

Thanks, I'll review it again.

jghali

2015-11-03 21:21

administrator   ~0037200

Each canvas mode has a desactivate() function where any necessary action before canvas mode exit can occur. So I agree with Craig : I do not think using appmodehelper is the right thing to do.

Mazoon

2015-11-08 07:35

reporter  

Issue12794v2.patch (2,554 bytes)   
Index: scribus/appmodehelper.cpp
===================================================================
--- scribus/appmodehelper.cpp	(revision 20498)
+++ scribus/appmodehelper.cpp	(working copy)
@@ -128,8 +128,17 @@
 	switch (oldMode)
 	{
 		case modeDrawBezierLine:
+		{
+			//if No nodes were created
+			if (currItem == NULL) {
+				setSpecialEditMode(false);
+				break;
+			}
+			scmw->slotDocCh();
+			doc->view()->FirstPoly = true;
 			setSpecialEditMode(false);
 			break;
+		}
 		case modeEdit:
 			{
 				if (newMode != modeEdit)
Index: scribus/canvasmode_drawbezier.cpp
===================================================================
--- scribus/canvasmode_drawbezier.cpp	(revision 20498)
+++ scribus/canvasmode_drawbezier.cpp	(working copy)
@@ -151,6 +151,25 @@
 		undoManager->action(target, is);
 	}
 	m_view->setRedrawMarkerShown(false);
+
+	if(currItem != NULL)
+	{
+		//When only one node(size=2) was created; it's not a valid line(min valid PoLine size is 6), delete it
+		if ((currItem->PoLine.size() < 6) && (currItem->PoLine.size() >= 2) )
+		{
+			m_view->Deselect(false);
+			m_doc->Items->removeOne(currItem);
+		}
+		else
+		{
+			m_doc->SizeItem(currItem->PoLine.WidthHeight().x(), currItem->PoLine.WidthHeight().y(), currItem, false, false);
+			currItem->setPolyClip(qRound(qMax(currItem->lineWidth() / 2.0, 1.0)));
+			m_doc->AdjustItemSize(currItem);
+			currItem->ContourLine = currItem->PoLine.copy();
+			currItem->ClipEdited = true;
+			currItem->FrameType = 3;
+		}
+	}
 }
 
 void BezierMode::mouseDoubleClickEvent(QMouseEvent *m)
Index: scribus/scribus.cpp
===================================================================
--- scribus/scribus.cpp	(revision 20498)
+++ scribus/scribus.cpp	(working copy)
@@ -1715,24 +1715,6 @@
 					view->requestMode(modeNormal);
 					break;
 				case modeDrawBezierLine:
-					currItem->PoLine.resize(qMax(0, static_cast<int>(currItem->PoLine.size())-2));
-					if (currItem->PoLine.size() < 4)
-					{
-						view->Deselect(false);
-						doc->Items->removeOne(currItem);
-					}
-					else
-					{
-						doc->SizeItem(currItem->PoLine.WidthHeight().x(), currItem->PoLine.WidthHeight().y(), currItem, false, false);
-						currItem->setPolyClip(qRound(qMax(currItem->lineWidth() / 2.0, 1.0)));
-						doc->AdjustItemSize(currItem);
-						currItem->ContourLine = currItem->PoLine.copy();
-						currItem->ClipEdited = true;
-						currItem->FrameType = 3;
-						slotDocCh();
-					}
-					view->FirstPoly = true;
-					break;
 				default:
 					if (currItem->Sizing)
 					{
Issue12794v2.patch (2,554 bytes)   

Mazoon

2015-11-08 07:48

reporter   ~0037310

Thank you cbradney and jghali for your feedbacks. I uploaded another patch where the modifications happen in canvasmode_drawbezier.cpp in deactivate() as jghali suggested. So:
in canvasmode_drawbezier.cpp: before exiting modeDrawBezier, it will check if there's only one node, if so, it will remove it.
in appmodehelper.cpp: if no item, it will exist normally without setting that the doc changed. Otherwise it will set that doc has changed.
in scribus.cpp: there is no more need to deal with ESC key individually.

Kunda

2015-11-10 16:11

updater   ~0037390

Devs, please review patch.

jghali

2015-11-11 17:23

administrator   ~0037436

The patch doesn't work. The single node object can still be seen in the document outline palette. Additionally there is an unwanted undo action which is created and can be seen in undo history.

jghali

2015-11-12 19:44

administrator   ~0037452

I fixed the patch. Now it works as expected. Thanks for your work Mazoon.

Kunda

2015-11-13 01:23

updater   ~0037455

Fixed.
Mazoons patch along with refinements was committed in r20549
Thanks to all who worked on this, especially Mazoon.

Kunda

2015-11-14 13:08

updater   ~0037476

Closing

Issue History

Date Modified Username Field Change
2014-10-27 16:34 FirasH New Issue
2014-11-13 14:44 FirasH Note Added: 0034202
2015-01-26 15:04 Kunda Note Added: 0034388
2015-01-26 15:04 Kunda Status new => confirmed
2015-01-26 15:04 Kunda Target Version => 1.5.1
2015-10-28 08:29 Mazoon Note Added: 0037021
2015-10-28 08:30 Mazoon File Added: Issue12794.patch
2015-10-28 08:35 Mazoon Note Edited: 0037021
2015-10-28 11:44 FirasH Patch No => Yes
2015-10-28 15:44 Kunda Note Added: 0037048
2015-10-30 22:23 FirasH Note Added: 0037130
2015-11-03 15:23 Kunda Tag Attached: HOST-Oman
2015-11-03 15:23 Kunda Tag Attached: patch_tested_ok
2015-11-03 15:47 cbradney Note Added: 0037192
2015-11-03 18:31 Mazoon Note Added: 0037196
2015-11-03 20:29 cbradney Note Added: 0037199
2015-11-03 21:21 jghali Note Added: 0037200
2015-11-05 12:23 Kunda Sticky Issue No => Yes
2015-11-08 07:35 Mazoon File Added: Issue12794v2.patch
2015-11-08 07:48 Mazoon Note Added: 0037310
2015-11-09 12:59 Kunda Summary It is possible to create Bezier Curves with only 1 node (2) => [Patch] It is possible to create Bezier Curves with only 1 node (2)
2015-11-10 16:11 Kunda Note Added: 0037390
2015-11-11 17:23 jghali Note Added: 0037436
2015-11-12 19:44 jghali Note Added: 0037452
2015-11-12 19:44 jghali Status confirmed => resolved
2015-11-12 19:44 jghali Fixed in Version => 1.5.1svn
2015-11-12 19:44 jghali Resolution open => fixed
2015-11-12 19:44 jghali Assigned To => jghali
2015-11-12 20:09 jghali Summary [Patch] It is possible to create Bezier Curves with only 1 node (2) => It is possible to create Bezier Curves with only 1 node (2)
2015-11-12 20:09 jghali Sticky Issue Yes => No
2015-11-13 01:23 Kunda Note Added: 0037455
2015-11-14 13:08 Kunda Note Added: 0037476
2015-11-14 13:08 Kunda Status resolved => closed