Skip to content

Conversation

Pessimistress
Copy link
Collaborator

@ilan-gold

See discussion in #4230 (comment)

Change List

  • Move BaseTileLayer back to geo-layers
  • Merge documentation

@Pessimistress Pessimistress requested a review from ibgreen February 4, 2020 20:25
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 80.85% when pulling 5f19317 on x/tile-layer-move into ff6f9ff on master.

- Default: `null`


##### `strategy` (Enum, optional)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a strong opinion here, but perhaps refinementStrategy, tileRefinementStrategy tileSelectionStrategy etc might be a bit more clear than just strategy?

- Default: `'best-available'`


##### `tileToBoundingBox` (Function, optional)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • I am wondering if there could be a tileset prop here, so that the user can pass in a Tileset2D instance?
  • And if so, several of the other advanced customization props could be specified (also, or exclusively) on the Tileset2D class
  • A lot of the customization for e.g. non-geospatial user cases would happen by users subclassing and redefining Tileset2D
  • If there was sufficient API alignment between tileset2D and tileset3D we might in the future just have a single TileLayer (essentially joining TileLayer and Tile3DLayer). This would of course require a serious generalization effort, so would come later, if at all.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this idea. It would require the Tileset2D class to be better defined and fully documented. I will open a follow-up PR for this.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, I was just hacking around. I wanted to load a static set of tiles at a specific zoom level to work around the low performance of the TerrainLayer and implement a make-shift terrain geofence for a user. Any ideas?

@Pessimistress Pessimistress merged commit efe8c59 into master Feb 5, 2020
@Pessimistress Pessimistress deleted the x/tile-layer-move branch February 5, 2020 20:23
chrisgervang pushed a commit that referenced this pull request Feb 11, 2020
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.

4 participants