Hi Sebastian, I have finally got on to do a review of your Image::readSubImageFromCurrentTexture() addition and feel there is a big code overlap between it and the original method which suggests to me there is an opportunity to refactor both to create common code parts, such as a help method to do the bulk of the work.
There is also an open issue that a recent submission "Bugfix for osg::Image::readImageFromCurrentTexture" highlights - that is in certain circumstances using GL_TEXTURE_BINDING_* doesn't work for select which texture target to use, this submission I've rejected as the solution breaks how osgconv uses readImageFromCurrentTexture() but the issue is not resolved, doing nothing is not a solution, just as much as merging this particular submission will break existing codes. I don't know yet what the appropriate solution to this problem is, but as it directly effects the original readImageFromCurrentTexture() implementation it will also effect your proposed readSubImageFromCurrentTexture() as it relies upon the same approach. Putting these two things together, I believe we'll need to refactor the code so that common functionality is placed into a helper method, and where the texture target is currently implicitly defined we'll likely need to come up with a way of explicitly defining the texture target as well as providing the implicit target selection for backwards compatibility. Does this make sense? I have dozens of other pending submissions to tackle so I'll not dive further into resolving this all right now so I'll tackle these and if others like yourself can spot a good way to resolve all this it'd be very welcome ;-) Cheers, Robert. Robert. On 2 November 2012 19:31, Sebastian Messerschmidt <[email protected]> wrote: > Hello, > > attached you'll find an additional readSubImageFromCurrentTexture member > function for the Image class. > I needed it to get a specific mipmap level from a bound texture. > Unfortunately there are various checks missing, so I'm not sure if it is > okay the way it is. > Maybe someone more familiar to the code could improve it. > Attached the Image header and cpp file against the current trunk. > > > cheers > Sebastian > > _______________________________________________ > 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
