Hi Robert,
I agree, the implementation is effectively duplicating a lot of code.
Maybe we can try to refactor it after the TEXTURE_BINDING - fix.
I can live with the current implementation without the subImage reading.
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
_______________________________________________
osg-submissions mailing list
[email protected]
http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org
