Skip to content

Apple: MaterialX on Metal#2324

Closed
slingthor wants to merge 1 commit intoPixarAnimationStudios:devfrom
slingthor:materialx
Closed

Apple: MaterialX on Metal#2324
slingthor wants to merge 1 commit intoPixarAnimationStudios:devfrom
slingthor:materialx

Conversation

@slingthor
Copy link
Copy Markdown
Contributor

Description of Change(s)

  • Enables MaterialX support on Metal. Makes the MaterialX shader generator polymorphic and implements the Metal based functionality.
  • Points to Apple's branched hosted version of MaterialX until official release of MaterialX with Metal
  • Makes sure that on Metal we don't attempt to mipmap textures containing only one pixel.
  • Metal versions 2.2 and earlier don't return upon usage of the discard keyword. Improve discard handling to pass MaterialX unit tests on Metal.

Co-authored by:
Morteza Mostajabodaveh
Jon Creighton

  • I have verified that all unit tests pass with the proposed changes
  • I have submitted a signed Contributor License Agreement

@slingthor slingthor force-pushed the materialx branch 4 times, most recently from bb8925c to e97eddb Compare March 6, 2023 16:42
@sunyab
Copy link
Copy Markdown
Contributor

sunyab commented Mar 14, 2023

Filed as internal issue #USD-8085

@dgovil dgovil mentioned this pull request Apr 10, 2023
1 task
pixar-oss pushed a commit that referenced this pull request May 19, 2023
This version update allows for MaterialX support on Metal.

Fixes #2324

(Internal change: 2275891)
@pixar-oss pixar-oss closed this in 5dab6e0 Jul 21, 2023
musicinmybrain pushed a commit to musicinmybrain/OpenUSD that referenced this pull request Apr 7, 2026
…expansion (PixarAnimationStudios#2324)

* Fix misaligned memory access in `LossyDctDecoder_execute` HALF→FLOAT expansion

After DCT decoding, `LossyDctDecoder_execute()` expands FLOAT-type channels
from their intermediate HALF (16-bit) XDR representation back to FLOAT (32-bit)
XDR in place.  The expansion was done by casting `_rows[y]` (a `uint8_t *`)
directly to `float *` and `uint16_t *`, then reading and writing through those
typed pointers.

Because row buffers are assigned by advancing a byte pointer with no alignment
padding (`outBufferEnd += chan->width * chan->bytes_per_element` in
`internal_dwa_compressor.h`), a FLOAT channel that follows a HALF channel of
odd width receives a `_rows[y]` pointer that is 2-byte aligned but not 4-byte
aligned.  Dereferencing a `float *` cast from such a pointer is undefined
behavior under the C standard:

- On ARM, RISC-V, and MIPS (strict alignment) this crashes immediately.
- On x86 it is silently tolerated at the hardware level but remains UB:
  auto-vectorizing compilers (SSE/AVX) may assume aligned access and generate
  incorrect code.
- UBSan reports: `store to misaligned address ... for type 'float', which
  requires 4 byte alignment` at `internal_dwa_decoder.h:749`.

Fix: replace the cast-and-dereference pattern with the `unaligned_load16` /
`memcpy` / `unaligned_store32` helpers already used throughout the rest of
OpenEXRCore (`internal_xdr.h`, `unpack.c`, `pack.c`, `internal_pxr24.c`).
These helpers use `memcpy` internally, which the C standard guarantees is safe
for unaligned addresses and which compilers compile to a single load/store
instruction on architectures that support it.

The byte-order handling is preserved correctly:
- `unaligned_load16` reads 2 bytes via `memcpy` and applies `one_to_native16`
  (XDR → native), returning a native-endian HALF value.
- `half_to_float` converts native HALF → native float.
- `memcpy(&bits, &f, 4)` reinterprets the float's bit pattern as `uint32_t`
  without numeric conversion (the correct type-pun idiom in C).
- `unaligned_store32` applies `one_from_native32` (native → XDR) and writes
  4 bytes via `memcpy`, storing the result in XDR float format.

Made-with: Cursor
Signed-off-by: Cary Phillips <cary@ilm.com>

* add TODO comment

Signed-off-by: Cary Phillips <cary@ilm.com>

---------

Signed-off-by: Cary Phillips <cary@ilm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants