nicklockwood / Euclid

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

Feature/codable keys #40

Closed zilmarinen closed 3 years ago

zilmarinen commented 3 years ago

This PR adds shortened coding key values to reduce the file size of meshes when saved to disc.

Only encoding the polygon material if a value is present and shortening the coding key values yields a significant reduction in the file size for meshes.

Initial results for this PR have seen simple meshes compressed by about 30%

zilmarinen commented 3 years ago

Is there any requirement for supporting encoding/decoding of structs such as Vector where components are expected to be supplied in varying formats?

For example, Vector (and other structs) appear to offer support for decoding values where the z component is omitted presumably to facilitate using Vector as a 2d struct for texture coordinates.

Does this library intend to support exporting meshes for specific definitions such as COLLADA or .obj files or any other external file format? If this is not the case the implementations of Codable could be simplified further if keyed containers are used where the key/value pairs of the structs being parsed are in a known, fixed format.

Whilst omitting these properties does reduce the size of the overall output, the complexity added for parsing each struct is considerably more cumbersome than simply encoding these "empty" values as is.

I will fix the broken CodingTests to illustrate my thoughts but I would be interested in your input for simplifying the codables as above.

nicklockwood commented 3 years ago

Thanks for looking at this. There's no particular requirement to support importing/exporting standard formats. The aims for the Codable support were basically:

1) support saving and loading meshes generated using the Euclid API 2) support loading hand-written mesh data in whatever forms are most convenient for humans to write

I'm not a huge fan of using single-letter keys. They're less natural for humans to write, which is annoying for goal 2, and they also don't reduce the size of serialized output as much as other strategies such as unkeyed containers, which is the solution I've used for things like Vector. Making that change now would also break backwards compatibility with existing stored data.

I think the goal of reducing serialized mesh size might be better served by adding a new, keyless serialization format that is similar to traditional formats, with vertex, normal, texture and index data all stored in separate keyless arrays. This could then be the default format for saving, while still loading data in the current format if detected.

codecov-io commented 3 years ago

Codecov Report

Merging #40 (b77e65a) into develop (22ab256) will decrease coverage by 0.49%. The diff coverage is 94.69%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop      #40      +/-   ##
===========================================
- Coverage    76.21%   75.71%   -0.50%     
===========================================
  Files           35       35              
  Lines         5651     5453     -198     
===========================================
- Hits          4307     4129     -178     
+ Misses        1344     1324      -20     
Impacted Files Coverage Δ
Sources/Transforms.swift 33.19% <0.00%> (+0.13%) :arrow_up:
Sources/Paths.swift 90.09% <94.11%> (+3.28%) :arrow_up:
Sources/Angle.swift 71.15% <100.00%> (+1.51%) :arrow_up:
Sources/Mesh.swift 60.60% <100.00%> (+5.81%) :arrow_up:
Sources/Plane.swift 86.86% <100.00%> (-0.81%) :arrow_down:
Sources/Polygon.swift 89.79% <100.00%> (+1.91%) :arrow_up:
Sources/Rotation.swift 77.41% <100.00%> (-9.82%) :arrow_down:
Sources/Vector.swift 88.40% <100.00%> (-1.07%) :arrow_down:
Sources/Vertex.swift 100.00% <100.00%> (ø)
Tests/CodingTests.swift 100.00% <100.00%> (ø)
... and 2 more

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 22ab256...b77e65a. Read the comment docs.

zilmarinen commented 3 years ago

Agreed; this PR got a little out of hand and snowballed into something else entirely.

The original aim was to reduce the file size of meshes but I realise now I was trying to achieve this the wrong way and for all the wrong reasons.

I am building an app where models are saved/loaded in a hierarchical graph of meshes performing CSG operations at each node. Persisting the rotation and scale etc are needed here to be able to reproduce the model when loading from disc and Euclid does this perfectly as is.

What I now realise I require is an additional means to encode the final mesh as simplified array of polygon vertices, solely the position, normal and texture coordinates. My mistake here was assuming that the changes I had proposed would achieve this goal.

Would you have any suggestions on options for exporting mesh data in a trimmed down, keyed format similar to this?

nicklockwood commented 3 years ago

The big problem with Codable is that it only supports one serialization format for each object. What you probably need to do is create a separate Codable struct hierarchy that matches the format you want to export, and then have code to convert between the Euclid structs and yours.

What I now realise I require is an additional means to encode the final mesh as simplified array of polygon vertices, solely the position, normal and texture coordinate

This part is probably more generally useful, as I already need to do something like this in the SceneKit adaptor. One option would be to extract that logic into something more general, like an iterator for easily walking over the individual vertices in a mesh. That could then be re-used by both the SceneKit exporter and your custom one (as well as other serialization implementations in future).

nicklockwood commented 3 years ago

You might also want to take a look at ShapeScript, which includes a scene graph implementation built on top of Euclid. I'm thinking of moving some of that logic into Euclid's core, although that will mean decoupling it from SceneKit a bit.