Further simplify tile subdivision mechanism #2525
Conversation
…o clementcolin-tile_subdivision
jailln
force-pushed
the
fix/simplify-tile-subdivision
branch
from
e116f8f to
bc808d8
Compare
|
I agree that pendingSubdivision should be removed. At the time of development, we introduced tile asynchronicity to handle streamed DEM tiles or to compute tiles in workers. |
|
We need to remove |
|
When you remove tiles instancing from scheduler, there is no more earlyDropFunction mechanism, to avoid unnecessary instancing. |
|
Sorry, |
| } catch (e) { | ||
| return Promise.reject(e); | ||
| } | ||
| // TODO removed a try catch here that didn't seem needed after a first quick look, but needs to be re-checked |
There was a problem hiding this comment.
Yes, there's no need to add a try block. Could remove comment
| // TO DO source.isVectorTileSource is a temp fix for pendingSubdivision. | ||
| // see issue https://github.com/iTowns/itowns/issues/2214 | ||
| if (node.pendingSubdivision && !source.isVectorTileSource) { | ||
| return currentLevel; | ||
| } | ||
| function _minimizeNetworkTraffic(node, nodeLevel) { | ||
| return nodeLevel; | ||
| } |
There was a problem hiding this comment.
this method could removed
| @@ -74,7 +69,7 @@ export function chooseNextLevelToFetch(strategy, node, nodeLevel = node.level, c | |||
| // default strategy | |||
| case STRATEGY_MIN_NETWORK_TRAFFIC: | |||
| default: | |||
| nextLevelToFetch = _minimizeNetworkTraffic(node, nodeLevel, currentLevel, layer.source); | |||
| nextLevelToFetch = _minimizeNetworkTraffic(node, nodeLevel); | |||
There was a problem hiding this comment.
If you remove _minimizeNetworkTraffic
| nextLevelToFetch = _minimizeNetworkTraffic(node, nodeLevel); | |
| nextLevelToFetch = nodeLevel |
| if (node.parent.pendingSubdivision) { | ||
| node.visible = false; | ||
| node.material.visible = false; | ||
| this.info.update(node); |
There was a problem hiding this comment.
the updating info is released in each case ?
| this.subdivideNode(context, node); | ||
| // display iff children aren't ready | ||
| node.material.visible = node.pendingSubdivision; | ||
| node.material.visible = false; | ||
| this.info.update(node); |
There was a problem hiding this comment.
this.info.update(node); Could this call be written only once in this method?
| */ | ||
| subdivideNode(context, node) { | ||
| if (!node.pendingSubdivision && !node.children.some(n => n.layer == this)) { | ||
| if (!node.children.some(n => n.layer == this)) { |
There was a problem hiding this comment.
may be could be write
if (node.children.length)Because the children can only be on the same node's layer (to be verified).
Based on #2521
Related to #2522