Refactor 3d tile traversal - Part 1 #540

xintongxia · 2019-11-28T00:04:15Z

This PR is the first step of refactoring the 3d-tiles traversal. Added a base traversal class (base-tileset-traverser.js) which could be shared by both Cesium's 3d-tiles and ArcGIS's i3s tiles traversal.

The example app examples/i3s/deck.gl/ is for testing purpose. Eventually these should be merged with 3d-tiles example examples/deck.gl/3d-tiles

Next steps:

update documentations
refactor tile-header and tileset classes, which could be shared by both tiles
refactor tile layer to be able to render both tiles
evaluate the logic of empty tile traversal, tile optimization, tile expiration

coveralls · 2019-11-28T00:19:55Z

Coverage decreased (-0.3%) to 49.498% when pulling 6b6f07d on xx/i3s into f00b344 on master.

ibgreen

Just some initial comments, ignore if they don't make sense. I probably haven't understood all the ideas in this PR yet.

Regarding the example name, the idea is one subfolder per 3D framework: examples/i3s/deck.gl/app.js -> examples/deck.gl/i3s/app.js

examples/i3s/deck.gl/app.js

ibgreen · 2019-11-28T00:14:53Z

examples/i3s/deck.gl/app.js

+
+    this._memWidget = new StatsWidget(lumaStats.get('Memory Usage'), {
+      framesPerUpdate: 1,
+      formatters: {


Any chance of getting the default formatters registered in luma.gl? This verbose code is now in more and more examples...

This is a great timing, now that luma.gl is making a major release version so no fear of accidentally breaking apps.

examples/deck.gl/3d-tiles/3d-tile-layer/tile-3d-layer.js

ibgreen · 2019-11-28T00:18:20Z

examples/i3s/deck.gl/app.js

@@ -108,17 +130,36 @@ export default class App extends PureComponent {
    ];
  }

+  _renderStats() {
+    // TODO - too verbose, get more default styling from stats widget?


Nit: Any chance to get this merged back as defaults into probe.gl, this verbose code is now in more and more examples...

Follow up PR.

ibgreen · 2019-11-28T00:20:00Z

examples/i3s/deck.gl/i3s-3d-layer/get-frame-state.js

@@ -0,0 +1,98 @@
+import {Vector3} from 'math.gl';
+import {CullingVolume, Plane} from '@math.gl/culling';


Q: Is this different from the Tile3DLayer? If so can we do any processing inside the loaders so they don't have to be different?

the example folder is temporary. Will merge i3s-3d-layer with tile-3d-layer in follow-up PR

this is a temp change, to test i3s loading and rendering, will be removed and merged with 3d-tiles in followups.

ibgreen · 2019-11-28T00:21:50Z

examples/i3s/deck.gl/i3s-3d-layer/i3s-3d-layer.js

@@ -4,12 +4,12 @@ import {Vector3} from 'math.gl';
 import {COORDINATE_SYSTEM, CompositeLayer} from '@deck.gl/core';
 import {SimpleMeshLayer} from '@deck.gl/mesh-layers';

-import {I3STileset} from '@loaders.gl/i3s';
+import {I3STileset3D} from '@loaders.gl/i3s';


Q: Is the plan to merge this tileset class with the other one?

We can e.g. create a @loaders.gl/category-3d-tiles if we need a common place.

Does 3d-tiles cover both cesium 3d-tiles and i3s?

Does 3d-tiles cover both cesium 3d-tiles and i3s?

Assuming you mean the @loaders.gl/3d-tiles npm module, I am open to making a change here, and have it include multiple formats.

A strong reason to join them would be to avoid inefficiency / overhead of having so many npm modules).

A reason to keep them separate could be to make it easier for the various partners that help us maintain the different standards to make changes to their module without being affected by other modules. But since we are sharing traversal code anyway I am not sure this really helps.

If joining the modules, some considerations would be:

We do want a clear distinction between code that implements a certain tiled format (subdirectories for ./3d-tiles/, ./i3s/, ./potree/ etc). This would contain the various file format loaders, but also format specific things like styling support etc.

Clearly documented ./common/ code that is shared between formats (traversal/caching/request scheduling etc).

Of course we should also consider the opinions of contributors, e.g. if @loshjawrence would have concerns about a single module setup covering all tiled formats.

loshjawrence · 2019-12-02T17:24:58Z

modules/3d-tiles/src/lib/tileset/base-tileset-traverser.js

+    // For traditional replacement refinement only refine if all children are loaded.
+    // Empty tiles are exempt since it looks better if children stream in as they are loaded to fill the empty space.
+    const checkRefines =
+      !skipLevelOfDetail && tile.refine === TILE3D_REFINEMENT.REPLACE && tile.hasRenderContent;


Consider removing references to skipLevelOfDetail in this pr or part 2 since we would need to have loaders generate gpu commands so that it would look ok during loading.

loshjawrence · 2019-12-02T17:33:19Z

modules/3d-tiles/src/lib/tileset/tile-3d-header.js

+  get hasChildren() {
+    // this.children are Tile3DHeader objects with content fetched from server
+    // this._header.children are children of this tile which are not yet fetched
+    return this.children.length > 0 || (this._header.children && this.children.length > 0);


Can this just be return this.children.length > 0;

it cannot work well with i3s tiles, the i3s tile doesn't include the tree structure in tileset, so loaders.gl needs to keep fetching each unfetched tile node during the traversal. So if an i3s tile node is already fetched tile._header.children will indicate if this tile has children or not. If loaders.gl continue fetching this tile's children, then whenever the child node is fetched, it will be constructed as Tile3DHeader object and added to tile.children.

Maybe explain this in comment?

xintongxia · 2019-12-06T23:07:17Z

modules/3d-tiles/src/lib/request-utils/request-scheduler.js

@@ -52,10 +52,19 @@ export default class RequestScheduler {
      return Promise.resolve(handle);
    }

+    // dedupe
+    const existingReq = this.requestQueue.find(req => req.handle === handle);


@jianhuang01

[WIP] use 3d-tiles traversal Refactor 3d tile traversal remove unused code address comments address comments add debugging console Dedupe tile requests enable debuggging Cleanup 3d-tiles traversal [WIP] use 3d-tiles traversal Refactor 3d tile traversal remove unused code address comments address comments add debugging console Use smaller bounding sphere Dedupe tile requests enable debuggging Add more examples Add stack dedupe and cancel frame update

xintongxia · 2019-12-12T01:46:27Z

Will do followup PR to address comments

xintongxia requested review from ibgreen, loshjawrence, Pessimistress and jianhuang01 November 28, 2019 00:14

ibgreen reviewed Nov 28, 2019

View reviewed changes

loshjawrence reviewed Dec 2, 2019

View reviewed changes

xintongxia mentioned this pull request Dec 2, 2019

Tracker: @loaders.gl/i3s #506

Closed

28 tasks

ibgreen approved these changes Dec 2, 2019

View reviewed changes

jianhuang01 force-pushed the xx/i3s branch from 1cec0e7 to 2285b0a Compare December 5, 2019 22:08

xintongxia commented Dec 6, 2019

View reviewed changes

xintongxia force-pushed the xx/i3s branch from 2285b0a to d9fe2c5 Compare December 11, 2019 19:32

lint

6b6f07d

xintongxia merged commit 994c463 into master Dec 12, 2019

xintongxia deleted the xx/i3s branch December 12, 2019 01:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor 3d tile traversal - Part 1 #540

Refactor 3d tile traversal - Part 1 #540

xintongxia commented Nov 28, 2019 •

edited

Loading

coveralls commented Nov 28, 2019 •

edited

Loading

ibgreen left a comment

ibgreen Nov 28, 2019

ibgreen Nov 28, 2019

xintongxia Dec 2, 2019

ibgreen Nov 28, 2019

xintongxia Dec 2, 2019

xintongxia Dec 2, 2019

ibgreen Nov 28, 2019

xintongxia Dec 2, 2019

ibgreen Dec 2, 2019

loshjawrence Dec 2, 2019 •

edited

Loading

loshjawrence Dec 2, 2019

xintongxia Dec 4, 2019

ibgreen Dec 4, 2019

xintongxia Dec 6, 2019

xintongxia commented Dec 12, 2019

		@@ -0,0 +1,98 @@
		import {Vector3} from 'math.gl';
		import {CullingVolume, Plane} from '@math.gl/culling';

Refactor 3d tile traversal - Part 1 #540

Refactor 3d tile traversal - Part 1 #540

Conversation

xintongxia commented Nov 28, 2019 • edited Loading

coveralls commented Nov 28, 2019 • edited Loading

ibgreen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

loshjawrence Dec 2, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xintongxia commented Dec 12, 2019

xintongxia commented Nov 28, 2019 •

edited

Loading

coveralls commented Nov 28, 2019 •

edited

Loading

loshjawrence Dec 2, 2019 •

edited

Loading