Hi Erik, I've just done a review of the changes. I don't feel the naming used is particularly intuitive - I don't understand the use of word parity in this context, so would like to discuss what might be more apporpriate - your suggest for merging control of swapping left/right eye might well be the solution to this, although we need to be midful of backwards compatibility. Perhaps a swap left/right eye flag would be sufficient, this needn't replace the existing codes such as the ones for horizontal/vertical split mapping.
The inclusion of formatting changes makes it very hard to see what is an actual functional change and what is done for purely cosmetic reasons. For future submissions could you keep the formating changes done as completely separate submission. I am also very wary of cosmetic changes as sometimes errors in the reformatting can introduce bugs, as every single line of code we change has the potential for introducing bugs, even if the risk is low per line, it's still there and the more lines you change the more likely that a bug will arise. Robert. On Fri, Apr 17, 2009 at 10:31 PM, Erik den Dekker <[email protected]> wrote: > Hi Robert, > > > > While using the HORIZONTAL_INTERLACE stereo mode I ran into a small problem, > which also effects the VERTICAL_INTERLACE and CHECKERBOARD stereo modes. I > discovered that the interlace pattern is fixed with respect to the first > line of the viewing window. For example, on my stereo monitor, the first > horizontal line is polarized in such a way that with standard stereo > polarized glasses it shows on the left eye; but it can easily be the other > way around for other monitors (The osg default was exactly the wrong way > around for me). The solution is to provide an option to switch left/right – > or change parity (odd/even) as I named it. (Note: The ‘hardware’ solution > for this would be to put on your polarized glasses upside-down, but the > ‘software’ solution is much more comfortable J) > > > > The above argument also goes for vertical interlace and checkerboard > interlace stereo, and the provided code fixes these paths too. > > > > Notice that for HORIZONTAL_SPLIT and VERTICAL_SPLIT stereo there are already > similar methods: SplitStereoHorizontalEyeMapping, > SplitStereoVerticalEyeMapping and related functions provide this. Actually > these methods are so similar that we may consider merging them into a single > function that can swap left/right eye and get a cleaner API. > > > > Last note: I made additional changes to code layout / whitespace for general > clarity, which I think is useful, but may clutter your diff. Functional > changes only are small and available on request. > > > > Changes made against SVN HEAD (revision 10045) > > > > Cheers, > > > > Erik den Dekker > > > > _______________________________________________ > 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
