-
Notifications
You must be signed in to change notification settings - Fork 315
Further simplify tile subdivision mechanism #2525
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
base: master
Are you sure you want to change the base?
Changes from all commits
a739d14
7dfdc28
cf9e590
3670422
7d21a95
0e78ff5
bc808d8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -11,12 +11,7 @@ export const STRATEGY_GROUP = 1; | |||||
| export const STRATEGY_PROGRESSIVE = 2; | ||||||
| export const STRATEGY_DICHOTOMY = 3; | ||||||
|
|
||||||
| function _minimizeNetworkTraffic(node, nodeLevel, currentLevel, source) { | ||||||
| // 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; | ||||||
| } | ||||||
|
Comment on lines
-15
to
16
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this method could be 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); | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you remove
Suggested change
|
||||||
| } | ||||||
| nextLevelToFetch = Math.min(nextLevelToFetch, maxZoom); | ||||||
| } | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -303,13 +303,6 @@ class TiledGeometryLayer extends GeometryLayer { | |
| if (!node.parent) { | ||
| return ObjectRemovalHelper.removeChildrenAndCleanup(this, node); | ||
| } | ||
| // early exit if parent' subdivision is in progress | ||
| if (node.parent.pendingSubdivision) { | ||
| node.visible = false; | ||
| node.material.visible = false; | ||
| this.info.update(node); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the updating info is released in each case ? |
||
| return undefined; | ||
| } | ||
|
|
||
| // do proper culling | ||
| node.visible = !this.culling(node, context.camera); | ||
|
|
@@ -320,10 +313,10 @@ class TiledGeometryLayer extends GeometryLayer { | |
| node.material.visible = true; | ||
| this.info.update(node); | ||
|
|
||
| if (node.pendingSubdivision || (TiledGeometryLayer.hasEnoughTexturesToSubdivide(context, node) && this.subdivision(context, this, node))) { | ||
| if (this.subdivision(context, this, node)) { | ||
| this.subdivideNode(context, node); | ||
| // display iff children aren't ready | ||
| node.material.visible = node.pendingSubdivision; | ||
| node.material.visible = false; | ||
| this.info.update(node); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| requestChildrenUpdate = true; | ||
| } | ||
|
|
@@ -422,39 +415,16 @@ class TiledGeometryLayer extends GeometryLayer { | |
| * @param {Object} context - The context of the update; see the {@link | ||
| * MainLoop} for more informations. | ||
| * @param {TileMesh} node - The node to subdivide. | ||
| * @return {Promise} { description_of_the_return_value } | ||
| */ | ||
| subdivideNode(context, node) { | ||
| if (!node.pendingSubdivision && !node.children.some(n => n.layer == this)) { | ||
| if (!node.children.some(n => n.layer == this)) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. may be could be write |
||
| const extents = node.extent.subdivision(); | ||
| // TODO: pendingSubdivision mechanism is fragile, get rid of it | ||
| node.pendingSubdivision = true; | ||
|
|
||
| const command = { | ||
| /* mandatory */ | ||
| view: context.view, | ||
| requester: node, | ||
| layer: this, | ||
| priority: 10000, | ||
| /* specific params */ | ||
| extentsSource: extents, | ||
| redraw: false, | ||
| }; | ||
|
|
||
| return context.scheduler.execute(command).then((children) => { | ||
| for (const child of children) { | ||
| node.add(child); | ||
| child.updateMatrixWorld(true); | ||
| } | ||
|
|
||
| node.pendingSubdivision = false; | ||
| context.view.notifyChange(node, false); | ||
| }, (err) => { | ||
| node.pendingSubdivision = false; | ||
| if (!err.isCancelledCommandException) { | ||
| throw new Error(err); | ||
| } | ||
| }); | ||
| for (const extent of extents) { | ||
| const child = this.convert(node, extent); | ||
| node.add(child); | ||
| child.updateMatrixWorld(true); | ||
| } | ||
| context.view.notifyChange(node, false); | ||
| } | ||
| } | ||
|
|
||
|
|
||
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, there's no need to add a try block. Could remove comment