From 4c1438b15e59bf609b11f3b962423989a28bca25 Mon Sep 17 00:00:00 2001 From: ivanimanishi Date: Tue, 11 Jul 2023 10:32:02 -0700 Subject: [PATCH] FrameRange : Prevent creation of FrameRanges with negative steps Previously, you could create a FrameRange object with a negative step, but it would go into an infinite loop if you tried to get the frame list Given that we have the ReversedFrameList that achieves the same as a negative step, it felt like using that was the correct approach. Note that we still include the support for the negative step in the parse method, so that we can provide a more useful error message during init. --- Changes | 5 +++++ src/IECore/FrameRange.cpp | 8 ++++++++ test/IECore/FrameList.py | 19 ++++++++++++++++++- 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/Changes b/Changes index 415b73b1d9..8649fd791b 100644 --- a/Changes +++ b/Changes @@ -1,6 +1,11 @@ 10.4.x.x (relative to 10.4.10.0) ======== +Fixes +----- + +- FrameRange : Prevented creation of FrameRanges with negative steps + 10.4.10.0 (relative to 10.4.9.1) ======== diff --git a/src/IECore/FrameRange.cpp b/src/IECore/FrameRange.cpp index ef746faa76..8266abfaa2 100644 --- a/src/IECore/FrameRange.cpp +++ b/src/IECore/FrameRange.cpp @@ -58,6 +58,10 @@ FrameRange::FrameRange( Frame start, Frame end, Frame step ) : m_start( start ), { throw Exception( "FrameRange step cannot be zero" ); } + if ( step < 0 ) + { + throw Exception( "FrameRange step cannot be negative. Consider using the reverse suffix instead." ); + } } FrameRange::~FrameRange() @@ -102,6 +106,10 @@ void FrameRange::setStep( Frame step ) { throw Exception( "FrameRange step cannot be zero" ); } + if ( step < 0 ) + { + throw Exception( "FrameRange step cannot be negative. Consider using the reverse suffix instead." ); + } m_step = step; } diff --git a/test/IECore/FrameList.py b/test/IECore/FrameList.py index 8963a5f494..ed03fd4ae7 100644 --- a/test/IECore/FrameList.py +++ b/test/IECore/FrameList.py @@ -55,7 +55,24 @@ def testReverseConstruction( self ) : self.assertEqual( f.asList(), [ 5, 4, 3, 2, 1 ] ) self.assertEqual( IECore.frameListFromList( [ 5, 4, 3, 2, 1 ] ), f ) + def testFrameRange( self ) : + + f = IECore.FrameList.parse( "1-5" ) + self.assertTrue( isinstance( f, IECore.FrameRange ) ) + self.assertEqual( f.asList(), [ 1, 2, 3, 4, 5 ] ) + # test step + f = IECore.FrameList.parse( "10-20x5" ) + self.assertTrue( isinstance( f, IECore.FrameRange ) ) + self.assertEqual( f.asList(), [ 10, 15, 20 ] ) + # start must be smaller or equal to end + self.assertRaises( Exception, IECore.FrameList.parse, "5-1" ) + # step must be positive + self.assertRaises( Exception, IECore.FrameList.parse, "1-5x0" ) + self.assertRaises( Exception, IECore.FrameList.parse, "1-5x-1" ) + self.assertRaises( Exception, IECore.FrameList.parse, "5-1x-1" ) + + ## \todo: there should probably be a lot more tests in here... if __name__ == "__main__": - unittest.main() + unittest.main()