Skip to content

Commit

Permalink
Updates based on review.
Browse files Browse the repository at this point in the history 
There are still private headers in EptReader.
  • Loading branch information
@abellgithub
abellgithub committed May 27, 2020
1 parent 6b02c9a commit 1f72e5f
Show file tree
Hide file tree
Showing 11 changed files with 80 additions and 145 deletions.
28 changes: 6 additions & 22 deletions io/EptAddonWriter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,9 @@ void EptAddonWriter::prepared(PointTableRef table)
}
m_pool.reset(new Pool(threads));

//ABELL
// Perhaps we need to wait to load addons until we have the
// headers/query from metadata.
// Note that we use a generic connector here. In ready() we steal
// the connector that was used in the EptReader that uses any
// headers/query that was set.
m_connector.reset(new Connector());
m_addons = Addon::store(*m_connector, m_args->m_addons, *(table.layout()));
}
Expand All @@ -120,6 +120,7 @@ void EptAddonWriter::ready(PointTableRef table)
m_hierarchyStep = eap->m_hierarchyStep;
m_info = std::move(eap->m_info);
m_hierarchy = std::move(eap->m_hierarchy);
m_connector = std::move(eap->m_connector);
}

void EptAddonWriter::write(const PointViewPtr view)
Expand Down Expand Up @@ -152,7 +153,7 @@ void EptAddonWriter::writeOne(const PointViewPtr view, const Addon& addon) const
PointRef pr(*view);
uint64_t nodeId(0);
uint64_t pointId(0);
for (uint64_t i(0); i < view->size(); ++i)
for (PointId i = 0; i < view->size(); ++i)
{
pr.setPointId(i);
nodeId = pr.getFieldAs<uint64_t>(m_nodeIdDim);
Expand Down Expand Up @@ -203,7 +204,7 @@ void EptAddonWriter::writeOne(const PointViewPtr view, const Addon& addon) const

// Write the top-level addon metadata.
NL::json meta;
meta["type"] = getTypeString(addon.type());
meta["type"] = Dimension::toName(Dimension::base(addon.type()));
meta["size"] = Dimension::size(addon.type());
meta["version"] = "1.0.0";
meta["dataType"] = "binary";
Expand Down Expand Up @@ -248,22 +249,5 @@ void EptAddonWriter::writeHierarchy(const std::string& directory,
}
}

std::string EptAddonWriter::getTypeString(Dimension::Type t) const
{
std::string s;
const auto base(Dimension::base(t));

if (base == Dimension::BaseType::Signed)
s = "signed";
else if (base == Dimension::BaseType::Unsigned)
s = "unsigned";
else if (base == Dimension::BaseType::Floating)
s = "float";
else
throwError("Invalid dimension type");

return s;
}

}

80 changes: 50 additions & 30 deletions io/EptReader.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,23 +62,6 @@ const StaticPluginInfo s_info
{ "ept" }
};

StringMap toStringMap(const NL::json& fwd)
{
StringMap map;

if (fwd.is_null())
return map;
if (!fwd.is_object())
throw pdal_error("Not an object type.");
for (auto& entry : fwd.items())
{
if (!entry.value().is_string())
throw pdal_error("Expected string type.");
map.insert({entry.key(), entry.value().get<std::string>()});
}
return map;
}

}

CREATE_STATIC_STAGE(EptReader, s_info);
Expand Down Expand Up @@ -131,8 +114,7 @@ struct EptReader::Args
NL::json m_ogr;
};

EptReader::EptReader() : m_args(new EptReader::Args), m_currentTile(nullptr),
m_artifactMgr(nullptr)
EptReader::EptReader() : m_args(new EptReader::Args), m_artifactMgr(nullptr)
{}

EptReader::~EptReader()
Expand All @@ -159,6 +141,28 @@ void EptReader::addArgs(ProgramArgs& args)
m_args->m_ogr);
}

namespace
{

// Extract a string map from JSON. Used in setForwards().
StringMap toStringMap(const NL::json& fwd)
{
StringMap map;

if (fwd.is_null())
return map;
if (!fwd.is_object())
throw pdal_error("Not an object type.");
for (auto& entry : fwd.items())
{
if (!entry.value().is_string())
throw pdal_error("Expected string type.");
map.insert({entry.key(), entry.value().get<std::string>()});
}
return map;
}

}

void EptReader::setForwards(StringMap& headers, StringMap& query)
{
Expand Down Expand Up @@ -494,7 +498,6 @@ void EptReader::overlaps()

// First, determine the overlapping nodes from the EPT resource.
overlaps(*m_hierarchy, m_connector->getJson(filename), key);
m_pool->await();
}

// Determine the addons that exist to correspond to tiles.
Expand All @@ -503,8 +506,8 @@ void EptReader::overlaps()
m_nodeId = 1;
std::string filename = addon.hierarchyDir() + key.toString() + ".json";
overlaps(addon.hierarchy(), m_connector->getJson(filename), key);
m_pool->await();
}
m_pool->await();
}


Expand Down Expand Up @@ -535,11 +538,19 @@ void EptReader::overlaps(Hierarchy& target, const NL::json& hier,
if (m_depthEnd && key.d >= m_depthEnd)
return;

// If our key isn't in the hierarchy, we've totally traversed this tree
// branch (there are no lower nodes).
auto it = hier.find(key.toString());
if (it == hier.end())
return;

int64_t numPoints = it->get<int64_t>();
int64_t numPoints(-2); // -2 will trigger an error below
try
{
numPoints = it->get<int64_t>();
}
catch (...)
{}

if (numPoints == -1)
{
Expand All @@ -562,8 +573,10 @@ void EptReader::overlaps(Hierarchy& target, const NL::json& hier,
}
else
{
// Note that when processing addons, we set node IDs which may
// not match the base hierarchy, but it doesn't matter since
// they are never used.
{
//ABELL we could probably use a local mutex to lock the target map.
std::lock_guard<std::mutex> lock(m_mutex);
target.emplace(key, (point_count_t)numPoints, m_nodeId++);
}
Expand All @@ -573,15 +586,19 @@ void EptReader::overlaps(Hierarchy& target, const NL::json& hier,
}
}

bool EptReader::processPoint(PointRef& dst, const TileContents& tile)
void EptReader::checkTile(const TileContents& tile)
{
using namespace Dimension;

if (tile.error().size())
{
m_pool->stop();
throwError("Error reading tile: " + tile.error());
}
}


bool EptReader::processPoint(PointRef& dst, const TileContents& tile)
{
using namespace Dimension;

BasePointTable& t = tile.table();

Expand Down Expand Up @@ -665,6 +682,7 @@ point_count_t EptReader::read(PointViewPtr view, point_count_t count)
TileContents tile = std::move(m_contents.front());
m_contents.pop();
l.unlock();
checkTile(tile);
process(view, tile, count - numRead);
numRead += tile.size();
m_tileCount--;
Expand All @@ -683,7 +701,7 @@ point_count_t EptReader::read(PointViewPtr view, point_count_t count)
{
EptArtifactPtr artifact
(new EptArtifact(std::move(m_info), std::move(m_hierarchy),
m_hierarchyStep));
std::move(m_connector), m_hierarchyStep));
m_artifactMgr->put("ept", artifact);
}

Expand Down Expand Up @@ -719,7 +737,9 @@ bool EptReader::processOne(PointRef& point)
std::unique_lock<std::mutex> l(m_mutex);
if (m_contents.size())
{
m_currentTile = &m_contents.front();
m_currentTile.reset(
new TileContents(std::move(m_contents.front())));
m_contents.pop();
l.unlock();
if (m_hierarchyIter != m_hierarchy->cend())
load(*m_hierarchyIter++);
Expand All @@ -728,6 +748,7 @@ bool EptReader::processOne(PointRef& point)
else
m_contentsCv.wait(l);
} while (true);
checkTile(*m_currentTile);
}

bool ok = processPoint(point, *m_currentTile);
Expand All @@ -738,8 +759,7 @@ bool EptReader::processOne(PointRef& point)
if (m_pointId == m_currentTile->size())
{
m_pointId = 0;
m_currentTile = nullptr;
m_contents.pop();
m_currentTile.reset();
if (--m_tileCount == 0)
return false;
}
Expand Down
7 changes: 2 additions & 5 deletions io/EptReader.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ class PDAL_DLL EptReader : public Reader, public Streamable
point_count_t count);
bool processPoint(PointRef& dst, const TileContents& tile);
void load(const Overlap& overlap);
void checkTile(const TileContents& tile);

std::unique_ptr<Connector> m_connector;
std::unique_ptr<EptInfo> m_info;
Expand All @@ -111,10 +112,6 @@ class PDAL_DLL EptReader : public Reader, public Streamable
std::unique_ptr<Pool> m_pool;
AddonList m_addons;

using StringMap = std::map<std::string, std::string>;
StringMap m_headers;
StringMap m_query;

mutable std::mutex m_mutex;
mutable std::condition_variable m_contentsCv;

Expand All @@ -124,7 +121,7 @@ class PDAL_DLL EptReader : public Reader, public Streamable
Dimension::Id m_nodeIdDim = Dimension::Id::Unknown;
Dimension::Id m_pointIdDim = Dimension::Id::Unknown;

TileContents *m_currentTile;
std::unique_ptr<TileContents> m_currentTile;
ArtifactManager *m_artifactMgr;

PointId m_pointId = 0;
Expand Down
5 changes: 3 additions & 2 deletions io/private/ept/Connector.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@
****************************************************************************/

#include "Connector.hpp"
#include "EptError.hpp"

#include <pdal/pdal_types.hpp>

namespace pdal
{
Expand Down Expand Up @@ -61,7 +62,7 @@ NL::json Connector::getJson(const std::string& path) const
}
catch (NL::json::parse_error& err)
{
throw ept_error("File '" + path + "' contained invalid JSON: " +
throw pdal_error("File '" + path + "' contained invalid JSON: " +
err.what());
}
}
Expand Down
7 changes: 5 additions & 2 deletions io/private/ept/EptArtifact.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
#include <memory>

#include <pdal/Artifact.hpp>
#include "Connector.hpp"
#include "EptInfo.hpp"
#include "Overlap.hpp"

Expand All @@ -47,13 +48,15 @@ class EptArtifact : public Artifact
{
public:
EptArtifact(std::unique_ptr<EptInfo> info,
std::unique_ptr<Hierarchy> hierarchy, size_t hierarchyStep) :
std::unique_ptr<Hierarchy> hierarchy,
std::unique_ptr<Connector> connector, size_t hierarchyStep) :
m_info(std::move(info)), m_hierarchy(std::move(hierarchy)),
m_hierarchyStep(hierarchyStep)
m_connector(std::move(connector)), m_hierarchyStep(hierarchyStep)
{}

std::unique_ptr<EptInfo> m_info;
std::unique_ptr<Hierarchy> m_hierarchy;
std::unique_ptr<Connector> m_connector;
size_t m_hierarchyStep;
};
using EptArtifactPtr = std::shared_ptr<EptArtifact>;
Expand Down
49 changes: 0 additions & 49 deletions io/private/ept/EptError.hpp
View file

This file was deleted.

0 comments on commit 1f72e5f

Please sign in to comment.