Visualization - Integration of VIS component

Patches for OCC and WOK were moved from CR22877_1 branches to CR24904 in Open CASCADE and WOK git repositories.

The git branch CR24904_1 was added.

Patch in the CR24904_1 consists VIS component that includes:
-IVtk, IVtkOCC, IVtkVTK, IVtkTools packages in TKIVtk toolkit.

Dear san,

please review the branch CR24904_1.

- Compilation errors on Linux were corrected.
- TKIVtk is not generated and built if VTK prerequisite is not found.
- Got rid of warnings in TKIVtk. style corrections.

This messages are useless, since this is first integration of this component.
Please add to commit message short description of new component - its purpose and main capabilities.

+//  IShape : Interface for working with a shape and its sub-shapes ids.
+//  File   : IVtk_IShape.h
+//  Author : Author
+//  Module : IVtk
+//  $Header: $

Please remove files from redundant comments ($Header, File, Module), merge useful ones (Author) with file header in the usual way for OCCT sources.

+#include "IVtk_Interface.hxx"
+#include "IVtk_Types.hxx"

This is wrong inclusion semantic for public headers in OCCT framework - use <header.hxx> instead.

+//! @class IVtk_IShape 
+//! @brief Interface for working with a shape and its sub-shapes ids.
+//! @ingroup interfaces
...
+//! @defgroup occimpl IVtkOCC

I'm not sure that @ingroup is valid for OCCT documentation.
Please check proper documentation generation for OCCT within this patch.

+  Standard_EXPORT IVtk_IdType GetId() const { return myId; }
+
+  Standard_EXPORT void SetId (IVtk_IdType theId) { myId = theId; }

Please remove Standard_EXPORT from all inline methods.

+  if (!(theShape.IsNull()))
+  {
...
+  if (!(thePointIds->IsEmpty()))
+  {

Please remove redundant braces.

+class IVtk_IShapePickerAlgo : public IVtk_Interface
+{
...
+  Standard_EXPORT virtual void SetView (IVtk_IView::Handle& theView) = 0;

This is inconsistent to push not-constant handle to the method when handle (not the object) is not expected to be modified.

+  Standard_EXPORT virtual bool Pick (const double& theX, const double& theY) = 0;
+  Standard_EXPORT virtual bool Pick (const double& theXMin,
+                                     const double& theYMin,
+                                     const double& theXMax,
+                                     const double& theYMax) = 0;

This is inconsistent to put double by reference.

+  //! @return the list of picked top-level shape IDs,
+  //! in the order of increasing depth (the ID of the shape closest to the eye 
+  //! is the first in the list)
+  Standard_EXPORT virtual IVtk_ShapeIdList ShapesPicked () const;
+
+  //! @param [in] theId Top-level shape ID
+  //! @return the list of picked sub-shape IDs for the given top-level shape ID,
+  //! in the order of increasing depth (the ID of the sub-shape closest to the eye 
+  //! is the first in the list)
+  Standard_EXPORT virtual IVtk_ShapeIdList SubShapesPicked (const IVtk_IdType theId) const;

Here and in other places - this is broken by design to return collection copy each time. Are all use cases expected to return only negligible small lists?

+  //! Converts 3D display coordinates into 3D world coordinates.
+  //! @param [in] theDisplayPnt array of two double display coordinates
+  //! @param [out] theWorldPnt array of three double world coordinates (memory should be allocated by the caller!)
+  //! @return true if conversion was successful, false otherwise
+  Standard_EXPORT virtual bool    DisplayToWorld (double* theDisplayPnt, double* theWorldPnt) const = 0;

Why points are passed as arrays of unknown size? Please use appropriate types (gp_XYZ or Graphic3d_Vec3d) instead. The same is true for other methods of IVtk_IView class.

+class IVtk_Interface : public MMgt_TShared

Inheritance from MMgt_TShared looks redundant - please use Standard_Transient instead.

+struct IVtk_Pnt2d
+{
+  double x;
+  double y;
+};

What for one more duplicate for gp_XY, Graphic3d_Vec2d?

+#ifndef IVtkOCC_H
+#define IVtkOCC_H
+
+//! @defgroup occimpl IVtkOCC
+
+#define qDebug() cout
...
+        qDebug() << "Error: EntityOwner having null SelectableObject picked!";

Debug messages are not expected to be printed within release mode (and unwelcome by customers in debug mode as well). Also qDebug looks not domain-specific and should not be defined globally in header.
Useful messages might be printed using Message_Messenger interface.

+IVtkOCC_SelectableObject::IVtkOCC_SelectableObject (const IVtkOCC_Shape::Handle& theShape)
+: SelectMgr_SelectableObject (PrsMgr_TOP_AllView),
+  myShape (theShape)
+{
+  if (myShape)
+  {
+    myShape->SetSelectableObject (this);
+  }

Note that this NULL-check syntax will work only in current master, but not in OCCT6.7.1. It is more convenient to use .IsNull().

+const TopoDS_Shape& IVtkOCC_Shape::GetSubShape (const IVtk_IdType theId) const
+{
+  if (theId < 0)
+  {
+    return myTopoDSShape;
+  }

Since IVtk_IdType is unsigned - negative check will NEVER work here.

+
+IVtkOCC_Shape::IVtkOCC_Shape (const TopoDS_Shape& theShape)
+: myTopoDSShape (theShape)
+{
+  buildSubShapeIdMap();
+}
+
+IVtkOCC_Shape::~IVtkOCC_Shape() { }

Standard documentation blocks are missing.

+Standard_Real IVtkOCC_ShapeMesher::GetDeflection() const
+{
+  if (myDeflection == 0.0)
...
+void IVtkOCC_ShapeMesher::meshShape()
+{
+  Standard_Real aDeflection = GetDeflection();
+  if (aDeflection == 0.0)
+  {
+    return;
+  }
...
+void IVtkOCC_ShapeMesher::addWireFrameFaces()
+{
+  // Check the deflection value once for all faces
+  if (GetDeflection() == 0.0)
+  {
+    return;
+  }

Testing double for equality is bad style. Should not be here myDeflection < Precision::Confusion()?

+      IVtkOCC_ShapeMesher* aThisMesher = const_cast<IVtkOCC_ShapeMesher*> (this);

Why not declare myDeflection as mutable?

+      // This magic line comes from AIS_Shape.cxx in OCCT 6.5.1
+      aThisMesher->myDeflection = Max (aMaxX - aMinX, Max (aMaxY - aMinY, aMaxZ - aMinZ) )
+                                  * GetDeviationCoeff() * 4;

It would be better to share "magic" code in AIS_Shape::GetDeflectio(), instead of copying it...

+    // Enable parallel mode of meshing if required
+    Standard_Boolean wasParallel = BRepMesh_IncrementalMesh::IsParallelDefault();
+    if ( myIsParallel && !wasParallel )
+    {
+      BRepMesh_IncrementalMesh::SetParallelDefault(Standard_True);
+    }

It looks... broken to change global flag.

+void IVtkOCC_ShapeMesher::processPolyline (Standard_Integer          theNbNodes,
+                                      const TColgp_Array1OfPnt&      thePoints,
+                                      const TColStd_Array1OfInteger& thePointIds,
+                                      const IVtk_IdType              theOcctId,
+                                      bool                           theNoTransform,
+                                      gp_Trsf                        theTransformation,
+                                      const IVtk_MeshType            theMeshType)
+{
+  if (theNbNodes < 2)
+  {
+    return;
+  }
+
+  IVtk_PointIdList *aPolyPointIds = new IVtk_PointIdList();

It doesn't look like aPolyPointIds is destroyed somewhere later.

+DEFINE_BASECOLLECTION(OccPolylinesBase,
+                      TColgp_SequenceOfPnt)
+DEFINE_LIST          (OccPolylines,
+                      OccPolylinesBase,
+                      TColgp_SequenceOfPnt)

Types have been defined without package suffix.

+  void PrintSelf (ostream& theOs, vtkIndent theIndent);

Please specify std namespace explicitly where used.

The patch was updated with remarks corrections and pushed in the branch CR24904_3.

Dear san,

please review.

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

As agreed with abv, VIS should be migrated to the latest VTK release (6.1.0 as of today) before integration, since VTK 5.10.1 cannot be built with VS2013.

WOK most likely should be adapted for VTK 6.1.0, too.

Branch [archived branch] has been updated by Participant.

[revision removed]


      from 16ff82e TKIVtk remarks:
       new 511d7ee Porting on VTK 6:


Detailed log of new commits:

[revision removed]
Author: aba
Date: Tue Aug 19 19:04:57 2014 +0400

    Porting on VTK 6:
    
    - shape source inherits vtkPolyDataAlgorithm now (vtkPolyDataSource was removed form VTK as deprecated functionality).
    - added factory auto-initialization in IVtkVTK_View
    - remove using of deprecated methods of pipeline mechanism.

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

The branch CR24904_3 was updated:
patch was ported to VTK 6.1.0 and rebased on current master.

Dear san,

please review.

Dear Anastasia,

+class Standard_EXPORT IVtkTools_ShapeDataSource : public vtkPolyDataAlgorithm
+class Standard_EXPORT IVtkTools_ShapeObject :  public vtkDataObject
+class Standard_EXPORT IVtkTools_SubPolyDataFilter : public vtkPolyDataAlgorithm

this trick with Standard_EXPORT would not work well when applied to entire class (not just class methods).
Please declare own macros to switch Standard_EXPORT/Standard_IMPORT usage (based on __IVtkOCC_DLL which should be automatically defined for this package by WOK)

Branch [archived branch] has been updated by Participant.

[revision removed]


Detailed log of new commits:

Author: aba
Date: Thu Aug 21 19:54:45 2014 +0400

    Remarks

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

Dear kgv,

please review the branch CR24904_3.

Dear Commenter 1,

please test all patches related to integration of VIS component (component itself, Draw plugin / tests, patch for WOK introducing new dependency).

Branch [archived branch] has been updated by Participant.

[revision removed]


Detailed log of new commits:

Author: aba
Date: Tue Aug 26 12:36:57 2014 +0400

    Corrected error.

Author: aba
Date: Tue Jul 15 15:11:06 2014 +0400

    0022877: Implementation of DRAW commands for non-regression testing:
    
    - TKIVtkDraw toolkit provides IVtk packages functionality in DRAW.
    
    - it allows to create VTK interactive view in regular or virtual mode (virtual windows),
      display OCC objects and dump them.
    
    - TKIVtkDraw provides also test commands: ivtlinit, ivtkdisplay, ivtkerase, ivtksetdispmode,
       ivtksetselmode, ivtkmoveto, ivtkselect, ivtkfit, ivtkdump, ivtkbgcolor.

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

IR-IR-24904-22887 has been created from CR24904_3 and CR22887_3

Problems:

Compilation on Linux failed
http://jenkins-test-02.nnov.opencascade.com:8080/user/mnt/my-views/view/IR-24904-22887/job/mnt-IR-24904-22887-master_build_occt_linux/4/parsed_console/

Testing on windows:
See HTML report
http://jenkins-test-02.nnov.opencascade.com:8080/user/mnt/my-views/view/IR-24904-22887/job/mnt-IR-24904-22887-master_occt_tests_windows_start/label=windows_test,tests_group=bugs,tests_subgroup=019/
It seems shapes is not maximize in Viewer.

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

IR-IR-24904-22887-1 has been created from CR24904_3 and updated CR22887_3

Problems:

Compilation on Linux and Windows is failed

http://jenkins-test-02.nnov.opencascade.com:8080/user/mnt/my-views/view/IR-24904-22887-1/job/mnt-IR-24904-22887-1-master_build_occt_linux/2/parsed_console/

http://jenkins-test-02.nnov.opencascade.com:8080/job/mnt-IR-24904-22887-1-master_build_occt_windows/1/parsed_console/

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

Branch [archived branch] has been updated by Participant.

[revision removed]


Detailed log of new commits:

Author: aba
Date: Fri Aug 29 14:46:30 2014 +0400

    Corrected errors after rebase.

IR-24904-22887-2 has been created from CR24904_3 and CR22887_3

Problems:

Compilation on Linux and Windows is failed

http://jenkins-test-02.nnov.opencascade.com:8080/user/mnt/my-views/view/IR-24904-22887-2/job/inv-IR-24904-22887-2-master_build_occt_linux_deb/1/parsed_console/

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

Branch [archived branch] has been updated by Participant.

[revision removed]


Detailed log of new commits:

Author: aba
Date: Tue Sep 2 18:02:28 2014 +0400

    Get rid from warning in SelectMgr_SelectableObject. Removed firendship from SelectMgr_SelectableObject.

Branch [archived branch] has been updated by Participant.

[revision removed]


Detailed log of new commits:

Author: aba
Date: Tue Sep 9 18:15:08 2014 +0400

    Corrected projector parameters for selection algorithm.

Branch [archived branch] has been updated by Participant.

[revision removed]


Detailed log of new commits:

Author: aba
Date: Wed Sep 10 20:17:21 2014 +0400

    Removed unneeded picking algorithm modification.

Branch [archived branch] has been updated forcibly by Participant.

[revision removed]

Branch [archived branch] has been deleted by Participant.

[revision removed]

Branch [archived branch] has been deleted by Participant.

[revision removed]

Branch [archived branch] has been deleted by Participant.

[revision removed]

Branch [archived branch] has been deleted by Participant.

[revision removed]