Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expansion of GoCAD interface #2586

Merged
merged 15 commits into from Aug 12, 2019
Next

added support for GoCAD PLine data, checking for VSet and Model3d

  • Loading branch information...
rinkk authored and TomFischer committed Aug 1, 2019
commit 189fcd01c2bc3d10b5bef4c8dbdfe3c152dcc883
@@ -39,7 +39,7 @@
#include "Applications/FileIO/GMSInterface.h"
#include "Applications/FileIO/Gmsh/GMSHInterface.h"
#include "Applications/FileIO/Gmsh/GmshReader.h"
#include "Applications/FileIO/GocadIO/GocadTSurfaceReader.h"
#include "Applications/FileIO/GocadIO/GocadAsciiReader.h"
#include "Applications/FileIO/Legacy/OGSIOVer4.h"
#include "Applications/FileIO/PetrelInterface.h"
#include "Applications/FileIO/TetGenInterface.h"
@@ -687,7 +687,7 @@ void MainWindow::loadFile(ImportFileType::type t, const QString &fileName)
else if (t == ImportFileType::GOCAD_TSURF)
{
std::string file_name(fileName.toStdString());
FileIO::Gocad::GocadTSurfaceReader gcts;
FileIO::Gocad::GocadAsciiReader gcts;
std::vector<std::unique_ptr<MeshLib::Mesh>> meshes;
if (gcts.readFile(file_name, meshes))
{
@@ -7,7 +7,7 @@
* http://www.opengeosys.org/LICENSE.txt
*/

#include "GocadTSurfaceReader.h"
#include "GocadAsciiReader.h"

#include <logog/include/logog.hpp>

@@ -16,6 +16,7 @@
#include "Applications/FileIO/GocadIO/CoordinateSystem.h"
#include "BaseLib/FileTools.h"
#include "BaseLib/StringTools.h"
#include "MeshLib/Elements/Line.h"
#include "MeshLib/Elements/Tri.h"
#include "MeshLib/Mesh.h"
#include "MeshLib/Node.h"
@@ -27,11 +28,9 @@ namespace Gocad
const std::string mat_id_name = "MaterialIDs";
const std::string eof_error = "Error: Unexpected end of file.";

GocadTSurfaceReader::GocadTSurfaceReader()
{
}
GocadAsciiReader::GocadAsciiReader() {}

bool GocadTSurfaceReader::readFile(
bool GocadAsciiReader::readFile(
std::string const& file_name,
std::vector<std::unique_ptr<MeshLib::Mesh>>& meshes)
{
@@ -43,11 +42,23 @@ bool GocadTSurfaceReader::readFile(
return false;
}

while (TSurfaceFound(in))
GOCAD_DATA_TYPE type;
while ((type = datasetFound(in)) != GOCAD_DATA_TYPE::UNDEFINED)
{
if (type == GOCAD_DATA_TYPE::VSET || type == GOCAD_DATA_TYPE::MODEL3D)
{
if (!skipToEND(in))
{
std::string const t = (type == GOCAD_DATA_TYPE::VSET) ? "VSet" : "Model3D";
ERR("Parsing of type %s is not implemented. Skipping section.", t);

This comment has been minimized.

Copy link
@endJunction

endJunction Aug 2, 2019

Member
Suggested change
ERR("Parsing of type %s is not implemented. Skipping section.", t);
ERR("Parsing of type %s is not implemented. Skipping section.", t.c_str());

This comment has been minimized.

Copy link
@rinkk

rinkk Aug 6, 2019

Author Member

✔️

return false;
}
continue;
}

std::string mesh_name = BaseLib::dropFileExtension(file_name) +
std::to_string(meshes.size() + 1);
std::unique_ptr<MeshLib::Mesh> mesh(readMesh(in, mesh_name));
std::unique_ptr<MeshLib::Mesh> mesh(readData(in, type, mesh_name));
if (mesh == nullptr)
{
ERR("File parsing aborted...")
@@ -58,8 +69,9 @@ bool GocadTSurfaceReader::readFile(
return true;
}

MeshLib::Mesh* GocadTSurfaceReader::readMesh(std::ifstream& in,
std::string& mesh_name)
MeshLib::Mesh* GocadAsciiReader::readData(std::ifstream& in,
GOCAD_DATA_TYPE const& type,
std::string& mesh_name)
{
if (!parseHeader(in, mesh_name))
{
@@ -108,7 +120,21 @@ MeshLib::Mesh* GocadTSurfaceReader::readMesh(std::ifstream& in,
return nullptr;
}
}
else if (str[0] == "TFACE")
else if (type == GOCAD_DATA_TYPE::PLINE && str[0] == "ILINE")

This comment has been minimized.

Copy link
@endJunction

endJunction Aug 1, 2019

Member

This is copy of the following if for TSURF. Please avoid code duplications.

This comment has been minimized.

Copy link
@rinkk

rinkk Aug 2, 2019

Author Member

✔️

This comment has been minimized.

Copy link
@endJunction

endJunction Aug 2, 2019

Member

The duplication is indeed resolved, but now the mesh creation logic is mixed up with the parsing of the cases. It is difficult to add new cases there.
It's up to you, but I'd have written something like

auto parse_mesh = [&](auto node_element_parser) {
    std::vector<MeshLib::Node*> nodes;
    std::vector<MeshLib::Element*> elems;
    std::map<std::size_t, std::size_t> node_id_map;
    if (!node_element_parser(in, nodes, elems, node_id_map, mesh_prop))
    {
        ERR(...);
        return nullptr;
    }
    return std::make_unique<MeshLib::Mesh>(mesh_name, ...);
};

And in the conditions:

if (type == TFACE)
{
    return parse_mesh(parseSurface);
}
if (type == PLINE)
{
    return parse_mesh(parseLine);
}

This comment has been minimized.

Copy link
@rinkk

rinkk Aug 6, 2019

Author Member

Honestly, I have no idea what happens there. And I know that it's difficult to add cases now, that's why it was written before such that cases were independent of each other.

This comment has been minimized.

Copy link
@rinkk

rinkk Aug 6, 2019

Author Member

To be more specific, I'd very much prefer the code the way it was originally written. That'd be six lines of duplicate code, which isn't a big issue in my opinion if it means the code is easier to read.

{
std::vector<MeshLib::Node*> nodes;
std::vector<MeshLib::Element*> elems;
std::map<std::size_t, std::size_t> node_id_map;
INFO("Parsing line %s", mesh_name.c_str());
if (!parseLine(in, nodes, elems, node_id_map, mesh_prop))
{
ERR("Error parsing Line %s.", mesh_name.c_str());
clearData(nodes, elems);
return nullptr;
}
return new MeshLib::Mesh(mesh_name, nodes, elems, mesh_prop);
}
else if (type == GOCAD_DATA_TYPE::TSURF && str[0] == "TFACE")
{
std::vector<MeshLib::Node*> nodes;
std::vector<MeshLib::Element*> elems;
@@ -120,7 +146,6 @@ MeshLib::Mesh* GocadTSurfaceReader::readMesh(std::ifstream& in,
clearData(nodes, elems);
return nullptr;
}

return new MeshLib::Mesh(mesh_name, nodes, elems, mesh_prop);
}
else
@@ -133,7 +158,7 @@ MeshLib::Mesh* GocadTSurfaceReader::readMesh(std::ifstream& in,
return nullptr;
}

bool GocadTSurfaceReader::TSurfaceFound(std::ifstream& in) const
GOCAD_DATA_TYPE GocadAsciiReader::datasetFound(std::ifstream& in) const
{
std::string line;
while (std::getline(in, line))
@@ -142,35 +167,38 @@ bool GocadTSurfaceReader::TSurfaceFound(std::ifstream& in) const
{
continue;
}
if (line.substr(0, 11) == "GOCAD TSurf")

if (line.substr(0, 10) == "GOCAD VSet")
{
return true;
// No idea why this is allowed in a *.ts file.
// It should be a whole different file type.
return GOCAD_DATA_TYPE::VSET;
}
if (line.substr(0, 13) == "GOCAD Model3d")
else if (line.substr(0, 11) == "GOCAD PLine")

This comment has been minimized.

Copy link
@endJunction

endJunction Aug 2, 2019

Member

I'd move this string to enum conversion to the place where the enum is defined.

Also no else after return is needed.

This comment has been minimized.

Copy link
@rinkk

rinkk Aug 6, 2019

Author Member

Do you suggest that I should implement a enum -> string conversion for this one use?

The return does not follow "else" but "while" and is required in case the end of the file is reached.

{
if (!skipModel3d(in))
{
ERR("Error parsing Model3d");
return false;
}
return GOCAD_DATA_TYPE::PLINE;
}
else if (line.substr(0, 11) == "GOCAD TSurf")
{
return GOCAD_DATA_TYPE::TSURF;
}
else if (line.substr(0, 13) == "GOCAD Model3d")
{
return GOCAD_DATA_TYPE::MODEL3D;
}
else
{
ERR("No TSurf-identifier found...");
return false;
ERR("No known identifier found...");
return GOCAD_DATA_TYPE::UNDEFINED;
}
}
return false;
return GOCAD_DATA_TYPE::UNDEFINED;
}

bool GocadTSurfaceReader::isCommentLine(std::string const& str) const
bool GocadAsciiReader::isCommentLine(std::string const& str) const
{
return (str.substr(0, 1) == "#");
}

bool GocadTSurfaceReader::parseHeader(std::ifstream& in, std::string& mesh_name)
bool GocadAsciiReader::parseHeader(std::ifstream& in, std::string& mesh_name)
{
std::string line;
while (std::getline(in, line))
@@ -190,7 +218,7 @@ bool GocadTSurfaceReader::parseHeader(std::ifstream& in, std::string& mesh_name)
return false;
}

bool GocadTSurfaceReader::parsePropertyClass(std::ifstream& in) const
bool GocadAsciiReader::parsePropertyClass(std::ifstream& in) const
{
std::string line;
while (std::getline(in, line))
@@ -222,7 +250,7 @@ std::string propertyCheck(std::string const& strng)
return std::string("");
}

bool GocadTSurfaceReader::parseProperties(std::ifstream& in,
bool GocadAsciiReader::parseProperties(std::ifstream& in,
std::vector<std::string> const& names,
MeshLib::Properties& mesh_prop)
{
@@ -272,7 +300,45 @@ bool GocadTSurfaceReader::parseProperties(std::ifstream& in,
return false;
}

bool GocadTSurfaceReader::parseSurface(
bool GocadAsciiReader::parseLine(
std::ifstream& in,
std::vector<MeshLib::Node*>& nodes,
std::vector<MeshLib::Element*>& elems,
std::map<std::size_t, std::size_t>& node_id_map,
MeshLib::Properties& mesh_prop)
{
if (!parseNodes(in, nodes, node_id_map, mesh_prop))
{
return false;
}
if (!parseLineSegments(in, nodes, elems, node_id_map, mesh_prop))
{
return false;
}

std::string line;
while (std::getline(in, line))
{
std::vector<std::string> str = BaseLib::splitString(line);
if (str[0] == "ILINE")
{
parseLine(in, nodes, elems, node_id_map, mesh_prop);
return true;
}
else if (line == "END")

This comment has been minimized.

Copy link
@endJunction

endJunction Aug 1, 2019

Member

else after a return is not needed...

This comment has been minimized.

Copy link
@rinkk

rinkk Aug 2, 2019

Author Member

✔️

{
return true;
}
else
{
WARN("GocadTSurfaceReader::parseLine() - Unknown keyword found: %s", line.c_str());
}
}
ERR("%s", eof_error.c_str());
return false;
}

bool GocadAsciiReader::parseSurface(
std::ifstream& in,
std::vector<MeshLib::Node*>& nodes,
std::vector<MeshLib::Element*>& elems,
@@ -311,10 +377,7 @@ bool GocadTSurfaceReader::parseSurface(
}
else
{
WARN(
"GocadTSurfaceReader::parseSurface() - Unknown keyword found: "
"%s",
line.c_str());
WARN("GocadTSurfaceReader::parseSurface() - Unknown keyword found: %s", line.c_str());
}
}
ERR("%s", eof_error.c_str());
@@ -330,7 +393,7 @@ MeshLib::Node* createNode(std::stringstream& sstr)
return new MeshLib::Node(data, id);
}

bool GocadTSurfaceReader::parseNodes(
bool GocadAsciiReader::parseNodes(
std::ifstream& in,
std::vector<MeshLib::Node*>& nodes,
std::map<std::size_t, std::size_t>& node_id_map,
@@ -345,7 +408,13 @@ bool GocadTSurfaceReader::parseNodes(
while (std::getline(in, line))
{
std::vector<std::string> str = BaseLib::splitString(line);
if (line.substr(0, 4) == "TRGL")
if (line.substr(0, 3) == "SEG")
{
in.seekg(pos);
return true;
}

else if (line.substr(0, 4) == "TRGL")
{
in.seekg(pos);
return true;
@@ -358,9 +427,7 @@ bool GocadTSurfaceReader::parseNodes(
if (!(line.substr(0, 4) == "VRTX" || line.substr(0, 5) == "PVRTX" ||
line.substr(0, 4) == "ATOM"))
{
WARN(
"GocadTSurfaceReader::parseNodes() - Unknown keyword found: %s",
line.c_str());
WARN("GocadTSurfaceReader::parseNodes() - Unknown keyword found: %s", line.c_str());

This comment has been minimized.

Copy link
@TomFischer

TomFischer Aug 2, 2019

Member

GocadTSurfaceReader -> GocadAsciiReader?

This comment has been minimized.

Copy link
@rinkk

rinkk Aug 2, 2019

Author Member

✔️

continue;
}

@@ -397,7 +464,62 @@ bool GocadTSurfaceReader::parseNodes(
return false;
}

bool GocadTSurfaceReader::parseElements(
bool GocadAsciiReader::parseLineSegments(
std::ifstream& in,
std::vector<MeshLib::Node*>& nodes,
std::vector<MeshLib::Element*>& elems,
std::map<std::size_t, std::size_t> const& node_id_map,
MeshLib::Properties& mesh_prop)
{
std::string keyword;

This comment has been minimized.

Copy link
@endJunction

endJunction Aug 1, 2019

Member

Please constrain variables to minimum possible scope.

This comment has been minimized.

Copy link
@rinkk

rinkk Aug 2, 2019

Author Member

✔️

This comment has been minimized.

Copy link
@endJunction

endJunction Aug 2, 2019

Member

Same goes for data array.

This comment has been minimized.

Copy link
@rinkk

rinkk Aug 6, 2019

Author Member

✔️

std::array<std::size_t, 3> data;
MeshLib::PropertyVector<int>& mat_ids =
*mesh_prop.getPropertyVector<int>(mat_id_name);
int current_mat_id(0);
if (!mat_ids.empty())
{
current_mat_id = (*std::max_element(mat_ids.begin(), mat_ids.end()))++;
}
std::streampos pos = in.tellg();
std::size_t id(0);
std::string line;
while (std::getline(in, line))
{
if (line.empty() || isCommentLine(line))
{
continue;
}
if (line.substr(0, 3) == "SEG")
{
std::stringstream sstr(line);
sstr >> keyword >> data[0] >> data[1];
This conversation was marked as resolved by endJunction

This comment has been minimized.

Copy link
@endJunction

endJunction Aug 2, 2019

Member

Also not clear is why data is 3 long, but only two elements are used.

This comment has been minimized.

Copy link
@rinkk

rinkk Aug 6, 2019

Author Member

✔️

std::array<MeshLib::Node*, 2> elem_nodes;
for (std::size_t i = 0; i < 2; ++i)
{
auto const it = node_id_map.find(data[i]);
if (it == node_id_map.end() || it->second >= nodes.size())
{
ERR("Error: Node ID (%d) out of range (0, %d).", data[i],
nodes.back()->getID());
return false;
}
elem_nodes[i] = nodes[it->second];
}
elems.push_back(new MeshLib::Line(elem_nodes, id++));
mat_ids.push_back(current_mat_id);
}
else
{
in.seekg(pos);
return true;
}
pos = in.tellg();
}
ERR("%s", eof_error.c_str());
return false;
}

bool GocadAsciiReader::parseElements(
std::ifstream& in,
std::vector<MeshLib::Node*>& nodes,
std::vector<MeshLib::Element*>& elems,
@@ -452,7 +574,7 @@ bool GocadTSurfaceReader::parseElements(
return false;
}

bool GocadTSurfaceReader::skipModel3d(std::ifstream& in) const
bool GocadAsciiReader::skipToEND(std::ifstream& in) const
This conversation was marked as resolved by endJunction

This comment has been minimized.

Copy link
@endJunction

endJunction Aug 1, 2019

Member
Suggested change
bool GocadAsciiReader::skipToEND(std::ifstream& in) const
bool GocadAsciiReader::skipToEnd(std::ifstream& in) const

This comment has been minimized.

Copy link
@rinkk

rinkk Aug 2, 2019

Author Member

I used this notation because the tag in the gocad-file is actually called END. If it's called "skipToEnd" it suggests that it jumps to the end of the file?

{
std::string line;
while (std::getline(in, line))
@@ -466,7 +588,7 @@ bool GocadTSurfaceReader::skipModel3d(std::ifstream& in) const
return false;
}

void GocadTSurfaceReader::clearData(std::vector<MeshLib::Node*>& nodes,
void GocadAsciiReader::clearData(std::vector<MeshLib::Node*>& nodes,
std::vector<MeshLib::Element*>& elems)
{
for (MeshLib::Element* e : elems)
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.