Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

[core] coveredByChildren is false if at least one child is uncovered #12371

Merged
merged 1 commit into from
Jul 24, 2018

Conversation

brunoabinader
Copy link
Member

Another spin-off from #12310 - I discovered that the coveredByChildren algorithm returns true if at least one child is entirely covered by its grandchildren.

Because we are limiting the number of tiles in #12310, some edge cases were causing the lower zoom tiles to disappear - notice that symbols still appear because their rendering is unaffected by clip IDs.

On the screenshots below, notice how the uppermost tile is not rendered (only its symbols), and its absence of depth buffer color:

original depth buffer
screenshot from 2018-07-11 22-28-52 screenshot from 2018-07-11 22-29-23

With the changes applied, the uppermost tile reappears, along with its depth buffer (dark grey):

fixed depth buffer
screenshot from 2018-07-11 22-30-23 screenshot from 2018-07-11 22-30-44

/cc @kkaefer @asheemmamoowala

@brunoabinader brunoabinader added bug Core The cross-platform C++ core, aka mbgl rendering labels Jul 11, 2018
@brunoabinader
Copy link
Member Author

One side effect of this change is that lower zoom tiles that are partially visible but have children covering its visible area entirely will cause coveredByChildren to return false and thus won't be removed from the clip IDs list. Though coveredByChildren logic is correct, we might need some extra logic in ClipIDGenerator::getClipIDs() to cover this corner case.

@brunoabinader brunoabinader force-pushed the covered-by-grandchildren branch from cc35ad2 to dd209eb Compare July 12, 2018 09:59
@@ -9,14 +9,13 @@ template <typename Iterator>
bool coveredByChildren(const UnwrappedTileID& id, Iterator it, const Iterator& end) {
for (const auto& child : id.children()) {
it = std::lower_bound(it, end, child, [](auto& a, auto& b) { return std::get<0>(a) < b; });
if (it == end) {
if (it == end)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: formatting use braces for if blocks

@brunoabinader brunoabinader force-pushed the covered-by-grandchildren branch from dd209eb to 17ac276 Compare July 13, 2018 08:06
@brunoabinader brunoabinader force-pushed the covered-by-grandchildren branch from 17ac276 to f4e850f Compare July 17, 2018 08:22
@brunoabinader brunoabinader merged commit 1683da3 into master Jul 24, 2018
@brunoabinader brunoabinader deleted the covered-by-grandchildren branch July 24, 2018 06:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Core The cross-platform C++ core, aka mbgl rendering
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants