soopercool101 / BrawlCrate

BrawlBox/BrawlTools Fork, Wii File Editor
https://discord.gg/s7c8763
GNU Lesser General Public License v3.0
150 stars 39 forks source link

Fix collada dae import for geometry with shared input offsets #90

Closed Minon closed 8 months ago

Minon commented 8 months ago

The Collada dae importer was not accounting for geometries that used inputs with shared offset values and assumed that there are always as many indices per facePoint as there were inputs.

This was causing the decoder to read/write vertices and indices to/from the wrong memory addresses, which can cause undefined behavior.

For example, if the geometry had the below inputs

<triangles material="material" count="3864">`
  <input semantic="VERTEX" source="#vertices" offset="0"/>
  <input semantic="NORMAL" source="#normals" offset="1"/>
  <input semantic="TEXCOORD" source="#map-0" offset="2" set="0"/>
  <input semantic="TEXCOORD" source="#map-1" offset="2" set="1"/>
  <p>287 0 0 288 1 1 286 2 2 0 3 3 ...

This means there are 3 index values for each face point, but the parser assumed there's 4 since there's 4 inputs. The first triangle in this list would be pulling positions from index 287, 1, and 2 when it should be pulling from index 287, 288, and 286

This code change should help BC from silently crashing/creating garbled models when importing some dae files that would otherwise work in other tools.

Note: The dae files that ran into this issue were exported from blender 3.6. This code has been tested and confirmed working with 2 dae files that were known to be working prior to this change. It has not been tested against any files that contain vertex colors.

soopercool101 commented 8 months ago

Looks good to me. I have a stable build I need to get released but will merge in once I get the last critical fix in for that, to allow some time for those on Canary to test properly