Skip to content

Commit

Permalink
added optional too-few-points check when importing curves
Browse files Browse the repository at this point in the history
  • Loading branch information
@clausnagel
clausnagel committed Aug 29, 2024
1 parent d998951 commit c85e3df
Show file tree
Hide file tree
Showing 3 changed files with 88 additions and 55 deletions.
10 changes: 5 additions & 5 deletions .../src/main/java/org/citydb/core/operation/importer/database/content/DBSurfaceGeometry.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
import org.citydb.core.operation.importer.CityGMLImportException;
import org.citydb.core.operation.importer.util.GeometryConverter;
import org.citydb.core.operation.importer.util.LocalAppearanceHandler;
import org.citydb.core.operation.importer.util.RingValidator;
import org.citydb.core.operation.importer.util.GeometryValidator;
import org.citydb.core.util.CoreConstants;
import org.citydb.util.log.Logger;
import org.citygml4j.model.citygml.texturedsurface._AbstractAppearance;
Expand Down Expand Up @@ -71,7 +71,7 @@ public class DBSurfaceGeometry implements DBImporter {
private final DBAppearance appearanceImporter;
private final IdManager ids;
private final LocalAppearanceHandler localAppearanceHandler;
private final RingValidator ringValidator;
private final GeometryValidator geometryValidator;

private final boolean replaceGmlId;
private final boolean importAppearance;
Expand Down Expand Up @@ -126,7 +126,7 @@ public DBSurfaceGeometry(Connection batchConn, Config config, CityGMLImportManag
localAppearanceHandler = importer.getLocalAppearanceHandler();
geometryConverter = importer.getGeometryConverter();
ids = new IdManager();
ringValidator = new RingValidator(importer.isFailOnError());
geometryValidator = new GeometryValidator(importer.isFailOnError());
}

protected long doImport(AbstractGeometry geometry, long cityObjectId) throws CityGMLImportException, SQLException {
Expand Down Expand Up @@ -228,7 +228,7 @@ private long doImport(AbstractGeometry geometry, long id, long parentId, long ro
AbstractRing exterior = polygon.getExterior().getRing();
if (exterior != null) {
List<Double> points = exterior.toList3d(reverse);
if (!ringValidator.validate(points, exterior))
if (!geometryValidator.isValidRing(points, exterior))
return 0;

if (applyTransformation)
Expand All @@ -254,7 +254,7 @@ private long doImport(AbstractGeometry geometry, long id, long parentId, long ro
AbstractRing interior = property.getRing();
if (interior != null) {
List<Double> interiorPoints = interior.toList3d(reverse);
if (!ringValidator.validate(interiorPoints, interior))
if (!geometryValidator.isValidRing(interiorPoints, interior))
continue;

if (applyTransformation)
Expand Down
50 changes: 24 additions & 26 deletions impexp-core/src/main/java/org/citydb/core/operation/importer/util/GeometryConverter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,12 +48,12 @@

public class GeometryConverter {
private AffineTransformer affineTransformer;
private final RingValidator ringValidator;
private final GeometryValidator geometryValidator;
private final int dbSrid;
private final boolean hasSolidSupport;

public GeometryConverter(AbstractDatabaseAdapter databaseAdapter, boolean failOnError) {
ringValidator = new RingValidator(failOnError);
geometryValidator = new GeometryValidator(failOnError);
dbSrid = databaseAdapter.getConnectionMetaData().getReferenceSystem().getSrid();

// solid geometries are only supported in Oracle 11g or higher
Expand Down Expand Up @@ -289,26 +289,26 @@ public GeometryObject getMultiPoint(ControlPoint controlPoint) {
return null;
}

public GeometryObject getCurve(AbstractCurve curve) {
public GeometryObject getCurve(AbstractCurve curve) throws CityGMLImportException {
if (curve != null) {
List<Double> pointList = curve.toList3d();
if (!pointList.isEmpty()) {
if (geometryValidator.isValidCurve(pointList, curve)) {
return GeometryObject.createCurve(convertPrimitive(pointList), 3, dbSrid);
}
}

return null;
}

public GeometryObject getMultiCurve(MultiCurve multiCurve) {
public GeometryObject getMultiCurve(MultiCurve multiCurve) throws CityGMLImportException {
if (multiCurve != null) {
List<List<Double>> pointList = new ArrayList<>();

if (multiCurve.isSetCurveMember()) {
for (CurveProperty property : multiCurve.getCurveMember()) {
if (property.isSetCurve()) {
List<Double> points = property.getCurve().toList3d();
if (!points.isEmpty()) {
if (geometryValidator.isValidCurve(points, property.getCurve())) {
pointList.add(points);
}
}
Expand All @@ -318,7 +318,7 @@ public GeometryObject getMultiCurve(MultiCurve multiCurve) {
for (AbstractCurve curve : property.getCurve()) {
if (curve != null) {
List<Double> points = curve.toList3d();
if (!points.isEmpty()) {
if (geometryValidator.isValidCurve(points, curve)) {
pointList.add(points);
}
}
Expand All @@ -333,7 +333,7 @@ public GeometryObject getMultiCurve(MultiCurve multiCurve) {
return null;
}

public GeometryObject getCurveGeometry(GeometricComplex geometricComplex) {
public GeometryObject getCurveGeometry(GeometricComplex geometricComplex) throws CityGMLImportException {
if (geometricComplex != null && geometricComplex.isSetElement()) {
List<List<Double>> pointList = new ArrayList<>();

Expand All @@ -342,7 +342,7 @@ public GeometryObject getCurveGeometry(GeometricComplex geometricComplex) {
AbstractGeometricPrimitive primitive = primitiveProperty.getGeometricPrimitive();
if (primitive instanceof AbstractCurve) {
List<Double> points = ((AbstractCurve) primitive).toList3d();
if (!points.isEmpty()) {
if (geometryValidator.isValidCurve(points, (AbstractCurve) primitive)) {
pointList.add(points);
}
}
Expand All @@ -360,15 +360,15 @@ public GeometryObject getCurveGeometry(GeometricComplex geometricComplex) {
return null;
}

public GeometryObject getCurveGeometry(MultiGeometry multiGeometry) {
public GeometryObject getCurveGeometry(MultiGeometry multiGeometry) throws CityGMLImportException {
List<List<Double>> pointList = new ArrayList<>();

if (multiGeometry != null) {
if (multiGeometry.isSetGeometryMember()) {
for (GeometryProperty<?> property : multiGeometry.getGeometryMember()) {
if (property.isSetGeometry() && property.getGeometry() instanceof AbstractCurve) {
List<Double> coords = ((AbstractCurve) property.getGeometry()).toList3d();
if (!coords.isEmpty()) {
if (geometryValidator.isValidCurve(coords, (AbstractCurve) property.getGeometry())) {
pointList.add(coords);
}
}
Expand All @@ -379,7 +379,7 @@ public GeometryObject getCurveGeometry(MultiGeometry multiGeometry) {
for (AbstractGeometry member : multiGeometry.getGeometryMembers().getGeometry()) {
if (member instanceof AbstractCurve) {
List<Double> coords = ((AbstractCurve) member).toList3d();
if (!coords.isEmpty()) {
if (geometryValidator.isValidCurve(coords, (AbstractCurve) member)) {
pointList.add(coords);
}
}
Expand All @@ -397,7 +397,7 @@ public GeometryObject getCurveGeometry(MultiGeometry multiGeometry) {
return null;
}

public GeometryObject getMultiCurve(List<LineStringSegmentArrayProperty> propertyList) {
public GeometryObject getMultiCurve(List<LineStringSegmentArrayProperty> propertyList) throws CityGMLImportException {
if (propertyList != null && !propertyList.isEmpty()) {
List<List<Double>> pointList = new ArrayList<>();

Expand All @@ -407,7 +407,7 @@ public GeometryObject getMultiCurve(List<LineStringSegmentArrayProperty> propert

for (LineStringSegment segment : property.getLineStringSegment()) {
List<Double> coords = segment.toList3d();
if (!coords.isEmpty()) {
if (geometryValidator.isValidCurve(coords, segment)) {
points.addAll(coords);
}
}
Expand All @@ -426,13 +426,13 @@ public GeometryObject getMultiCurve(List<LineStringSegmentArrayProperty> propert
return null;
}

public GeometryObject getPoint(PointProperty pointProperty) {
public GeometryObject getPoint(PointProperty pointProperty) throws CityGMLImportException {
return pointProperty != null ?
getPoint(pointProperty.getPoint()) :
null;
}

public GeometryObject getMultiPoint(MultiPointProperty multiPointProperty) {
public GeometryObject getMultiPoint(MultiPointProperty multiPointProperty) throws CityGMLImportException {
return multiPointProperty != null ?
getMultiPoint(multiPointProperty.getMultiPoint()) :
null;
Expand All @@ -444,25 +444,25 @@ public GeometryObject getPointGeometry(GeometricComplexProperty complexProperty)
null;
}

public GeometryObject getCurve(CurveProperty curveProperty) {
public GeometryObject getCurve(CurveProperty curveProperty) throws CityGMLImportException {
return curveProperty != null ?
getCurve(curveProperty.getCurve()) :
null;
}

public GeometryObject getMultiCurve(MultiCurveProperty multiCurveProperty) {
public GeometryObject getMultiCurve(MultiCurveProperty multiCurveProperty) throws CityGMLImportException {
return multiCurveProperty != null ?
getMultiCurve(multiCurveProperty.getMultiCurve()) :
null;
}

public GeometryObject getCurveGeometry(GeometricComplexProperty complexProperty) {
public GeometryObject getCurveGeometry(GeometricComplexProperty complexProperty) throws CityGMLImportException {
return complexProperty != null && complexProperty.isSetGeometricComplex() ?
getCurveGeometry(complexProperty.getGeometricComplex()) :
null;
}

public GeometryObject getPointOrCurveGeometry(AbstractGeometry abstractGeometry) {
public GeometryObject getPointOrCurveGeometry(AbstractGeometry abstractGeometry) throws CityGMLImportException {
switch (abstractGeometry.getGMLClass()) {
case POINT:
return getPoint((Point) abstractGeometry);
Expand Down Expand Up @@ -574,7 +574,7 @@ private List<List<Double>> generatePointList(Polygon polygon, boolean is2d, bool
AbstractRing exteriorRing = polygon.getExterior().getRing();
if (exteriorRing != null) {
List<Double> coords = exteriorRing.toList3d(reverse);
if (!ringValidator.validate(coords, exteriorRing)) {
if (!geometryValidator.isValidRing(coords, exteriorRing)) {
return null;
}

Expand All @@ -585,11 +585,9 @@ private List<List<Double>> generatePointList(Polygon polygon, boolean is2d, bool
AbstractRing interiorRing = abstractRingProperty.getRing();
if (interiorRing != null) {
coords = interiorRing.toList3d(reverse);
if (!ringValidator.validate(coords, interiorRing)) {
continue;
if (geometryValidator.isValidRing(coords, interiorRing)) {
pointList.add(coords);
}

pointList.add(coords);
}
}
}
Expand Down Expand Up @@ -681,7 +679,7 @@ public void visit(AbstractRing ring) {
// required to handle surface patches such as triangles and rectangles
List<Double> points = ring.toList3d(reverse);
try {
if (ringValidator.validate(points, ring)) {
if (geometryValidator.isValidRing(points, ring)) {
pointList.add(points);
rings.add(ringNo);
ringNo++;
Expand Down
83 changes: 59 additions & 24 deletions ...peration/importer/util/RingValidator.java → ...tion/importer/util/GeometryValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,37 +30,69 @@
import org.citydb.core.operation.importer.CityGMLImportException;
import org.citydb.core.util.CoreConstants;
import org.citydb.util.log.Logger;
import org.citygml4j.model.gml.GML;
import org.citygml4j.model.gml.geometry.AbstractGeometry;
import org.citygml4j.model.gml.geometry.primitives.AbstractCurve;
import org.citygml4j.model.gml.geometry.primitives.AbstractCurveSegment;
import org.citygml4j.model.gml.geometry.primitives.AbstractRing;
import org.citygml4j.util.child.ChildInfo;

import java.util.List;

public class RingValidator {
public class GeometryValidator {
private final Logger log = Logger.getInstance();
private final boolean failOnError;

public RingValidator(boolean failOnError) {
public GeometryValidator(boolean failOnError) {
this.failOnError = failOnError;
}

public RingValidator() {
public GeometryValidator() {
this(false);
}

public boolean validate(List<Double> coordinates, AbstractRing ring) throws CityGMLImportException {
public boolean isValidCurve(List<Double> coordinates, AbstractCurve curve) throws CityGMLImportException {
if (coordinates == null || curve.hasLocalProperty(CoreConstants.GEOMETRY_INVALID)) {
return false;
}

// too few points
if (coordinates.size() / 3 < 2) {
logOrThrowErrorMessage(curve, "Curve has too few points.");
return false;
}

return true;
}

public boolean isValidCurve(List<Double> coordinates, AbstractCurveSegment segment) throws CityGMLImportException {
if (coordinates == null) {
return false;
}

// too few points
if (coordinates.size() / 3 < 2) {
logOrThrowErrorMessage(segment, "Curve segment has too few points.");
return false;
}

return true;
}

public boolean isValidRing(List<Double> coordinates, AbstractRing ring) throws CityGMLImportException {
if (coordinates == null || ring.hasLocalProperty(CoreConstants.GEOMETRY_INVALID)) {
return false;
}

// check closedness
if (coordinates.size() >= 9 && !isClosed(coordinates)) {
log.debug(getGeometrySignature(ring) + ": Fixed unclosed ring by appending its first point.");
log.debug(getGeometrySignature(getParentOrSelf(ring)) +
": Fixed unclosed ring by appending its first point.");
}

// too few points
if (coordinates.size() / 3 < 4) {
logOrThrowErrorMessage(ring, "Ring has too few points.");
logOrThrowErrorMessage(getParentOrSelf(ring), "Ring has too few points.");
return false;
}

Expand Down Expand Up @@ -88,34 +120,37 @@ private boolean isClosed(List<Double> coords) {
return true;
}

public String getGeometrySignature(AbstractRing ring) {
AbstractGeometry geometry = new ChildInfo().getParentGeometry(ring);
if (geometry == null) {
geometry = ring;
}

String signature = "gml:" + geometry.getGMLClass().toString();
String gmlId = geometry.hasLocalProperty(CoreConstants.OBJECT_ORIGINAL_GMLID) ?
(String) geometry.getLocalProperty(CoreConstants.OBJECT_ORIGINAL_GMLID) :
geometry.getId();

if (gmlId != null) {
signature += " '" + gmlId + "'";
} else {
signature += " (unknown gml:id)";
private String getGeometrySignature(GML gml) {
String signature = "gml:" + gml.getGMLClass().toString();
if (gml instanceof AbstractGeometry) {
AbstractGeometry geometry = (AbstractGeometry) gml;
String gmlId = geometry.hasLocalProperty(CoreConstants.OBJECT_ORIGINAL_GMLID) ?
(String) geometry.getLocalProperty(CoreConstants.OBJECT_ORIGINAL_GMLID) :
geometry.getId();

signature += gmlId != null ?
" '" + gmlId + "'" :
" (unknown gml:id)";
}

return signature;
}

private void logOrThrowErrorMessage(AbstractRing ring, String message) throws CityGMLImportException {
ring.setLocalProperty(CoreConstants.GEOMETRY_INVALID, message);
message = getGeometrySignature(ring) + ": " + message;
private void logOrThrowErrorMessage(GML gml, String message) throws CityGMLImportException {
if (gml instanceof AbstractGeometry) {
((AbstractGeometry) gml).setLocalProperty(CoreConstants.GEOMETRY_INVALID, message);
}

message = getGeometrySignature(gml) + ": " + message;
if (!failOnError) {
log.error(message);
} else {
throw new CityGMLImportException(message);
}
}

private AbstractGeometry getParentOrSelf(AbstractGeometry geometry) {
AbstractGeometry parent = new ChildInfo().getParentGeometry(geometry);
return parent != null ? parent : geometry;
}
}

0 comments on commit c85e3df

Please sign in to comment.