View Issue Details

IDProjectCategoryView StatusLast Update
0002213OpenFOAMPatchpublic2016-12-03 20:00
Reporterwyldckat Assigned Tohenry  
PrioritynormalSeverityminorReproducibilitysometimes
Status resolvedResolutionfixed 
Summary0002213: "vtkTopo.C" in foamToVTK library is missing the same fix as provided in #1633
DescriptionWhile diagnosing issue #2099, I stumbled upon a missing fix for "applications/utilities/postProcessing/dataConversion/foamToVTK/foamToVTK/vtkTopo.C", which was applied to "vtkPV?FoamMeshVolume.C" in issue #1633 (commit e199a0fbf67), but which I completely forgot to test and provide the same fix for this "vtkTopo.C" file.

The odd thing is that I thought that I had seen this be already fixed sometime ago... but I might have mistaken it for something else... either that, or we have an additional class where this kind of FOAM to VTK format is being done...

Anyway, attached is the file "vtkTopo.C" for updating "applications/utilities/postProcessing/dataConversion/foamToVTK/foamToVTK/vtkTopo.C" on both 4.x and dev, using the same fix as used in commit e199a0fbf67.
Additional InformationNote: This FOAM to VTK mesh conversion mechanism needs to be formalized into a single class, even if it's just a class of inline static methods for 1-to-1 cell conversions... or one from which the others derive...
Problem is: Where exactly should we place such a class? Maybe in "src/fileFormats/vtk"?
TagsNo tags attached.

Relationships

related to 0002099 resolvedhenry snap stage generates internal triangles 
related to 0002219 closedwyldckat Proposition for deprecating the representation of tetWedge with a collapsed VTK_WEDGE when exporting to VTK 

Activities

wyldckat

2016-08-24 01:45

updater  

vtkTopo.C (12,063 bytes)   
/*---------------------------------------------------------------------------*\
  =========                 |
  \\      /  F ield         | OpenFOAM: The Open Source CFD Toolbox
   \\    /   O peration     |
    \\  /    A nd           | Copyright (C) 2011-2016 OpenFOAM Foundation
     \\/     M anipulation  |
-------------------------------------------------------------------------------
License
    This file is part of OpenFOAM.

    OpenFOAM is free software: you can redistribute it and/or modify it
    under the terms of the GNU General Public License as published by
    the Free Software Foundation, either version 3 of the License, or
    (at your option) any later version.

    OpenFOAM is distributed in the hope that it will be useful, but WITHOUT
    ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
    FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
    for more details.

    You should have received a copy of the GNU General Public License
    along with OpenFOAM.  If not, see <http://www.gnu.org/licenses/>.

Description
    Note: bug in vtk displaying wedges? Seems to display ok if we decompose
    them. Should be thoroughly tested!
    (they appear rarely in polyhedral meshes, do appear in some cut meshes)

\*---------------------------------------------------------------------------*/

#include "vtkTopo.H"
#include "polyMesh.H"
#include "cellShape.H"
#include "cellModeller.H"
#include "Swap.H"

// * * * * * * * * * * * * * * Static Data Members * * * * * * * * * * * * * //

bool Foam::vtkTopo::decomposePoly = true;


// * * * * * * * * * * * * * * * * Constructors  * * * * * * * * * * * * * * //

Foam::vtkTopo::vtkTopo(const polyMesh& mesh)
:
    mesh_(mesh),
    vertLabels_(),
    cellTypes_(),
    addPointCellLabels_(),
    superCells_()
{
    const cellModel& tet = *(cellModeller::lookup("tet"));
    const cellModel& pyr = *(cellModeller::lookup("pyr"));
    const cellModel& prism = *(cellModeller::lookup("prism"));
    const cellModel& wedge = *(cellModeller::lookup("wedge"));
    const cellModel& tetWedge = *(cellModeller::lookup("tetWedge"));
    const cellModel& hex = *(cellModeller::lookup("hex"));

    const cellShapeList& cellShapes = mesh_.cellShapes();

    // Number of additional points needed by the decomposition of polyhedra
    label nAddPoints = 0;

    // Number of additional cells generated by the decomposition of polyhedra
    label nAddCells = 0;

    // face owner is needed to determine the face orientation
    const labelList& owner = mesh.faceOwner();

    // Scan for cells which need to be decomposed and count additional points
    // and cells
    if (decomposePoly)
    {
        forAll(cellShapes, celli)
        {
            const cellModel& model = cellShapes[celli].model();

            if
            (
                model != hex
             && model != wedge    // See above.
             && model != prism
             && model != pyr
             && model != tet
             && model != tetWedge
            )
            {
                const cell& cFaces = mesh_.cells()[celli];

                forAll(cFaces, cFacei)
                {
                    const face& f = mesh_.faces()[cFaces[cFacei]];

                    label nQuads = 0;
                    label nTris = 0;
                    f.nTrianglesQuads(mesh_.points(), nTris, nQuads);

                    nAddCells += nQuads + nTris;
                }

                nAddCells--;
                nAddPoints++;
            }
        }
    }


    // Set size of additional point addressing array
    // (from added point to original cell)
    addPointCellLabels_.setSize(nAddPoints);

    // Set size of additional cells mapping array
    // (from added cell to original cell)
    superCells_.setSize(nAddCells);

    // List of vertex labels in VTK ordering
    vertLabels_.setSize(cellShapes.size() + nAddCells);

    // Label of vtk type
    cellTypes_.setSize(cellShapes.size() + nAddCells);

    // Set counters for additional points and additional cells
    label addPointi = 0, addCelli = 0;

    forAll(cellShapes, celli)
    {
        const cellShape& cellShape = cellShapes[celli];
        const cellModel& cellModel = cellShape.model();

        labelList& vtkVerts = vertLabels_[celli];

        if (cellModel == tet)
        {
            vtkVerts = cellShape;

            cellTypes_[celli] = VTK_TETRA;
        }
        else if (cellModel == pyr)
        {
            vtkVerts = cellShape;

            cellTypes_[celli] = VTK_PYRAMID;
        }
        else if (cellModel == prism)
        {
            // VTK has a different node order for VTK_WEDGE
            // their triangles point outwards!
            vtkVerts = cellShape;

            Foam::Swap(vtkVerts[1], vtkVerts[2]);
            Foam::Swap(vtkVerts[4], vtkVerts[5]);

            cellTypes_[celli] = VTK_WEDGE;
        }
        else if (cellModel == tetWedge)
        {
            // Treat as squeezed prism (VTK_WEDGE)
            vtkVerts.setSize(6);
            vtkVerts[0] = cellShape[0];
            vtkVerts[1] = cellShape[2];
            vtkVerts[2] = cellShape[1];
            vtkVerts[3] = cellShape[3];
            vtkVerts[4] = cellShape[4];
            vtkVerts[5] = cellShape[3];

            cellTypes_[celli] = VTK_WEDGE;
        }
        else if (cellModel == wedge)
        {
            // Treat as squeezed hex
            vtkVerts.setSize(8);
            vtkVerts[0] = cellShape[0];
            vtkVerts[1] = cellShape[1];
            vtkVerts[2] = cellShape[2];
            vtkVerts[3] = cellShape[2];
            vtkVerts[4] = cellShape[3];
            vtkVerts[5] = cellShape[4];
            vtkVerts[6] = cellShape[5];
            vtkVerts[7] = cellShape[6];

            cellTypes_[celli] = VTK_HEXAHEDRON;
        }
        else if (cellModel == hex)
        {
            vtkVerts = cellShape;

            cellTypes_[celli] = VTK_HEXAHEDRON;
        }
        else if (decomposePoly)
        {
            // Polyhedral cell. Decompose into tets + pyramids.

            // Mapping from additional point to cell
            addPointCellLabels_[addPointi] = celli;

            // The new vertex from the cell-centre
            const label newVertexLabel = mesh_.nPoints() + addPointi;

            // Whether to insert cell in place of original or not.
            bool substituteCell = true;

            const labelList& cFaces = mesh_.cells()[celli];
            forAll(cFaces, cFacei)
            {
                const face& f = mesh_.faces()[cFaces[cFacei]];
                const bool isOwner = (owner[cFaces[cFacei]] == celli);

                // Number of triangles and quads in decomposition
                label nTris = 0;
                label nQuads = 0;
                f.nTrianglesQuads(mesh_.points(), nTris, nQuads);

                // Do actual decomposition into triFcs and quadFcs.
                faceList triFcs(nTris);
                faceList quadFcs(nQuads);
                label trii = 0;
                label quadi = 0;
                f.trianglesQuads(mesh_.points(), trii, quadi, triFcs, quadFcs);

                forAll(quadFcs, quadI)
                {
                    label thisCelli;

                    if (substituteCell)
                    {
                        thisCelli = celli;
                        substituteCell = false;
                    }
                    else
                    {
                        thisCelli = mesh_.nCells() + addCelli;
                        superCells_[addCelli++] = celli;
                    }

                    labelList& addVtkVerts = vertLabels_[thisCelli];

                    addVtkVerts.setSize(5);

                    const face& quad = quadFcs[quadI];

                    // Ensure we have the correct orientation for the
                    // base of the primitive cell shape.
                    // If the cell is face owner, the orientation needs to be
                    // flipped.
                    // At the moment, VTK doesn't actually seem to care if
                    // negative cells are defined, but we'll do it anyhow
                    // (for safety).
                    if (isOwner)
                    {
                        addVtkVerts[0] = quad[3];
                        addVtkVerts[1] = quad[2];
                        addVtkVerts[2] = quad[1];
                        addVtkVerts[3] = quad[0];
                    }
                    else
                    {
                        addVtkVerts[0] = quad[0];
                        addVtkVerts[1] = quad[1];
                        addVtkVerts[2] = quad[2];
                        addVtkVerts[3] = quad[3];
                    }
                    addVtkVerts[4] = newVertexLabel;

                    cellTypes_[thisCelli] = VTK_PYRAMID;
                }

                forAll(triFcs, triI)
                {
                    label thisCelli;

                    if (substituteCell)
                    {
                        thisCelli = celli;
                        substituteCell = false;
                    }
                    else
                    {
                        thisCelli = mesh_.nCells() + addCelli;
                        superCells_[addCelli++] = celli;
                    }


                    labelList& addVtkVerts = vertLabels_[thisCelli];

                    const face& tri = triFcs[triI];

                    addVtkVerts.setSize(4);

                    // See note above about the orientation.
                    if (isOwner)
                    {
                        addVtkVerts[0] = tri[2];
                        addVtkVerts[1] = tri[1];
                        addVtkVerts[2] = tri[0];
                    }
                    else
                    {
                        addVtkVerts[0] = tri[0];
                        addVtkVerts[1] = tri[1];
                        addVtkVerts[2] = tri[2];
                    }
                    addVtkVerts[3] = newVertexLabel;

                    cellTypes_[thisCelli] = VTK_TETRA;
                }
            }

            addPointi++;
        }
        else
        {
            // Polyhedral cell - not decomposed
            cellTypes_[celli] = VTK_POLYHEDRON;

            const labelList& cFaces = mesh_.cells()[celli];

            // space for the number of faces and size of each face
            label nData = 1 + cFaces.size();

            // count total number of face points
            forAll(cFaces, cFacei)
            {
                const face& f = mesh.faces()[cFaces[cFacei]];
                nData += f.size();   // space for the face labels
            }

            vtkVerts.setSize(nData);

            nData = 0;
            vtkVerts[nData++] = cFaces.size();

            // build face stream
            forAll(cFaces, cFacei)
            {
                const face& f = mesh.faces()[cFaces[cFacei]];
                const bool isOwner = (owner[cFaces[cFacei]] == celli);

                // number of labels for this face
                vtkVerts[nData++] = f.size();

                if (isOwner)
                {
                    forAll(f, fp)
                    {
                        vtkVerts[nData++] = f[fp];
                    }
                }
                else
                {
                    // fairly immaterial if we reverse the list
                    // or use face::reverseFace()
                    forAllReverse(f, fp)
                    {
                        vtkVerts[nData++] = f[fp];
                    }
                }
            }
        }
    }

    if (decomposePoly)
    {
        Pout<< "    Original cells:" << mesh_.nCells()
            << " points:" << mesh_.nPoints()
            << "   Additional cells:" << superCells_.size()
            << "  additional points:" << addPointCellLabels_.size()
            << nl << endl;
    }

}


// ************************************************************************* //
vtkTopo.C (12,063 bytes)   

henry

2016-08-24 09:42

manager   ~0006734

Thanks for fix, I am putting it in now.

I totally that this complex code dumlication is mental and needs to be sorted out. One option would be to keep all the generic functionality for foam to VTK in the foamToVTK library:

./applications/utilities/postProcessing/dataConversion/foamToVTK/foamToVTK

and link this library into the ParaView reader.

wyldckat

2016-08-24 11:08

updater   ~0006735

Then shouldn't the library code for "foamToVTK" be moved to "src", so that it's easier to be picked up by other applications/libraries?

henry

2016-08-24 11:15

manager   ~0006736

I thought about that but it is not a general purpose library, it is currently only used for the foamToVTK application and potentially the ParaView reader. Another option would be to move the ParaView reader into the dataConversion/foamToVTK but that would separate it from the other ParaView readers.

wyldckat

2016-08-24 14:04

updater   ~0006738

Consolidating the remaining VTK exports comes to mind, as a reason for moving the foamToVTK library to somewhere at "FOAM_SRC". For example, the sampling library and all other function objects that can export to VTK.

Nonetheless, for the time being, I'm not yet seeing a way to have a complete generalization for both on RAM (e.g. ParaView) and file export. Hence the simpler class of static methods, which does justify keeping the foamToVTK library source code where it is for now.

henry

2016-08-24 14:11

manager   ~0006739

Should the remaining VTK exports be moved into the foamToVTK library and that library linked in when VTK output is needed?

wyldckat

2016-08-24 14:28

updater   ~0006740

With some additional code revising (better safe than sorry), yes, I believe it would be best to consolidate the classes and code for exporting mesh/data to VTK into a single library, even though it may create a dependency on a larger library for all others that depend on that code. It would help focus any exporting efforts for VTK into a single library, as well as help enforce non-format-bound export mechanisms, like we currently use for function objects (forces library comes to mind, can't export to CSV or any other format).

The "src/fileFormats" is also a library that could take some further extending, so it would more easily cover formats that are already being handled up somewhere else up to a point.

Merging the two libraries probably doesn't make sense, but it does make sense to have "fileFormats" as an entry point for all export/import of external formats, even though this might not be achievable in the near future... but having mesh/data import/export to any format accessible from a single library, is something that could make it easier to have cooperative data transfer with other software (remote co-processing of some VTK files comes to mind; or for boundary condition coupling of large experimental databases).

Possible directory structure:

  - src
      - fileFormats
          - fileFormats
          - foamToVTK

henry

2016-08-24 14:35

manager   ~0006741

Given that the 'foamToVTK' would be a sub-directory of 'fileFormats' it would be logical to rename it 'VTK' as it relates to the VTK format and the 'foamTo' is implied by its location.

wyldckat

2016-08-24 15:28

updater   ~0006742

Just a few reminders:

  1- The "fileFormats" library already handles read and write operations, so it's not as straight forward to assume that the "foamTo" is implied.

  2- "src/fileFormats/vtk" has a reading class for VTK unstructured meshes, so it might make sense to move it into the consolidated library.

  3- Be careful to not use "libVTK.so" as the name for the final library binary.

wyldckat

2016-12-03 20:00

updater   ~0007389

I've made a note to look into this later on, namely the consolidation of the VTK features into a single library, as discussed in the previous comments.

This report is hereby "resolved" as "fixed", as listed below:

Resolved in OpenFOAM-4.x by commit 85daa4f41a76d16ae8e6428d0ec81ccd583300b4

Resolved in OpenFOAM-dev by commit f50d72786360b6d37fb4a397d5acf7f42f353cd1

Issue History

Date Modified Username Field Change
2016-08-24 01:45 wyldckat New Issue
2016-08-24 01:45 wyldckat Status new => assigned
2016-08-24 01:45 wyldckat Assigned To => henry
2016-08-24 01:45 wyldckat File Added: vtkTopo.C
2016-08-24 01:46 wyldckat Relationship added related to 0002099
2016-08-24 09:42 henry Note Added: 0006734
2016-08-24 11:08 wyldckat Note Added: 0006735
2016-08-24 11:15 henry Note Added: 0006736
2016-08-24 14:04 wyldckat Note Added: 0006738
2016-08-24 14:11 henry Note Added: 0006739
2016-08-24 14:28 wyldckat Note Added: 0006740
2016-08-24 14:35 henry Note Added: 0006741
2016-08-24 15:28 wyldckat Note Added: 0006742
2016-08-26 00:59 wyldckat Relationship added related to 0002219
2016-12-03 20:00 wyldckat Status assigned => resolved
2016-12-03 20:00 wyldckat Resolution open => fixed
2016-12-03 20:00 wyldckat Fixed in Version => 4.x
2016-12-03 20:00 wyldckat Note Added: 0007389