Skip to content

Commit

Permalink
bugfix: block states sometimes were inconsistent when decreasing reso…
Browse files Browse the repository at this point in the history
…lution

An assumption was made that there is always at least one VISIBLE block in a subtree of the REMOVED node. However, this assumption was too strong and it's not always true: for example, consider the following block tree: REMOVED #1 -> REMOVED #2 -> VISIBLE #3. For example, in the next scene update the VISIBLE block #3 goes outside the screen so it's excluded from the requested tree, while it's neighboring block #4 with the same parent node #2 that was outside the screen is now inside, so it's added as a new leaf node in the PENDING state. The updated block tree becomes: REMOVED #1 -> REMOVED #2 -> PENDING #4. Then in the next scene update the resolution needs to decrease, so that the root block #1 now becomes the leaf of the requested tree, and its state is set to PENDING. The algorithm needs to keep all VISIBLE leaf nodes and its ancestors in the subtree of this block #1, and it mistakenly keeps the block #2 under the incorrect assumption mentioned above (assuming that there is a VISIBLE block somewhere below in the subtree), while there is only the PENDING block #4. Then the PENDING block #4 is excluded from the new configuration because it's not needed anymore, and the REMOVED block #2 ends up being the leaf node which is an inconsistent state.

With this fix, the block #2 will be excluded from the updated block tree configuration as well because there are no VISIBLE blocks in its subtree, and all the resulting states will be valid.
  • Loading branch information
Igor Pisarev committed Oct 31, 2019
1 parent 50a934d commit b119175
Showing 1 changed file with 8 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -905,20 +905,23 @@ else if (treeNodeForNewLeafKey.state == BlockTreeNodeState.REMOVED)
blockTree.traverseSubtreeSkipRoot(newRequestedLeafKey, (childKey, childNode) -> {
if (childNode.state == BlockTreeNodeState.REMOVED)
{
touchedBlocks.add(childKey);
// Check that a subtree exists
assert !blockTree.isLeaf(childKey) : "A state of the block in the tree says that there supposed to be a subtree with visible blocks, " +
"but the block is a leaf node: " + childNode + ", key: " + childKey;
return true;
}
else if (childNode.state == BlockTreeNodeState.VISIBLE)
{
touchedBlocks.add(childKey);
// Check that there are no REMOVED or VISIBLE blocks in the subtree
assert assertSubtreeOfVisibleBlock(childKey);
assert assertSubtreeOfVisibleBlock(childKey) : "There should be no REMOVED or VISIBLE blocks in the VISIBLE subtree";
// Keep the block and its ancestors
blockTree.traverseAncestors(childKey, (ancestorKey, ancestorNode) -> touchedBlocks.add(ancestorKey));
return false;
}
else
{
assert assertPendingSubtree(childKey) : "Expected to be a pending subtree";
return false;
}
return false;
});
}
}
Expand Down

0 comments on commit b119175

Please sign in to comment.