Skip to content
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

Expose Tileset2D instance #5504

Closed
wants to merge 2 commits into from
Closed

Expose Tileset2D instance #5504

wants to merge 2 commits into from

Conversation

kylebarron
Copy link
Collaborator

@kylebarron kylebarron commented Feb 19, 2021

Background

There's a lot of great tile selection logic in the TileLayer. However it's very much tied to the Web Mercator projection and OSM indexing system. There are use cases for displaying tiled data that isn't in the OSM tiling system. For example, if one wished to display tiled data in the GlobeView, a "global geodetic" tiling system might be more appropriate, where there are two base tiles for each hemisphere, then a quadtree within each, and where each tile is in the WGS84 projection.

I don't think it should necessarily be deck.gl's job to support other tiling systems, but I think it should be easier for an end user to extend the TileLayer's functionality.

This is a very basic PR to expose the Tileset2D class, and is intended to start further discussion.

  • Does it make sense to set Tileset2D as a layer prop? I'm not aware of any other circumstances where a class is a prop. Should it be tied to the TileLayer class instead, so that a user who wishes to change the Tileset2D would subclass the TileLayer and swap out the tileset attribute?
  • What's the right abstraction level to expose? For example, in the above use case of supporting alternative tiling systems, the main function one would wish to overwrite is I think getBoundingVolume. However that's "locked away" inside a non-exported OSMNode class, so even with exposing the Tileset2D class, you'd still have to copy a substantial amount of TileLayer code into a custom app.

Change List

  • Expose Tileset2D from @deck.gl/geo-layers
  • Add Tileset2D as prop to TileLayer.

@Pessimistress
Copy link
Collaborator

Any exports must be documented. As a start, we may want to expose the layer prop ( TilesetClass ?) and Tileset2D as experimental APIs, and provide a dedicated session for them inside the TileLayer documentation. With description of the required interface and maybe a simple example.

@ilan-gold
Copy link
Collaborator

ilan-gold commented Feb 20, 2021

If I understand what you are suggesting @kylebarron I think your second bullet point makes a lot sense in addition to "exporting the whole class." The Tileset2D provides nice rendering (i.e overlaps, prioritization, aborting etc.) + requesting capabilities for each tile (i.e load me this x,y tile). But how exactly those tiles are indexed seems like a nice abstraction to have separate from the rendering + requesting the Tileset2D gives. Perhaps exposing the getTileIndices function as a prop would make sense? I think I originally committed something along these lines when adding the non-geospatial tile indexing and we actually had a BaseTileLayer that exposed this as a prop for a bit but it was reverted. Maybe I'm misunderstanding though....just trying to give some feedback :)

@kylebarron
Copy link
Collaborator Author

Thanks for chiming in @ilan-gold! I think that one of the issues here is that in contrast to some of the other deck.gl layers, the TileLayer is very "deep", which makes it harder to override layer internals by subclassing. You have TileLayer -> Tileset2D -> getTileIndices -> getOSMTileIndices -> OSMNode. For alternative (geospatial) tiling systems, I think the frustum culling code would be of greatest value to reuse. But the frustum culling is in getOSMTileIndices and OSMNode, so with the current architecture that's very hard to expose to enable reuse.

But how exactly those tiles are indexed seems like a nice abstraction to have separate from the rendering + requesting the Tileset2D gives

I think I'm searching for the best (and most maintainable) way to achieve this. I'd personally love to override just OSMNode, because it seems like the TileLayer would/might "just work" with alternative tiling by just defining each tile to be in a separate location. (It would also be necessary to relax the assumption that there's just one root tile).

@jfuehner
Copy link

Does this have a chance of being accepted/implemented soon?

Would love to have the ability to use the existing MapView capabilities with alternate TileLayers that are visualizing different projections. Something more user friendly than the OrthographicView.

See my comment from this discussion

@kylebarron
Copy link
Collaborator Author

At this stage I mostly want more feedback from users. What's the right abstraction to expose? How many of the internals would be useful for users to be able to override.

@igorDykhta
Copy link
Collaborator

Can we export this at least as _Tileset2D?

@kylebarron
Copy link
Collaborator Author

Closing as superseded by #6848

@kylebarron kylebarron closed this May 5, 2022
@Pessimistress Pessimistress deleted the kyle/expose-tileset2d branch May 11, 2022 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants