Skip to content

Commit

Permalink
ImageStats : Fix handling of min/max outside data window
Browse files Browse the repository at this point in the history
  • Loading branch information
danieldresser-ie authored and johnhaddon committed Jan 4, 2024
1 parent f5bf50c commit 9062a8c
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 1 deletion.
4 changes: 3 additions & 1 deletion Changes.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@ Fixes
- Stopped failed jobs jumping to the end of the Local Jobs UI.
- Fixed message log update.
- Fixed `Job.statistics()` errors on Windows, ensuring that a `pid` is always returned when available.
- ImageStats : Fixed output of infinite values, which were previously being clamped.
- ImageStats :
- Fixed output of infinite values, which were previously being clamped.
- Results for min/max now correctly reflect zero values outside the data window.
- NodeMenu, NodeEditor : `userDefault` metadata is now evaluated in the script context, so it can depend on script variables.

API
Expand Down
34 changes: 34 additions & 0 deletions python/GafferImageTest/ImageStatsTest.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,40 @@ def testROI( self ) :
self.__assertColour( s["min"].getValue(), imath.Color4f( 0.25, 0, 0, 0.5 ) )
self.__assertColour( s["max"].getValue(), imath.Color4f( 0.5, 0.5, 0, 0.75 ) )

# Offset the colors in the image so we can see the effects of whether or not we include pixels outside
# the data window.

g = GafferImage.Grade()
g["in"].setInput( r["out"] )
g['channels'].setValue( "*" )
g['blackClamp'].setValue( False )
g['whiteClamp'].setValue( False )
g['offset'].setValue( imath.Color4f( 1 ) )

s["in"].setInput( g["out"] )

s["area"].setValue( imath.Box2i( imath.V2i( 20, 20 ), imath.V2i( 25, 25 ) ) )
self.__assertColour( s["average"].getValue(), imath.Color4f( 1.5, 1, 1, 1.5 ) )
self.__assertColour( s["max"].getValue(), imath.Color4f( 1.5, 1, 1, 1.5 ) )
self.__assertColour( s["min"].getValue(), imath.Color4f( 1.5, 1, 1, 1.5 ) )

s["area"].setValue( imath.Box2i( imath.V2i( 19, 20 ), imath.V2i( 24, 25 ) ) )
self.__assertColour( s["average"].getValue(), imath.Color4f( 1.2, 0.8, 0.8, 1.2 ) )
self.__assertColour( s["max"].getValue(), imath.Color4f( 1.5, 1, 1, 1.5 ) )
self.__assertColour( s["min"].getValue(), imath.Color4f( 0, 0, 0, 0 ) )

g['offset'].setValue( imath.Color4f( -2 ) )

s["area"].setValue( imath.Box2i( imath.V2i( 20, 20 ), imath.V2i( 25, 25 ) ) )
self.__assertColour( s["average"].getValue(), imath.Color4f( -1.5, -2, -2, -1.5 ) )
self.__assertColour( s["max"].getValue(), imath.Color4f( -1.5, -2, -2, -1.5 ) )
self.__assertColour( s["min"].getValue(), imath.Color4f( -1.5, -2, -2, -1.5 ) )

s["area"].setValue( imath.Box2i( imath.V2i( 19, 20 ), imath.V2i( 24, 25 ) ) )
self.__assertColour( s["average"].getValue(), imath.Color4f( -1.2, -1.6, -1.6, -1.2 ) )
self.__assertColour( s["max"].getValue(), imath.Color4f( 0, 0, 0, 0 ) )
self.__assertColour( s["min"].getValue(), imath.Color4f( -1.5, -2, -2, -1.5 ) )

# Test only tiles which intersect a changed boundary have modified hashes
def testROIHash( self ) :

Expand Down
12 changes: 12 additions & 0 deletions src/GafferImage/ImageStats.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,7 @@ void ImageStats::hash( const ValuePlug *output, const Context *context, IECore::
}

Imath::Box2i boundsIntersection;
bool beyondDataWindow;
double areaMult;

{
Expand All @@ -363,6 +364,7 @@ void ImageStats::hash( const ValuePlug *output, const Context *context, IECore::
}
const Imath::Box2i dataWindow = flattenedInPlug()->dataWindowPlug()->getValue();
boundsIntersection = BufferAlgo::intersection( area, dataWindow );
beyondDataWindow = boundsIntersection != area;
areaMult = double(area.size().x) * area.size().y;
}

Expand All @@ -384,6 +386,8 @@ void ImageStats::hash( const ValuePlug *output, const Context *context, IECore::
return;
}

h.append( beyondDataWindow );

// We traverse in TopToBottom order because otherwise the hash could change just based on
// the order in which hashes are combined
ImageAlgo::parallelGatherTiles(
Expand Down Expand Up @@ -442,6 +446,7 @@ void ImageStats::compute( ValuePlug *output, const Context *context ) const
}

Imath::Box2i boundsIntersection;
bool beyondDataWindow;
double areaMult;

{
Expand All @@ -468,6 +473,7 @@ void ImageStats::compute( ValuePlug *output, const Context *context ) const
}
const Imath::Box2i dataWindow = flattenedInPlug()->dataWindowPlug()->getValue();
boundsIntersection = BufferAlgo::intersection( area, dataWindow );
beyondDataWindow = boundsIntersection != area;
areaMult = double(area.size().x) * area.size().y;
}

Expand Down Expand Up @@ -510,6 +516,12 @@ void ImageStats::compute( ValuePlug *output, const Context *context ) const
float max = -std::numeric_limits<float>::infinity();
double sum = 0.;

if( beyondDataWindow )
{
min = 0.;
max = 0.;
}

// We traverse in TopToBottom order because floating point precision means that changing
// the order to sum in could produce slightly non-deterministic results
ImageAlgo::parallelGatherTiles(
Expand Down

0 comments on commit 9062a8c

Please sign in to comment.