Rewriting how render buffers and aborted renders are handled.

Fixes Autodesk#8
Fixes Autodesk#9
Fixes Autodesk#4

CMakeLists.txt

@@ -34,7 +34,11 @@ if (NOT BUILD_USE_CUSTOM_BOOST)
         find_package(Boost REQUIRED)
     endif ()
 endif ()
-find_package(Python2 COMPONENTS Development Interpreter REQUIRED)
+if (BUILD_SCHEMAS)
+    find_package(Python2 COMPONENTS Development Interpreter REQUIRED)
+else ()
+    find_package(Python2 COMPONENTS Development REQUIRED)
+endif ()
 find_package(TBB REQUIRED)
 
 configure_file(
@@ -102,6 +106,10 @@ if (BUILD_DOCS)
     add_subdirectory(docs)
 endif ()
 
+if (BUILD_UNIT_TESTS)
+    find_package(GoogleTest REQUIRED)
+endif ()
+
 install(FILES LICENSE.md
         DESTINATION .)
 

cmake/modules/FindGoogleTest.cmake

@@ -0,0 +1,73 @@
+# Simple module to find Google Test.
+
+if (WIN32)
+    set(GOOGLETEST_LIB_EXTENSION ".lib"
+            CACHE STRING "Extension of Google Test libraries")
+else () # MacOS and Linux
+    set(GOOGLETEST_LIB_EXTENSION ".a"
+            CACHE STRING "Extension of Google Test libraries")
+endif ()
+
+if (WIN32)
+    set(GOOGLETEST_LIB_PREFIX ""
+            CACHE STRING "Prefix of Google Test libraries")
+else ()
+    set(GOOGLETEST_LIB_PREFIX "lib"
+            CACHE STRING "Prefix of Google Test libraries")
+endif ()
+
+
+find_path(GTEST_INCLUDE_DIR gtest/gtest.h
+          PATHS "${GOOGLETEST_LOCATION}/include"
+                "$ENV{GOOGLETEST_LOCATION}/include"
+                "${GOOGLETEST_INCLUDEDIR}"
+                "$ENV{GOOGLETEST_INCLUDEDIR}"
+          DOC "Google Test include directory")
+
+find_library(GMOCK_LIBRARY ${GOOGLETEST_LIB_PREFIX}gmock${GOOGLETEST_LIB_EXTENSION}
+             PATHS "${GOOGLETEST_LOCATION}/lib64"
+                   "$ENV{GOOGLETEST_LOCATION}/lib64"
+                   "${GOOGLETEST_LOCATION}/lib"
+                   "$ENV{GOOGLETEST_LOCATION}/libb"
+                   "${GOOGLETEST_LIBRARYDIR}"
+                   "$ENV{GOOGLETEST_LIBRARYDIR}"
+             DOC "Google Test Mock library")
+
+find_library(GMOCK_MAIN_LIBRARY ${GOOGLETEST_LIB_PREFIX}gmock_main${GOOGLETEST_LIB_EXTENSION}
+             PATHS "${GOOGLETEST_LOCATION}/lib64"
+                    "$ENV{GOOGLETEST_LOCATION}/lib64"
+                    "${GOOGLETEST_LOCATION}/lib"
+                    "$ENV{GOOGLETEST_LOCATION}/libb"
+                    "${GOOGLETEST_LIBRARYDIR}"
+                    "$ENV{GOOGLETEST_LIBRARYDIR}"
+             DOC "Google Test Mock Main library")
+
+find_library(GTEST_LIBRARY ${GOOGLETEST_LIB_PREFIX}gtest${GOOGLETEST_LIB_EXTENSION}
+             PATHS "${GOOGLETEST_LOCATION}/lib64"
+                    "$ENV{GOOGLETEST_LOCATION}/lib64"
+                    "${GOOGLETEST_LOCATION}/lib"
+                    "$ENV{GOOGLETEST_LOCATION}/libb"
+                    "${GOOGLETEST_LIBRARYDIR}"
+                    "$ENV{GOOGLETEST_LIBRARYDIR}"
+             DOC "Google Test library")
+
+find_library(GTEST_MAIN_LIBRARY ${GOOGLETEST_LIB_PREFIX}gtest_main${GOOGLETEST_LIB_EXTENSION}
+             PATHS "${GOOGLETEST_LOCATION}/lib64"
+                   "$ENV{GOOGLETEST_LOCATION}/lib64"
+                   "${GOOGLETEST_LOCATION}/lib"
+                   "$ENV{GOOGLETEST_LOCATION}/libb"
+                   "${GOOGLETEST_LIBRARYDIR}"
+                   "$ENV{GOOGLETEST_LIBRARYDIR}"
+             DOC "Google Test Mock Main library")
+
+set(GTEST_LIBRARIES "${GMOCK_LIBRARY};${GMOCK_MAIN_LIBRARY};${GTEST_LIBRARY};${GTEST_MAIN_LIBRARY}")
+
+include(FindPackageHandleStandardArgs)
+
+find_package_handle_standard_args(GoogleTest
+    REQUIRED_VARS
+        GTEST_INCLUDE_DIR
+        GMOCK_LIBRARY
+        GMOCK_MAIN_LIBRARY
+        GTEST_LIBRARY
+        GTEST_MAIN_LIBRARY)

cmake/utils/options.cmake

@@ -25,6 +25,7 @@ option(BUILD_NDR_PLUGIN "Builds the NDR Plugin" ON)
 option(BUILD_PROCEDURAL "Builds the Procedural" ON)
 option(BUILD_USD_WRITER "Builds the USD Writer" ON)
 option(BUILD_DOCS "Builds the Documentation" ON)
+option(BUILD_UNIT_TEST "Build the unit tests" OFF)
 
 set(PREFIX_PROCEDURAL "procedural" CACHE STRING "Directory to install the procedural under.")
 set(PREFIX_PLUGINS "plugin" CACHE STRING "Directory to install the plugins (Hydra and Ndr) under.")

render_delegate/CMakeLists.txt

@@ -50,7 +50,7 @@ target_include_directories(hdArnold SYSTEM PRIVATE "${USD_INCLUDE_DIR}")
 target_include_directories(hdArnold SYSTEM PRIVATE "${ARNOLD_INCLUDE_DIR}")
 target_include_directories(hdArnold SYSTEM PRIVATE "${Boost_INCLUDE_DIRS}")
 target_include_directories(hdArnold SYSTEM PRIVATE "${TBB_INCLUDE_DIRS}")
-target_include_directories(hdArnold SYSTEM PRIVATE Python2::Python)
+target_include_directories(hdArnold SYSTEM PRIVATE "${Python2_INCLUDE_DIRS}")
 target_include_directories(hdArnold PRIVATE "${CMAKE_CURRENT_BINARY_DIR}")
 
 target_compile_definitions(hdArnold PRIVATE "HDARNOLD_EXPORTS=1")

render_delegate/constant_strings.h

@@ -64,6 +64,7 @@ ASTR(AA_seed);
 ASTR(abort_on_error);
 ASTR(abort_on_license_fail);
 ASTR(angular);
+ASTR(aov_pointer);
 ASTR(attribute);
 ASTR(auto_transparency_depth);
 ASTR(base);
@@ -122,6 +123,7 @@ ASTR(GI_volume_samples);
 ASTR(ginstance);
 ASTR(GPU);
 ASTR(grids);
+ASTR(HdArnoldDriver);
 ASTR(id);
 ASTR(ignore_atmosphere);
 ASTR(ignore_bump);
@@ -182,6 +184,7 @@ ASTR(profile_file);
 ASTR(progressive);
 ASTR(progressive_min_AA_samples);
 ASTR(progressive_show_all_outputs);
+ASTR(projMtx);
 ASTR(quad_light);
 ASTR(radius);
 ASTR(reference_time);
@@ -242,6 +245,7 @@ ASTR(uvidxs);
 ASTR(uvlist);
 ASTR(varname);
 ASTR(vidxs);
+ASTR(viewMtx);
 ASTR(visibility);
 ASTR(vlist);
 ASTR(volume);

render_delegate/instancer.h

@@ -10,7 +10,7 @@
 // distributed under the License is distributed on an "AS IS" BASIS,
 // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 // See the License for the specific language governing permissions and
-// limitations under the License.// Copyright (c) 2019 Autodesk, Inc., all rights reserved
+// limitations under the License.
 /// @file instancer.h
 ///
 /// Utilities to support point instancers.

render_delegate/nodes/driver.cpp

@@ -27,48 +27,49 @@
 // limitations under the License.
 #include <ai.h>
 
-#include <tbb/concurrent_queue.h>
-
+#include <algorithm>
 #include <memory>
 
 #include "../constant_strings.h"
+#include "../render_buffer.h"
 #include "../utils.h"
-#include "nodes.h"
 
-PXR_NAMESPACE_USING_DIRECTIVE
+PXR_NAMESPACE_OPEN_SCOPE
 
 AI_DRIVER_NODE_EXPORT_METHODS(HdArnoldDriverMtd);
 
-AtString HdArnoldDriver::projMtx("projMtx");
-AtString HdArnoldDriver::viewMtx("viewMtx");
-
 namespace {
 const char* supportedExtensions[] = {nullptr};
 
+struct BucketData {
+    TfToken name;
+    int type;
+    const void* data;
+
+    BucketData(const TfToken& _name, int _type, const void* _data) : name(_name), type(_type), data(_data) {}
+};
+
 struct DriverData {
     GfMatrix4f projMtx;
     GfMatrix4f viewMtx;
+    HdArnoldRenderBufferStorage* renderBuffers;
+    // Local storage for converting from P to depth.
+    std::vector<float> depths[AI_MAX_THREADS];
+    // Local storage for the id remapping.
+    std::vector<int> ids[AI_MAX_THREADS];
+    // Local storage for the color buffer.
+    std::vector<AtRGBA> colors[AI_MAX_THREADS];
+    // Local storage for bucket data.
+    std::vector<BucketData> buckets[AI_MAX_THREADS];
 };
-} // namespace
 
-tbb::concurrent_queue<HdArnoldBucketData*> bucketQueue;
-
-void hdArnoldEmptyBucketQueue(const std::function<void(const HdArnoldBucketData*)>& f)
-{
-    HdArnoldBucketData* data = nullptr;
-    while (bucketQueue.try_pop(data)) {
-        if (data) {
-            f(data);
-            delete data;
-            data = nullptr;
-        }
-    }
-}
+} // namespace
 
 node_parameters
 {
-    AiParameterMtx(HdArnoldDriver::projMtx, AiM4Identity());
-    AiParameterMtx(HdArnoldDriver::viewMtx, AiM4Identity());
+    AiParameterMtx(str::projMtx, AiM4Identity());
+    AiParameterMtx(str::viewMtx, AiM4Identity());
+    AiParameterPtr(str::aov_pointer, nullptr);
 }
 
 node_initialize
@@ -80,15 +81,18 @@ node_initialize
 node_update
 {
     auto* data = reinterpret_cast<DriverData*>(AiNodeGetLocalData(node));
-    data->projMtx = HdArnoldConvertMatrix(AiNodeGetMatrix(node, HdArnoldDriver::projMtx));
-    data->viewMtx = HdArnoldConvertMatrix(AiNodeGetMatrix(node, HdArnoldDriver::viewMtx));
+    data->projMtx = HdArnoldConvertMatrix(AiNodeGetMatrix(node, str::projMtx));
+    data->viewMtx = HdArnoldConvertMatrix(AiNodeGetMatrix(node, str::viewMtx));
+    data->renderBuffers = static_cast<HdArnoldRenderBufferStorage*>(AiNodeGetPtr(node, str::aov_pointer));
+    // TODO(pal): Clear AOVs.
 }
 
 node_finish {}
 
 driver_supports_pixel_type
 {
-    return pixel_type == AI_TYPE_RGBA || pixel_type == AI_TYPE_VECTOR || pixel_type == AI_TYPE_UINT;
+    return pixel_type == AI_TYPE_RGBA || pixel_type == AI_TYPE_RGB || pixel_type == AI_TYPE_VECTOR ||
+           pixel_type == AI_TYPE_UINT;
 }
 
 driver_extension { return supportedExtensions; }
@@ -101,64 +105,97 @@ driver_prepare_bucket {}
 
 driver_process_bucket
 {
-    const auto* driverData = reinterpret_cast<const DriverData*>(AiNodeGetLocalData(node));
+    auto* driverData = reinterpret_cast<DriverData*>(AiNodeGetLocalData(node));
     const char* outputName = nullptr;
     int pixelType = AI_TYPE_RGBA;
     const void* bucketData = nullptr;
-    auto* data = new HdArnoldBucketData();
-    data->xo = bucket_xo;
-    data->yo = bucket_yo;
-    data->sizeX = bucket_size_x;
-    data->sizeY = bucket_size_y;
-    const auto bucketSize = bucket_size_x * bucket_size_y;
+    // TODO: IDs are used to detect background pixels, so we have to handle them first. We should do a two phase
+    //  iteration, first gather all the available data and handle ids, then process the rest.
+    const auto pixelCount = bucket_size_x * bucket_size_y;
+    // We should almost always have depth and id.
+    auto& ids = driverData->ids[tid];
+    ids.clear();
+    auto& buckets = driverData->buckets[tid];
+    buckets.clear();
     while (AiOutputIteratorGetNext(iterator, &outputName, &pixelType, &bucketData)) {
-        if (pixelType == AI_TYPE_RGBA && strcmp(outputName, "RGBA") == 0) {
-            data->beauty.resize(bucketSize);
-            const auto* inRGBA = reinterpret_cast<const AtRGBA*>(bucketData);
-            for (auto i = decltype(bucketSize){0}; i < bucketSize; ++i) {
-                const auto in = inRGBA[i];
-                const auto x = bucket_xo + i % bucket_size_x;
-                const auto y = bucket_yo + i / bucket_size_x;
-                AtRGBA8 out;
-                out.r = AiQuantize8bit(x, y, 0, in.r, true);
-                out.g = AiQuantize8bit(x, y, 1, in.g, true);
-                out.b = AiQuantize8bit(x, y, 2, in.b, true);
-                out.a = AiQuantize8bit(x, y, 3, in.a, true);
-                data->beauty[i] = out;
-            }
-
-        } else if (pixelType == AI_TYPE_VECTOR && strcmp(outputName, "P") == 0) {
-            data->depth.resize(bucketSize, 1.0f);
-            const auto* pp = reinterpret_cast<const GfVec3f*>(bucketData);
-            auto* pz = data->depth.data();
-            for (auto i = decltype(bucketSize){0}; i < bucketSize; ++i) {
-                // Rays hitting the background will return a (0,0,0) vector. We don't worry about it, as background
-                // pixels will be marked with an ID of 0 by arnold.
-                const auto p = driverData->projMtx.Transform(driverData->viewMtx.Transform(pp[i]));
-                pz[i] = std::max(-1.0f, std::min(1.0f, p[2]));
-            }
+        if (pixelType == AI_TYPE_VECTOR && strcmp(outputName, "P") == 0) {
+            // TODO(pal): Push back to P too if the buffer P exists.
+            buckets.emplace_back(HdAovTokens->depth, AI_TYPE_VECTOR, bucketData);
         } else if (pixelType == AI_TYPE_UINT && strcmp(outputName, "ID") == 0) {
-            data->primId.resize(bucketSize, -1);
-            // Technically, we're copying from an unsigned int buffer to a signed int buffer... but the values were
-            // originally force-reinterpreted to unsigned on the way in, so we're undoing that on the way out
-            memcpy(data->primId.data(), bucketData, bucketSize * sizeof(decltype(data->primId[0])));
+            const auto it = driverData->renderBuffers->find(HdAovTokens->primId);
+            if (it != driverData->renderBuffers->end()) {
+                ids.resize(pixelCount, -1);
+                const auto* in = static_cast<const unsigned int*>(bucketData);
+                for (auto i = decltype(pixelCount){0}; i < pixelCount; i += 1) {
+                    ids[i] = static_cast<int>(in[i]) - 1;
+                }
+                it->second->WriteBucket(bucket_xo, bucket_yo, bucket_size_x, bucket_size_y, HdFormatInt32, ids.data());
+            }
+        } else if (pixelType == AI_TYPE_RGB || pixelType == AI_TYPE_RGBA) {
+            // TODO(pal): Push back to RGBA too if the buffer RGBA exists.
+            buckets.emplace_back(
+                strcmp(outputName, "RGBA") == 0 ? HdAovTokens->color : TfToken{outputName}, pixelType, bucketData);
         }
     }
-    if (data->beauty.empty() || data->depth.empty() || data->primId.empty()) {
-        delete data;
-    } else {
-        for (auto i = decltype(bucketSize){0}; i < bucketSize; ++i) {
-            // The easiest way to filter is to check for the zero primID.
-            data->primId[i] -= 1;
-            if (data->primId[i] == -1) {
-                data->depth[i] = 1.0f - AI_EPSILON;
-                data->beauty[i].a = 0.0f;
+    for (const auto& bucket : buckets) {
+        if (bucket.name == HdAovTokens->depth) {
+            const auto it = driverData->renderBuffers->find(bucket.name);
+            if (it == driverData->renderBuffers->end()) {
+                continue;
+            }
+            auto& depth = driverData->depths[tid];
+            depth.resize(pixelCount, 1.0f);
+            const auto* in = static_cast<const GfVec3f*>(bucket.data);
+            if (ids.empty()) {
+                for (auto i = decltype(pixelCount){0}; i < pixelCount; i += 1) {
+                    const auto p = driverData->projMtx.Transform(driverData->viewMtx.Transform(in[i]));
+                    depth[i] = std::max(-1.0f, std::min(1.0f, p[2]));
+                }
+            } else {
+                for (auto i = decltype(pixelCount){0}; i < pixelCount; i += 1) {
+                    if (ids[i] == -1) {
+                        depth[i] = 1.0f - AI_EPSILON;
+                    } else {
+                        const auto p = driverData->projMtx.Transform(driverData->viewMtx.Transform(in[i]));
+                        depth[i] = std::max(-1.0f, std::min(1.0f, p[2]));
+                    }
+                }
+            }
+            it->second->WriteBucket(bucket_xo, bucket_yo, bucket_size_x, bucket_size_y, HdFormatFloat32, depth.data());
+        } else if (bucket.name == HdAovTokens->color) {
+            const auto it = driverData->renderBuffers->find(bucket.name);
+            if (it == driverData->renderBuffers->end()) {
+                continue;
+            }
+            if (ids.empty()) {
+                it->second->WriteBucket(
+                    bucket_xo, bucket_yo, bucket_size_x, bucket_size_y, HdFormatFloat32Vec4, bucketData);
+            } else {
+                auto& color = driverData->colors[tid];
+                color.resize(pixelCount, AI_RGBA_ZERO);
+                const auto* in = static_cast<const AtRGBA*>(bucket.data);
+                for (auto i = decltype(pixelCount){0}; i < pixelCount; i += 1) {
+                    if (ids[i] == -1) {
+                        color[i] = AI_RGBA_ZERO;
+                    } else {
+                        color[i] = in[i];
+                    }
+                }
+                it->second->WriteBucket(
+                    bucket_xo, bucket_yo, bucket_size_x, bucket_size_y, HdFormatFloat32Vec4, color.data());
+            }
+        } else if (bucket.type == AI_TYPE_RGB) {
+            const auto it = driverData->renderBuffers->find(bucket.name);
+            if (it != driverData->renderBuffers->end()) {
+                it->second->WriteBucket(
+                    bucket_xo, bucket_yo, bucket_size_x, bucket_size_y, HdFormatFloat32Vec3, bucket.data);
             }
         }
-        bucketQueue.push(data);
     }
 }
 
 driver_write_bucket {}
 
 driver_close {}
+
+PXR_NAMESPACE_CLOSE_SCOPE