Copilot commented on code in PR #2380: URL: https://github.com/apache/sedona/pull/2380#discussion_r2420658796
########## common/src/main/java/org/apache/sedona/common/approximate/StraightSkeleton.java: ########## @@ -0,0 +1,168 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.sedona.common.approximate; + +import java.util.*; +import javax.vecmath.Point3d; +import org.locationtech.jts.geom.*; +import org.twak.camp.Corner; +import org.twak.camp.Edge; +import org.twak.camp.Machine; +import org.twak.camp.Output; +import org.twak.camp.Skeleton; +import org.twak.utils.collections.Loop; +import org.twak.utils.collections.LoopL; + +/** + * Straight skeleton computation for polygons using the campskeleton library. + * + * <p>The straight skeleton is a method of representing a polygon by a topological skeleton. It is + * defined by a continuous shrinking process in which each edge of the polygon is moved inward in a + * parallel manner. This implementation uses the campskeleton library which implements the weighted + * straight skeleton algorithm based on Felkel's approach. + * + * <p>References: - Tom Kelly and Peter Wonka (2011). Interactive Architectural Modeling with + * Procedural Extrusions - Felkel, P., & Obdržálek, Š. (1998). Straight skeleton implementation + */ +public class StraightSkeleton { + + public StraightSkeleton() {} + + /** + * Compute the straight skeleton for a polygon. + * + * @param polygon Input polygon (must be simple, non-self-intersecting) + * @return MultiLineString representing the straight skeleton edges + */ + public Geometry computeSkeleton(Polygon polygon) { + if (polygon == null || polygon.isEmpty()) { + return null; + } + + GeometryFactory factory = polygon.getFactory(); + + try { + // Convert JTS polygon to campskeleton format + LoopL<Edge> input = convertPolygonToEdges(polygon); + + // Compute straight skeleton + Skeleton skeleton = new Skeleton(input, true); + skeleton.skeleton(); + + // Check if skeleton computation succeeded + if (skeleton.output == null || skeleton.output.edges == null) { + System.err.println( + "WARNING: Campskeleton failed to produce output for polygon: " + polygon.toText()); Review Comment: Using System.err.println for logging is not recommended. Consider using a proper logging framework like SLF4J or java.util.logging to allow for better log level control and configuration. ########## common/src/main/java/org/apache/sedona/common/approximate/StraightSkeleton.java: ########## @@ -0,0 +1,168 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.sedona.common.approximate; + +import java.util.*; +import javax.vecmath.Point3d; +import org.locationtech.jts.geom.*; +import org.twak.camp.Corner; +import org.twak.camp.Edge; +import org.twak.camp.Machine; +import org.twak.camp.Output; +import org.twak.camp.Skeleton; +import org.twak.utils.collections.Loop; +import org.twak.utils.collections.LoopL; + +/** + * Straight skeleton computation for polygons using the campskeleton library. + * + * <p>The straight skeleton is a method of representing a polygon by a topological skeleton. It is + * defined by a continuous shrinking process in which each edge of the polygon is moved inward in a + * parallel manner. This implementation uses the campskeleton library which implements the weighted + * straight skeleton algorithm based on Felkel's approach. + * + * <p>References: - Tom Kelly and Peter Wonka (2011). Interactive Architectural Modeling with + * Procedural Extrusions - Felkel, P., & Obdržálek, Š. (1998). Straight skeleton implementation + */ +public class StraightSkeleton { + + public StraightSkeleton() {} + + /** + * Compute the straight skeleton for a polygon. + * + * @param polygon Input polygon (must be simple, non-self-intersecting) + * @return MultiLineString representing the straight skeleton edges + */ + public Geometry computeSkeleton(Polygon polygon) { + if (polygon == null || polygon.isEmpty()) { + return null; + } + + GeometryFactory factory = polygon.getFactory(); + + try { + // Convert JTS polygon to campskeleton format + LoopL<Edge> input = convertPolygonToEdges(polygon); + + // Compute straight skeleton + Skeleton skeleton = new Skeleton(input, true); + skeleton.skeleton(); + + // Check if skeleton computation succeeded + if (skeleton.output == null || skeleton.output.edges == null) { + System.err.println( + "WARNING: Campskeleton failed to produce output for polygon: " + polygon.toText()); + return factory.createMultiLineString(new LineString[0]); + } + + // Convert skeleton output to JTS geometry + List<LineString> skeletonEdges = extractSkeletonEdges(skeleton, factory); + + if (skeletonEdges.isEmpty()) { + System.err.println("WARNING: No skeleton edges extracted for polygon: " + polygon.toText()); Review Comment: Using System.err.println for logging is not recommended. Consider using a proper logging framework like SLF4J or java.util.logging to allow for better log level control and configuration. ########## common/src/test/java/org/apache/sedona/common/StraightSkeletonTest.java: ########## @@ -0,0 +1,454 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.sedona.common; + +import static org.junit.Assert.*; + +import org.junit.Test; +import org.locationtech.jts.geom.Geometry; +import org.locationtech.jts.geom.LineString; +import org.locationtech.jts.geom.MultiLineString; + +/** + * Comprehensive tests for Straight Skeleton implementation. + * + * <p>Tests cover: - Simple polygons (square, rectangle, triangle) - Complex polygons (L-shape, + * U-shape, T-shape, star) - Polygons with reflex angles - MultiPolygon geometries - Edge cases + * (very narrow, concave shapes) + */ +public class StraightSkeletonTest { + + /** + * Helper method to test a polygon and verify basic properties of its medial axis. + * + * @param testName Name of the test for reporting + * @param wkt WKT representation of the polygon + * @param expectedSegments Expected number of skeleton segments + */ + private void testPolygon(String testName, String wkt, int expectedSegments) throws Exception { + testPolygon(testName, wkt, expectedSegments, true); + } + + /** + * Helper method to test a polygon and verify basic properties of its medial axis. + * + * @param testName Name of the test for reporting + * @param wkt WKT representation of the polygon + * @param expectedSegments Expected number of skeleton segments + * @param strictLengthCheck If false, skip perimeter comparison (for complex road networks) + */ + private void testPolygon( + String testName, String wkt, int expectedSegments, boolean strictLengthCheck) + throws Exception { + Geometry polygon = Constructors.geomFromWKT(wkt, 0); + Geometry medialAxis = Functions.straightSkeleton(polygon); + + // Print skeleton for visualization (only for road network tests) + if (testName.contains("Road") + || testName.contains("Junction") + || testName.contains("Intersection")) { + System.out.println("SKELETON_WKT:" + testName + ":" + medialAxis.toText()); + } + + // Basic assertions + assertNotNull(testName + ": Medial axis should not be null", medialAxis); + assertTrue( + testName + ": Result should be MultiLineString", medialAxis instanceof MultiLineString); + + int numSegments = medialAxis.getNumGeometries(); + + // Debug: print actual count for comparison + if (numSegments != expectedSegments && expectedSegments >= 0) { + System.out.printf("%s: Expected %d, got %d%n", testName, expectedSegments, numSegments); + } + + // If expectedSegments is -1, skip exact count assertion (just verify it works) + if (expectedSegments >= 0) { + assertEquals( + testName + ": Should have exactly " + expectedSegments + " segments", Review Comment: Using System.out.printf for test output is not recommended. Consider using proper test assertions or logging framework for better test output management. ```suggestion // If expectedSegments is -1, skip exact count assertion (just verify it works) // If expectedSegments is -1, skip exact count assertion (just verify it works) if (expectedSegments >= 0) { assertEquals( testName + ": Should have exactly " + expectedSegments + " segments, but got " + numSegments, ``` ########## common/src/test/java/org/apache/sedona/common/StraightSkeletonTest.java: ########## @@ -0,0 +1,454 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.sedona.common; + +import static org.junit.Assert.*; + +import org.junit.Test; +import org.locationtech.jts.geom.Geometry; +import org.locationtech.jts.geom.LineString; +import org.locationtech.jts.geom.MultiLineString; + +/** + * Comprehensive tests for Straight Skeleton implementation. + * + * <p>Tests cover: - Simple polygons (square, rectangle, triangle) - Complex polygons (L-shape, + * U-shape, T-shape, star) - Polygons with reflex angles - MultiPolygon geometries - Edge cases + * (very narrow, concave shapes) + */ +public class StraightSkeletonTest { + + /** + * Helper method to test a polygon and verify basic properties of its medial axis. + * + * @param testName Name of the test for reporting + * @param wkt WKT representation of the polygon + * @param expectedSegments Expected number of skeleton segments + */ + private void testPolygon(String testName, String wkt, int expectedSegments) throws Exception { + testPolygon(testName, wkt, expectedSegments, true); + } + + /** + * Helper method to test a polygon and verify basic properties of its medial axis. + * + * @param testName Name of the test for reporting + * @param wkt WKT representation of the polygon + * @param expectedSegments Expected number of skeleton segments + * @param strictLengthCheck If false, skip perimeter comparison (for complex road networks) + */ + private void testPolygon( + String testName, String wkt, int expectedSegments, boolean strictLengthCheck) + throws Exception { + Geometry polygon = Constructors.geomFromWKT(wkt, 0); + Geometry medialAxis = Functions.straightSkeleton(polygon); + + // Print skeleton for visualization (only for road network tests) + if (testName.contains("Road") + || testName.contains("Junction") + || testName.contains("Intersection")) { + System.out.println("SKELETON_WKT:" + testName + ":" + medialAxis.toText()); + } Review Comment: Using System.out.println for test output is not recommended. Consider using a proper logging framework or test framework assertions. If debugging output is needed, use test-specific logging utilities. ########## common/src/test/java/org/apache/sedona/common/StraightSkeletonTest.java: ########## @@ -0,0 +1,454 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.sedona.common; + +import static org.junit.Assert.*; + +import org.junit.Test; +import org.locationtech.jts.geom.Geometry; +import org.locationtech.jts.geom.LineString; +import org.locationtech.jts.geom.MultiLineString; + +/** + * Comprehensive tests for Straight Skeleton implementation. + * + * <p>Tests cover: - Simple polygons (square, rectangle, triangle) - Complex polygons (L-shape, + * U-shape, T-shape, star) - Polygons with reflex angles - MultiPolygon geometries - Edge cases + * (very narrow, concave shapes) + */ +public class StraightSkeletonTest { + + /** + * Helper method to test a polygon and verify basic properties of its medial axis. + * + * @param testName Name of the test for reporting + * @param wkt WKT representation of the polygon + * @param expectedSegments Expected number of skeleton segments + */ + private void testPolygon(String testName, String wkt, int expectedSegments) throws Exception { + testPolygon(testName, wkt, expectedSegments, true); + } + + /** + * Helper method to test a polygon and verify basic properties of its medial axis. + * + * @param testName Name of the test for reporting + * @param wkt WKT representation of the polygon + * @param expectedSegments Expected number of skeleton segments + * @param strictLengthCheck If false, skip perimeter comparison (for complex road networks) + */ + private void testPolygon( + String testName, String wkt, int expectedSegments, boolean strictLengthCheck) + throws Exception { + Geometry polygon = Constructors.geomFromWKT(wkt, 0); + Geometry medialAxis = Functions.straightSkeleton(polygon); + + // Print skeleton for visualization (only for road network tests) + if (testName.contains("Road") + || testName.contains("Junction") + || testName.contains("Intersection")) { + System.out.println("SKELETON_WKT:" + testName + ":" + medialAxis.toText()); + } + + // Basic assertions + assertNotNull(testName + ": Medial axis should not be null", medialAxis); + assertTrue( + testName + ": Result should be MultiLineString", medialAxis instanceof MultiLineString); + + int numSegments = medialAxis.getNumGeometries(); + + // Debug: print actual count for comparison + if (numSegments != expectedSegments && expectedSegments >= 0) { + System.out.printf("%s: Expected %d, got %d%n", testName, expectedSegments, numSegments); + } + + // If expectedSegments is -1, skip exact count assertion (just verify it works) + if (expectedSegments >= 0) { + assertEquals( + testName + ": Should have exactly " + expectedSegments + " segments", + expectedSegments, + numSegments); + } else { + // Just verify we got some segments + assertTrue(testName + ": Should produce at least one segment", numSegments > 0); + System.out.printf("%s: Produced %d segments%n", testName, numSegments); Review Comment: Using System.out.printf for test output is not recommended. Consider using proper test assertions or logging framework for better test output management. ########## common/src/main/java/org/apache/sedona/common/approximate/StraightSkeleton.java: ########## @@ -0,0 +1,168 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.sedona.common.approximate; + +import java.util.*; +import javax.vecmath.Point3d; +import org.locationtech.jts.geom.*; +import org.twak.camp.Corner; +import org.twak.camp.Edge; +import org.twak.camp.Machine; +import org.twak.camp.Output; +import org.twak.camp.Skeleton; +import org.twak.utils.collections.Loop; +import org.twak.utils.collections.LoopL; + +/** + * Straight skeleton computation for polygons using the campskeleton library. + * + * <p>The straight skeleton is a method of representing a polygon by a topological skeleton. It is + * defined by a continuous shrinking process in which each edge of the polygon is moved inward in a + * parallel manner. This implementation uses the campskeleton library which implements the weighted + * straight skeleton algorithm based on Felkel's approach. + * + * <p>References: - Tom Kelly and Peter Wonka (2011). Interactive Architectural Modeling with + * Procedural Extrusions - Felkel, P., & Obdržálek, Š. (1998). Straight skeleton implementation + */ +public class StraightSkeleton { + + public StraightSkeleton() {} + + /** + * Compute the straight skeleton for a polygon. + * + * @param polygon Input polygon (must be simple, non-self-intersecting) + * @return MultiLineString representing the straight skeleton edges + */ + public Geometry computeSkeleton(Polygon polygon) { + if (polygon == null || polygon.isEmpty()) { + return null; + } + + GeometryFactory factory = polygon.getFactory(); + + try { + // Convert JTS polygon to campskeleton format + LoopL<Edge> input = convertPolygonToEdges(polygon); + + // Compute straight skeleton + Skeleton skeleton = new Skeleton(input, true); + skeleton.skeleton(); + + // Check if skeleton computation succeeded + if (skeleton.output == null || skeleton.output.edges == null) { + System.err.println( + "WARNING: Campskeleton failed to produce output for polygon: " + polygon.toText()); + return factory.createMultiLineString(new LineString[0]); + } + + // Convert skeleton output to JTS geometry + List<LineString> skeletonEdges = extractSkeletonEdges(skeleton, factory); + + if (skeletonEdges.isEmpty()) { + System.err.println("WARNING: No skeleton edges extracted for polygon: " + polygon.toText()); + return factory.createMultiLineString(new LineString[0]); + } + + return factory.createMultiLineString(skeletonEdges.toArray(new LineString[0])); + + } catch (Exception e) { + System.err.println( + "ERROR: Failed to compute straight skeleton for polygon: " + polygon.toText()); + e.printStackTrace(); Review Comment: Using System.err.println and printStackTrace for error logging is not recommended. Consider using a proper logging framework like SLF4J or java.util.logging to allow for better log level control and structured error handling. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
