tudelft3d / 3dfier

The open-source tool for creating 3D models
http://tudelft3d.github.io/3dfier
GNU General Public License v3.0
529 stars 106 forks source link

Add curve stroking #93

Closed vvmruder closed 4 years ago

vvmruder commented 4 years ago

Solves #92

It is stroking input geometries by standard value of OGR library. Maybe we could add parameter to make it configurable from outside.

Other option would be to simply fail with a clean error message describing the problem.

tcommandeur commented 4 years ago

This code to handle curved polygons is very welcome. In the past I used OGR2OGR to stroke the curved polygons and never looked into integrating it into 3dfier.

Right now the branches of integrate_with_docker and add_curve_stroking are overlapping, could you maybe clean this branch to consist only the changes for curve stroking? Also I am wondering about the reasoning of making the bounding box larger, could you explain?

We should look into making the 'dfMaxAngleStepSizeDegrees' parameter configurable. Also a message of the amount of stroked curved polygons like we did with splitting the multipolygons would be a nice addition. I can help with all this when the pull request is cleaned.

vvmruder commented 4 years ago

The BBOX change was related to another issue I had with swiss coordinates. Iam not absolutely sure, but this BBOX is "semi-filter" for input if no BBOX was provided by configuration. Right?

If so, it would skip polygons because swiss coordinates range to 2'XXX'XXX:1'XXX'XXX values. I thought this might have an impact. If I understood it wrong I can remove this too.

tcommandeur commented 4 years ago

Then I understand and your reasoning is right, it is a default value that should be larger then any coordinate of the input data. It is later set to the polygonal extent. In the process its used for decision making if a point cloud file should be read because the extent of the point cloud overlaps the extent of the input polygons bounding box.

vvmruder commented 4 years ago

So we will keep it as is?

vvmruder commented 4 years ago

Branch is cleaned.

tcommandeur commented 4 years ago

@vvmruder can you test this version with the curvepolygons?

It now reports a log of amount curvepolygons stroked and has the setting as described in the configs readme and configs defaults readme

tcommandeur commented 4 years ago

So we will keep it as is?

Lets use your version.

As soon as you know if the stroking works correctly, also with the max_angle_curvepolygon setting, we can merge this pull request.

vvmruder commented 4 years ago

I tested it. And it is running good. If parameter is not provided and the standard value 0 is used, output is: "Stroked ... CurvedPolygon(s) with maximum angle of 0". This might be misleading info. Do you have idea where to get the real angle limit from OGR default value?

vvmruder commented 4 years ago

I asked on mailing list

vvmruder commented 4 years ago

I got an answer on my question from mailing list: its 4 degrees as default value if parameter is omitted or 0.

tcommandeur commented 4 years ago

I'll make the message read "default 4 degrees" if 0 is applied.