nicklockwood / Euclid

A Swift library for creating and manipulating 3D geometry
MIT License
644 stars 53 forks source link

Feature/vertex color component #61

Closed zilmarinen closed 3 years ago

zilmarinen commented 3 years ago

Adds a .color property to the Vertex to allow for per vertex colors when creating SCNGeometry.

zilmarinen commented 3 years ago

@nicklockwood I have added an initialiser to the Mesh to create the SCNGeometrySource for color components and ported over the appropriate elements of the Color struct from ShapeScript.

I have also updated the example project to test the per vertex colors by adding a simple shader program as it does not looks like SceneKit supports vertex colors out of the box.

Running the example you can see that the geometry is created with .clear color components as this is the default in the Vertex constructor. Unfortunately I am not familiar enough with the CSG and Shape operations such as loft and lathe but I presume that when creating a primitive such as

let cube = Mesh.cube(size: 0.8, material: Color.red)

The material passed in here could be passed along to any vertex constructors used when creating the geometry a la:

return Vertex(pos, normal, uv, material as? Color)

Testing this on a cube does indeed create the appropriate vertex colors but when cascading the material down through the sphere/lathe methods the inner faces are always clear (verified by changing the clear color to something other than clear).

Screenshot 2021-08-04 at 07 58 46

Perhaps you could take a gander at this so far and see if this is the correct approach? I have tested this by passing the material through any Vertex constructor in Shapes by checking material as? Color ?? .clear but I have not committed this as I was not sure if this was necessarily the right thing to do.

nicklockwood commented 3 years ago

@CaptainRedmuff this seems to be along the right lines. The issue of how to specify the vertex colors remains open though - just passing it through from the Mesh constructor probably isn't ideal though, as it's presumably more expensive to set vertex colors individually if you don't need to.

I'd guess that the shader should be set up to blend vertex colors with the overall material color, so you can specify a base color and then tweak it for just a subset of vertices. In which case it probably makes sense for the default to be white instead of clear (which is how the old fixed-pipeline OpenGL process worked).

nicklockwood commented 3 years ago

@CaptainRedmuff hey, are you still working on this? Would you like me to take it over?

zilmarinen commented 3 years ago

@nicklockwood Apologies for the delayed response.

I have tried to delve into the mesh generation to initialise Euclids built in shapes with vertex colors but I do not fully understand the inner workings of the CSG operations and the lathe/loft methods enough to make any further changes in confidence.

I have removed the WIP status on this PR if you want to run with my changes so far.

Passing the color along as the material to the constructors does correctly set the outer vertex colors but always leaves the inner vertices clear after stencilling/subtracting. I believe this is the right approach but perhaps you have something else in mind?

codecov-commenter commented 3 years ago

Codecov Report

Merging #61 (48f47de) into develop (a5ea5d3) will decrease coverage by 0.52%. The diff coverage is 50.57%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop      #61      +/-   ##
===========================================
- Coverage    72.90%   72.38%   -0.53%     
===========================================
  Files           21       21              
  Lines         4145     4222      +77     
===========================================
+ Hits          3022     3056      +34     
- Misses        1123     1166      +43     
Impacted Files Coverage Δ
Sources/Euclid+SceneKit.swift 22.79% <0.00%> (-2.21%) :arrow_down:
Sources/Color.swift 65.57% <100.00%> (+7.57%) :arrow_up:
Sources/Vertex.swift 94.94% <100.00%> (+1.52%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update a5ea5d3...48f47de. Read the comment docs.

nicklockwood commented 3 years ago

@CaptainRedmuff I've rebased on develop and fixed the CI/test issues. I'm still not exactly clear on how this will be used though. The test project currently renders a blank image unless you change the hard-coded Vertex default color to something other than transparent, and there's not really a convenient way to set vertex colors once a mesh has been created. I'm inclined to hold off on landing this until we can come up with a good way to actually demonstrate the functionality.

zilmarinen commented 3 years ago

@nicklockwood This all looks superb. Thank you for picking this up and rebasing in the latest changes.

For my purposes this looks to be right on the mark. Surfacing the vertex colours to the shader program is really all that I wanted to achieve.

I am not sure what to suggest for the behaviour where passing a Color along as the material property for the mesh constructors results in a 'clear' mesh. I suppose I had expected that the color would be set for each vertex but this is not something I had overly considered as I would not personally be creating meshes using any of the predefined shapes and instead would create polygons individually whilst setting the vertex colours along the way.

Should this be undefined behaviour when using custom shader programs?

zilmarinen commented 3 years ago

I have just had a cursory glance at integrating this into my project. It looks like there might be a bug in the Codable vertex implementation where properties are not being encoded/decoded correctly when specifying position, normal, texture and colors where the components are zeros.

This test fails for a vertex at .zero with a normal pointing upwards, a .zero texture coordinate and a .black color:

func testEncodingVertexWithZeroedComponents() {
    XCTAssertEqual(
        try decode("[0,0,0,0,1,0,0,0,0]"),
        Vertex(.zero, Vector(0, 1, 0), .zero, .black)
    )
}

func testDecodingVertexWithZeroedComponents() {
    XCTAssertEqual(
        try encode(Vertex(.zero, Vector(0, 1, 0), .zero, .black)),
        "[0,0,0,0,1,0,0,0,0]"
    )
}
nicklockwood commented 3 years ago

@CaptainRedmuff I believe I've fixed this now if you'd like to try it.

zilmarinen commented 3 years ago

@nicklockwood I can confirm the vertex codable fix has resolved the issue 👍

I have added a fix for incorrect setup of the color geometry source to cater for SCNVector3 and SCNVector4 adopting different types (CGFloat/Double and Float) for their components across macOS and iOS.

nicklockwood commented 3 years ago

@CaptainRedmuff good catch, thanks!

zilmarinen commented 3 years ago

@nicklockwood apologies for the radio silence on this. I have been trying my hand at some SwiftUI which lead me to writing Yield.

I believe you said previously that you were unclear on how this change (per vertex color components) would be used in practice so hopefully you may be able to draw an example from my project.

Yield is still in early development but the aim is to generate the appropriate meshes for a complete set of terrain tiles for use in an editing tool. Currently I am using the color components solely for visualisation purposes in the editor but these will later be used alongside UVs to do fancy things with shaders.

I have managed to model several tile variations so far which I can visualise in the app to validate the mesh output. However, when exporting the mesh to JSON and then importing it into my Quick Look preview, the resulting mesh is incorrect. It appears that some faces are missing, or perhaps the polgons have clear color components for their vertices so only a few faces of the mesh appear to be rendered correctly.

In the editor I can see the mesh is correct:

screen_edge

When viewing the mesh with Quick Look, the mesh is incorrect:

quicklook

Presumably this is down to how the Vertex is encoded/decoded as when I view the mesh JSON it looks as though some of the vertex components are incorrect, perhaps due to the logic around if/how the z component is omitted if it is equal to zero.

In search of progress I have made some changes to the transforms to ensure the colors are passed through but I am weary of making any changes to the Codables for the sake of stability.

I am not sure what your thoughts are on this change after a lull in activity on my behalf. I would very much like to see this become a part of Euclid and would be happy to provide any examples of usage if it would help to clarify the purpose of this PR.

nicklockwood commented 3 years ago

@zilmarinen did you mean to close the PR? I still intend to merge this at some point - It's mainly blocked at the moment by the serialization bug.

zilmarinen commented 3 years ago

Apologies @nicklockwood - I managed to make a complete mess of this whilst rebasing onto develop. I do not think I can reopen this PR but I will make a new one once I have resolved the rebasing conflicts.

nicklockwood commented 3 years ago

Ah OK. Develop is in a weird state at the moment anyway with the semantic vector types stuff, so maybe rebase on master for now.