Simplify reverse iterators. There isn't a "past the beginning" iterator so this is hard to make a true bidirectional iterator. Instead restrict this to a single direction based on the m_bReverse parameter. Use the "past the end" iterator in place of the "past the beginning" iterator when in reverse.

stepmania/src/NoteData.cpp

@@ -11,7 +11,6 @@
 #include "XmlFile.h"
 #include "Foreach.h"
 #include "RageUtil_AutoPtr.h"
-#include <iterator>
 
 REGISTER_CLASS_TRAITS( NoteData, new NoteData(*pCopy) )
 
@@ -898,88 +897,136 @@ void NoteData::LoadFromNode( const XNode* pNode )
 	ASSERT(0);
 }
 
-NoteData::reverse_iterator NoteData::rlower_bound( int iTrack, int iRow )
+template<typename ND, typename iter, typename TN>
+void NoteData::_all_tracks_iterator<ND, iter, TN>::Find( bool bReverse )
 {
-	/* This is annoyingly tricky. First, find the upper bound for iRow. The map upper bound is the first
-	 * key that is greater than iRow. If no such key exists in the map, then every key is less than or
-	 * equal to iRow so we want the first. Note that if the map is empty, rbegin() will be the same as
-	 * rend(). If the first key in the map is is greater than iRow then all are so return rend(). If i
-	 * is neither begin() nor end(), then i - 1 (which isn't defined!) is last key in the map that is
-	 * less than or equal to iRow. */
-	iterator i = m_TapNotes[iTrack].upper_bound( iRow );
-	if( i == m_TapNotes[iTrack].end() )
-		return m_TapNotes[iTrack].rbegin();
-	if( i == m_TapNotes[iTrack].begin() )
-		return m_TapNotes[iTrack].rend();
-	return reverse_iterator( --i );
-}
-
-NoteData::const_reverse_iterator NoteData::rlower_bound( int iTrack, int iRow ) const
-{
-	return const_cast<NoteData*>(this)->rlower_bound( iTrack, iRow );
-}
-
-template<typename ND, typename iter, typename TN, bool bReverse, typename iterMethodInt, iterMethodInt mybegin, iterMethodInt myend, typename iterMethodIntInt, iterMethodIntInt mylower_bound>
-void NoteData::_all_tracks_iterator<ND, iter, TN, bReverse, iterMethodInt, mybegin, myend, iterMethodIntInt, mylower_bound>::Find()
-{
-	int iMinRow = m_iEndRow+1;
-	int iMaxRow = m_iStartRow-1;
-	
 	// If no notes can be found in the range, m_iTrack will stay -1 and IsAtEnd() will return true.
 	m_iTrack = -1;
-	for( int iTrack = 0; iTrack < m_NoteData.GetNumTracks(); ++iTrack )
+	if( bReverse )
 	{
-		iter &i = m_vIters[iTrack];
+		int iMaxRow = m_iStartRow - 1;
-		if( m_Cond )
+		for( int iTrack = m_NoteData.GetNumTracks() - 1; iTrack >= 0; --iTrack )
 		{
-			while( i != (m_NoteData.*myend)(iTrack) && (bReverse ? (i->first > iMaxRow) : (i->first < iMinRow)) && !m_Cond(i->second) )
+			iter &i( m_vIters[iTrack] );
-				++i;
+			if( m_Cond )
+			{
+				while( i != m_NoteData.end(iTrack) && i->first > iMaxRow && !m_Cond(i->second) )
+				{
+					if( i == m_NoteData.begin(iTrack) )
+						i = m_NoteData.end( iTrack );
+					else
+						--i;
+				}
+			}
+			if( i != m_NoteData.end(iTrack) && i->first > iMaxRow )
+			{
+				iMaxRow = i->first;
+				m_iTrack = iTrack;
+			}
 		}
-
+	}
-		if( i != (m_NoteData.*myend)(iTrack) && (bReverse ? (i->first > iMaxRow) : (i->first < iMinRow)) )
+	else
+	{
+		int iMinRow = m_iEndRow + 1;
+		for( int iTrack = 0; iTrack < m_NoteData.GetNumTracks(); ++iTrack )
 		{
-			(bReverse ? iMaxRow : iMinRow) = i->first;
+			iter &i = m_vIters[iTrack];
-			m_iTrack = iTrack;
+			if( m_Cond )
+			{
+				while( i != m_NoteData.end(iTrack) && i->first < iMinRow && !m_Cond(i->second) )
+					++i;
+			}
+			if( i != m_NoteData.end(iTrack) && i->first < iMinRow )
+			{
+				iMinRow = i->first;
+				m_iTrack = iTrack;
+			}
 		}
 	}
 }
 
-template<typename ND, typename iter, typename TN, bool bReverse, typename iterMethodInt, iterMethodInt mybegin, iterMethodInt myend, typename iterMethodIntInt, iterMethodIntInt mylower_bound>
+template<typename ND, typename iter, typename TN>
-NoteData::_all_tracks_iterator<ND, iter, TN, bReverse, iterMethodInt, mybegin, myend, iterMethodIntInt, mylower_bound>::_all_tracks_iterator( ND &nd, int iStartRow, int iEndRow, NoteData::IteratorCond cond ) :
+NoteData::_all_tracks_iterator<ND, iter, TN>::_all_tracks_iterator( ND &nd, int iStartRow, int iEndRow, bool bReverse, NoteData::IteratorCond cond ) :
-	m_NoteData(nd), m_iTrack(0), m_iStartRow(iStartRow), m_iEndRow(iEndRow), m_Cond(cond)
+	m_NoteData(nd), m_iTrack(0), m_iStartRow(iStartRow), m_iEndRow(iEndRow), m_bReverse(bReverse), m_Cond(cond)
 {
 	ASSERT( m_NoteData.GetNumTracks() > 0 );
-	for( int iTrack = 0; iTrack < m_NoteData.GetNumTracks(); ++iTrack )
+	if( bReverse )
 	{
-		iter i = (m_NoteData.*mylower_bound)(iTrack, bReverse?iEndRow:iStartRow);
+		for( int iTrack = 0; iTrack < m_NoteData.GetNumTracks(); ++iTrack )
-		iter end = (m_NoteData.*myend)(iTrack);
+		{
-		m_vIters.push_back( i );
+			iter i = m_NoteData.upper_bound( iTrack, iEndRow );
+			
+			if( i == m_NoteData.begin(iTrack) )
+				m_vIters.push_back( m_NoteData.end(iTrack) );
+			else
+				m_vIters.push_back( --i );
+		}
 	}
-	Find();
+	else
+	{
+		for( int iTrack = 0; iTrack < m_NoteData.GetNumTracks(); ++iTrack )
+			m_vIters.push_back( m_NoteData.lower_bound(iTrack, iStartRow) );
+	}
+	Find( bReverse );
 }
 
-template<typename ND, typename iter, typename TN, bool bReverse, typename iterMethodInt, iterMethodInt mybegin, iterMethodInt myend, typename iterMethodIntInt, iterMethodIntInt mylower_bound>
+template<typename ND, typename iter, typename TN>
-NoteData::_all_tracks_iterator<ND, iter, TN, bReverse, iterMethodInt, mybegin, myend, iterMethodIntInt, mylower_bound> &NoteData::_all_tracks_iterator<ND, iter, TN, bReverse, iterMethodInt, mybegin, myend, iterMethodIntInt, mylower_bound>::operator++() // preincrement
+NoteData::_all_tracks_iterator<ND, iter, TN> &NoteData::_all_tracks_iterator<ND, iter, TN>::operator++() // preincrement
 {
-	++m_vIters[m_iTrack];
+	if( m_bReverse )
-	Find();
+	{
+		if( m_vIters[m_iTrack] == m_NoteData.begin(m_iTrack) )
+			m_vIters[m_iTrack] = m_NoteData.end( m_iTrack );
+		else
+			--m_vIters[m_iTrack];
+	}
+	else
+	{
+		++m_vIters[m_iTrack];
+	}
+	Find( m_bReverse );
 	return *this;
 }
 
-template<typename ND, typename iter, typename TN, bool bReverse, typename iterMethodInt, iterMethodInt mybegin, iterMethodInt myend, typename iterMethodIntInt, iterMethodIntInt mylower_bound>
+template<typename ND, typename iter, typename TN>
-NoteData::_all_tracks_iterator<ND, iter, TN, bReverse, iterMethodInt, mybegin, myend, iterMethodIntInt, mylower_bound> NoteData::_all_tracks_iterator<ND, iter, TN, bReverse, iterMethodInt, mybegin, myend, iterMethodIntInt, mylower_bound>::operator++( int dummy ) // postincrement
+NoteData::_all_tracks_iterator<ND, iter, TN> NoteData::_all_tracks_iterator<ND, iter, TN>::operator++( int dummy ) // postincrement
 {
-	_all_tracks_iterator<ND, iter, TN, bReverse, iterMethodInt, mybegin, myend, iterMethodIntInt, mylower_bound> ret(*this);
+	_all_tracks_iterator<ND, iter, TN> ret( *this );
 	operator++();
 	return ret;
 }
+/* XXX: This doesn't satisfy the requirements that ++iter; --iter; is a no-op so it cannot be bidirectional for now. */
+#if 0
+template<typename ND, typename iter, typename TN>
+NoteData::_all_tracks_iterator<ND, iter, TN> &NoteData::_all_tracks_iterator<ND, iter, TN>::operator--() // predecrement
+{
+	if( m_bReverse )
+	{
+		++m_vIters[m_iTrack];
+	}
+	else
+	{
+		if( m_vIters[m_iTrack] == m_NoteData.begin(m_iTrack) )
+			m_vIters[m_iTrack] = m_NoteData.end( m_iTrack );
+		else
+			--m_vIters[m_iTrack];
+	}
+	Find( !m_bReverse );
+	return *this;
+}
 
+template<typename ND, typename iter, typename TN>
+NoteData::_all_tracks_iterator<ND, iter, TN> NoteData::_all_tracks_iterator<ND, iter, TN>::operator--( int dummy ) // postdecrement
+{
+	_all_tracks_iterator<ND, iter, TN> ret( *this );
+	operator--();
+	return ret;
+}
+#endif
 
 // Explicit instantiation.
-template class NoteData::_all_tracks_iterator<NoteData, NoteData::iterator, TapNote, false, NoteData::iterator_method_int, &NoteData::begin, &NoteData::end, NoteData::iterator_method_int_int, &NoteData::lower_bound>;
+template class NoteData::_all_tracks_iterator<NoteData, NoteData::iterator, TapNote>;
-template class NoteData::_all_tracks_iterator<const NoteData, NoteData::const_iterator, const TapNote, false, NoteData::const_iterator_method_int, &NoteData::begin, &NoteData::end, NoteData::const_iterator_method_int_int, &NoteData::lower_bound>;
+template class NoteData::_all_tracks_iterator<const NoteData, NoteData::const_iterator, const TapNote>;
-template class NoteData::_all_tracks_iterator<NoteData, NoteData::reverse_iterator, TapNote, true, NoteData::reverse_iterator_method_int, &NoteData::rbegin, &NoteData::rend, NoteData::reverse_iterator_method_int_int, &NoteData::rlower_bound>;
-template class NoteData::_all_tracks_iterator<const NoteData, NoteData::const_reverse_iterator, const TapNote, true, NoteData::const_reverse_iterator_method_int, &NoteData::rbegin, &NoteData::rend, NoteData::const_reverse_iterator_method_int_int, &NoteData::rlower_bound>;
 
 /*
  * (c) 2001-2004 Chris Danford, Glenn Maynard

stepmania/src/NoteData.h

@@ -6,6 +6,7 @@
 #include "NoteTypes.h"
 #include <map>
 #include <set>
+#include <iterator>
 
 #define FOREACH_NONEMPTY_ROW_IN_TRACK( nd, track, row ) \
 	for( int row = -1; (nd).GetNextTapNoteRowForTrack(track,row); )
@@ -46,16 +47,14 @@ public:
 	const_reverse_iterator rend( int iTrack ) const			{ return m_TapNotes[iTrack].rend(); }
 	iterator lower_bound( int iTrack, int iRow )			{ return m_TapNotes[iTrack].lower_bound( iRow ); }
 	const_iterator lower_bound( int iTrack, int iRow ) const	{ return m_TapNotes[iTrack].lower_bound( iRow ); }
-	reverse_iterator rlower_bound( int iTrack, int iRow );
+	iterator upper_bound( int iTrack, int iRow )			{ return m_TapNotes[iTrack].upper_bound( iRow ); }
-	const_reverse_iterator rlower_bound( int iTrack, int iRow ) const;
+	const_iterator upper_bound( int iTrack, int iRow ) const	{ return m_TapNotes[iTrack].upper_bound( iRow ); }
 	void swap( NoteData &nd )					{ m_TapNotes.swap( nd.m_TapNotes ); }
 
 
-
-
 	typedef bool (*IteratorCond)( const TapNote& );
 	// This is ugly to make it templated but I don't want to have to write the same class twice.
-	template<typename ND, typename iter, typename TN, bool bReverse, typename iterMethodInt, iterMethodInt mybegin, iterMethodInt myend, typename iterMethodIntInt, iterMethodIntInt mylower_bound>
+	template<typename ND, typename iter, typename TN>
 	class _all_tracks_iterator
 	{
 		ND		&m_NoteData;
@@ -63,13 +62,16 @@ public:
 		int		m_iTrack;
 		const int	m_iStartRow;
 		const int	m_iEndRow;
+		const bool	m_bReverse;
 		IteratorCond	m_Cond;
 		
-		void Find();	// point m_iTrack at the iterator with the smallest row that meets mCond and is within range
+		void Find( bool bReverse );
-public:
+	public:
-		_all_tracks_iterator( ND &nd, int iStartRow, int iEndRow, IteratorCond cond );
+		_all_tracks_iterator( ND &nd, int iStartRow, int iEndRow, bool bReverse, IteratorCond cond );
 		_all_tracks_iterator &operator++();		// preincrement
 		_all_tracks_iterator operator++( int dummy );	// postincrement
+		//_all_tracks_iterator &operator--();		// predecrement
+		//_all_tracks_iterator operator--( int dummy );	// postdecrement
 		inline int Track() const		{ return m_iTrack; }
 		inline int Row() const			{ return m_vIters[m_iTrack]->first; }
 		inline bool IsAtEnd() const		{ return m_iTrack == -1; }
@@ -78,11 +80,10 @@ public:
 		inline const TN &operator*() const	{ DEBUG_ASSERT( !IsAtEnd() ); return m_vIters[m_iTrack]->second; }
 		inline const TN *operator->() const	{ DEBUG_ASSERT( !IsAtEnd() ); return &m_vIters[m_iTrack]->second; }
 	};
-	typedef _all_tracks_iterator<NoteData, NoteData::iterator, TapNote, false, NoteData::iterator_method_int, &NoteData::begin, &NoteData::end, NoteData::iterator_method_int_int, &NoteData::lower_bound> 								all_tracks_iterator;
+	typedef _all_tracks_iterator<NoteData, NoteData::iterator, TapNote> 			all_tracks_iterator;
-	typedef _all_tracks_iterator<const NoteData, NoteData::const_iterator, const TapNote, false, NoteData::const_iterator_method_int, &NoteData::begin, &NoteData::end, NoteData::const_iterator_method_int_int, &NoteData::lower_bound>				all_tracks_const_iterator;
+	typedef _all_tracks_iterator<const NoteData, NoteData::const_iterator, const TapNote>	all_tracks_const_iterator;
-	typedef _all_tracks_iterator<NoteData, NoteData::reverse_iterator, TapNote, true, NoteData::reverse_iterator_method_int, &NoteData::rbegin, &NoteData::rend, NoteData::reverse_iterator_method_int_int, &NoteData::rlower_bound> 					all_tracks_reverse_iterator;
+	typedef all_tracks_iterator								all_tracks_reverse_iterator;
-	typedef _all_tracks_iterator<const NoteData, NoteData::const_reverse_iterator, const TapNote, true, NoteData::const_reverse_iterator_method_int, &NoteData::rbegin, &NoteData::rend, NoteData::const_reverse_iterator_method_int_int, &NoteData::rlower_bound>	all_tracks_const_reverse_iterator;
+	typedef all_tracks_const_iterator							all_tracks_const_reverse_iterator;
-
 private:
 	// There's no point in inserting empty notes into the map.
 	// Any blank space in the map is defined to be empty.
@@ -119,22 +120,21 @@ public:
 	void GetTapNoteRange( int iTrack, int iStartRow, int iEndRow, TrackMap::iterator &begin, TrackMap::iterator &end );
 	all_tracks_iterator GetTapNoteRangeAllTracks( int iStartRow, int iEndRow, IteratorCond cond = NULL )
 	{
-		return all_tracks_iterator( *this, iStartRow, iEndRow, cond );
+		return all_tracks_iterator( *this, iStartRow, iEndRow, false, cond );
 	}
 	all_tracks_const_iterator GetTapNoteRangeAllTracks( int iStartRow, int iEndRow, IteratorCond cond = NULL) const
 	{
-		return all_tracks_const_iterator( *this, iStartRow, iEndRow, cond );
+		return all_tracks_const_iterator( *this, iStartRow, iEndRow, false, cond );
 	}
 	all_tracks_reverse_iterator GetTapNoteRangeAllTracksReverse( int iStartRow, int iEndRow, IteratorCond cond = NULL )
 	{
-		return all_tracks_reverse_iterator( *this, iStartRow, iEndRow, cond );
+		return all_tracks_iterator(*this, iStartRow, iEndRow, true, cond);
 	}
 	all_tracks_const_reverse_iterator GetTapNoteRangeAllTracksReverse( int iStartRow, int iEndRow, IteratorCond cond = NULL) const
 	{
-		return all_tracks_const_reverse_iterator( *this, iStartRow, iEndRow, cond );
+		return all_tracks_const_iterator(*this, iStartRow, iEndRow, true, cond);
 	}
 
-
 	/* Return an iterator range include iStartRow to iEndRow.  Extend the range to include
 	 * hold notes overlapping the boundary. */
 	void GetTapNoteRangeInclusive( int iTrack, int iStartRow, int iEndRow, const_iterator &begin, const_iterator &end, bool bIncludeAdjacent=false ) const;