Godot: Expanding SurfaceTool

[clayjohn]

I know we are on feature freeze, so this is something that can wait. But I wanted to see if there would be interest in expanding the SurfaceTool. Right now it is a little cumbersome to use. It requires running a loop and adding each vertex attribute one-by-one before adding each individual vertex.

I would like to add a few functions that would make it easy to create a mesh using the surface tool just by passing in arrays for each of the attributes for example something like:

var st = SurfaceTool.new()
var mesh = st.create_from_arrays(vertices, colors, uvs)

Right now if you had these arrays you would have to write:

var st = SurfaceTool.new()
st.begin(Mesh.PRIMITIVE_TRIANGLES)
for i in range(vertices.size()):
    st.add_color(colors[i])
    st.add_uv(uvs[i])
    st.add_vertex(vertices[i])

And that is just the simplest scenario. It can get much more complicated then that.

I think the implementation could be done fairly simply and make use of some of the internal methods already present like _create_list and _create_list_from_arrays.

Please let me know if anyone has anything against adding a function like this. Or if anyone has ideas for other functions to expand the use of the SurfaceTool I could add them at the same time.

[slapin]

I think MeshDataTool is better bet for operating mesh data; I think some extension and more conststency to that might make it great tool for quick creation of efficient geometry.

[slapin]

SurfaceTool is more about immediate geometry in OpenGL 1.x style. If you look into Unity-style mesh operations, MeshDataTool is your friend. I think merging these into some mature mesh data manipulation toolkit would be wise decision and handle all cases with simple interface.

[Zylann]

For bare bones mesh generation with arrays, you can use ArrayMesh, I stopped using SurfaceTool a long time ago now^^
Never tried MeshDataTool though, it's not documented and its API doesn't look like a mesh generation utility. Here is what I use:

    var positions = PoolVector3Array([
        Vector3(0, 0, 1),
        Vector3(0, 0, 0),
        Vector3(0, 1, 0),
        Vector3(0, 1, 1)
    ])
    var normals = PoolVector3Array([
        Vector3(1, 0, 0),
        Vector3(1, 0, 0),
        Vector3(1, 0, 0),
        Vector3(1, 0, 0)
    ])
    var uvs = PoolVector2Array([
        Vector2(0, 0),
        Vector2(0.25, 0),
        Vector2(0.25, 0.25),
        Vector2(0, 0.25)
    ])
    var indices = PoolIntArray([
        0, 1, 2,
        0, 2, 3
    ])

    var arrays = []
    arrays.resize(Mesh.ARRAY_MAX)
    arrays[Mesh.ARRAY_VERTEX] = positions
    arrays[Mesh.ARRAY_NORMAL] = normals
    arrays[Mesh.ARRAY_TEX_UV] = uvs
    arrays[Mesh.ARRAY_INDEX] = indices

    var mesh = ArrayMesh.new()
    mesh.add_surface_from_arrays(Mesh.PRIMITIVE_TRIANGLES, arrays)

    var mi = get_node("MeshInstance")
    mi.mesh = mesh

Eventually, an advantage of SurfaceTool is that it can generate normals and tangents for you for example, but so far my use cases had more benefit using raw arrays (Though sometimes I had to re-implement normals calculation myself while not wanting to use SurfaceTool for its annoying API).

[clayjohn]

@Zylann This is awesome! I can't believe I missed this, I was messing around trying to recreate this functionality by accessing the VisualServer directly but I ran into problems accessing nodes from RIDs. This is exactly the kind of functionality that I was talking about. Still would be nice to have a function that takes the arrays separated and constructs the mesh array from them. But I guess a better place to add it would be within the ArrayMesh rather than adding that functionality to the SurfaceTool.

I still think the SurfaceTool needs some work though. Currently we have the create_from and append_from methods which populate the SurfaceTool from existing meshes, but the only thing you can do from there is generate normals, tangents, or commit the surface back into a mesh. So having some tools that allow you to add vertices to a mesh would be useful.

It may be time to rethink what functionality should be in SurfaceTool vs MeshDataTool vs ArrayMesh. It seems right now they are a little mixed up. I think the main idea was that SurfaceTool should be used for generating a surface and committing to a mesh, MeshDataTool should be used for reading and modifying mesh data, but not adding or creating, and ArrayMesh is where meshes should be created.

Given this structure it doesnt make a lot of sense for SurfaceTool to generate normals, tangents, or to be able to read from existing meshes, that functionality should all be in the MeshDataTool.

Anyway, thats a lot of information, please let me know what you think. I think there is definitely some room to improve workflow for people who procedural create meshes.

[slapin]

Well, all you can do with SurfaceTool you can do with MeshDataTool now, but
I agree the API is crippled and inconsistent. I'd prefer to move all
low level stuff to mesh object, like it is already done in C++ and use
MeshDataTool and SurfaceTool as helpers to simplify common tasks. Currently
MeshDataTool basically does the same thing mesh object does in C++ which
looks silly to me. Some proper design is needed around these.

On Wed, Oct 10, 2018 at 3:04 AM Clay John [e-mail] wrote:

@Zylann https://github.com/Zylann This is awesome! I can't believe I
missed this, I was messing around trying to recreate this functionality by
accessing the VisualServer directly but I ran into problems accessing nodes
from RIDs. This is exactly the kind of functionality that I was talking
about. Still would be nice to have a function that takes the arrays
separated and constructs the mesh array from them. But I guess a better
place to add it would be within the ArrayMesh rather than adding that
functionality to the SurfaceTool.

I still think the SurfaceTool needs some work though. Currently we have
the create_from and append_from methods which populate the SurfaceTool
from existing meshes, but the only thing you can do from there is generate
normals, tangents, or commit the surface back into a mesh. So having some
tools that allow you to add vertices to a mesh would be useful.

It may be time to rethink what functionality should be in SurfaceTool vs
MeshDataTool vs ArrayMesh. It seems right now they are a little mixed up. I
think the main idea was that SurfaceTool should be used for generating a
surface and committing to a mesh, MeshDataTool should be used for reading
and modifying mesh data, but not adding or creating, and ArrayMesh is where
meshes should be created.

Given this structure it doesnt make a lot of sense for SurfaceTool to
generate normals, tangents, or to be able to read from existing meshes,
that functionality should all be in the MeshDataTool.

Anyway, thats a lot of information, please let me know what you think. I
think there is definitely some room to improve workflow for people who
procedural create meshes.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/godotengine/godot/issues/22874#issuecomment-428393456,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAAX03vAiXff3gGuKephz3gK1wJkTzp5ks5ujTl_gaJpZM4XRPBd
.

[slapin]

Currently C++ API is done quite well for mesh operations, it is GDScript
API which needs some polish.

On Wed, Oct 10, 2018 at 2:58 PM Sergey Lapin [e-mail] wrote:

Well, all you can do with SurfaceTool you can do with MeshDataTool now,
but I agree the API is crippled and inconsistent. I'd prefer to move all
low level stuff to mesh object, like it is already done in C++ and use
MeshDataTool and SurfaceTool as helpers to simplify common tasks. Currently
MeshDataTool basically does the same thing mesh object does in C++ which
looks silly to me. Some proper design is needed around these.

On Wed, Oct 10, 2018 at 3:04 AM Clay John [e-mail]
wrote:

@Zylann https://github.com/Zylann This is awesome! I can't believe I
missed this, I was messing around trying to recreate this functionality by
accessing the VisualServer directly but I ran into problems accessing nodes
from RIDs. This is exactly the kind of functionality that I was talking
about. Still would be nice to have a function that takes the arrays
separated and constructs the mesh array from them. But I guess a better
place to add it would be within the ArrayMesh rather than adding that
functionality to the SurfaceTool.

I still think the SurfaceTool needs some work though. Currently we have
the create_from and append_from methods which populate the SurfaceTool
from existing meshes, but the only thing you can do from there is generate
normals, tangents, or commit the surface back into a mesh. So having some
tools that allow you to add vertices to a mesh would be useful.

It may be time to rethink what functionality should be in SurfaceTool vs
MeshDataTool vs ArrayMesh. It seems right now they are a little mixed up. I
think the main idea was that SurfaceTool should be used for generating a
surface and committing to a mesh, MeshDataTool should be used for reading
and modifying mesh data, but not adding or creating, and ArrayMesh is where
meshes should be created.

Given this structure it doesnt make a lot of sense for SurfaceTool to
generate normals, tangents, or to be able to read from existing meshes,
that functionality should all be in the MeshDataTool.

Anyway, thats a lot of information, please let me know what you think. I
think there is definitely some room to improve workflow for people who
procedural create meshes.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/godotengine/godot/issues/22874#issuecomment-428393456,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAAX03vAiXff3gGuKephz3gK1wJkTzp5ks5ujTl_gaJpZM4XRPBd
.

[Toshiwoz]

I feel the same, it took me time ti find out the most efficient way to create a mesh programmatically was by using ArrayMesh, as it is a resource, so it was quite confusing at the beginning.

I believe MeshDataTool and SurfaceTool should be merged into a single object that can be used to modify and also create an ArrayMesh. And to my undesrtanding a change like that cannot happen till Godot 4 as it would break compatibility with existing projects.

[clayjohn]

I totally agree. I'm hoping we can deprecate SurfaceTool in the 3.2 release and have it removed completely by 3.3 or 4 (whichever one comes first). MeshDataTool also needs to have a lot of work done to make it more flexible and to work more smoothly with the ArrayMesh.

[jejay]

I am piggybacking on this as it touches an issue I would have otherwise raised myself.

I am importing triangles from numpy via the python bindings. Looping over them is not an option; I tried and it takes 10 minutes instead of 20 seconds when I memory dump the numpy array directly underneath a PoolVector3Array pointer.

I still need to generate the normals, so I import the ArrayMesh based on the PoolArrays with SurfaceTool::create_from() to then generate the normals. The step with the ArrayMesh could be bypassed if SurfaceTool::create_from_triangle_arrays() was exposed to the API, but that is only a minor issue.

The bigger issue is, that I would like to have my normals generated for smooth shading. There is only the method SurfaceTool::add_smooth_group() which only seems to work when feeding in everything with the loop approach, as far as I understand from the C++ engine code. I can't loop, so I can't smooth :-(

[Zylann]

@jejay until SurfaceTool gets improved, here is the code I used to generate my normals as I'm still using the API I mentionned in https://github.com/godotengine/godot/issues/22874#issuecomment-428176795, in case it helps:

static func calculate_normals(positions, indices):
    var out_normals = PoolVector3Array()

    var tcounts = []
    tcounts.resize(positions.size())
    out_normals.resize(positions.size())

    for i in range(0, tcounts.size()):
        tcounts[i] = 0

    var tri_count = indices.size() / 3

    var i = 0
    while i < indices.size():

        var i0 = indices[i]
        var i1 = indices[i+1]
        var i2 = indices[i+2]
        i += 3

        # TODO does triangle area matter?
        # If it does then we don't need to normalize in triangle calculation since it will account for its length
        var n = get_triangle_normal(positions[i0], positions[i1], positions[i2])

        out_normals[i0] += n
        out_normals[i1] += n
        out_normals[i2] += n

        tcounts[i0] += 1
        tcounts[i1] += 1
        tcounts[i2] += 1

    for j in range(out_normals.size()):
        out_normals[j] = (out_normals[j] / float(tcounts[j])).normalized()

    return out_normals


static func get_triangle_normal(a, b, c):
    var u = (a - b).normalized()
    var v = (a - c).normalized()
    return v.cross(u)

[aaronfranke]

Right now if you had these arrays you would have to write:

var st = SurfaceTool.new()
st.begin(Mesh.PRIMITIVE_TRIANGLES)
for i in range(vertices.size()):
   st.add_color(colors[i])
   st.add_uv(uvs[i])
   st.add_vertex(vertices[i])

It would also be nice to have the option of just sending in arrays directly:

var st = SurfaceTool.new()
st.begin(Mesh.PRIMITIVE_TRIANGLES)
st.add_colors(colors)
st.add_uvs(uvs)
st.add_vertices(vertices)

[clayjohn]

That was something I had considered withy original proposal, however there is little benefit to that over just using an ArrayMesh directly. Given the internals of the SurfaceTool it is best to keep the two methods separate. SurfaceTool doesn't track arrays of data the way that ArrayMesh does. It needs vertex information to come in single vertex packets.