Skip to content

Commit

Permalink
Merge pull request #825 from zeux/vcone-tail
Browse files Browse the repository at this point in the history 
vertexcodec: Reduce tail padding for v1
  • Loading branch information
@zeux
zeux authored Dec 25, 2024
2 parents d8e89bf + 9d3ffc4 commit a744d99
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 57 deletions.
34 changes: 16 additions & 18 deletions demo/tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,33 +50,31 @@ static const PV kVertexBuffer[] = {
};

static const unsigned char kVertexDataV0[] = {
0xa0, 0x01, 0x3f, 0x00, 0x00, 0x00, 0x58, 0x57, 0x58, 0x01, 0x26, 0x00, 0x00, 0x00, 0x01,
0x0c, 0x00, 0x00, 0x00, 0x58, 0x01, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01,
0x3f, 0x00, 0x00, 0x00, 0x17, 0x18, 0x17, 0x01, 0x26, 0x00, 0x00, 0x00, 0x01, 0x0c, 0x00,
0x00, 0x00, 0x17, 0x01, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, // clang-format :-/
0xa0, 0x01, 0x3f, 0x00, 0x00, 0x00, 0x58, 0x57, 0x58, 0x01, 0x26, 0x00, 0x00, 0x00, 0x01, 0x0c,
0x00, 0x00, 0x00, 0x58, 0x01, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x3f, 0x00,
0x00, 0x00, 0x17, 0x18, 0x17, 0x01, 0x26, 0x00, 0x00, 0x00, 0x01, 0x0c, 0x00, 0x00, 0x00, 0x17,
0x01, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, // clang-format :-/
};

static const unsigned char kVertexDataV1[] = {
0xae, 0xee, 0xaa, 0xee, 0x00, 0x4b, 0x4b, 0x4b, 0x00, 0x00, 0x4b, 0x00, 0x00, 0x7d, 0x7d,
0x7d, 0x00, 0x00, 0x7d, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x62, 0x00, 0x62, // clang-format :-/
0xae, 0xee, 0xaa, 0xee, 0x00, 0x4b, 0x4b, 0x4b, 0x00, 0x00, 0x4b, 0x00, 0x00, 0x7d, 0x7d, 0x7d,
0x00, 0x00, 0x7d, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x62, 0x00, 0x62, // clang-format :-/
};

// This binary blob is a valid v1 encoding of vertex buffer but it used a custom version of
// the encoder that exercised all features of the format; because of this it is much larger
// and will never be produced by the encoder itself.
static const unsigned char kVertexDataV1Custom[] = {
0xae, 0xd4, 0x94, 0xd4, 0x01, 0x0e, 0x00, 0x58, 0x57, 0x58, 0x02, 0x02, 0x12, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x03, 0x00, 0x00, 0x58, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00,
0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x03, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x0e, 0x00, 0x7d, 0x7d, 0x7d, 0x01, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x7d, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x01, 0x62, // clang-format :-/
0xae, 0xd4, 0x94, 0xd4, 0x01, 0x0e, 0x00, 0x58, 0x57, 0x58, 0x02, 0x02, 0x12, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x03, 0x00, 0x00, 0x58, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x03, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x0e, 0x00, 0x7d, 0x7d, 0x7d, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x7d, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x62, // clang-format :-/
};

static void decodeIndexV0()
Expand Down
37 changes: 18 additions & 19 deletions src/vertexcodec.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,8 @@ const size_t kVertexBlockSizeBytes = 8192;
const size_t kVertexBlockMaxSize = 256;
const size_t kByteGroupSize = 16;
const size_t kByteGroupDecodeLimit = 24;
const size_t kTailMinSize = 32; // must be >= kByteGroupDecodeLimit
const size_t kTailMinSizeV0 = 32;
const size_t kTailMinSizeV1 = 24;

static const int kBitsV0[4] = {0, 2, 4, 8};
static const int kBitsV1[5] = {0, 1, 2, 4, 8};
Expand Down Expand Up @@ -354,7 +355,7 @@ static void encodeDeltas(unsigned char* buffer, const unsigned char* vertex_data
case 2:
return encodeDeltas1<unsigned int, true>(buffer, vertex_data, vertex_count, vertex_size, last_vertex, k, channel >> 4);
default:
assert(!"Unsupported channel encoding");
assert(!"Unsupported channel encoding"); // unreachable
}
}

Expand Down Expand Up @@ -385,15 +386,12 @@ static int estimateRotate(const unsigned char* vertex_data, size_t vertex_count,
vertex += vertex_size;
}

// ignore trivial groups for performance
if (bitg == 0 || bitg == ~0u)
continue;

for (int j = 0; j < 8; ++j)
{
unsigned int bitr = rotate(bitg, j);

sizes[j] += estimateBits((unsigned char)(bitr >> 0)) + estimateBits((unsigned char)(bitr >> 8)) + estimateBits((unsigned char)(bitr >> 16)) + estimateBits((unsigned char)(bitr >> 24));
sizes[j] += estimateBits((unsigned char)(bitr >> 0)) + estimateBits((unsigned char)(bitr >> 8));
sizes[j] += estimateBits((unsigned char)(bitr >> 16)) + estimateBits((unsigned char)(bitr >> 24));
}
}

Expand Down Expand Up @@ -764,8 +762,7 @@ static const unsigned char* decodeVertexBlock(const unsigned char* data, const u
decodeDeltas1<unsigned int, true>(buffer, transposed + k, vertex_count, vertex_size, last_vertex + k, (32 - (channel >> 4)) & 31);
break;
default:
// invalid channel type
return NULL;
return NULL; // invalid channel type
}
}

Expand Down Expand Up @@ -947,7 +944,7 @@ inline const unsigned char* decodeBytesGroupSimd(const unsigned char* data, unsi
}

default:
SIMD_UNREACHABLE();
SIMD_UNREACHABLE(); // unreachable
}
}
#endif
Expand Down Expand Up @@ -1015,7 +1012,7 @@ inline const unsigned char* decodeBytesGroupSimd(const unsigned char* data, unsi
}

default:
SIMD_UNREACHABLE();
SIMD_UNREACHABLE(); // unreachable
}
}
#endif
Expand Down Expand Up @@ -1159,8 +1156,7 @@ inline const unsigned char* decodeBytesGroupSimd(const unsigned char* data, unsi
}

default:
SIMD_UNREACHABLE();
return data;
SIMD_UNREACHABLE(); // unreachable
}
}
#endif
Expand Down Expand Up @@ -1279,7 +1275,7 @@ inline const unsigned char* decodeBytesGroupSimd(const unsigned char* data, unsi
}

default:
SIMD_UNREACHABLE();
SIMD_UNREACHABLE(); // unreachable
}
}
#endif
Expand Down Expand Up @@ -1593,8 +1589,7 @@ static const unsigned char* decodeVertexBlockSimd(const unsigned char* data, con
decodeDeltas4Simd<2>(buffer, transposed + k, vertex_count_aligned, vertex_size, last_vertex + k, (32 - (channel >> 4)) & 31);
break;
default:
// invalid channel type
return NULL;
return NULL; // invalid channel type
}
}

Expand Down Expand Up @@ -1681,7 +1676,8 @@ size_t meshopt_encodeVertexBufferLevel(unsigned char* buffer, size_t buffer_size
}

size_t tail_size = vertex_size + (version == 0 ? 0 : vertex_size / 4);
size_t tail_size_pad = tail_size < kTailMinSize ? kTailMinSize : tail_size;
size_t tail_size_min = version == 0 ? kTailMinSizeV0 : kTailMinSizeV1;
size_t tail_size_pad = tail_size < tail_size_min ? tail_size_min : tail_size;

if (size_t(data_end - data) < tail_size_pad)
return 0;
Expand Down Expand Up @@ -1775,7 +1771,9 @@ size_t meshopt_encodeVertexBufferBound(size_t vertex_count, size_t vertex_size)
size_t vertex_block_data_size = vertex_block_size;

size_t tail_size = vertex_size + (vertex_size / 4);
size_t tail_size_pad = tail_size < kTailMinSize ? kTailMinSize : tail_size;
size_t tail_size_min = kTailMinSizeV0 > kTailMinSizeV1 ? kTailMinSizeV0 : kTailMinSizeV1;
size_t tail_size_pad = tail_size < tail_size_min ? tail_size_min : tail_size;
assert(tail_size_pad >= kByteGroupDecodeLimit);

return 1 + vertex_block_count * vertex_size * (vertex_block_control_size + vertex_block_header_size + vertex_block_data_size) + tail_size_pad;
}
Expand Down Expand Up @@ -1828,7 +1826,8 @@ int meshopt_decodeVertexBuffer(void* destination, size_t vertex_count, size_t ve
return -1;

size_t tail_size = vertex_size + (version == 0 ? 0 : vertex_size / 4);
size_t tail_size_pad = tail_size < kTailMinSize ? kTailMinSize : tail_size;
size_t tail_size_min = version == 0 ? kTailMinSizeV0 : kTailMinSizeV1;
size_t tail_size_pad = tail_size < tail_size_min ? tail_size_min : tail_size;

if (size_t(data_end - data) < tail_size_pad)
return -2;
Expand Down
39 changes: 19 additions & 20 deletions tools/codecfuzz.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,14 @@ void fuzzRoundtrip(const uint8_t* data, size_t size, size_t stride, int level)
assert(encoded && decoded);

size_t res = meshopt_encodeVertexBufferLevel(static_cast<unsigned char*>(encoded), bound, data, count, stride, level);
assert(res <= bound);
assert(res > 0 && res <= bound);

int rc = meshopt_decodeVertexBuffer(decoded, count, stride, static_cast<unsigned char*>(encoded), res);
// encode again at the boundary to check for memory safety
// this should produce the same output because encoder is deterministic
size_t rese = meshopt_encodeVertexBufferLevel(static_cast<unsigned char*>(encoded) + bound - res, res, data, count, stride, level);
assert(rese == res);

int rc = meshopt_decodeVertexBuffer(decoded, count, stride, static_cast<unsigned char*>(encoded) + bound - res, res);
assert(rc == 0);

assert(memcmp(data, decoded, count * stride) == 0);
Expand All @@ -49,30 +54,24 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size)
fuzzDecoder(data, size, 4, meshopt_decodeIndexSequence);

// decodeVertexBuffer supports any strides divisible by 4 in 4-256 interval
// It's a waste of time to check all of them, so we'll just check a few with different alignment mod 16
// it's a waste of time to check all of them, so we'll just check a few with different alignment mod 16
fuzzDecoder(data, size, 4, meshopt_decodeVertexBuffer);
fuzzDecoder(data, size, 16, meshopt_decodeVertexBuffer);
fuzzDecoder(data, size, 24, meshopt_decodeVertexBuffer);
fuzzDecoder(data, size, 32, meshopt_decodeVertexBuffer);

// encodeVertexBuffer/decodeVertexBuffer should roundtrip for any stride, check a few with different alignment mod 16
fuzzRoundtrip(data, size, 4, 0);
fuzzRoundtrip(data, size, 16, 0);
fuzzRoundtrip(data, size, 24, 0);
fuzzRoundtrip(data, size, 32, 0);

// repeat roundtrip testing for new encoding
meshopt_encodeVertexVersion(0xe);

for (int level = 0; level < 4; ++level)
{
fuzzRoundtrip(data, size, 4, level);
fuzzRoundtrip(data, size, 16, level);
fuzzRoundtrip(data, size, 24, level);
fuzzRoundtrip(data, size, 32, level);
}

meshopt_encodeVertexVersion(0);
// this also checks memory safety properties of the encoder
// to conserve time, we only check one version/level combination, biased towards version 1
uint8_t data0 = size > 0 ? data[0] : 0;
int level = data0 % 5;

meshopt_encodeVertexVersion(level < 4 ? 0xe : 0);

fuzzRoundtrip(data, size, 4, level);
fuzzRoundtrip(data, size, 16, level);
fuzzRoundtrip(data, size, 24, level);
fuzzRoundtrip(data, size, 32, level);

return 0;
}

0 comments on commit a744d99

Please sign in to comment.