Fix size_t -> int -> size_t round trip in Cycles

There are two things achieved by this change:

- No possible downcast of size_t to int when calculating motion steps.
- Disambiguate call to `min()` which was for some reason considered
  ambiguous on 32bit platforms `min(int, unsigned int)`.
- Do the same for the `max()` call to keep them symmetrical.

On an implementation side the `min()` is defined for a fixed width
integer type to disambiguate uint from size_t on 32bit platforms,
and yet be able to use it for 32bit operands on 64bit platforms without
upcast.

This ended up in a bit bigger change as the conditional compile-in of
functions is easiest if the functions is templated. Making the functions
templated required to remove the other source of ambiguity which is
`algorithm.h` which was pulling min/max from std.

Now it is the `math.h` which is the source of truth for min/max.
It was only one place which was relying on `algorithm.h` for these
functions, hence the choice of `math.h` as the safest and least
intrusive.

Fixes 32bit platforms (such as i386) in Debian package build system.

Differential Revision: https://developer.blender.org/D14062
This commit is contained in:
Sergey Sharybin 2022-02-10 10:37:00 +01:00
parent 87d2de88fd
commit 04d55038ee
12 changed files with 54 additions and 22 deletions

View File

@ -471,7 +471,7 @@ void BVHEmbree::add_instance(Object *ob, int i)
assert(instance_bvh != NULL);
const size_t num_object_motion_steps = ob->use_motion() ? ob->get_motion().size() : 1;
const size_t num_motion_steps = min(num_object_motion_steps, RTC_MAX_TIME_STEP_COUNT);
const size_t num_motion_steps = min(num_object_motion_steps, (size_t)RTC_MAX_TIME_STEP_COUNT);
assert(num_object_motion_steps <= RTC_MAX_TIME_STEP_COUNT);
RTCGeometry geom_id = rtcNewGeometry(rtc_device, RTC_GEOMETRY_TYPE_INSTANCE);
@ -522,7 +522,7 @@ void BVHEmbree::add_triangles(const Object *ob, const Mesh *mesh, int i)
}
assert(num_motion_steps <= RTC_MAX_TIME_STEP_COUNT);
num_motion_steps = min(num_motion_steps, RTC_MAX_TIME_STEP_COUNT);
num_motion_steps = min(num_motion_steps, (size_t)RTC_MAX_TIME_STEP_COUNT);
const size_t num_triangles = mesh->num_triangles();
@ -775,7 +775,7 @@ void BVHEmbree::add_curves(const Object *ob, const Hair *hair, int i)
}
assert(num_motion_steps <= RTC_MAX_TIME_STEP_COUNT);
num_motion_steps = min(num_motion_steps, RTC_MAX_TIME_STEP_COUNT);
num_motion_steps = min(num_motion_steps, (size_t)RTC_MAX_TIME_STEP_COUNT);
const size_t num_curves = hair->num_curves();
size_t num_segments = 0;

View File

@ -335,7 +335,7 @@ DeviceInfo Device::get_multi_device(const vector<DeviceInfo> &subdevices,
if (device.type == DEVICE_CPU && subdevices.size() > 1) {
if (background) {
int orig_cpu_threads = (threads) ? threads : TaskScheduler::max_concurrency();
int cpu_threads = max(orig_cpu_threads - (subdevices.size() - 1), 0);
int cpu_threads = max(orig_cpu_threads - (subdevices.size() - 1), size_t(0));
VLOG(1) << "CPU render threads reduced from " << orig_cpu_threads << " to " << cpu_threads
<< ", to dedicate to GPU.";

View File

@ -311,7 +311,7 @@ template<typename T> class device_only_memory : public device_memory {
: device_memory(device, name, allow_host_memory_fallback ? MEM_READ_WRITE : MEM_DEVICE_ONLY)
{
data_type = device_type_traits<T>::data_type;
data_elements = max(device_type_traits<T>::num_elements, 1);
data_elements = max(device_type_traits<T>::num_elements, size_t(1));
}
device_only_memory(device_only_memory &&other) noexcept : device_memory(std::move(other))

View File

@ -761,7 +761,7 @@ bool BVHMetal::build_TLAS(Progress &progress,
num_instances++;
if (ob->use_motion()) {
num_motion_transforms += max(1, ob->get_motion().size());
num_motion_transforms += max((size_t)1, ob->get_motion().size());
}
else {
num_motion_transforms++;

View File

@ -1586,7 +1586,7 @@ void OptiXDevice::build_bvh(BVH *bvh, Progress &progress, bool refit)
if (ob->is_traceable() && ob->use_motion()) {
total_motion_transform_size = align_up(total_motion_transform_size,
OPTIX_TRANSFORM_BYTE_ALIGNMENT);
const size_t motion_keys = max(ob->get_motion().size(), 2) - 2;
const size_t motion_keys = max(ob->get_motion().size(), (size_t)2) - 2;
total_motion_transform_size = total_motion_transform_size +
sizeof(OptixSRTMotionTransform) +
motion_keys * sizeof(OptixSRTData);
@ -1660,7 +1660,7 @@ void OptiXDevice::build_bvh(BVH *bvh, Progress &progress, bool refit)
/* Insert motion traversable if object has motion. */
if (motion_blur && ob->use_motion()) {
size_t motion_keys = max(ob->get_motion().size(), 2) - 2;
size_t motion_keys = max(ob->get_motion().size(), (size_t)2) - 2;
size_t motion_transform_size = sizeof(OptixSRTMotionTransform) +
motion_keys * sizeof(OptixSRTData);

View File

@ -119,7 +119,7 @@ void Hair::Curve::motion_keys(const float3 *curve_keys,
{
/* Figure out which steps we need to fetch and their interpolation factor. */
const size_t max_step = num_steps - 1;
const size_t step = min((int)(time * max_step), max_step - 1);
const size_t step = std::min((size_t)(time * max_step), max_step - 1);
const float t = time * max_step - step;
/* Fetch vertex coordinates. */
float4 curr_keys[2];
@ -147,7 +147,7 @@ void Hair::Curve::cardinal_motion_keys(const float3 *curve_keys,
{
/* Figure out which steps we need to fetch and their interpolation factor. */
const size_t max_step = num_steps - 1;
const size_t step = min((int)(time * max_step), max_step - 1);
const size_t step = min((size_t)(time * max_step), max_step - 1);
const float t = time * max_step - step;
/* Fetch vertex coordinates. */
float4 curr_keys[4];
@ -191,8 +191,8 @@ void Hair::Curve::keys_for_step(const float3 *curve_keys,
size_t k1,
float4 r_keys[2]) const
{
k0 = max(k0, 0);
k1 = min(k1, num_keys - 1);
k0 = max(k0, (size_t)0);
k1 = min(k1, (size_t)(num_keys - 1));
const size_t center_step = ((num_steps - 1) / 2);
if (step == center_step) {
/* Center step: regular key location. */
@ -237,8 +237,8 @@ void Hair::Curve::cardinal_keys_for_step(const float3 *curve_keys,
size_t k3,
float4 r_keys[4]) const
{
k0 = max(k0, 0);
k3 = min(k3, num_keys - 1);
k0 = max(k0, (size_t)0);
k3 = min(k3, (size_t)(num_keys - 1));
const size_t center_step = ((num_steps - 1) / 2);
if (step == center_step) {
/* Center step: regular key location. */

View File

@ -606,8 +606,8 @@ void LightManager::device_update_background(Device *device,
ImageMetaData metadata;
if (!env->handle.empty()) {
ImageMetaData metadata = env->handle.metadata();
environment_res.x = max(environment_res.x, metadata.width);
environment_res.y = max(environment_res.y, metadata.height);
environment_res.x = max(environment_res.x, (int)metadata.width);
environment_res.y = max(environment_res.y, (int)metadata.height);
}
}
if (node->type == SkyTextureNode::get_node_type()) {

View File

@ -53,7 +53,7 @@ void Mesh::Triangle::motion_verts(const float3 *verts,
{
/* Figure out which steps we need to fetch and their interpolation factor. */
const size_t max_step = num_steps - 1;
const size_t step = min((int)(time * max_step), max_step - 1);
const size_t step = min((size_t)(time * max_step), max_step - 1);
const float t = time * max_step - step;
/* Fetch vertex coordinates. */
float3 curr_verts[3];

View File

@ -55,7 +55,7 @@ float4 PointCloud::Point::motion_key(const float3 *points,
/* Figure out which steps we need to fetch and their
* interpolation factor. */
const size_t max_step = num_steps - 1;
const size_t step = min((int)(time * max_step), max_step - 1);
const size_t step = min((size_t)(time * max_step), max_step - 1);
const float t = time * max_step - step;
/* Fetch vertex coordinates. */
const float4 curr_key = point_for_step(

View File

@ -21,8 +21,6 @@
CCL_NAMESPACE_BEGIN
using std::max;
using std::min;
using std::remove;
using std::sort;
using std::stable_sort;

View File

@ -124,7 +124,41 @@ ccl_device_inline int min(int a, int b)
return (a < b) ? a : b;
}
ccl_device_inline uint min(uint a, uint b)
ccl_device_inline uint32_t max(uint32_t a, uint32_t b)
{
return (a > b) ? a : b;
}
ccl_device_inline uint32_t min(uint32_t a, uint32_t b)
{
return (a < b) ? a : b;
}
ccl_device_inline uint64_t max(uint64_t a, uint64_t b)
{
return (a > b) ? a : b;
}
ccl_device_inline uint64_t min(uint64_t a, uint64_t b)
{
return (a < b) ? a : b;
}
/* NOTE: On 64bit Darwin the `size_t` is defined as `unsigned long int` and `uint64_t` is defined
* as `unsigned long long`. Both of the definitions are 64 bit unsigned integer, but the automatic
* substitution does not allow to automatically pick function defined for `uint64_t` as it is not
* exactly the same type definition.
* Work this around by adding a templated function enabled for `size_t` type which will be used
* when there is no explicit specialization of `min()`/`max()` above. */
template<class T>
ccl_device_inline typename std::enable_if_t<std::is_same_v<T, size_t>, T> max(T a, T b)
{
return (a > b) ? a : b;
}
template<class T>
ccl_device_inline typename std::enable_if_t<std::is_same_v<T, size_t>, T> min(T a, T b)
{
return (a < b) ? a : b;
}

View File

@ -23,7 +23,7 @@
#include <stdlib.h>
#include <string.h>
#include "util/algorithm.h"
#include "util/math.h"
#include "util/murmurhash.h"
#if defined(_MSC_VER)