mapbox / mapbox-scenekit

Other
231 stars 51 forks source link

Path drawing using nodes #24

Closed natalia-osa closed 5 years ago

natalia-osa commented 6 years ago

Fixes #20

Adds possibility to display a route on a selected TerrainNode. Possibilities:

Also leaves the possibility to simply:

Things which are not yet done (but I'm planning to do it after my vacations, so after next week):

camdeardorff commented 6 years ago

This is really cool @natalia-osa, I tried it out and it looks really really good. I have tried testing with some of my own data which is far more dense in terms of points, so many that the path almost looks jagged. This is the problem on the opposite side of what you mentioned had mentioned with too few of points.

I have been experimenting with reducing the number of points in my path by comparing the distance of the consecutive vectors, not sure if this is a good way of doing so but I thought I might mention it.

jim-martin commented 6 years ago

Hi @natalia-osa, thanks for your contribution!

I've been looking into drawing polylines on a terrainNode and want to share this experimental branch that might be what you're looking for in #20.

It minimizes the number of nodes and triangles added to the scene, but requires a physical iOS device to render. We're still looking at how to distribute the shader files in the MapboxScenekit framework, but have a look at the DemoPlacementViewController on that branch the meantime.

natalia-osa commented 6 years ago

Hi @jim-martin, thanks for your work, I will check it out during this week and close my PR if your work answers my needs.

It will still need the recalculation of heights periodically, or will it work properly in a scenario where there are too few points and the line goes eg through a mountain? There should be points added in the meantime, so no distance between points is longer than X (eg 2 meters). Otherwise the line in the middle will be going through a wrong height.

natalia-osa commented 6 years ago

@jim-martin Hi, could you please update your branch to master? It can't be applied to projects which support pre-iOS 10 versions, like mine. As a result I can neither test nor use it.

jim-martin commented 6 years ago

@natalia-osa , I've merged the line drawing branch to master. Let me know how your testing goes!

natalia-osa commented 6 years ago

@jim-martin Hi Jim, there are issues with your code after merging. It would be good to have some kind of CI in your framework or merge master to your branch and run it first. The project on master does not compile. There are 8 errors, one is because the inherited method tries to change the accessibility of enclosing type (public -> internal), other 7 are because the new branch is not compatible with iOS 8 and 9, and that compatibility was merged to the master around 2 weeks ago.

I've briefly looked at it and while methods like UIColor(displayP3Red:green:blue:alpha:) can be easily replaced with something iOS 8/9 compatible. There is 1 other change not compatible with iOS 9. Other 6 issues are for iOS 8 only. My project needs to support iOS 9, so if you will remove the support for 9, we won't update to any newer library version, unfortunately.

natalia-osa commented 5 years ago

To be closed, parts used in #48.