Testing; better tests of environment mapping, minor fixes (#3694)

Would you believe we never had a testsuite test for environment
mapping?  Well, it wasn't totally untested, in the sense that OSL
testsuite had one, and obviously it's used in production all the
time. But OIIO's testsuite didn't have any tests of
TextureSystem::environmemt(). We now remedy that.

* Add testsuite/texture-env, contains several environment tests,
  including handles vs filenames, subimage selection, different interp
  and mip modes, ask for >4 channels.

* New CMake build option OIIO_TEX_IMPLEMENT_VARYINGREF can be used for
  testing to disable implementation of the old 1.x style of batch
  texture calls.  We don't use them, they are deprecated. This option
  give us a way to exclude them from code analysis, because we really
  don't care.

* Fix bug with >4 channels for environment batch shading!

* Avoid unnecessary spec lookup in TileID constructor.

* Avoid building testtex test_hash during code coverage -- it is an
  extremely specialized test, not run in the testsuite, it shouldn't
  really count againt good code coverage.

Author: lgritz

.github/workflows/analysis.yml

@@ -61,6 +61,7 @@ jobs:
                             CMAKE_UNITY_BUILD=OFF
                             CODECOV=1
                             CTEST_TEST_TIMEOUT=1200
+                            OIIO_CMAKE_FLAGS="-DOIIO_TEX_IMPLEMENT_VARYINGREF=OFF"
 
     runs-on: ${{ matrix.os }}
     container:

CMakeLists.txt

@@ -122,6 +122,10 @@ set (TEX_BATCH_SIZE "" CACHE STRING "Force TextureSystem SIMD batch size (e.g. 1
 if (TEX_BATCH_SIZE)
     add_definitions ("-DOIIO_TEXTURE_SIMD_BATCH_WIDTH=${TEX_BATCH_SIZE}")
 endif ()
+option (OIIO_TEX_IMPLEMENT_VARYINGREF "Implement the deprecated batch texture functions taking VaryingRef params" ON)
+if (NOT OIIO_TEX_IMPLEMENT_VARYINGREF)
+    add_definitions (-DOIIO_TEX_NO_IMPLEMENT_VARYINGREF=1)
+endif ()
 
 # Set the default namespace
 set (${PROJ_NAME}_NAMESPACE ${PROJECT_NAME} CACHE STRING

src/cmake/testing.cmake

@@ -182,6 +182,7 @@ macro (oiio_add_all_tests)
                     texture-wrapfill
                     texture-fat texture-skinny
                     texture-stats
+                    texture-env
                    )
     oiio_add_tests (${all_texture_tests})
     # Duplicate texture tests with batch mode

src/libtexture/environment.cpp

@@ -216,11 +216,15 @@ TextureSystemImpl::environment(ustring filename, TextureOptions& options,
                                float* result, float* dresultds,
                                float* dresultdt)
 {
+#ifdef OIIO_TEX_NO_IMPLEMENT_VARYINGREF
+    return false;
+#else
     Perthread* thread_info        = get_perthread_info();
     TextureHandle* texture_handle = get_texture_handle(filename, thread_info);
     return environment(texture_handle, thread_info, options, runflags,
                        beginactive, endactive, R, dRdx, dRdy, nchannels, result,
                        dresultds, dresultdt);
+#endif
 }
 
 
@@ -235,6 +239,9 @@ TextureSystemImpl::environment(TextureHandle* texture_handle,
                                float* result, float* dresultds,
                                float* dresultdt)
 {
+#ifdef OIIO_TEX_NO_IMPLEMENT_VARYINGREF
+    return false;
+#else
     if (!texture_handle)
         return false;
     bool ok = true;
@@ -256,6 +263,7 @@ TextureSystemImpl::environment(TextureHandle* texture_handle,
         }
     }
     return ok;
+#endif
 }
 
 
@@ -597,8 +605,10 @@ TextureSystemImpl::environment(TextureHandle* texture_handle,
 
     bool ok          = true;
     Tex::RunMask bit = 1;
+    float* r         = OIIO_ALLOCA(float, 3 * nchannels * Tex::BatchWidth);
+    float* drds      = r + nchannels * Tex::BatchWidth;
+    float* drdt      = r + 2 * nchannels * Tex::BatchWidth;
     for (int i = 0; i < Tex::BatchWidth; ++i, bit <<= 1) {
-        float r[4], drds[4], drdt[4];  // temp result
         if (mask & bit) {
             opt.sblur  = options.sblur[i];
             opt.tblur  = options.tblur[i];

src/libtexture/imagecache_pvt.h

@@ -489,7 +489,7 @@ struct TileID {
     /// Initialize a TileID based on full elaboration of image file,
     /// subimage, and tile x,y,z indices.
     TileID(ImageCacheFile& file, int subimage, int miplevel, int x, int y,
-           int z = 0, int chbegin = 0, int chend = -1)
+           int z, int chbegin, int chend)
         : m_x(x)
         , m_y(y)
         , m_z(z)
@@ -499,9 +499,10 @@ struct TileID {
         , m_chend(chend)
         , m_file(&file)
     {
-        int nc = file.spec(subimage, miplevel).nchannels;
-        if (chend < chbegin || chend > nc)
+        if (chend < chbegin) {
+            int nc  = file.spec(subimage, miplevel).nchannels;
             m_chend = nc;
+        }
     }
 
     /// Destructor is trivial, because we don't hold any resources

src/libtexture/texture3d.cpp

@@ -197,11 +197,15 @@ TextureSystemImpl::texture3d(ustring filename, TextureOptions& options,
                              float* result, float* dresultds, float* dresultdt,
                              float* dresultdr)
 {
+#ifdef OIIO_TEX_NO_IMPLEMENT_VARYINGREF
+    return false;
+#else
     Perthread* thread_info        = get_perthread_info();
     TextureHandle* texture_handle = get_texture_handle(filename, thread_info);
     return texture3d(texture_handle, thread_info, options, runflags,
                      beginactive, endactive, P, dPdx, dPdy, dPdz, nchannels,
                      result, dresultds, dresultdt, dresultdr);
+#endif
 }
 
 
@@ -214,6 +218,9 @@ TextureSystemImpl::texture3d(
     VaryingRef<Imath::V3f> dPdy, VaryingRef<Imath::V3f> dPdz, int nchannels,
     float* result, float* dresultds, float* dresultdt, float* dresultdr)
 {
+#ifdef OIIO_TEX_NO_IMPLEMENT_VARYINGREF
+    return false;
+#else
     bool ok = true;
     result += beginactive * nchannels;
     if (dresultds) {
@@ -235,6 +242,7 @@ TextureSystemImpl::texture3d(
         }
     }
     return ok;
+#endif
 }
 
 

src/libtexture/texturesys.cpp

@@ -1024,11 +1024,15 @@ TextureSystemImpl::texture(ustring filename, TextureOptions& options,
                            int nchannels, float* result, float* dresultds,
                            float* dresultdt)
 {
+#ifdef OIIO_TEX_NO_IMPLEMENT_VARYINGREF
+    return false;
+#else
     Perthread* thread_info        = get_perthread_info();
     TextureHandle* texture_handle = get_texture_handle(filename, thread_info);
     return texture(texture_handle, thread_info, options, runflags, beginactive,
                    endactive, s, t, dsdx, dtdx, dsdy, dtdy, nchannels, result,
                    dresultds, dresultdt);
+#endif
 }
 
 
@@ -1043,6 +1047,9 @@ TextureSystemImpl::texture(TextureHandle* texture_handle,
                            int nchannels, float* result, float* dresultds,
                            float* dresultdt)
 {
+#ifdef OIIO_TEX_NO_IMPLEMENT_VARYINGREF
+    return false;
+#else
     if (!texture_handle)
         return false;
     bool ok = true;
@@ -1065,6 +1072,7 @@ TextureSystemImpl::texture(TextureHandle* texture_handle,
         }
     }
     return ok;
+#endif
 }