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

Make tiler zoom level configurable #1705

Merged
merged 3 commits into from
Oct 16, 2023
Merged

Conversation

MertenF
Copy link
Contributor

@MertenF MertenF commented Oct 6, 2023

The zoom levels for generating 2d tiles is fixed to 5-21.
By adding following 2 options, it will be possible to configure the zoom levels.
--tiles-min-zoom
--tile-max-zoom

I have not yet tested these changes. Looking for feedback.

@Saijin-Naib
Copy link
Contributor

This looks great!

Do you think you would be able to read the GSD for orthophoto/de and set the min-zoom to that?

Max I can't think of a reason to change...

I'm just musing if parameterless would be possible and meet your usecase?

I think many folks would love this!

@MertenF
Copy link
Contributor Author

MertenF commented Oct 6, 2023

Dynamic min-zoom level, depending on the GSD, sounds interesting.
Keeping the max-zoom level hardcoded is fine by me. My use-case was getting deeper zoomlevels for more detailed projects. The current level is more than big enough.

I'll take a look into making the min-zoom dynamic instead of a parameter. Parameterless looks like the better choice.

@Saijin-Naib
Copy link
Contributor

Thank you so much! I am sorry to put more work on your shoulders, but we struggle a bit with folks keeping track of all the parameters we already have, so when we can meet a need without adding one... It's a lot nicer for folks to work with.

I'm excited to see what you make!

Instead of hardcoding a value, calculate the maximum zoomlevel in which there is still an increase in detail. By using the configured orthophoto resolution or GSD.

The higher the latitude, the higher the resolution will be of the tile.
Resulting in a chance of generating useless tiles, as there is no compensation for this.
At the moment it'll use the worst-case resolution from the equator.

Zoom level calulation from: https://wiki.openstreetmap.org/wiki/Zoom_levels
@MertenF
Copy link
Contributor Author

MertenF commented Oct 12, 2023

I've added calculations depending on the GSD. However in the splitmerge I could not find the GSD, so there I used the configured orthophoto-resolution argument.

@Saijin-Naib
Copy link
Contributor

So, so cool! You should have a review by someone who can assess the code better than I shortly.

Thanks so much again for this amazing feature! You're going to make so many folks so happy.

@pierotofy
Copy link
Member

Thanks for the PR @MertenF ! It looks like a useful addition.

I think we still need to enforce a more conservative maximum zoom level. 21 was chosen empirically because that's where things started to take too long to compute. I've run a test with 22 (which is still fine), but at 23 things got really slow. At 24 it almost took more time to generate tiles than to complete the rest of the pipeline.

I don't think we should go higher than 22 in any situation. Let's remember that ODM is not a replacement for post-processing. If one has a strong need to have super-resolution tiles, they can/should run gdal2tiles.py on the orthophoto manually outside of ODM?

@MertenF
Copy link
Contributor Author

MertenF commented Oct 15, 2023

Each extra zoom level means that the amount of tiles goes times 5. So I guess it's not a big surprise that those higher levels need a lot more computation time. With that in mind, the limit that I've set (30) is indeed too high.

Level 22 results in an resolution of 3.73 cm/px, level 23 results in 1.86 cm/px.
I'm more inclined to have 23 as hard limit instead of 22. As this still means a substantial increase in my opinion. Level 22 will only be the case when both orthophoto-resolution and the GSD are less than 3.73. Which won't happen a lot I would guess?

What do you think?

@pierotofy
Copy link
Member

I think 23 is reasonable. 👍

@pierotofy
Copy link
Member

Looks great, thanks @MertenF !

@pierotofy pierotofy merged commit 08d0390 into OpenDroneMap:master Oct 16, 2023
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