Unlimited number of textures for Cycles
ClosedPublic

Authored by Stefan Werner (swerner) on Apr 25 2017, 10:31 AM.

Details

Summary

This patch allows for an unlimited number of textures in Cycles where the hardware allows. It replaces a number static arrays with dynamic arrays and changes the way the flat_slot indices are calculated. Eventually, I'd like to get to a point where there are only flat slots left and textures off all kinds are stored in a single array.

Note that the arrays in DeviceScene are changed from containing device_vector<T> objects to device_vector<T>* pointers. Ideally, I'd like to store objects, but dynamic resizing of a std:vector in pre-C++11 calls the copy constructor, which for a good reason is not implemented for device_vector. Once we require C++11 for Cycles builds, we can implement a move constructor for device_vector and store objects again.

The limits for CUDA Fermi hardware still apply.

Diff Detail

Repository
rB Blender

Just initial pass of review.

The thing i wonder here is whether it's more beneficial to have per-type vector or a vector of virtual objects.

intern/cycles/kernel/kernel_globals.h
23

Any reason to use stl::vector instead if ccl::vector from util_vector.h? API-wise they should be exactly the same, but this gives extra memory profiling for Blender.

intern/cycles/kernel/kernels/cpu/kernel.cpp
101

Code style, no space after keyword. I would also suggest always use braces, even for single line bodies. Makes code more explicit and avoids noise in the future.

Same applies to the cases below.

intern/cycles/kernel/svm/svm_image.h
154

Wondering whether it's better to do

const texture_id = kernel_tex_type(id);
if(texture_id == ...)
169

Same as above. And cases below.

intern/cycles/render/image.cpp
201

Would suggest making it a constant similar to primitive type so if we add more types we don't have to scroll over all code and replace bits.

206–208

We don't use c++ casts in Cycles, we use C-like casts.

208

Same as above.

For what it's worth, here's a Python script that will create thousands of Suzannes and put individual 1x1 pixel textures on them: http://pasteall.org/371017/python

I just noticed that this patch doesn't work with OpenCL, so I'll need to address that too.

Stefan Werner (swerner) marked 7 inline comments as done.
This comment was removed by Stefan Werner (swerner).

Addressed Sergey's comments and changed what should (hopefully) make OpenCL work.

Tod Baudais (smellslikedonkey) added inline comments.
intern/cycles/render/image.cpp
225

Should these magic numbers be constants somewhere?

Addressed Tod's comments, magic number is now replaced with a more descriptive #define.

Stefan Werner (swerner) marked an inline comment as done.Apr 26 2017, 11:10 AM

Please always assign entire Cycles project as Reviewers. I refactored most of the image code last year, so I'd like a chance to check on the proposed changes here too. Quite busy atm, but will check in the coming days.

In general looks fine. Should be a first step towards easier code in this area. Once we can drop Fermi fully (as soon as nvidia drops it with CUDA 9), we can simplify it even further and get rid of the special cases.

intern/cycles/render/image.cpp
287

Not sure why the comment is changed, we still convert single channel and half slots which are not available on fermi to other slot types. It's easier to read a one sentence comment, rather than read 4 if/else lines.

294

Code style (no gap between if and brace), also wrong in other places.

Thomas Dinges (dingto) requested changes to this revision.Apr 26 2017, 4:55 PM
Thomas Dinges (dingto) added inline comments.
intern/cycles/kernel/kernels/cpu/kernel_cpu_image.h
26–40

Picky: Back in last years GSOC I changed all code to be following the following order:
float4, byte4, half4, float, byte, half. I would like to keep this order either forward or backward, but keep it nevertheless. See also definition of ImageDataType().

This revision now requires changes to proceed.Apr 26 2017, 4:55 PM

Addressed DingTo's comments.

Stefan Werner (swerner) marked 3 inline comments as done.Apr 26 2017, 8:38 PM
Stefan Werner (swerner) added inline comments.
intern/cycles/render/image.cpp
287

Initially I thought I could get half precision textures on Fermi too by assigning the texture slots as needed, but that ended up not being the case. This commend was a relict of that.

Stefan Werner (swerner) marked 2 inline comments as done.Apr 26 2017, 8:39 PM

Looks good to me then. :) We could only get half textures working on Fermi, if we take some of the float4 slots and make them half4 ones, but I wouldn't bother.

This revision is now accepted and ready to land.Apr 26 2017, 10:30 PM
This revision was automatically updated to reflect the committed changes.