Cycles Denoising: Part 1: Preparation changes
ClosedPublic

Authored by Lukas Stockner (lukasstockner97) on Mar 31 2017, 2:18 AM.

Details

Summary

These changes don't affect behaviour at all, but are needed later.

They include:

  • New approach of handling CPU kernel entry points (needed since the filter has a lot of kernels)
  • Dedicated control over tile highlighting (needed to re-highlight tiles when they're being denoised)
  • Renaming the PATH_TRACE task (it will handle both path tracing and denoising later)
  • Device-only memory: Already used in the split kernel, now a separate class
  • Several changes in the kernel to improve numerical stability and prevent NaNs

Diff Detail

Only did a quick look right now, will look over the rest later. I like the changes to how kernels are handled.

intern/cycles/device/device_cpu.cpp
121

What will happen if the architecture is changed in the debug panel?

398–400

split_kernel is leaked here.

intern/cycles/kernel/kernels/cpu/kernel_cpu_impl.h
202

No KERNEL_STUB for this?

Hristo Gueorguiev (nirved) added inline comments.
intern/cycles/kernel/kernel_path.h
682 ↗(On Diff #8520)

This didn't make it to split kernel.

Thanks for the initial review, should all be fixed now.

What is the reason for moving N from the specific BSDFs to all of them? Hair and transparent do not have a valid normal, and since they are not really meaningful in those cases it seems better not to have them at all, so that they can't be used accidentally.

Edit: seems this is needed for D2591.

intern/cycles/device/device_cpu.cpp
57

This is not strictly thread safe, we support running multiple render sessions at the same time. With a const char* instead of a string it should be safe enough though.

intern/cycles/kernel/kernel_path.h
326 ↗(On Diff #8553)

I'm not sure what this does, I can't tell what kind of difference this makes even assuming there are NaNs.

Lukas Stockner (lukasstockner97) marked an inline comment as done.

Addressed review.

This revision is now accepted and ready to land.Apr 22 2017, 1:47 AM

Just a rebase, no real changes.