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

Binary writer, improved tilecutting, documentation update #21

Open
wants to merge 87 commits into
base: main
Choose a base branch
from

Conversation

bengtl
Copy link
Member

@bengtl bengtl commented Dec 27, 2024

This update includes enhancements to the MicroJSON library, such as improved metadata handling, the addition of a TileHandler class, and various documentation updates. It also features code cleanups to enhance overall quality and maintainability.

while proceed:
simplified = simplify_recursive(coords, sq_tolerance)
# check that the simplification has enough vertices
if len(simplified) <= min_vertices:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comparison seems suspicious. A few lines before we return on if len(coords) <= min_vertices, and here we keep on simplifying. I tried this branch on my own file and end up in an infinite loop. I changed it for if len(simplified) <= min_vertices and it seems to work.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the feedback, again much appreciated. However, you should not get an infitine loop:

  • The first if-clause outside of the loop will check if there are sufficient vertices to even consider simplification. As it is not possible to increase the number of vertices to beyond the original, it accepts that it has to return less than the expected minimum.
  • The second if-clause inside the loop will check if the tolerance allows to return at least the minimum number of vertices. If not, it will decrease the tolerance (less simplification). Did you run it in a debugger? If so, how many loops did you try, and what was the last value of the tolerance?
    Of course, I am happy to find and remove any bugs, just don't want to make sure it is not a feature. :)

Copy link

@lasconic lasconic Jan 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My test case is small enough to share. I didn't run a debugger but printed len(simplified) and minvertices. They are forever 2 and 3, and sq_tolerance 0. If the condition is correct, then a check is needed to stop the infinite loop and give up the simplification ?

{
    "type": "FeatureCollection",
    "features": [
      {
        "type": "Feature",
        "id": 1,
        "geometry": {
          "type": "Polygon",
          "coordinates": [[[0.0, 0.0], [0.0, 3200.0], [3200.0, 3200.0], [3200.0, 0.0], [0.0, 0.0]]]
        },
        "properties": {
            "data": "A1"
        }
      },
      {
        "type": "Feature",
        "id": 2,
        "geometry": {
          "type": "Polygon",
          "coordinates": [[[3200.0, 0.0], [3200.0, 3200.0], [6400.0, 3200.0], [6400.0, 0.0], [3200.0, 0.0]]]
        },
        "properties": {
            "test": "A2"
        }
      },
      {
        "type": "Feature",
       "id": 3,
        "geometry": {
          "type": "Polygon",
          "coordinates": [[[1.0, 0.0], [1.0, 1.0], [1.0, 1.0], [1.0, 0.0], [1.0, 0.0]]]
        },
        "properties": {
            "test": "A3"
        }
      }
    ]
  }
bounds=[0, 0, 32000, 25600]
center = [bounds[2]/2, bounds[3]/2, 0]

vector_layers = [
    TileLayer(
        id="polygon-layer",
        fields={"data": "String"},
        minzoom=0,
        maxzoom=6,
        description="Layer containing polygon data",
    )
]


# Instantiate TileModel with your settings
tile_model = TileModel(
    tilejson="3.0.0",
    tiles=[Path(f"{output_dir}/{{z}}/{{x}}/{{y}}.{'pbf' if convert_to_pbf else 'json'}")],  # Local path or URL
    name="Example Tile Layer",
    description="A TileJSON example incorporating MicroJSON data",
    version="1.0.0",
    attribution="AA",
    minzoom=0,
    maxzoom=6,
    bounds=bounds,
    center= center,
    vector_layers=vector_layers
)

handler = TileWriter(tile_model, pbf=convert_to_pbf)
handler.microjson2tiles(input_file, validate_output)

Copy link
Member Author

@bengtl bengtl Jan 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The last example contains duplicate consecutive coordinates:
"coordinates": [[[1.0, 0.0], [1.0, 1.0], [1.0, 1.0], [1.0, 0.0], [1.0, 0.0]]]
While technically not wrong it results in the issues we see, as the algorithm will get stuck.
Rather than changing the logic, I would like to check the input data for this kind of anomalies.
@lasconic, If you like, you would be very welcome to create an issue addressing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants