Repository navigation
meshletcodec: Implement new decodeMeshlet interface - #1008
Merged
Merged
Conversation
To support decoding meshlet data directly into runtime storage for conventional layouts, we need to be able to decode triangles into 3 bytes each, and vertex references into 16-bit instead of 32-bit. All of these cases benefit from decoding the data directly into the target format. Additionally, all of them need tighter bounds on the output writes compared to "up to 16 bytes extra" we have right now. To that end, rename decodeMeshlet to meshopt_decodeMeshletRaw and introduce a new function that takes vertex/index sizes plus a template wrapper so that existing code in demo/fuzzing compiles. We will actually implement support for variable strides as well as safe SIMD decoding separately.
We now support decoding meshlet triangles into bytes and decoding vertex references into 16-bit integers when using scalar decoding. The scalar decoder writes one vertex/triangle at a time, so it's automatically memory safe as a result (if slow).
We will need a few variants of each decode function to support safe decoding for various types; they all will reuse the basic state update as a building block. There's a minor risk of compiler not inlining these, but it should really not happen and if it does we can forceinline these.
For now it's not clear if we will need to fully duplicate the underlying code to support fastest decoding when the output buffers are padded; for now we'll do that, and remove the Raw variants if we can.
Vertex reference decoding simply needs to narrow the resulting values to 16-bit; SSE doesn't have a single-instruction way to do this that's not a shuffle (we could use packus but that requires masking the value to avoid saturation). Triangle decoding doesn't need to shuffle and just needs to shift the vector to align the bytes for the store. In both cases, the resulting functions are *not* memory safe and still overwrite too much memory. This will be adjusted separately.
The current memory safety guarantee for meshlet decoding requires the output space to be 32-bit aligned as far as the buffer space goes (as in, the total number of bytes written is up to alignup(size, 4)). With that in mind, when we decode 32-bit references, we need to special case the last group and write each element one at a time. We could do a 8+4 write but that's a little more code and not obviously faster. When we decode 16-bit references, we can use the 32-bit alignment guarantee to only do special tail processing when the last group has 1 or 2 elements, in which case we can write it in a single 32-bit store.
Similarly to vertex decoding, we process complete groups and write two triangles at a time, and handle the tail separately. For 32-bit triangle decoding this works perfectly, because the main loop can still do 8-byte writes. Note that this is not *actually* correct for 8/24-bit triangle decoding. Specifically, when triangle count is divisible by 4, we will write an extra two bytes after the last triangle as our inner loop uses 8-byte writes. Additionally, when targeting write-combined memory, this re-writes two bytes every six bytes which can double the number of transactions depending on the CPU.
We test performance on meshopt_decodeMeshletRaw now and ensure that all 4 non-raw combinations correctly decode the meshlet data.
Instead of decoding triangle pairs, we now decode two triangles at a time. This allows us to write 12 bytes per triangle quad, which can be done using two writes which is efficient even if the target memory is write-combined. This also allows us to merge bounds checks (gap is big enough to decode 4 triangles) and some loop overhead, which ends up with the code being faster even when the target memory is not write-combined. We use the same insight as is used in the 16-bit vertex reference decoder to process the tail, where we need to write 3/6 bytes but *can* write 4/8 bytes due to alignment requirements. For now the "raw" decoder still uses triangle pairs, as that has been validated to be simple and fast.
This was meant to be 3 or 4 instead of 1 or 4; this makes the template wrapper a little more complicated but the C API makes a little more sense.
We previously over-allocated the output buffer which guaranteed safety for all combinations; the new size-aware decoding has tighter output buffer requirements, so we check them all now. The output buffer now has to have align(size, 4) bytes in it; for 32-bit triangles/vertices, this is equivalent to not writing past the output array; for 16-bit vertices or 24-bit triangles, it's a little more complicated as the extra padding depends on the element count.
While in practice the raw integer store is probably safe, the output location for the tail is not guaranteed to be 4 bytes aligned; in some scenarios, such as undefined-sanitizer, this could cause issues. Resolve these via aligned(1) integer casts similarly to how vertex codec handles this. We simply need to generate MOVQ here without using _mm_storeu_si32 (which is not supported on MSVC until AVX-512).
We default to placing SIMD_TARGET on its own line unless clang-format disagrees; fix the uses where it was not the case. Also remove SIMD_AVX from the macro definition; this was left from the vertexcodec version, but we don't need it here.
We now decode the same meshlet twice, into a 32-bit and 24-bit triangle format, to validate that the index data roundtrips.
For simplicity, we test triangles and vertices separately; when testing vertices, we encode one triangle because while we currently allow encoding 0 triangles, this may change in the future as it's a non-sensical configuration.
Instead of explicitly enumerating the tail conditions we can simplify them based on the number of groups we have already processed, to check if we have extra elements to write. Also clean up some comments and fix an implicit int->unsigned cast.
This groups SIMD group decode helpers into a single block which should make it easier to introduce support for other SIMD ISAs in the future by reusing the flow in higher level *Simd functions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
To support decoding meshlet data directly into runtime storage for
conventional layouts, we need to be able to decode triangles into 3
bytes each, and vertex references into 16-bit instead of 32-bit.
All of these cases benefit from decoding the data directly into the
target format. Additionally, all of them need tighter bounds on the
output writes compared to "up to 16 bytes extra" we have right now.
To that end, rename decodeMeshlet to
meshopt_decodeMeshletRawandintroduce a new function that takes vertex/index sizes plus a template
wrapper so that existing code in demo/fuzzing compiles.
To maintain reasonable SIMD efficiency given the inherent variability
of meshlet counts, the output requirements on the new functions demand
that the output size can be overwritten by up to a 4 byte boundary; that is,
decodeMeshlet can write
align(triangle_count * triangle_size, 4)(and samefor vertex data) bytes. This can occasionally result in surprising behavior
if the output data is not aligned by 4 bytes: a stream of 3-byte meshlet
triangles may require up to a 3-byte overwrite, so the last meshlet in that
stream may not be memory-safe to decode directly. In cases like this, a
memcpyfallback from the temporary buffer is recommended if alignmentis impractical.
When the output buffers are 4-byte aligned, the new implementation results
in clean uninterrupted series of writes to the output memory, which results
in optimal decoding performance for write-combined memory.
When additional processing of meshlet data is required, it's optimal to decode
into temporary buffers in which case extra tail processing is unnecessary; for
now,
decodeMeshletRawis best in these cases and can be 5% faster, althoughin the future we might drop this function for simplicity. The dispatch setup
in this PR is not fully optimal and will likely get reworked in later PRs.
The new functions result in ~7-8 GB/s combined decoding throughput when
decoding a realistic mix of meshlet data from
niagaraproject on Zen 4; thismakes meshlet encoding Pareto optimal. The actual compression ratios will
strongly depend on the underlying data of course.
This contribution is sponsored by Valve.