Cycles: MetalRT support (kernel side)
ClosedPublic
Actions

Authored by Michael Jones (michael_jones) on Nov 24 2021, 5:16 PM.

Details

Reviewers

Brecht Van Lommel (brecht)
William Leeson (leesonw)

Maniphest Tasks

T92212: Cycles Metal device

Commits

rC6a038a2e6370: Metalrt support (kernel side)
rBf613c4c0953e: Cycles: MetalRT support (kernel side)

Summary

This patch adds MetalRT support to Cycles kernel code. It is mostly additive in nature or confined to Metal-specific code, however there are a few areas where this interacts with other code:

MetalRT closely follows the Optix implementation, and in some cases (notably handling of transforms) it makes sense to extend Optix special-casing to MetalRT. For these generalisations we now have __KERNEL_GPU_RAYTRACING__ instead of __KERNEL_OPTIX__.
MetalRT doesn't support primitive offsetting (as with primitiveIndexOffset in Optix), so we define and populate a new kernel texture, __object_prim_offset, containing per-object primitive / curve-segment offsets. This is referenced and applied in MetalRT intersection handlers.
Two new BVH layout enum values have been added: BVH_LAYOUT_METAL and BVH_LAYOUT_MULTI_METAL_EMBREE for XPU mode). Some host-side enum case handling has been updated where it is trivial to do so.

Ref T92212

Diff Detail

Repository

rB Blender

Branch

arcpatch-D13353 (branched from master)

Build Status

Buildable 19038
Build 19038: arc lint + arc unit

Event Timeline

Michael Jones (michael_jones) requested review of this revision.Nov 24 2021, 5:16 PM

Michael Jones (michael_jones) created this revision.

Harbormaster completed remote builds in B18940: Diff 45223.Nov 24 2021, 5:16 PM

Eugen (mycoconut) awarded a token.Nov 24 2021, 5:17 PM

Michał Myśliwiec (misiek3d) awarded a token.Nov 24 2021, 5:56 PM

Paul Larson (GeorgiaPacific) added a subscriber: Paul Larson (GeorgiaPacific).Nov 24 2021, 8:49 PM

Alaska (Alaska) awarded a token.Nov 25 2021, 12:15 PM

Looks generally fine.

After most of the Metal changes have landed I'll refactor the kernel/bvh/bvh.h code to move the device specific code to kernel/device/*/bvh.h, since it's getting a bit messy.

intern/cycles/kernel/bvh/bvh.h
253–257	It's unclear to me what this reference to the OptiX flags means. Is it work in unfinished? Handled elsewhere in the code?
intern/cycles/kernel/device/metal/compat.h
281	I would just put this here rather than having an intermediate macro: #ifdef __METALRT__ metalrt_as_type accel_struct; metalrt_ift_type ift_default; metalrt_ift_type ift_shadow; metalrt_ift_type ift_local; #endif
intern/cycles/kernel/device/metal/kernel.metal
221	This looks like a bug in the OptiX code. I think it should be `optixSetPayload_5(true)` in OptiX, and `payload.result = true;` here. I can fix the OptiX case in master. I think it went unnoticed because the throughput is so low that it's effectively opaque anyway, and impact on performance or the render is likely small.
intern/cycles/scene/scene.h
103	Add this in `ObjectManager::device_free` along with the other object device vectors.

This revision now requires changes to proceed.Nov 26 2021, 7:16 PM

Michael Jones (michael_jones) added inline comments.Nov 26 2021, 7:55 PM

intern/cycles/kernel/bvh/bvh.h
253–257	I agree that this is confusing - I will add a better comment. MetalRT uses anyhit-like behaviour as a default, with ray continuation / termination controlled by the intersection handlers. So the first block needs no further handling since anyhit is the default behaviour. In the the second block, the early termination is controlled elsewhere (by the intersection handler): /* Shadow ray early termination. */ if (visibility & PATH_RAY_SHADOW_OPAQUE) { result.accept = true; result.continue_search = false; return result; }

Proper host-side object_prim_offset teardown
Tidier conditional compilation and better comments
Proper early-accept in case of low throughput

Harbormaster completed remote builds in B19038: Diff 45403.Nov 29 2021, 3:37 PM

Michael Jones (michael_jones) marked 4 inline comments as done.Nov 29 2021, 3:38 PM

Looks good except these two compile issues:

/home/brecht/dev/blender/intern/cycles/bvh/bvh.cpp:93:11: warning: enumeration value 'BVH_LAYOUT_METAL' not handled in switch [-Wswitch]
  switch (params.bvh_layout) {

/home/brecht/dev/blender/intern/cycles/device/multi/device.cpp:186:56: error: use of undeclared identifier 'DEVICE_METAL'
          params.bvh_layout = sub.device->info.type == DEVICE_METAL ? BVH_LAYOUT_METAL :

This revision now requires changes to proceed.Nov 29 2021, 3:55 PM