View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0012794 | Scribus | Shape Drawing | public | 2014-10-27 16:34 | 2015-11-14 13:08 |
| Reporter | FirasH | Assigned To | jghali | ||
| Priority | normal | Severity | minor | Reproducibility | always |
| Status | closed | Resolution | fixed | ||
| Platform | x86_64 | OS | openSUSE | OS Version | 13.1 |
| Product Version | 1.5.0svn | ||||
| Target Version | 1.5.1 | Fixed in Version | 1.5.1svn | ||
| Summary | 0012794: It is possible to create Bezier Curves with only 1 node (2) | ||||
| Description | It is possible to create Bezier Curves with only 1 node. | ||||
| Steps To Reproduce | 0) 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 Information | 1.4.5.svn (19594) 1.5.0.svn (19611) | ||||
| Tags | HOST-Oman, patch_tested_ok | ||||
| Patch | Yes | ||||
|
|
"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? |
|
|
Confirmed OSX 10.9.5 1.5svn r19768 Qt5.4 |
|
|
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. |
|
|
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)
{
|
|
|
Mazoon, because of influx of so many patches, you may want to start this thread on the scribus-dev list to stimulate conversation. Thanks! |
|
|
Applying the patch fixes the issue here reported! Thanks Mazoon! |
|
|
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? |
|
|
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) |
|
|
Thanks, I'll review it again. |
|
|
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. |
|
|
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)
{
|
|
|
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. |
|
|
Devs, please review patch. |
|
|
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. |
|
|
I fixed the patch. Now it works as expected. Thanks for your work Mazoon. |
|
|
Fixed. Mazoons patch along with refinements was committed in r20549 Thanks to all who worked on this, especially Mazoon. |
|
|
Closing |
| 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 |
