View Issue Details

IDProjectCategoryView StatusLast Update
0001668DrawingBugpublic2014-08-19 14:06
Reportermghansen256 Assigned Toyorik  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
PlatformLinuxOSKubuntuOS Version14.04
Fixed in Version0.15 
Summary0001668: Projections are always added to the first drawing page
DescriptionI have two drawing sheets in my document. According to the wiki: "In the same selection, add the page object you want to draw your objects to. If there is no existing page, a new one will be created. If you didn't select a page but there is at least one in the document, the first found one will be used to draw to."

However, even if I select a part and the second of the two pages, the projection is always created on the first page.

http://www.freecadweb.org/wiki/index.php?title=Draft_Drawing

Additional InformationOS: Ubuntu 14.04.1 LTS
Word size: 64-bit
Version: 0.14.3702 (Git)
Branch: releases/FreeCAD-0-14
Hash: b3368125c63289ec8ce9faec2b2ae4c78d436406
Python version: 2.7.6
Qt version: 4.8.6
Coin version: 4.0.0a
SoQt version: 1.6.0a
OCC version: 6.7.0
TagsNo tags attached.

Activities

yorik

2014-08-07 14:20

administrator   ~0004924

Indeed, the "add new view" code doesn't scan the selection for a page, contrarily to what's on the wiki. I'll try to fix that (as well as the other Drawing tools that put stuff on a page, that should also do the same).

mghansen256

2014-08-09 20:41

reporter   ~0004935

Last edited: 2014-08-09 20:43

Hi Yorik,

thanks for looking into this! However, ortho views are still always added to the first page. Looking at your commit, the code looks for the currently selected page, but the page is the not given as a parameter to TaskDlgOrthoViews. TaskDlgOrthoViews then just takes the first page from the document when it initializes.

void CmdDrawingOrthoViews::activated(int iMsg)
{
    std::vector<App::DocumentObject*> shapes = getSelection().getObjectsOfType(Part::Feature::getClassTypeId());
    if (shapes.size() != 1) {
        QMessageBox::warning(Gui::getMainWindow(), QObject::tr("Wrong selection"),
            QObject::tr("Select a Part object."));
        return;
    }

    // pages is local to this function
    std::vector<App::DocumentObject*> pages = getSelection().getObjectsOfType(Drawing::FeaturePage::getClassTypeId());
    if (pages.empty()) {
        pages = this->getDocument()->getObjectsOfType(Drawing::FeaturePage::getClassTypeId());
        if (pages.empty()){
            QMessageBox::warning(Gui::getMainWindow(), QObject::tr("No page found"),
                QObject::tr("Create a page first."));
            return;
        }
    }
 
    // TaskDlgOrthoViews does not get pages as a parameter:
    Gui::Control().showDialog(new TaskDlgOrthoViews());
}

mghansen256

2014-08-14 05:34

reporter  

1643-patch.diff (2,618 bytes)   
diff --git a/src/Mod/Drawing/Gui/Command.cpp b/src/Mod/Drawing/Gui/Command.cpp
index 7cacf85..67daf0e 100644
--- a/src/Mod/Drawing/Gui/Command.cpp
+++ b/src/Mod/Drawing/Gui/Command.cpp
@@ -319,21 +319,20 @@ CmdDrawingOrthoViews::CmdDrawingOrthoViews()
 
 void CmdDrawingOrthoViews::activated(int iMsg)
 {
-    std::vector<App::DocumentObject*> shapes = getSelection().getObjectsOfType(Part::Feature::getClassTypeId());
+    const std::vector<App::DocumentObject*> shapes = getSelection().getObjectsOfType(Part::Feature::getClassTypeId());
     if (shapes.size() != 1) {
         QMessageBox::warning(Gui::getMainWindow(), QObject::tr("Wrong selection"),
-            QObject::tr("Select a Part object."));
+            QObject::tr("Select exactly one Part object."));
         return;
     }
 
-    std::vector<App::DocumentObject*> pages = getSelection().getObjectsOfType(Drawing::FeaturePage::getClassTypeId());
+    // Check that a page object exists. TaskDlgOrthoViews will then check for a selected page object
+    // and use that, otherwise it will use the first page in the document.
+    const std::vector<App::DocumentObject*> pages = this->getDocument()->getObjectsOfType(Drawing::FeaturePage::getClassTypeId());
     if (pages.empty()) {
-        pages = this->getDocument()->getObjectsOfType(Drawing::FeaturePage::getClassTypeId());
-        if (pages.empty()){
-            QMessageBox::warning(Gui::getMainWindow(), QObject::tr("No page found"),
-                QObject::tr("Create a page first."));
-            return;
-        }
+	QMessageBox::warning(Gui::getMainWindow(), QObject::tr("No page found"),
+	    QObject::tr("Create a page first."));
+	return;
     }
  
     Gui::Control().showDialog(new TaskDlgOrthoViews());
diff --git a/src/Mod/Drawing/Gui/TaskOrthoViews.cpp b/src/Mod/Drawing/Gui/TaskOrthoViews.cpp
index 0a9fb12..2474d23 100644
--- a/src/Mod/Drawing/Gui/TaskOrthoViews.cpp
+++ b/src/Mod/Drawing/Gui/TaskOrthoViews.cpp
@@ -858,7 +858,10 @@ TaskOrthoViews::TaskOrthoViews(QWidget *parent)
     const char * part = obj.front()->getNameInDocument();
 
     App::Document * doc = App::GetApplication().getActiveDocument();
-    vector<App::DocumentObject*> pages = doc->getObjectsOfType(Drawing::FeaturePage::getClassTypeId());
+    vector<App::DocumentObject*> pages = Gui::Selection().getObjectsOfType(Drawing::FeaturePage::getClassTypeId());
+    if (pages.empty()) {
+	pages = doc->getObjectsOfType(Drawing::FeaturePage::getClassTypeId());
+    }
     string PageName = pages.front()->getNameInDocument();
     const char * page = PageName.c_str();
 
1643-patch.diff (2,618 bytes)   

mghansen256

2014-08-14 05:35

reporter   ~0004945

Hi Yorik,

I attached a patch. I also made a pull-request on github for this change, if that is easier for you to integrate.

https://github.com/FreeCAD/FreeCAD_sf_master/pull/29

Best regards,

Michael

yorik

2014-08-16 22:58

administrator   ~0004956

Cool, thanks! Just a doubt, you changed some std::vector to const std::vector, which makes them unmodifiable, however one of those can be modified (in the second part of your diff, line 329). My feeble knowledge of C++ tells me this is wrong, or is it me who is wrong? In fact I don't understand well why the change in your second commit?

mghansen256

2014-08-17 20:03

reporter   ~0004960

Hi Yorik,

I think it is ok, as I only mark those vectors as const that we do not modify later. Also, when you declare a vector as const and then try to modify it, the compiler will generate an error message. Note that we only declare the vector as const, not the pages themselves that are pointed to in the vector.

I added some comments marked with NEW in the code below. If you do not like the const statement, you can remove it - the code will work without it. I tested it for no pages, no selected pages and selected pages when triggering the action.

Usually I try to mark as many things const as possible, to avoid accidental overwrites like "if (a=1)", and also to allow more optimization for the compiler, as it knows that the variables will not be modified.

Best regards,

Michael

void CmdDrawingOrthoViews::activated(int iMsg)
{
    // NEW the vector shapes can be const because we simply want to count how many elements it has, but not make any modifications
    const std::vector<App::DocumentObject*> shapes = getSelection().getObjectsOfType(Part::Feature::getClassTypeId());
    if (shapes.size() != 1) {
        QMessageBox::warning(Gui::getMainWindow(), QObject::tr("Wrong selection"),
            QObject::tr("Select exactly one Part object."));
        return;
    }

    // Check that a page object exists. TaskDlgOrthoViews will then check for a selected page object
    // and use that, otherwise it will use the first page in the document.
    // NEW pages can be const, because we just want to check that it is not empty
    const std::vector<App::DocumentObject*> pages = this->getDocument()->getObjectsOfType(Drawing::FeaturePage::getClassTypeId());
    if (pages.empty()) {
    QMessageBox::warning(Gui::getMainWindow(), QObject::tr("No page found"),
        QObject::tr("Create a page first."));
    return;
    }
 
    Gui::Control().showDialog(new TaskDlgOrthoViews());
}

TaskOrthoViews::TaskOrthoViews(QWidget *parent)
  : ui(new Ui_TaskOrthoViews)
{
    /* ><-- snipped a part -->< */

    App::Document * doc = App::GetApplication().getActiveDocument();
    // NEW here pages can not be const, because we first try to load the selected pages into it
    vector<App::DocumentObject*> pages = Gui::Selection().getObjectsOfType(Drawing::FeaturePage::getClassTypeId());
    if (pages.empty()) {
        // NEW and if that fails, we load all the pages from the document into it
    pages = doc->getObjectsOfType(Drawing::FeaturePage::getClassTypeId());
    }

    /* ><-- snipped -->< */
}

wmayer

2014-08-19 13:22

administrator   ~0004970

The patch/branch name contains the wrong id. It must be 1668 and not 1643.

Just merged your code but realized too late the wrong id number. See git show dd7bd833c68a86d5

Related Changesets

FreeCAD: master 7778988f

2014-08-08 03:00:31

yorik

Details Diff
Drawing: Allow to select the page in which to create a view feature - fixes 0001668 Affected Issues
0001668
mod - src/Mod/Drawing/Gui/Command.cpp Diff File

FreeCAD: master dd7bd833

2014-08-19 15:07:02

mghansen256


Committer: wmayer Details Diff
Fix bug 1643: Projections are always added to the first drawing page Affected Issues
0001668
mod - src/Mod/Drawing/Gui/Command.cpp Diff File
mod - src/Mod/Drawing/Gui/TaskOrthoViews.cpp Diff File

Issue History

Date Modified Username Field Change
2014-08-07 09:33 mghansen256 New Issue
2014-08-07 14:20 yorik Note Added: 0004924
2014-08-07 14:20 yorik Assigned To => yorik
2014-08-07 14:20 yorik Status new => assigned
2014-08-08 01:32 yorik Changeset attached => FreeCAD Master master 7778988f
2014-08-08 01:32 yorik Status assigned => closed
2014-08-08 01:32 yorik Resolution open => fixed
2014-08-08 01:38 yorik Fixed in Version => 0.15
2014-08-09 20:41 mghansen256 Note Added: 0004935
2014-08-09 20:41 mghansen256 Status closed => feedback
2014-08-09 20:41 mghansen256 Resolution fixed => reopened
2014-08-09 20:43 mghansen256 Note Edited: 0004935
2014-08-14 05:34 mghansen256 File Added: 1643-patch.diff
2014-08-14 05:35 mghansen256 Note Added: 0004945
2014-08-14 05:35 mghansen256 Status feedback => assigned
2014-08-16 22:58 yorik Note Added: 0004956
2014-08-17 20:03 mghansen256 Note Added: 0004960
2014-08-19 13:22 wmayer Note Added: 0004970
2014-08-19 14:06 wmayer Changeset attached => FreeCAD Master master dd7bd833
2014-08-19 14:06 wmayer Status assigned => closed
2014-08-19 14:06 wmayer Resolution reopened => fixed