-
Notifications
You must be signed in to change notification settings - Fork 89
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
Added 3D model building example #685
Conversation
@@ -146,6 +153,33 @@ public boolean process(MapTile tile, RenderBuckets buckets, MapElement element, | |||
|
|||
ExtrusionStyle extrusion = (ExtrusionStyle) style.current(); | |||
|
|||
boolean isErased = false; | |||
if (element.isBuilding() || element.isBuildingPart()) { |
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.
There is an identical check right below, how these work together?
Also how about users who don't care for that feature, how can they avoid such expensive checks in each process
call?
The "eraser" term is not very understandable, what does it mean? |
I prepared "erasers" to remove overlapping extrusion polygons. Erasers are defined by one or more The eraser-point sample I added includes a GeoPoint that locates inside SHIBUYA109 building. If the extrusion exists, the 3D model will be wrapped by the extrusion polygons and so the model will be hidden. |
Thanks for the explanation. So if it's related to 3D models feature, these code paths should be optional somehow, for only when that feature is in use and not reduce performance for other users. |
You're right. Current code checks each building polygon in tile if there's any eraser points overlapping. This is a poor implementation that will make the performance really bad. Now working on it... |
@@ -31,6 +34,8 @@ | |||
*/ | |||
public class MapElement extends GeometryBuffer { | |||
|
|||
public List<GeoPoint> geoPoints = new ArrayList<>(); |
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.
MapElement
extends GeometryBuffer
, so holds already and very efficiently the points.
Better use the existing infrastructure, like elsewhere in library, for optimal performance.
this.y = MercatorProjection.latitudeToY(lat); | ||
this.x = MercatorProjection.longitudeToX(lon); | ||
this.rotation = rotation; | ||
this.xRotation = xRotation; |
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.
Sorry that I barge in here.
We usually don't need xRotation. Please rotate model while export:
https://github.com/libgdx/libgdx/wiki/Importing-Blender-models-in-LibGDX#setting-the-coordinate-system-up-axis
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.
Sorry that I barge in here.
Please everyone do, more eyes are welcome. 🙂
Purpose remains always stability + performance.
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.
I'm not sure if it's solved, my Blender2.79 + fbx-conv is causing the z-up building models to be mowing down. This might be the reason (fbx-conv or blender change?)
libgdx/fbx-conv#102
By the way, horizontal inverse problem has moved to #686
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.
Seems that fbx-conf is responsible for those results.
I think that are two features, one for erase buildings / stuff, and one for adding a 3d model. |
Any new feature that hooks on top without changing much core library appears automatically more attractive. |
Separate issues |
Reverted mainClassName
Temporary close. The main issue was the model flip. I’ll re-open this PR when there’s an optimized building eraser ready. |
Since 3D models are available, buildings should be replaced too.
Looking for the reason why, but the displayed objects are