Merge pull request #5102 from danieldresser-ie/visitPixels

Prototype For Fast Bulk Sampling Of Pixels

SConstruct

@@ -476,6 +476,14 @@ if env["PLATFORM"] != "win32" :
 		# Turn off the parts of `-Wextra` that we don't like.
 		env.Append( CXXFLAGS = [ "-Wno-cast-function-type", "-Wno-unused-parameter" ] )
 
+		# Set this weird compiler flag that in general is expected to cause compiled code to be about
+		# half a percent slower, but works around this ridiculous bug:
+		# https://gcc.gnu.org/bugzilla//show_bug.cgi?id=51041
+		# It's a 10 year old bug that sometimes causes important inner loops to get 4X slower for no
+		# reason ( it affects use of visitPixels depending on exact register usage patterns at the call
+		# site ), and there appears to be no real solution ... maybe we should be moving away from GCC?
+		env.Append( CXXFLAGS = [ "-fira-region=all" ] )
+
 		env.Append(
 			CXXFLAGS = [ "-pthread" ],
 			SHLINKFLAGS = [ "-pthread", "-Wl,--no-undefined" ],
@@ -1099,7 +1107,7 @@ libraries = {
 			"LIBS" : [ "Gaffer", "GafferImage", "OpenImageIO$OIIO_LIB_SUFFIX" ],
 		},
 		"pythonEnvAppends" : {
-			"LIBS" : [ "GafferImage", "GafferImageTest" ],
+			"LIBS" : [ "GafferImage", "GafferImageTest", "fmt" ],
 		},
 		"additionalFiles" :
 			glob.glob( "python/GafferImageTest/scripts/*" ) + glob.glob( "python/GafferImageTest/images/*" ) +

contrib/scripts/graphPerformanceResults.py

@@ -0,0 +1,81 @@
+#! /usr/bin/env python
+
+import argparse
+import inspect
+import subprocess
+import os
+import json
+import matplotlib.pyplot as plt
+import matplotlib.ticker
+
+parser = argparse.ArgumentParser(
+	description = inspect.cleandoc(
+	"""
+	Graph the performance results for a folder of json files.
+	""" )
+)
+
+parser.add_argument(
+	'jsonFolder',
+	help='Folder containing json files'
+)
+
+args = parser.parse_args()
+
+jsonFolder = vars( args )["jsonFolder"]
+
+jsonFileNames = [ i for i in os.listdir( jsonFolder ) if i.endswith( ".json" ) ]
+
+results = []
+for n in jsonFileNames:
+	f = open( "%s/%s" % ( jsonFolder, n ) )
+	d = json.load( f )
+	f.close()
+
+	r = {}
+	for key, value in d.items():
+		if "timings" in value:
+			r[ key.split()[1].split('.')[-1][:-1] + "." + key.split()[0]] = ( min( value["timings"] ), max( value["timings"] ) )
+
+	resultName = n[:-5]
+	resultOrder = resultName
+
+	try:
+		resultOrder = int( subprocess.check_output( [ "git", "rev-list", "--count", resultName ] ) )
+		resultName = subprocess.check_output( [ "git", "log", "--format=%B", "-n 1", resultName ] ).splitlines()[0].decode( "UTF-8" )
+	except:
+		pass
+
+	results.append( [resultOrder, resultName, r] )
+
+results.sort()
+results = [r[1:] for r in results ]
+
+ax = plt.subplot(111)
+
+curveNames = results[0][1].keys()
+curveNames = sorted( curveNames, key = lambda k : -results[-1][1][k][0] )
+for k in curveNames:
+	ax.plot( [ j[1][k][0] for j in results ], label = k )
+	# Use this line instead if you want to see variability
+	#ax.fill_between( range( len( results ) ), [ j[1][k][0] for j in results ], [ j[1][k][1] for j in results ], label = k )
+
+plt.yscale( "log", base = 2 )
+
+# Force labelling of every power of two
+plt.yticks( [2**i for i in range( -4, 4 )] )
+
+ax.xaxis.set_major_formatter( matplotlib.ticker.FuncFormatter( lambda i, pos : results[int(i)][0] ) )
+plt.xticks( rotation = 45, ha = "right" )
+
+box = ax.get_position()
+
+# Create margin
+ax.set_position([box.x0, box.y0 + box.height * 0.2, box.width * 0.8, box.height * 0.8])
+
+# Put a legend to the right of the current axis
+ax.legend(loc='center left', bbox_to_anchor=(1, 0.5))
+
+# Currently hacking things up to work with very old matplotlib
+#plt.show_all()
+plt.show()

contrib/scripts/runTestsForCommits.py

@@ -0,0 +1,46 @@
+#! /usr/bin/env python
+
+import argparse
+import inspect
+import subprocess
+import os
+
+parser = argparse.ArgumentParser(
+	description = inspect.cleandoc(
+	"""
+	Run a selection of Gaffer tests for a specified set of commits,
+	outputting a separate json file for the tests for each commit.
+	""" )
+)
+
+parser.add_argument(
+	'--commits', action='store', nargs='+',
+	help='Hashes of commits to build'
+)
+
+parser.add_argument(
+	'--tests', action='store', nargs='+',
+	help='Names of tests to run'
+)
+
+parser.add_argument(
+	'--outputFolder', action='store', required = True,
+	help='Folder to put output json files to'
+)
+
+args = parser.parse_args()
+
+outputFolder = vars( args )["outputFolder"]
+try:
+	os.makedirs( outputFolder )
+except:
+	pass
+
+currentBranch = subprocess.check_output( [ "git", "stat", "-s", "-b" ] ).splitlines()[0].split()[1]
+print( currentBranch )
+for c in vars( args )["commits"]:
+	subprocess.check_call( [ "git", "checkout", c ] )
+	subprocess.check_call( [ "scons", "-j 16", "build" ] )
+	subprocess.check_call( [ "gaffer", "test" ] + vars( args )["tests"] + [ "-outputFile", "%s/%s.json" % ( outputFolder, c ) ] )
+
+subprocess.check_call( [ "git", "checkout", currentBranch ] )

include/GafferImage/ImagePlug.h

@@ -256,8 +256,8 @@ class GAFFERIMAGE_API ImagePlug : public Gaffer::ValuePlug
 		static const IECore::FloatVectorData *blackTile();
 		static const IECore::FloatVectorData *whiteTile();
 
-		static int tileSize() { return 1 << tileSizeLog2(); };
-		static int tilePixels() { return tileSize() * tileSize(); };
+		static constexpr int tileSize() { return 1 << tileSizeLog2(); };
+		static constexpr int tilePixels() { return tileSize() * tileSize(); };
 
 		/// Returns the index of the tile containing a point
 		/// This just means dividing by tile size ( always rounding down )
@@ -286,9 +286,9 @@ class GAFFERIMAGE_API ImagePlug : public Gaffer::ValuePlug
 		};
 		//@}
 
-	private :
+		static constexpr int tileSizeLog2() { return 7; };
 
-		static int tileSizeLog2() { return 7; };
+	private :
 
 		static void compoundObjectToCompoundData( const IECore::CompoundObject *object, IECore::CompoundData *data );
 

include/GafferImage/Sampler.h

@@ -94,6 +94,14 @@ class GAFFERIMAGE_API Sampler
 		/// 0.5, 0.5.
 		float sample( float x, float y );
 
+		/// Call a functor for all pixels in the region.
+		/// Much faster than calling sample(int,int) repeatedly for every pixel in the
+		/// region, up to 5 times faster in practical cases.
+		/// The signature of the functor must be `F( float value, int x, int y )`.
+		/// Each pixel is visited in order of increasing X, then increasing Y.
+		template<typename F>
+		inline void visitPixels( const Imath::Box2i &region, F &&lambda );
+
 		/// Appends a hash that represent all the pixel
 		/// values within the requested sample area.
 		void hash( IECore::MurmurHash &h ) const;
@@ -106,8 +114,8 @@ class GAFFERIMAGE_API Sampler
 		/// Cached data access
 		/// @param p Any point within the cache that we wish to retrieve the data for.
 		/// @param tileData Is set to the tile's channel data.
-		/// @param tilePixelIndex XY indices that can be used to access the colour value of point 'p' from tileData.
-		void cachedData( Imath::V2i p, const float *& tileData, Imath::V2i &tilePixelIndex );
+		/// @param tilePixelIndex Is set to the index used to access the colour value of point 'p' from tileData.
+		void cachedData( Imath::V2i p, const float *& tileData, int &tilePixelIndex );
 
 		const ImagePlug *m_plug;
 		const std::string m_channelName;
@@ -117,6 +125,7 @@ class GAFFERIMAGE_API Sampler
 		std::vector< IECore::ConstFloatVectorDataPtr > m_dataCache;
 		std::vector< const float * > m_dataCacheRaw;
 		Imath::Box2i m_cacheWindow;
+		int m_cacheOriginIndex;
 		int m_cacheWidth;
 
 		int m_boundingMode;