Search Unity

Feedback ProBuilder API allocates collections every time we get positions/sharedVertices

Discussion in 'World Building' started by dnlbschff, Dec 16, 2021.

  1. dnlbschff

    dnlbschff

    Joined:
    Nov 1, 2016
    Posts:
    2
    I am using ProBuilder (v5.0.3) to do lots of procedural mesh generation at runtime. Over the past weeks I have been running into a couple of issues and was wondering if this was the right place to post my feedback. Here are some of my pain points.

    1. Accessing ProBuilderMesh.positions or ProBuilderMesh.sharedVertices will allocate a new collection every single time.
      I noticed this when trying to add debug labels to each vertex showing me the shared vertex index for which the code ended up allocating about 200kb every frame. Caching the collections before looping over the verts helps of course, but I think it should be possible to hand out a read-only reference to the existing positionsInternal array without any allocations or provide a way to retrieve vertex positions by index directly from the existing collection.
    2. Similarly, ProBuilderMesh.sharedVertexLookup is only internal.
      Many extension methods that ProBuilder comes with uses this lookup for efficiency. While this makes sense, implementing similar extensions with custom functionality outside of the ProBuilder namespace requires me to rebuild the lookup with SharedVertex.GetSharedVertexLookup. Similar to my previous point, I think a public read-only reference to the lookup should be made available.
    3. Many useful extension methods aren't public. (there are more public ones in 5.0.3 than in 4.5.2, but I think more of them should be public)
      e.g.: ElementSelection.GetConnectedEdges, ElementSelection.GetEdgeRing, ElementSelection.GetNeighborFaces(ProBuildermesh mesh, int[] indexes)
    4. Methods like EditorSceneViewPicker.FaceRaycast could be way more useful if they took a generic Ray instead of being so specific to SceneView tools. (and weren't exclusive to the UnityEditor namespace)
      These methods could be very useful in general at runtime as well: GetNearestEdge, GetNearestVertices, FaceRayCastBothCullModes. I have my own implementation of these taking generic world space Rays, so there would be a lot of value in having these possibilities in the API itself.
    I'm happy to provide more feedback should this be a desired direction of development.
     
    kaarrrllll likes this.
  2. kaarrrllll

    kaarrrllll

    Unity Technologies

    Joined:
    Aug 24, 2017
    Posts:
    552
    Agreed on providing access to vertex buffers through read-only collections. Same for the shared vertex lookup. If you look at the source code you can see we already have internal no-alloc accessors for most of these anyways.

    The extension methods are a little different, most of these were left private because they have unusual requirements for calling (e.g., the mesh must be in some specific state) or the API itself was not something we wanted to support. If there are specific ones you feel strongly about please feel free to open PR (https://github.com/Unity-Technologies/com.unity.probuilder), or we can discuss in this thread.

    Regarding the picking functions, most of these are wrapping functionality that is available in the runtime already. If I remember correctly it's in the SceneViewPicking class.