View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0002213 | OpenFOAM | Patch | public | 2016-08-24 01:45 | 2016-12-03 20:00 |
Reporter | wyldckat | Assigned To | henry | ||
Priority | normal | Severity | minor | Reproducibility | sometimes |
Status | resolved | Resolution | fixed | ||
Summary | 0002213: "vtkTopo.C" in foamToVTK library is missing the same fix as provided in #1633 | ||||
Description | While 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 Information | Note: 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"? | ||||
Tags | No tags attached. | ||||
|
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; } } // ************************************************************************* // |
|
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. |
|
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? |
|
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. |
|
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. |
|
Should the remaining VTK exports be moved into the foamToVTK library and that library linked in when VTK output is needed? |
|
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 |
|
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. |
|
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. |
|
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 |
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 |