Fix tile filtering to use area overlap rather than only NW corner #21
Conversation
…rner point of each tile. Fixes missing tiles at zoom levels 8 and above. The bug: At zoom 7 and below, all tiles are included in region zip files regardless of geography (line 161). At zoom 8+, tiles are filtered by checking if they fall within region boundaries. However, the original is_in() function only checked if a single point (the tile's upper-left corner) was inside the region boundary. This incorrectly excluded tiles that overlapped the region boundary but whose upper-left corner was outside. That caused a column of missing tiles -- less frustrating as you zoom in, but at zoom level 8, 1.4 degrees of longitude are missing. For example, you can't find KONP on the sectional chart at zoom 8; it's in column 39, which is absent from the zip file. To fix this, I replaced is_in with Area.overlaps. I replaced the chain of singleton arguments with an Area datatype to make the code and the overlaps condition more readable, and use that type to represent both the constant region definitions and the tile bounds.
|
fixes part of apps4av/avare#365 |
|
I ran sec.py, and it correctly generates the missing column of tiles at zoom level 8 & 9. Hooray! |
apps4av
left a comment
apps4av
left a comment
There was a problem hiding this comment.
Thank you. But how big of a problem is it ?
The chart code previously written was getting so complicated that it was unreliable and we were missing release dates. Hence the chart code was rewritten to make it as simple as possible and resist feature creeps. Since there are no complaints on the forum regarding this, we will need to wait to merge it.
|
Notably, this is not a feature creep, it's a bug fix: there are tiles missing. Try putting your airplane on KONP (Oregon coast, near the boundary of the NW region) and zooming in and out. Far enough out (zoom level 7 or below), it works fine. But at zoom level 8, the coastal part of oregon is just missing, a black hole. "NW goes to the Pacific" isn't a feature, it's a requirement that the app currently doesn't satisfy. The complaint is issue #365 over on the app repository. It has been around for a long time. |
|
The bug boils down to: the existing code tests whether a tile's northwest corner falls inside the region boundary. Instead, it should test whether any of the tile's area overlaps the region boundary. Maybe one thing that is off-putting is the introduction of the Area type. I could express the fix in fewer lines of code with a long chain of inequalities inlined at the site of the comparison. I preferred to introduce the Area type because it makes a clean separation. At the call site, we express the critical idea that fixes the bug: "does the area of this tile overlap with the area of the region?" Then all the clumsy inequality logic is bundled together inside the Area.overlaps method. |
|
Is there a workaround like downloading the next adjacent area ?
Regards,
Apps For Aviators Support,
We encourage users to use the Forum for all questions. Any updates to the
forum will help other users, who might have similar questions.
Forum: https://groups.google.com/forum/#!forum/apps4av-forum
…On Tue, Oct 7, 2025, 1:43 PM Jon Howell ***@***.***> wrote:
*jonhnet* left a comment (apps4av/charts#21)
<#21 (comment)>
The bug boils down to: the existing code tests whether a tile's northwest
corner falls inside the region boundary. Instead, it should test whether
any of the tile's area overlaps the region boundary.
Maybe one thing that is off-putting is the introduction of the Area type.
I could express the fix in fewer lines of code with a long chain of
inequalities inlined at the site of the comparison. I preferred to
introduce the Area type because it makes a clean separation. At the call
site, we express the critical idea that fixes the bug: "does the area of
this tile overlap with the area of the region?" Then all the clumsy
inequality logic is bundled together inside the Area.overlaps method.
—
Reply to this email directly, view it on GitHub
<#21 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAWPRTZGC6P3Y4VWKJOXTFL3WP3UTAVCNFSM6AAAAACIL643POVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTGNZXHEZTCNZWGI>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
|
There is no region west of NW. (Well, AK and PAC are west of NW, but too far west and not at the same latitude.) I went looking to see if this bug affects the northern border of NW, but it doesn't because the region border is 50°N and the sectional ends at 49°N. A workaround would be to move the western boundary of every region at lest 1.41° more west. I'm not sure that's a robust workaround; just that the SEC tiles happened to be 1.41° wide at zoom 8. But also ... why wouldn't we want to fix a bug that omits tiles that belong in a given region? It seems much easier to understand than hiding the bug by changing region boundaries. (By the way, I freaking love the regions. Chart list maintenance was such a nightmare in the prior model, but now I just select a few regions and I'm off to the airport. Thanks!) |
|
I would extend he region boundaries as that's no new code added. The file
sizes will be bigger but slightly and there will be no new code added to
the chart prep.
Regards,
Apps For Aviators Support,
We encourage users to use the Forum for all questions. Any updates to the
forum will help other users, who might have similar questions.
Forum: https://groups.google.com/forum/#!forum/apps4av-forum
…On Tue, Oct 7, 2025, 5:31 PM Jon Howell ***@***.***> wrote:
*jonhnet* left a comment (apps4av/charts#21)
<#21 (comment)>
There is no area west of NW. (Well, AK and PAC are west of NW, but too far
west and not at the same latitude.)
A workaround would be to move the western boundary of every region at lest
1.41° more west. I'm not sure that's a robust workaround; just that the SEC
tiles happened to be 1.41° wide at zoom 8.
But also ... why wouldn't we want to fix a bug that omits tiles that
belong in a given region? It seems much easier to understand than hiding
the bug by changing region boundaries.
(By the way, I freaking love the regions. Chart list maintenance was such
a nightmare in the prior model, but now I just select a few regions and I'm
off to the airport. Thanks!)
—
Reply to this email directly, view it on GitHub
<#21 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAWPRT3V44B5NNT5762BOLL3WQWLTAVCNFSM6AAAAACIL643POVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTGNZYG44DOMRUGY>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
…west corner. At zoom 8+, tiles were filtered by checking if the northwest corner was inside the region boundary. This incorrectly excluded tiles that overlapped the region but whose northwest corner was outside.
|
will be merged on next cycle. |
|
Problems reported, missing areas in horizontal lines. |
|
I'll take a look |
|
Gah! My apologies. The call to projection.findBounds on common.py:122 assigns the second result to I have made the following changes:
The harness runs the original code (before this PR, which lost west coast airports), the broken d532140 version (which lost the top row of tiles due to this bug), and the proposed fix. That harness compares the entire list of tiles generated for sectionals, and confirms that (a) original code is missing left tiles and d532140 indeed misses the top row, so the harness is working, and (b) the proposed fix produces a superset of the original and d532140. That should give us confidence that another bug isn't lurking in this fix.
Based on the harness results, you should be able to re-run the tile generation and get new zip files that have the missing files (and no less than we had before). I don't know how you mark downloads to be re-enabled inside a cycle, but I hope it's feasible. My apologies for missing this bug -- I should have tried that test harness experiment the first time to detect any loss of tiles. |
|
Proposed fix in #24 . |
I was expecting that 2512 be re-run and overwrite the defective data. That would have avoided any issues for folks who had not yet updated their data. Otherwise, you would just mark your region for update (or worse case delete, then download again). But what happened was the fixed data was issued as 2513 which required everyone to (re-)download from an unexpected future cycle which caused confusion and questions on the forum. I would like to understand why 2513 made more sense than just reissue 2512. I assume I misunderstand something. |
|
This is mostly risk control. The scripts are automated to detect the next
cycle and build. Since the error was after 2512, 2513 was released.
Changing the scripts to change that mechanism is not worth the risk of
breaking something else.
The proper control for future issues is making sure changes are thoroughly
tested before PRs are merged.
…On Fri, Dec 19, 2025 at 12:17 AM Bradley Spatz ***@***.***> wrote:
*b-spatz* left a comment (apps4av/charts#21)
<#21 (comment)>
you should be able to re-run the tile generation [...]
I don't know how you mark downloads to be re-enabled inside a cycle, but I
hope it's feasible.
I was expecting that 2512 be re-run and overwrite the defective data. That
would have avoided any issues for folks who had not yet updated their data.
Otherwise, you would just mark your region for update (or worse case
delete, then download again).
But what happened was the fixed data was issued as 2513 which required
everyone to (re-)download from an unexpected future cycle which caused
confusion and questions on the forum.
I would like to understand why 2513 made more sense than just reissue
2512. I assume I misunderstand something.
Thanks in advance.
—
Reply to this email directly, view it on GitHub
<#21 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAWPRT26RBQURTOF7DCKY6L4COC6FAVCNFSM6AAAAACIL643POVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTMNZTGU3TIOBVG4>
.
You are receiving this because you modified the open/close state.Message
ID: ***@***.***>
--
Regards,
Apps For Aviators Support,
We encourage users to use the Forum for all questions. Any updates to the
forum will help other users, who might have similar questions.
Forum: https://groups.google.com/forum/#!forum/apps4av-forum
|
3 participants
Fixes missing tiles at zoom levels 8 and above, part of issue #365
The bug: At zoom 7 and below, all tiles are included in region zip files regardless of geography (line 161). At zoom 8+, tiles are filtered by checking if they fall within region boundaries. However, the original is_in() function only checked if a single point (the tile's upper-left corner) was inside the region boundary. This incorrectly excluded tiles that overlapped the region boundary but whose upper-left corner was outside. That caused a column of missing tiles -- less frustrating as you zoom in, but at zoom level 8, 1.4 degrees of longitude are missing. For example, you can't find KONP on the sectional chart at zoom 8; it's in column 39, which is absent from the zip file.
To fix this, I replaced is_in with Area.overlaps. I replaced the chain of singleton arguments with an Area datatype to make the code and the overlaps condition more readable, and use that type to represent both the constant region definitions and the tile bounds.
I tested this logic change locally with a one-off unit test that reproduced the old and new logic. I didn't include a unit test because the present code doesn't provide good boundaries for that. I have not tested it end-to-end on a real sectional chart.