HI Colin, Thanks for the review, and agree on you prognosis of what's wrong with readArrayImplementation. I'll have a think about the issue and if I don't get to it first would welcome changes from yourself.
Robert. On 25 February 2012 11:16, Colin McDonald <[email protected]> wrote: > Hi Robert, > > Unfortunately I won't be in a position to build or test your changes > until next Tuesday or Wednesday. > > But from a quick code review I don't think it will work. The problem > is that there is no mechanism in InputStream::readArrayImplementation > for determining what exactly needs to be byte-swapped. > > So for example for an Vec3Array, the call to readArrayImplementation > doesn't specify the useByteSwap flag. It needs to byte swap, but not > over the whole 12 byte array element, rather over 3 x 4 byte float > values. There is currently no way of doing that in > InputStream::readArrayImplementation. > > That's why in my original submission I had removed any attempt to do > byte swapping in InputStream, and just left it to the > BinaryInputIterator. A compromise solution could be to handle all > byte swapping in BinaryInputIterator, but still allow > InputStream::readArrayImplementation to read a whole array in a single > I/O for non-byte swapped input. Something like: > > > template<typename T> > void InputStream::readArrayImplementation( T* a, int read_size, bool > ObsoleteFlag ) > { > int size = 0; > *this >> size >> BEGIN_BRACKET; > if ( size ) > { > a->resize( size ); > if ( isBinary() && !_byteSwap ) > { > readCharArray( (char*)&((*a)[0]), read_size*size ); checkStream(); > } > else > { > for ( int i=0; i<size; ++i ) > *this >> (*a)[i]; > } > } > *this >> END_BRACKET; > } > > > If you like I could code this up & properly test it next week, and resubmit. > > Regards > > Colin McDonald > > On Fri, Feb 24, 2012 at 9:10 PM, Robert Osfield > <[email protected]> wrote: >> Hi Colin and Rui, >> >> I've made some more changes in attempt to keep InputStream and >> InputStreamOperator's handling of the endian byte swap consistent, the >> changes are detailed below. I have also merged Colin's suggested >> changes the the write/readLong(..) and write/readULong(..) methods >> These changes are now checked into svn/trunk. >> >> Could you review these changes and test them out and let me know if >> they are sufficient or whether there is still some further work to do. >> >> Thanks, >> Robert. > _______________________________________________ > osg-submissions mailing list > [email protected] > http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org _______________________________________________ osg-submissions mailing list [email protected] http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org
