Re: [osg-users] Deprecating vertex indices in osg::Geometry
Hi All, I have just checked in some further refinement to how the deprecated functionality is handled. Rather than providing a #ifdef OSG_USE_DEPRECATED_METHODS to switch on/off the compile of these deprecated methods they have been moved entirely out of osg::Geometry, and placed into a new namespace specifically to help out with compiling old code that uses the deprecated features. The new namespace is deprecated_osg, with deprecated_osg::Geometry now defined within it, this class implements the AttributeBinding_BIND_PER_PRIMITIVE and set*Indices() methods. deprecated_osg::Geometry subclasses from osg::Geometry so gets all the non deprecated functionality as well. This change will mean that code that uses Geometry::set*Indicies() or Geometry::set*Binding(osg::Geometry::BIND_PER_PRIMITIVE) will now fail to compile with a warning that these aren't available, to get these to compile you'll need to replace osg::Geometry with deprecated_osg::Geometry and everything should work. It will still be neccessary to run the Geometry::fixDeprecatedData() on these deprecated geometries. This method is called automatically by Geode::addDrawable() so often you won't need to call it. Robert. ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] Deprecating vertex indices in osg::Geometry
Hi All,
I have just finished a rather intensive bout of work on the new
cleaned up osg::Geometry, one of the most challenging parts was
providing a fallback for the deprecated array indices and
BIND_PER_PRIMITIVE usage. The new osg::Geometry doesn't support
rendering of geometries with indices and BIND_PER_PRIMITIVE so one has
to convert geometries containing these over to OpenGL fast path
compliant forms using just straight vertex arrays and standard
glDrawArray/glDrawElements based primitives, and to ease this task I
have written a could of new methods to osg::Geometry:
/** Return true if the deprecated use array indicies or
BIND_PER_PRIMITIVE binding has been assigned to arrays.*/
bool containsDeprecatedData() const { return _containsDeprecatedData; }
/** fallback for deprecated functionality. Return true if the
Geometry contains any array indices or BIND_PER_PRIMITIVE arrays. */
bool checkForDeprecatedData();
/** fallback for deprecated functionality. Removes any array
indices and BIND_PER_PRIMITIVE arrays.*/
void fixDeprecatedData();
I have add a check in osg::Geode::addDrawable() to automatically calls
geometry-fixDeprecatedData() when it finds a
geometry-containsDeprecatedData() returns true. This will catch most
cases where applications are still using the deprecated functionality.
The old methods for setting the indices are still in place but guarded
by a #if defined(OSG_USE_DEPRECATED_GEOMETRY_METHODS) and currently
CMake in conjucntion with include/osg/Config sets this by default, but
you can disable this via cmake. For now the OSG itself won't build if
you do this, but once we are a bit further down the road we can make
sure that the rest of the OSG will compile with the
OSG_USE_DEPRECATED_GEOMETRY_METHODS being defined. For 3.2 my current
inclination is to leave this #define enabled by default just to allow
users to be able to compile there applications without problems.
Not all the old osg::Geometry methods exist now - for instance there
is now Geometry::ArrayData structure, just direct access to
osg::Array, for these there is no way we can fix things so that the
old code will compile and such code will have to be ported across. I
expect this will only effect a small number of users though as normal
OSG usage wouldn't require usage of ArrayData directly.
I have decided against going the GeometryDeprecated route that I was
considering at the start of this thread, I feel the above fallback
should be sufficient and will be easier to maintain and document.
An svn update will get all these changes. Fingers crossed there won't
be any big fallout, most apps should just compile and work as before.
Let me know if you have problems though.
The work isn't quite finished yet. There are still plugins and
examples that use the deprecated functionality that we need to port
across. Some of the plugins look like there just read things like
indices for the purpose of writing out, but now osg::Geometry won't
officially contain these so this code can probably be dropped. I
would apprecaite help from the community with a review of the pugins
for the old deprecated functionality, especially as I've been working
a few too many evenings and weekends of late and have started to
suffer a bit from RSI in both forearms/wrists/hands so I'll need to
cut back on my current high work intensity.
Thanks,
Robert.
___
osg-users mailing list
osg-users@lists.openscenegraph.org
http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] Deprecating vertex indices in osg::Geometry
Hi Robert, I tried out your code and it won't load a lot of geometry files even if they are using fast path. The problem lies in that with the old Geometry::ArrayData structure you could first set the binding and then the array. Now you are forced to do this the other way around. The ive reader (ive::Geometry::read) for instance always first sets the binding and then the data. And for .osg files it depend on how you structure your file. Cheers, Pjotr -- Read this topic online here: http://forum.openscenegraph.org/viewtopic.php?p=54653#54653 ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] Deprecating vertex indices in osg::Geometry
Hi Pjotr, On 18 June 2013 15:04, Pjotr Svetachov pjotrsvetac...@gmail.com wrote: I tried out your code and it won't load a lot of geometry files even if they are using fast path. The problem lies in that with the old Geometry::ArrayData structure you could first set the binding and then the array. Now you are forced to do this the other way around. The ive reader (ive::Geometry::read) for instance always first sets the binding and then the data. And for .osg files it depend on how you structure your file. I thought I had already recoded the .osg and .ive plugins to avoid setting the binding before applying an array, but I see that I missed this in the .ive plugin, I'm just coding up a solution to this... watch this space :-) Robert. ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] Deprecating vertex indices in osg::Geometry
Hi Pjotr, Could you do an svn update, rebuild and then report how your models load, hopefully the changes I've made will address the issue. Cheers, Robert. ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] Deprecating vertex indices in osg::Geometry
Hi Robert,
Great work! I just have a few questions/comments.
On 6/18/2013 7:42 AM, Robert Osfield wrote:
Hi All,
I have just finished a rather intensive bout of work on the new
cleaned up osg::Geometry, one of the most challenging parts was
providing a fallback for the deprecated array indices and
BIND_PER_PRIMITIVE usage. The new osg::Geometry doesn't support
rendering of geometries with indices and BIND_PER_PRIMITIVE so one has
to convert geometries containing these over to OpenGL fast path
compliant forms using just straight vertex arrays and standard
glDrawArray/glDrawElements based primitives, and to ease this task I
have written a could of new methods to osg::Geometry:
/** Return true if the deprecated use array indicies or
BIND_PER_PRIMITIVE binding has been assigned to arrays.*/
bool containsDeprecatedData() const { return _containsDeprecatedData;
}
/** fallback for deprecated functionality. Return true if the
Geometry contains any array indices or BIND_PER_PRIMITIVE arrays. */
bool checkForDeprecatedData();
/** fallback for deprecated functionality. Removes any array
indices and BIND_PER_PRIMITIVE arrays.*/
void fixDeprecatedData();
What is the difference between containsDeprecatedData() and
checkForDeprecatedData()? I see that containsDeprecatedData() just
checks the flag, what does the latter do? Forgive me if this is obvious,
the names suggest the same operation to me.
I looked at the revision at
https://github.com/openscenegraph/osg/commit/95548b9cda28c1124fea2c09402547d018266384
but I could not find the implementations of the new member functions.
Also, the include guard in include/osg/Geometry got changed to
OSG_GEOMETRYNEW from OSG_GEOMETRY, was that intentional?
Regards,
Judson
___
osg-users mailing list
osg-users@lists.openscenegraph.org
http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] Deprecating vertex indices in osg::Geometry
Hi Robert, Reading ive files works. Only reading .osg files that contain vertex attribute arrays does not work yet. (both the binding and the normalization can be set before the array itself right now). Cheers, Pjotr robertosfield wrote: Hi Pjotr, Could you do an svn update, rebuild and then report how your models load, hopefully the changes I've made will address the issue. Cheers, Robert. ___ osg-users mailing list http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org -- Post generated by Mail2Forum -- Read this topic online here: http://forum.openscenegraph.org/viewtopic.php?p=54658#54658 ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] Deprecating vertex indices in osg::Geometry
Hi Pjotr, On 18 June 2013 15:37, Pjotr Svetachov pjotrsvetac...@gmail.com wrote: Reading ive files works. Only reading .osg files that contain vertex attribute arrays does not work yet. (both the binding and the normalization can be set before the array itself right now). Just applied a fix, could you try an svn update again ;-) Robert. ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] Deprecating vertex indices in osg::Geometry
Hi Judson, On 18 June 2013 15:21, Judson Weissert jud...@mfrac.com wrote: What is the difference between containsDeprecatedData() and checkForDeprecatedData()? I see that containsDeprecatedData() just checks the flag, what does the latter do? Forgive me if this is obvious, the names suggest the same operation to me. checkForDeprecatedData() actively goes through the arrays looking for an IndexArray attached via their UserData and any Array::Binding set to BIND_PER_PRIMITIVE, if it finds any it sets the internal member variable _containsDeprecatedData; containsDeprecatedData() just returns the _containsDeprecatedData values. Both return true if their is deprecated data attached. I looked at the revision at https://github.com/openscenegraph/osg/commit/95548b9cda28c1124fea2c09402547d018266384 but I could not find the implementations of the new member functions. Also, the include guard in include/osg/Geometry got changed to OSG_GEOMETRYNEW from OSG_GEOMETRY, was that intentional? Thanks for the note, I've just fixed the header so it's back to it's original form. My first experiement with a cut down osg::Geometry was with a class called GeometryNew that was copied from Geometry and then cut down. Once I know this was working for fast paths I then set about adding fallback for the deprecated slow paths, with the later falling into place I was able to replace the old Geometry with GeometryNew and remove the later. I did forget to change the head guard though :-) Robert. ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] Deprecating vertex indices in osg::Geometry
Hi Robert, On 6/18/2013 10:58 AM, Robert Osfield wrote: Hi Judson, On 18 June 2013 15:21, Judson Weissert jud...@mfrac.com wrote: What is the difference between containsDeprecatedData() and checkForDeprecatedData()? I see that containsDeprecatedData() just checks the flag, what does the latter do? Forgive me if this is obvious, the names suggest the same operation to me. checkForDeprecatedData() actively goes through the arrays looking for an IndexArray attached via their UserData and any Array::Binding set to BIND_PER_PRIMITIVE, if it finds any it sets the internal member variable _containsDeprecatedData; containsDeprecatedData() just returns the _containsDeprecatedData values. Both return true if their is deprecated data attached. I looked at the implementations now (the Geometry.cpp changes were not included in the online diff). Correct me if I am wrong, outside of the optimizer, there is no code path that will call checkForDeprecatedData(). Therefore, it is quite likely that drawImplementation() and other functions will be called before checkForDeprecatedData() is called. Thus, I am wondering if the _containsDeprecatedData flag should be taken out entirely. Perhaps the member functions could be replaced with two free functions: bool CheckForDeprecatedData (const osg::Geometry *geometry); and void FixDeprecatedData (osg::Geometry *geometry); and they would be part of the osg::Geometry API, but they would not be as tightly coupled to the same source files. i.e., you could move them wherever you want. That is, assuming they do not require access to the non-Public osg::Geometry API. I realize it is expensive to call CheckForDeprecatedData(), but in theory, FixDeprecatedData() is going to be called afterwards anyhow, so you end up with the same end result (at least from what I have observed so far). I am basing this logic on the fact that the _containsDeprecatedData flag is already not guaranteed to be correct in the places where it is being used as a guard. Regards, Judson ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] Deprecating vertex indices in osg::Geometry
Hi Judson, On 18 June 2013 16:17, Judson Weissert jud...@mfrac.com wrote: I looked at the implementations now (the Geometry.cpp changes were not included in the online diff). Correct me if I am wrong, outside of the optimizer, there is no code path that will call checkForDeprecatedData(). Therefore, it is quite likely that drawImplementation() and other functions will be called before checkForDeprecatedData() is called. Thus, I am wondering if the _containsDeprecatedData flag should be taken out entirely. Currently the Geometry::set*Indices() and set*Binding() methods set _containsDeprecatedData directly. The checkDeprecatedData() is there as a fallback in case this method isn't reliable. Please remember this code is still rather experimental, once it starts getting tested out it may or may not need to change. It might be that we'll be able to remove the checkDeprecateData() and perhaps even the _containsDeprecateData member, but until we are a bit further in I won't commit any particular way, it's really up to the community now to start testing and shake it down. One thing I am thinking about doing is making these methods and associated via all optional compiled in, which will be on by default. Making this element optionally compiled in will allow developers that need to keep the OSG small and won't ever have deprecated data to deal with can just go withe clean and pure osg::Geometry. Longer term I'd these methods in what ever form they end up will be deprecated and eventually removed completely. Perhaps the member functions could be replaced with two free functions: bool CheckForDeprecatedData (const osg::Geometry *geometry); and void FixDeprecatedData (osg::Geometry *geometry); and they would be part of the osg::Geometry API, but they would not be as tightly coupled to the same source files. i.e., you could move them wherever you want. That is, assuming they do not require access to the non-Public osg::Geometry API. I realize it is expensive to call CheckForDeprecatedData(), but in theory, FixDeprecatedData() is going to be called afterwards anyhow, so you end up with the same end result (at least from what I have observed so far). I am basing this logic on the fact that the _containsDeprecatedData flag is already not guaranteed to be correct in the places where it is being used as a guard. The _containsDeprecatedData flag is there to avoid the drawImplementation() from having to check to see whether it's safe to run or not, calling the checkForDeprecatedData() is just too expensive to call on call drawImplementation(). Attempting a drawImplementation() whilst it contains BIND_PER_PRIMITIVE or indices will likey result in a seg fault so worth guarding against. Robert. ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] Deprecating vertex indices in osg::Geometry
Hi Robert, On 6/18/2013 11:47 AM, Robert Osfield wrote: Hi Judson, On 18 June 2013 16:17, Judson Weissert jud...@mfrac.com wrote: I looked at the implementations now (the Geometry.cpp changes were not included in the online diff). Correct me if I am wrong, outside of the optimizer, there is no code path that will call checkForDeprecatedData(). Therefore, it is quite likely that drawImplementation() and other functions will be called before checkForDeprecatedData() is called. Thus, I am wondering if the _containsDeprecatedData flag should be taken out entirely. Currently the Geometry::set*Indices() and set*Binding() methods set _containsDeprecatedData directly. The checkDeprecatedData() is there as a fallback in case this method isn't reliable. Ah, that is the part I missed, I did not see the _containsDeprecatedData flag usage in the inline functions in the header. Thanks for the explanation, Judson ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] Deprecating vertex indices in osg::Geometry
Hi. I'm not sure I follow the discussion, since each post is starting to become a separate article, but I'll try to tell what I think. I don't know the policy on OSG ABI, but if it's to keep ABI compatible between some releases, I would: * add compile flag to select between old and new Geometry (defaults to old one); * warn during compile time and during running (into the log) that using the old Geometry is deprecated and will be removed in the following stable release; * warn in changelog about the coming ABI breakage; If the policy is only to keep API compatibility with some minor changes like obsoleting unused functions, then Geometry could be removed. 2013/6/15 Robert Osfield robert.osfi...@gmail.com Hi Mattias et. al, On 14 June 2013 14:23, Mathias Fröhlich mathias.froehl...@gmx.net wrote: I really like the idea of GeometryDeprecated. It takes somehow more work to make this happen for users. But at very first it's just about changing a single datatype. Once it compiles with osg::Geometry again you know that you will nowhere have a slow path geometry anymore. You do not need to rely on your test coverage and an exception then. I know, this is against what you usually do with your api. My clean up work today has focused on sliming down osg::ArrayDispatchers and as a knock on effect implementing GeometryDeprecated has become a bit more complicated as it'll need to take on more of the work that ArrayDispatchers used to handle. I don't think it's right for us to bloat the core OSG just to maintain deprecated functionality so my current inclination is to perhaps even go further and not provide GeometryDeprecated at all. I'm currently toying with the idea of leaving the deprecated slow path methods in osg::Geometry and have flag that gets set to declare this osg::Geometry in invalid condition that can't be rendered or handled in any osg::Geometry processing, to make this osg::Geometry usable you'd then call an optimize method that converts all the indices and per primitive usage into OpenGL fast path compliant usage. My current implementation has the valid fast path version of this osg::Geometry nested within the invalid one and used in it's place when rendering, but this feels a bit cludgy and potentially inefficient when it comes to updates. The awkwardness with this nested osg::Geometry is why I'm starting to wonder if just labelling these problem osg::Geometry as invalid and let them be ignored unless the uses runs the optimize method. For the .osg, .ive loaders and new serilizers and thinking that by default we could automatically run the optimizer on the any problem osg::Geometry that have been loaded. If we really start to optimize geometries under the hood, when would you do this then? You cannot rely on anything in osgDB since you have to account for in application generated geometries. The only non concurrent opportunity is the update stage so far. Ok, let's assume that happens there. But then we at least have *huge* geometries. Processing them with this kind of optimization to get rid of the indices and that can take a long time. It can, so I'm inclined to push this back to user and do a one time convert after they have created the osg::Geometry. When could possible have a contains invalid osg::Geometry flag in the whole scene graph so that the update traversal knows it needs to hunt down and convert them, this does add complexity though. The other alternative might be to have a linked list of invalid osg::Geometry that need to be dealt with. Having this invalid/deprecated osg::Geometry be updated each frame would be very costly, but then they are any way. What we really want users to do is migrate away from ever creating these bad osg::Geometry in the first place, so having to jump through an extra hoop or two to make it explicit about what is happening might be the best way, even if it does mean that end users can't just recompile and run with the latest OSG and expect everything including there old deprecated usage to still work. Offically array Indices and BIND_PER_PRIMTIVE have been deprecated for several releases, every time the topic is mentioned I try to persuade users to use fast paths. The inline docs have long been clear that they are deprecated so perhaps I needn't be so accommodating. We experience this as of today with non fast path geometries where the driver is doing this work under the hood. And in contrast to osg these drivers have a long histroy of optimizing this code paths and are well optimized. OpenGL drivers have been better at replicating the glBegin/glEnd than the OSG equivalent found in the osg::GLBeginEndAdapter, as much as I tried to optimize the later I could never get the same performance as what the OpenGL driver was doing. By contrast fast path osg::Geometry are relatively easy to just push out to OpenGL without a big CPU overhead. Also update is
Re: [osg-users] Deprecating vertex indices in osg::Geometry
HI Michael, On 15 June 2013 15:59, michael kapelko korn...@gmail.com wrote: I'm not sure I follow the discussion, since each post is starting to become a separate article, but I'll try to tell what I think. The thread might be a little meandering, but it's still roughly on topic. The meandering is partly down to be testing out different API and implementation approaches. I don't know the policy on OSG ABI, but if it's to keep ABI compatible between some releases, I would: * add compile flag to select between old and new Geometry (defaults to old one); * warn during compile time and during running (into the log) that using the old Geometry is deprecated and will be removed in the following stable release; * warn in changelog about the coming ABI breakage; If the policy is only to keep API compatibility with some minor changes like obsoleting unused functions, then Geometry could be removed. The approach we've taken has been for patch releases within a stable release set to remain binary compatible, i.e. 3.0.0 is binary compatible with 3.0.1. For developer series I don't attempt to retain binary compatible, and don't expect to retain binary compatibility. In general I do try and maintain the bulk of the API so the majority of OSG users can move between developer and close stable releases without too much effort. My current expectation is that the new cleaned up, fast path only osg::Geometry will not have all the methods and data structures that OSG-3.0.x osg::Geometry had but for most users will just compile and work as before. Those using slow path API's such as array indices and BIND_PER_PRIMITIVE will have to do something extra, either porting away from these long deprecated features, or adding a call to like geometry-fixDeprecatedData(); I don't know yet whether we should make these deprecated API's #ifdef'd out or not yet, if we do we'd want to maintain the ABI so use of inline functions may be the way to go. I'm getting close to checking in my new cleaned up osg::Geometry, once I do hopefully most users will be able to say of that nice cleanup and all I had to do is recompile, but with others using the old deprecated functionality we'll need to help them port across/refine what the OSG provides in terms of fallback. Once I have the cleanup version of osg::Geometry checked in there will still be work within the OSG to continue moving across to using just the non deprecated API, for this I'll need assistance from the community. Robert. ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] Deprecating vertex indices in osg::Geometry
Hi All, My opinions about what to do in about osg::Geometry and the old deprecated functionality is continuing to evolve. I'm starting to feel that have an osg::GeometryDeprecated is going to be awkward to maintain and for end users, while the new cleaned up osg::Geometry is clearly much better from an internal implementation and performance point of view it might be best in the short term to maintain support for the old deprecated functionality. For the old deprecated usage, while keep the clean fast path only implementation I am think about having an internal osg::Geometry that is generated from the parent osg::Geometry that is fully fast path complient and automatically generated from the parent osg::Geometry's data. This fallback could be optionally compiled out, as could the public API for the old deprecated functionality. One could also at runtime disable the fallback and possible emit an std::exception to help users who want to migrate across. Thoughts? Robert. ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] Deprecating vertex indices in osg::Geometry
Hi Robert, On Friday, June 14, 2013 13:08:43 Robert Osfield wrote: My opinions about what to do in about osg::Geometry and the old deprecated functionality is continuing to evolve. I'm starting to feel that have an osg::GeometryDeprecated is going to be awkward to maintain and for end users, while the new cleaned up osg::Geometry is clearly much better from an internal implementation and performance point of view it might be best in the short term to maintain support for the old deprecated functionality. For the old deprecated usage, while keep the clean fast path only implementation I am think about having an internal osg::Geometry that is generated from the parent osg::Geometry that is fully fast path complient and automatically generated from the parent osg::Geometry's data. This fallback could be optionally compiled out, as could the public API for the old deprecated functionality. One could also at runtime disable the fallback and possible emit an std::exception to help users who want to migrate across. I really like the idea of GeometryDeprecated. It takes somehow more work to make this happen for users. But at very first it's just about changing a single datatype. Once it compiles with osg::Geometry again you know that you will nowhere have a slow path geometry anymore. You do not need to rely on your test coverage and an exception then. I know, this is against what you usually do with your api. If we really start to optimize geometries under the hood, when would you do this then? You cannot rely on anything in osgDB since you have to account for in application generated geometries. The only non concurrent opportunity is the update stage so far. Ok, let's assume that happens there. But then we at least have *huge* geometries. Processing them with this kind of optimization to get rid of the indices and that can take a long time. We experience this as of today with non fast path geometries where the driver is doing this work under the hood. And in contrast to osg these drivers have a long histroy of optimizing this code paths and are well optimized. Also update is currently empty. And for applications that do not need an additional graph traversal this should stay like that IMO. Depending on the structure of your graph - most probably for viz sim applications - you might need to traverse way more nodes in update than you need for cull, since you might have loaded much more geometry than you currently have in the current view. So having update walk the whole present tree can itself take a noticable time. Rather than that I would prefer to have this explicitly stated in the API what is sensible to use and what not. At least a compile switch which only allows me to use fast path stuff would be helpful IMO. So I would vote for GeometryDeprecated. May be you can call the two classes also FastGeometry vs. Geometry indstead of Geometry vs. DeprecatedGemetry. Anyway thanks for all you work! Mathias ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] Deprecating vertex indices in osg::Geometry
Hi Mattias et. al, On 14 June 2013 14:23, Mathias Fröhlich mathias.froehl...@gmx.net wrote: I really like the idea of GeometryDeprecated. It takes somehow more work to make this happen for users. But at very first it's just about changing a single datatype. Once it compiles with osg::Geometry again you know that you will nowhere have a slow path geometry anymore. You do not need to rely on your test coverage and an exception then. I know, this is against what you usually do with your api. My clean up work today has focused on sliming down osg::ArrayDispatchers and as a knock on effect implementing GeometryDeprecated has become a bit more complicated as it'll need to take on more of the work that ArrayDispatchers used to handle. I don't think it's right for us to bloat the core OSG just to maintain deprecated functionality so my current inclination is to perhaps even go further and not provide GeometryDeprecated at all. I'm currently toying with the idea of leaving the deprecated slow path methods in osg::Geometry and have flag that gets set to declare this osg::Geometry in invalid condition that can't be rendered or handled in any osg::Geometry processing, to make this osg::Geometry usable you'd then call an optimize method that converts all the indices and per primitive usage into OpenGL fast path compliant usage. My current implementation has the valid fast path version of this osg::Geometry nested within the invalid one and used in it's place when rendering, but this feels a bit cludgy and potentially inefficient when it comes to updates. The awkwardness with this nested osg::Geometry is why I'm starting to wonder if just labelling these problem osg::Geometry as invalid and let them be ignored unless the uses runs the optimize method. For the .osg, .ive loaders and new serilizers and thinking that by default we could automatically run the optimizer on the any problem osg::Geometry that have been loaded. If we really start to optimize geometries under the hood, when would you do this then? You cannot rely on anything in osgDB since you have to account for in application generated geometries. The only non concurrent opportunity is the update stage so far. Ok, let's assume that happens there. But then we at least have *huge* geometries. Processing them with this kind of optimization to get rid of the indices and that can take a long time. It can, so I'm inclined to push this back to user and do a one time convert after they have created the osg::Geometry. When could possible have a contains invalid osg::Geometry flag in the whole scene graph so that the update traversal knows it needs to hunt down and convert them, this does add complexity though. The other alternative might be to have a linked list of invalid osg::Geometry that need to be dealt with. Having this invalid/deprecated osg::Geometry be updated each frame would be very costly, but then they are any way. What we really want users to do is migrate away from ever creating these bad osg::Geometry in the first place, so having to jump through an extra hoop or two to make it explicit about what is happening might be the best way, even if it does mean that end users can't just recompile and run with the latest OSG and expect everything including there old deprecated usage to still work. Offically array Indices and BIND_PER_PRIMTIVE have been deprecated for several releases, every time the topic is mentioned I try to persuade users to use fast paths. The inline docs have long been clear that they are deprecated so perhaps I needn't be so accommodating. We experience this as of today with non fast path geometries where the driver is doing this work under the hood. And in contrast to osg these drivers have a long histroy of optimizing this code paths and are well optimized. OpenGL drivers have been better at replicating the glBegin/glEnd than the OSG equivalent found in the osg::GLBeginEndAdapter, as much as I tried to optimize the later I could never get the same performance as what the OpenGL driver was doing. By contrast fast path osg::Geometry are relatively easy to just push out to OpenGL without a big CPU overhead. Also update is currently empty. And for applications that do not need an additional graph traversal this should stay like that IMO. Depending on the structure of your graph - most probably for viz sim applications - you might need to traverse way more nodes in update than you need for cull, since you might have loaded much more geometry than you currently have in the current view. So having update walk the whole present tree can itself take a noticable time. If we did an update traversal then one would have to limit it to just the slow path geometry that is being updated on that frame. If you don't have much slow path geometry this needn't be too slow. If you have lots of slow path geometry then well perhaps you deserve to do the work to correct or suffer a slow app... Rather than
Re: [osg-users] Deprecating vertex indices in osg::Geometry
Hi All, I have now got the old .osg, .ive loaders working with the new osg::Geometry, and got the new serializers working. The way I've tackled it was by storing the old deprecated IndexArray functionality as UserData on the osg::Array that they were originally paired with. This approach means that you can read and write the original data intact even without the original access methods for the arrays. However, I have temporarily reinstated the s/getVertexIndices() etc. methods to help out with the translation work - these simply s/get the user data hiding the fact that there isn't any ArrayData any more. I'm actually now starting to wonder whether leaving these in place and using a optional build to enable/disable them, this would at least allow users to keep their code compiling. If we do keep the option of building with the old array indices methods then it also raises the question do we attempt some form of fallback scheme to implement the unsupported paths. Potentially one could map the indexed arrays and rebuild and BIND_PER_PRIMITIVE primitive sets to fast paths equivilants stored in an osg::Geometry. There was an internal optimized osg::Geometry that kind of did this job previously and would could re-instate this. The other alternative is to just post process the osg::Geometry with hidden indices and BIND_PER_PRIMITIVE at the scene graph pre-processing stage and if one tries to render an osg::Geometry with this old baggage you just return an error and do an non op of the graphics front. We could even use an optional compile to throw an exception when the scene graph hits one of these old and now invalid osg::Geometry to help users with debugging. Thoughts on aiding the move to the cleaned up osg::Geometry? What would help your translation across? Robert. ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] Deprecating vertex indices in osg::Geometry
Hi All, After brief foray back to do some client work and merging submissions I have returned to looking at the new cleaned up Geometry and a GeometryDeprecated fallback implementation. Currently I'm doing a experimental build of the OSG with GeometryNew now named Geometry, and the old Geometry renamed GeometryDeprecated. Most of the core OSG classes and plugins have ported across pretty easily, in many cases the code has now become simpler because there osg::Geometry now is also using fast paths and there are no indices to worry about. Sticking points will be the .osg, .ive and new serializers as they are all built around there been indices to read and write. I don't know yet how to tackle this so I'm going to try and get the rest of the OSG built against Geometry/GeometryDeprecated before returning to how to tackle the native OSG file formats. I'll keep you all updated on how things progress. Fingers crossed the bulk of the core OSG work related to these changes will be checked in this week. Robert. ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] Deprecating vertex indices in osg::Geometry
On 5 June 2013 06:45, Robert Osfield robert.osfi...@gmail.com wrote:
Right now I feel that sticking with the same naming convention that
users have been used will make for a smoother transition to the new
osg::Geometry. Ideally I want most users just to recompile their code
and not have any problems.. Extending the enum Binding with
BIND_INSTANCE_DIVISOR_0 etc, would be one way, it's a bit cludgy but
would avoid the overlap in functionality issue.
I have quickly mocked up the change to Array::Binding thus:
enum Binding
{
BIND_UNDEFINED=-1,
BIND_OFF=0,
BIND_OVERALL=1,
BIND_PER_PRIMITIVE_SET=2,
BIND_PER_VERTEX=4,
BIND_INSTANCE_DIVISOR_0=6,
BIND_INSTANCE_DIVISOR_1=BIND_INSTANCE_DIVISOR_0+1,
BIND_INSTANCE_DIVISOR_2=BIND_INSTANCE_DIVISOR_0+2,
BIND_INSTANCE_DIVISOR_3=BIND_INSTANCE_DIVISOR_0+3,
BIND_INSTANCE_DIVISOR_4=BIND_INSTANCE_DIVISOR_0+4,
BIND_INSTANCE_DIVISOR_5=BIND_INSTANCE_DIVISOR_0+5,
BIND_INSTANCE_DIVISOR_6=BIND_INSTANCE_DIVISOR_0+6,
BIND_INSTANCE_DIVISOR_7=BIND_INSTANCE_DIVISOR_0+7
};
Also I change the s/getBinding() to use an int rather than Binding so
you can simply write:
array-setBinding(BIND_INSTANCE_DIVISOR_0+divisor);
On a separate clean up of GeometryNew I have now removed the non
essential verifyBindings()/sharedArray() methods that really belong in
osgUtil as part of set of helper classses/methods. With this clean up
comparing the GeometryNew to old the Geometry shows how effective the
clean up has been:
$ wc include/osg/GeometryNew src/osg/GeometryNew.cpp
258 1127 11245 include/osg/GeometryNew
1146 2487 38666 src/osg/GeometryNew.cpp
1404 3614 49911 total
$ wc include/osg/Geometry src/osg/Geometry.cpp
463 1877 20075 include/osg/Geometry
2763 5793 100862 src/osg/Geometry.cpp
3226 7670 120937 total
Total size is now less than half the previous size. The code is also
clearer. So far so good.
Robert.
___
osg-users mailing list
osg-users@lists.openscenegraph.org
http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] Deprecating vertex indices in osg::Geometry
While out this morning it occurred to me that the conundrum of how to support the GeometryDeprecated from the existing .osg, .ive and .osgt/.osgb/.osgx serializers might be to load the data as the new streamlined osg::Geometry but place the extra data that osg::Geometry no longer stores as UserData attached the geometry this UserData then can get parsed afterwards by a NodeVisitor that converts the osg::Geometry into GeometryDeprecated. This approach would allow us to move GeometryDeprecated out of the core osg library and avoid any games such as having the osg serializer library contain a link to another osg library. This above is just a passing thought, I won't attempt anything until tomorrow as I busy with other work today. ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
[osg-users] Deprecating vertex indices in osg::Geometry
Hi All, One of the changes I've discussed before, but as yet no made, is to simplify osg::Geometry so that it drops the index arrays that you can currently associated for vertex, normal, colour, texcoord arrays. These index arrays aren't supported by OpenGL, instead have to be emulated by breaking the primitive data up into small chunks and sending the vertex data and primitive data in small batches. This is very inefficient, so what having separate vertex indicies might seem like a good way of reducing vertex data overhead by sharing more it causes a big CPU overhead and results in lower performance. While there are clear warnings in the osg::Geometry header that this feature is deprecated and forces and OpenGL slow path I know it still gets used on occasion - against best practice advice. What I'd like now to do is drop the index arrays from osg::Geometry, and provide a GeometryDeprecated class for the those who need backwards compatibility. Potentially this GeometryDeprecated class could go in the core osg library, but I'm tempted to move it out into one of the optional NodeKits to reduce the size of the core osg library. To be clear OpenGL indexed primitives will still be fully support by osg::Geometry - DrawElement* has always been available and will remain, it's the long been the preferred way to pass your primitive data to OpenGL. Another step in making these changes that I'm considering is moving the normalize parameter from Geometry::ArrayData into osg::Array, with this change ArrayData ceases to have a role and can be removed further simplifying osg::Geometry. Thoughts? Robert. ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] Deprecating vertex indices in osg::Geometry
I have now started looking into what changes would be required to osg::Geometry and have decided to make a copy of Geometry called GoemetryNew and cut this down to see how easy it will be clean this class up and what the knock on effects might be. Now I have dived in and begun the process one what is clear is just how much the implementation of Geometry.cpp was complicated by the support for indices - the size of GeometryNew.cpp text file is now 1982 bytes vs 2763 for the old Geometry.cpp. Getting rid of indices will reduce footprint and also will make the code faster as they will be less checks to see if indices are there and need something special doing for them. Another element that jumps out as out of step with modern OpenGL is support for PER_PRIMITIVE binding of arrays - this still forces OpenGL slow paths and again complicates the code. I believe that osg::Geometry really should just support fast paths so as part of the cleanup it would reasonable to drop this from GeometryNew.cpp as well. I will tackle the indices first though. My current action plan is to get GeometryNew.cpp to build and run with the osggeometry example, then look for feedback from the community. If things look viable then the next step will be rename GeometryNew.cpp to Geometry.cpp and renamed the existing Geometry.cpp to GeometryDeprecated.cpp. This step will require some games on the serialization front to return backwards compatibility. Another area of change would be looking at Geometry::ArrayData - this currently contains array, indices, binding and normalize variables. My first step is to simply remove indices, but I'm thinking that binding and normalize might both be suitable for moving into osg::Array and for ArrayData to be removed entirely. Thoughts? Robert. ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] Deprecating vertex indices in osg::Geometry
On 4 June 2013 09:42, Robert Osfield robert.osfi...@gmail.com wrote: My current action plan is to get GeometryNew.cpp to build and run with the osggeometry example I have now got my quick clean up of Geometry as GeometryNew class with osgeometry compiling and running correctly. My GeometryNew class removes support for vertex indices. My next step is to look at BIND_PER_PRIMITIVE. While vertex indices aren't actually used in the core OSG BIND_PER_PRIMITIVE is quite widely used in the examples. They really shouldn't be though as it means that they are all forces slow paths in osg::Geometry. I don't know yet whether to check in GeometryNew, or whether to wait till I've done enough work to renamed the old Geometry to GeometryDeprecated and rename GeometryNew to Geometry. I'm currently inclined to do this work in stages and check in GeometryNew even if it's just a temporary class name. Again I welcome feedback from the community. Robert. ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] Deprecating vertex indices in osg::Geometry
Robert, I think it's better to check it in.. Others would have the opportunity to participate or evely siltently read the code and learn. Maybe there aer others out there reading changes and try to learn as I do it usually ( If time permits). Regards, Torben -- Read this topic online here: http://forum.openscenegraph.org/viewtopic.php?p=54420#54420 ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] Deprecating vertex indices in osg::Geometry
On 4 June 2013 10:21, Torben Dannhauer tor...@dannhauer.info wrote: I think it's better to check it in.. Others would have the opportunity to participate or evely siltently read the code and learn. Maybe there aer others out there reading changes and try to learn as I do it usually ( If time permits). GeometryNew is now checked in, along with osggeometry that has been ported to it - simply a name change as osggeometry.cpp was already clean w.r.t using only fast paths. GeometryNew doesn't have any array indices buit still supports BIND_PER_PRIMITIVE, so this will be my next bit of work. I've been looking at other OSG examples and find out that several use BIND_PER_PRIMITIVE, ouch... slow path examples! These are: osganimate/osganimate.cpp osgdelaunay/osgdelaunay.cpp osgimpostor/osgimpostor.cpp osgocclusionquery/osgocclusionquery.cpp osgoscdevice/osgoscdevice.cpp osgphotoalbum/osgphotoalbum.cpp osgpick/osgpick.cpp osgsharedarray/osgsharedarray.cpp I will work through these removing the deprecated usage to make it possible to port them over to the evolving GeometryNew. For most users I'm hoping that they will have already been create osg::Geometry that is all clean and using fast paths, if that's the case then they won't need to make any changes in their applications - a simple rebuild will be sufficient. Robert. ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] Deprecating vertex indices in osg::Geometry
Hi Robert, On Tuesday, June 04, 2013 09:42:10 Robert Osfield wrote: Thoughts? Great idea! I see that the GeometryDeprecated is still needed. ... that means we will need this at least. The begin/end emulation code could then probably move to wherever you move the GeometryDeprecated class. What about the plugins? Do all of them consistently use the fast path? May be this is also an opportunity to throw out the deprecated dlists? Also to get good 'fast path' performance, handlling of primitive restart would be good. I know there were discussions about that. But was there a final outcome how you want to have this implemented? Greetings Mathias ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] Deprecating vertex indices in osg::Geometry
HI Mathias, On 4 June 2013 13:53, Mathias Fröhlich mathias.froehl...@gmx.net wrote: I see that the GeometryDeprecated is still needed. ... that means we will need this at least. I don't think it'll be too difficult to maintain, and it'll be really useful to old Geometry held in the existing .osg/.ive files. The begin/end emulation code could then probably move to wherever you move the GeometryDeprecated class. Yes! :-) What about the plugins? Do all of them consistently use the fast path? We'll have review them one by one. Most should be using fast paths, ones that aren't we'll need to port across to using GeometryDeprecated if it isn't easy to port them across to the new cleaned up osg::Geometry. May be this is also an opportunity to throw out the deprecated dlists? I don't reason a need to drop display lists in osg::Geometry, at least at this point. Display lists are rather orthogonal to the rest of the API. One thing the clean up will mean is a simpler internal implementation that will have a lower CPU overhead so the cost on non display listing will be lower so more models that presently work best with display lists will now be better without. I do still expect display lists to work best for some models and OpenGL drivers though. Also to get good 'fast path' performance, handlling of primitive restart would be good. I know there were discussions about that. But was there a final outcome how you want to have this implemented? Ooo I'm a bit cold on primitive restart, I vaguely recall some discussion previous about it but it's long enough ago for me to not have a clear idea. In general I'd say with a clean up osg::Geometry will be in better place to look at implementing the new OpenGL features, these will be focused on the new Geometry implementation with GeometryDepercated being specifically for backwards compatible slow path usage without attempting to update it to latest OpenGL features. This approach will free us from some of the sticky bits of implementation - the glBegin/glEnd emulation is an example of this. Robert. ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] Deprecating vertex indices in osg::Geometry
Hi, On Tuesday, June 04, 2013 14:12:21 Robert Osfield wrote: May be this is also an opportunity to throw out the deprecated dlists? I don't reason a need to drop display lists in osg::Geometry, at least at this point. Display lists are rather orthogonal to the rest of the API. One thing the clean up will mean is a simpler internal implementation that will have a lower CPU overhead so the cost on non display listing will be lower so more models that presently work best with display lists will now be better without. I do still expect display lists to work best for some models and OpenGL drivers though. This is definitely orthogonal. The reason is more that users already need to change their geomertry for this particular change. So may be it's a good time to clean up a little more than to annoy these people again in say two years with some more changes in this area. But that's just a thought ... Also to get good 'fast path' performance, handlling of primitive restart would be good. I know there were discussions about that. But was there a final outcome how you want to have this implemented? Ooo I'm a bit cold on primitive restart, I vaguely recall some discussion previous about it but it's long enough ago for me to not have a clear idea. In general I'd say with a clean up osg::Geometry will be in better place to look at implementing the new OpenGL features, these will be focused on the new Geometry implementation with GeometryDepercated being specifically for backwards compatible slow path usage without attempting to update it to latest OpenGL features. This approach will free us from some of the sticky bits of implementation - the glBegin/glEnd emulation is an example of this. Well, primitive restart allows you to merge a lot of indexed primitive sets into a single one for triangle strips for example. Without you need to emit a lot of extra draws for each strip. That means this takes a whole lot of cpu cycles to do the draw setup on the gpu for a few triangles in the strip. this in turn make strips basically unusable if you are draw bound. Or you will get draw bound because of that. Primitive restart allows you to specify a 'magic index' that can be placed in the index array to signal the GPU that a new strip begins. By that you get back huger bunches of draws scheduled with a single OpenGL draw call doing the GPU setup just this single time. Conceptually this is an OpenGL state like any other StateAttribute you already implemented. So the easiest would be to add an other StateAttribute for this. The bad thing is that you cannot rely on this to be present as an OpenGL extension but requires the geometry to be built in a different way. So the additional code path to do is more or less intrusive. The good thing is that a whole lot of gpu's do handle primitive restart. May be an optimizer pass that spits primitive restart draw element calls could be useful for this. Anyway, that's also kind of orthogonal. But once people need to change their geometries for this renewed Geometry class it might be also a natural point for somebody using osg to move to restarted strips if this is possible. Greetings Mathias ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] Deprecating vertex indices in osg::Geometry
Hi Robert, Would you consider changing BIND_PER_PRIMITIVE code to do some kind of internal conversion to fast path rendering, such as expanding the arrays by 3x (duplicating the color or normal for each vertex) so that it can use the per-vertex rendering path? I have certainly written code where I wanted per-face coloring or sharp edged shading (same normal across the face) and PER_PRIMITIVE captures my intent better than using PER_VERTEX and having to duplicate the colors/normals. It's not a big deal, but having the concept handled by osg::Geometry is more convenient than handling it in client code --- as you've noted, even in the examples PER_PRIMITIVE is used quite a bit. - Peter On 6/4/2013 5:14 AM, Robert Osfield wrote: On 4 June 2013 09:42, Robert Osfield robert.osfi...@gmail.com wrote: My current action plan is to get GeometryNew.cpp to build and run with the osggeometry example I have now got my quick clean up of Geometry as GeometryNew class with osgeometry compiling and running correctly. My GeometryNew class removes support for vertex indices. My next step is to look at BIND_PER_PRIMITIVE. While vertex indices aren't actually used in the core OSG BIND_PER_PRIMITIVE is quite widely used in the examples. They really shouldn't be though as it means that they are all forces slow paths in osg::Geometry. I don't know yet whether to check in GeometryNew, or whether to wait till I've done enough work to renamed the old Geometry to GeometryDeprecated and rename GeometryNew to Geometry. I'm currently inclined to do this work in stages and check in GeometryNew even if it's just a temporary class name. Again I welcome feedback from the community. Robert. ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org -- Peter Amstutz Senior Software Engineer Technology Solutions Experts Natick, MA 02131 ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] Deprecating vertex indices in osg::Geometry
Hi Peter, On 4 June 2013 14:29, Peter Amstutz peter.amst...@tseboston.com wrote: Would you consider changing BIND_PER_PRIMITIVE code to do some kind of internal conversion to fast path rendering, such as expanding the arrays by 3x (duplicating the color or normal for each vertex) so that it can use the per-vertex rendering path? That is what the OSG has been doing since 3.0.x but this type of emulation is very slow. While it might be convenient for end users It's bad for a scene graph to hiding the complexity to such an extent that you get huge slow down with seemingly Innocent usage. I much prefer making efficient use of OpenGL the default, and dropping off efficient fast paths something you have to try hard to do, rather than something you can do without even realizing. Going forward there are advanced features of OpenGL that really build upon the that vertex arrays can be accessed and how primitive data is used that doesn't fit well with this old functionality. I have certainly written code where I wanted per-face coloring or sharp edged shading (same normal across the face) and PER_PRIMITIVE captures my intent better than using PER_VERTEX and having to duplicate the colors/normals. It's not a big deal, but having the concept handled by osg::Geometry is more convenient than handling it in client code --- as you've noted, even in the examples PER_PRIMITIVE is used quite a bit. I understand the convenience angle, but as it hides a multitude of sins I don't believe it's a good practice. Perhaps the best way to deal with this to to help a osg::Geometry builder class that can help users create the data they want in a convenient form. In the short term you could simply use GeometryDeprecated. Having a converter from GeometryDeprecated to new cleaned up Geometry will be relatively straight forward and might be another approach. I believe that a fast path only osg::Geometry is worthy enough goal to explicitly drop all slow path support, so it only supports what modern OpenGL can support with no clever emulation being hidden behind the scenes. This does mean the convenience of things like vertex indices and BIND_PER_PRIMITIVE have to be dropped, to deal with this I don't think new osg::Geometry should bend to accomodate, rather we should seek out ways of making it more convenient to create osg::Geometry - I call upon the community to make suggestions about help classes. Cheers, Robert. ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] Deprecating vertex indices in osg::Geometry
Hi Robert, This is totally reasonable and on balance I agree with your reasoning. I thought the feature deserved a little discussion before being sent quietly into the night :-) On 6/4/2013 9:44 AM, Robert Osfield wrote: I believe that a fast path only osg::Geometry is worthy enough goal to explicitly drop all slow path support, so it only supports what modern OpenGL can support with no clever emulation being hidden behind the scenes. This does mean the convenience of things like vertex indices and BIND_PER_PRIMITIVE have to be dropped, to deal with this I don't think new osg::Geometry should bend to accomodate, rather we should seek out ways of making it more convenient to create osg::Geometry - I call upon the community to make suggestions about help classes. Cheers, Robert. ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org -- Peter Amstutz Senior Software Engineer Technology Solutions Experts Natick, MA 02131 ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] Deprecating vertex indices in osg::Geometry
Hi Peter, On 4 June 2013 15:34, Peter Amstutz peter.amst...@tseboston.com wrote: This is totally reasonable and on balance I agree with your reasoning. I thought the feature deserved a little discussion before being sent quietly into the night :-) ;-) The reason for me to raise this topic is specifically to get full spectrum of opinions and issues that might arise. While as project lead it's my job to steer the over course that the code takes but if I head off in direction that doesn't take the community with it then I'm damaging the project, so expect and want users to jump and down in discontent to make sure that any new decisions are sound and well justified. Robert. ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] Deprecating vertex indices in osg::Geometry
Hi All,
I have now taken the next step and added the following methods to osg::Array:
/** Specify whether the array data should be normalized by OpenGL.*/
void setNormalize(bool normalize) { _normalize = normalize; }
/** Get whether the array data should be normalized by OpenGL.*/
bool getNormalize() const { return _normalize; }
/** Set hint to ask that the array data is passed via integer
or double, or normal setVertexAttribPointer function.*/
void setPreserveDataType(bool preserve) { _preserveDataType =
preserve; }
/** Get hint to ask that the array data is passed via integer
or double, or normal setVertexAttribPointer function.*/
bool getPreserveDataType() const { return _preserveDataType; }
/** Set the rate at which generic vertex attributes advance
during instanced rendering. Uses the glVertexAttribDivisor feature of
OpenGL 4.0*/
void setAttribDivisor(GLuint divisor) { _attribDivisor = divisor; }
/** Get the rate at which generic vertex attributes advance
during instanced rendering.*/
GLuint getAttribDivisor() const { return _attribDivisor; }
enum Binding
{
BIND_OFF=0,
BIND_OVERALL,
BIND_PER_PRIMITIVE_SET,
BIND_PER_VERTEX
};
/** Specify how this array should be passed to OpenGL.*/
void setBinding(Binding binding) { _binding = binding; }
/** Get how this array should be passed to OpenGL.*/
Binding getBinding() const { return _binding; }
At present all of them doing nothing at all apart from sitting there
looking pretty. The PreserveDataType and AttribDivisor are entirely
new and are intended to support new extensions being added to the OSG
- but their implementation will have to wait till the GeometryNew
refactor is further down the road.
The Normalize and Binding map directly to what has been available in
osg::Geometry::ArrayData's normalize and binding parameters. The
binding we've typically set via Geometry::setNormalBinding() etc.
which we could easily remap internally to array-setBinding(..), but
it will of course require one to call setNormalBinding() after
setNormalArray(..) otherwise the setting would be lost. Alternatively
we just drop backwards compatibility and force uses to set the binding
directly on the array.
Providing backwards compatibility for normalize will more awkward as
custom normalize settings would require a direct setting to ArrayData,
however, I'm inclined to think dropping this element of backwards
compatibility won't effect too many users as I expect most users not
to be applying anything other than defaults.
The naming of Binding is something I'm note yet sure of, with the
introduction of AttribDivisor the binding becomes a bit less clearly
defined as well. Not sure what to make of this yet...
Robert.
___
osg-users mailing list
osg-users@lists.openscenegraph.org
http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] Deprecating vertex indices in osg::Geometry
Hi,
Just a coment. Deprecate inmediate mode is also and advantage w.r.t. mobile
devices. OpenGL_ES does not implement inmediate mode since its first
version, and display lists are no supported since OpenGL_ES 1.1. So it will
be good for those starting with ES and OpenSceneGraph.
Cheers.
2013/6/4 Robert Osfield robert.osfi...@gmail.com
Hi All,
I have now taken the next step and added the following methods to
osg::Array:
/** Specify whether the array data should be normalized by
OpenGL.*/
void setNormalize(bool normalize) { _normalize = normalize; }
/** Get whether the array data should be normalized by OpenGL.*/
bool getNormalize() const { return _normalize; }
/** Set hint to ask that the array data is passed via integer
or double, or normal setVertexAttribPointer function.*/
void setPreserveDataType(bool preserve) { _preserveDataType =
preserve; }
/** Get hint to ask that the array data is passed via integer
or double, or normal setVertexAttribPointer function.*/
bool getPreserveDataType() const { return _preserveDataType; }
/** Set the rate at which generic vertex attributes advance
during instanced rendering. Uses the glVertexAttribDivisor feature of
OpenGL 4.0*/
void setAttribDivisor(GLuint divisor) { _attribDivisor = divisor; }
/** Get the rate at which generic vertex attributes advance
during instanced rendering.*/
GLuint getAttribDivisor() const { return _attribDivisor; }
enum Binding
{
BIND_OFF=0,
BIND_OVERALL,
BIND_PER_PRIMITIVE_SET,
BIND_PER_VERTEX
};
/** Specify how this array should be passed to OpenGL.*/
void setBinding(Binding binding) { _binding = binding; }
/** Get how this array should be passed to OpenGL.*/
Binding getBinding() const { return _binding; }
At present all of them doing nothing at all apart from sitting there
looking pretty. The PreserveDataType and AttribDivisor are entirely
new and are intended to support new extensions being added to the OSG
- but their implementation will have to wait till the GeometryNew
refactor is further down the road.
The Normalize and Binding map directly to what has been available in
osg::Geometry::ArrayData's normalize and binding parameters. The
binding we've typically set via Geometry::setNormalBinding() etc.
which we could easily remap internally to array-setBinding(..), but
it will of course require one to call setNormalBinding() after
setNormalArray(..) otherwise the setting would be lost. Alternatively
we just drop backwards compatibility and force uses to set the binding
directly on the array.
Providing backwards compatibility for normalize will more awkward as
custom normalize settings would require a direct setting to ArrayData,
however, I'm inclined to think dropping this element of backwards
compatibility won't effect too many users as I expect most users not
to be applying anything other than defaults.
The naming of Binding is something I'm note yet sure of, with the
introduction of AttribDivisor the binding becomes a bit less clearly
defined as well. Not sure what to make of this yet...
Robert.
___
osg-users mailing list
osg-users@lists.openscenegraph.org
http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
--
Jordi Torres
___
osg-users mailing list
osg-users@lists.openscenegraph.org
http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] Deprecating vertex indices in osg::Geometry
On 4 June 2013 17:56, Jordi Torres jtorresfa...@gmail.com wrote: Just a coment. Deprecate inmediate mode is also and advantage w.r.t. mobile devices. OpenGL_ES does not implement inmediate mode since its first version, and display lists are no supported since OpenGL_ES 1.1. So it will be good for those starting with ES and OpenSceneGraph. Indeed. The cleaned up Geometry will also me smaller and faster too, certainly a help for mobile devices. Robert. ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] Deprecating vertex indices in osg::Geometry
Hi Robert, These changes sound good. I'm all for cleaning up the Geometry class. You mentioned implementing some sort of backwards compatibility for the serialization plugins. Does that mean existing osg files (osg, ive, osgb, etc...) that use indices will automatically fall back to the GeometryDeprecated class? I know the 3ds max exporter plugin generates vertex indices in some cases. Just curious how those existing files will be handled now. Cheers, Farshid On Tue, Jun 4, 2013 at 12:14 AM, Robert Osfield robert.osfi...@gmail.comwrote: Hi All, One of the changes I've discussed before, but as yet no made, is to simplify osg::Geometry so that it drops the index arrays that you can currently associated for vertex, normal, colour, texcoord arrays. These index arrays aren't supported by OpenGL, instead have to be emulated by breaking the primitive data up into small chunks and sending the vertex data and primitive data in small batches. This is very inefficient, so what having separate vertex indicies might seem like a good way of reducing vertex data overhead by sharing more it causes a big CPU overhead and results in lower performance. While there are clear warnings in the osg::Geometry header that this feature is deprecated and forces and OpenGL slow path I know it still gets used on occasion - against best practice advice. What I'd like now to do is drop the index arrays from osg::Geometry, and provide a GeometryDeprecated class for the those who need backwards compatibility. Potentially this GeometryDeprecated class could go in the core osg library, but I'm tempted to move it out into one of the optional NodeKits to reduce the size of the core osg library. To be clear OpenGL indexed primitives will still be fully support by osg::Geometry - DrawElement* has always been available and will remain, it's the long been the preferred way to pass your primitive data to OpenGL. Another step in making these changes that I'm considering is moving the normalize parameter from Geometry::ArrayData into osg::Array, with this change ArrayData ceases to have a role and can be removed further simplifying osg::Geometry. Thoughts? Robert. ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] Deprecating vertex indices in osg::Geometry
Hi Farshid, On 4 June 2013 18:27, Farshid Lashkari fla...@gmail.com wrote: You mentioned implementing some sort of backwards compatibility for the serialization plugins. Does that mean existing osg files (osg, ive, osgb, etc...) that use indices will automatically fall back to the GeometryDeprecated class? They will either have to create a GeometryDeprecated fill it in and pass it back, or create a GeometryDeprecated fill it in, convert this to the new Geometry and then pass it back. Ideally I'd like GeometryDeprecated placed outwith core osg but this would mean linking plugins with more libraries than what was required before so not ideal. For the time being I think we'll be stuck with GeometryDeprecated are part of the core osg. The automatic detection of indices and falling back to GeometryDeprecated is going to be awkward. Longer term we might be able to automatically map all the old Geometry usage with indices and per primitive bindings directly to the new Geometry and do away with the need to return GeometryDeprecated. The only problem would be if users back the assumption about their availability and rely upon indices or per primitive bindings in the scene graphs they load, perhaps an optional compile would work around this. I know the 3ds max exporter plugin generates vertex indices in some cases. Just curious how those existing files will be handled now. We really want all the plugins we can generating osg::Geometry based scene graphs rather than falling back to GeometryDeprecated. So it would be good to port these old plugins across. Robert. ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] Deprecating vertex indices in osg::Geometry
Hi All, I have now completed the removal of ArrayData container and all slow path support from GeometryNew. The size is now down to less than 2/3rd the size of the original Geometry.cpp. The largest bloat left is with helper functions for verifying and correcting bindings, but I now believe these should be placed into osgUtil as helper functions rather osg::Geometry so I'll likely remove these and the size will go small still. Robert. ___ osg-users mailing list osg-users@lists.openscenegraph.org http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] Deprecating vertex indices in osg::Geometry
robertosfield wrote:
The PreserveDataType and AttribDivisor are entirely
new and are intended to support new extensions being added to the OSG
- but their implementation will have to wait till the GeometryNew
refactor is further down the road.
This is great !
robertosfield wrote:
Alternatively we just drop backwards compatibility and
force uses to set the binding directly on the array.
I prefer this solution, sometimes changes are simply needed.
robertosfield wrote:
The naming of Binding is something I'm note yet sure of, with the
introduction of AttribDivisor the binding becomes a bit less clearly
defined as well. Not sure what to make of this yet...
Maybe a single parameter, something like get/setRate, using an enum :
enum Rate
{
RATE_OFF=0, // Attribute is never emitted =BIND_OFF -
attribDivisor = 0
RATE_SINGLESHOT // Attribute is emitted once =BIND_OVERALL -
attribDivisor = 0
RATE_PRIMITIVE_SET, // Attribute is emitted at each start of a
primitive set = BIND_PRIMITIVE_SET - attribDivisor = 0
RATE_VERTEX , // Attribute is emitted at each vertex =
BIND_VERTEX - attribDivisor = 0
RATE_INSTANCE_0, // Attribute is emitted at each vertex =
BIND_VERTEX - attribDivisor = 0
RATE_INSTANCE_1, // Attribute is emitted at each instance =
BIND_VERTEX - attribDivisor = 1
RATE_INSTANCE_2,// Attribute is emitted at each 2 instances =
BIND_VERTEX - attribDivisor = 2
...
RATE_INSTANCE_9,// Attribute is emitted at each 2 instances =
BIND_VERTEX - attribDivisor = 9
};
Of course, user can do setRate(RATE_INSTANCE_0 + 25) to have attribDivisor = 25
robertosfield wrote:
I have now completed the removal of ArrayData container and all slow
path support from GeometryNew. The size is now down to less than
2/3rd the size of the original Geometry.cpp. The largest bloat left
is with helper functions for verifying and correcting bindings, but I
now believe these should be placed into osgUtil as helper functions
rather osg::Geometry so I'll likely remove these and the size will go
small still.
Great work !
I have a question about immediate mode deprecation : does that mean the array
dispatchers are no more needed ? Do the osg::geometryNew class set
vertex/normal/textcoords... pointers directly on the osg::State ?
And about display list : I understand that it's too early to remove them, in
some situations it's still usefull. But maybe it's time to use it per default ?
--
Read this topic online here:
http://forum.openscenegraph.org/viewtopic.php?p=54456#54456
___
osg-users mailing list
osg-users@lists.openscenegraph.org
http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
Re: [osg-users] Deprecating vertex indices in osg::Geometry
Hi Aurelien,
On 4 June 2013 21:33, Aurelien Albert aurelien.alb...@alyotech.fr wrote:
robertosfield wrote:
The naming of Binding is something I'm note yet sure of, with the
introduction of AttribDivisor the binding becomes a bit less clearly
defined as well. Not sure what to make of this yet...
Maybe a single parameter, something like get/setRate, using an enum :
enum Rate
{
RATE_OFF=0, // Attribute is never emitted =BIND_OFF -
attribDivisor = 0
RATE_SINGLESHOT // Attribute is emitted once =BIND_OVERALL -
attribDivisor = 0
RATE_PRIMITIVE_SET, // Attribute is emitted at each start of a
primitive set = BIND_PRIMITIVE_SET - attribDivisor = 0
RATE_VERTEX , // Attribute is emitted at each vertex =
BIND_VERTEX - attribDivisor = 0
RATE_INSTANCE_0, // Attribute is emitted at each vertex =
BIND_VERTEX - attribDivisor = 0
RATE_INSTANCE_1, // Attribute is emitted at each instance =
BIND_VERTEX - attribDivisor = 1
RATE_INSTANCE_2,// Attribute is emitted at each 2 instances =
BIND_VERTEX - attribDivisor = 2
...
RATE_INSTANCE_9,// Attribute is emitted at each 2 instances =
BIND_VERTEX - attribDivisor = 9
};
Of course, user can do setRate(RATE_INSTANCE_0 + 25) to have attribDivisor =
25
I was wondering about the overlap between AttribDivisor and Binding,
combining the two would avoid issues with users setting
BIND_OVERALL/BIND_PER_PRIMITIVE_SET and AttribDivisor and expecting it
somehow to work.
Right now I feel that sticking with the same naming convention that
users have been used will make for a smoother transition to the new
osg::Geometry. Ideally I want most users just to recompile their code
and not have any problems.. Extending the enum Binding with
BIND_INSTANCE_DIVISOR_0 etc, would be one way, it's a bit cludgy but
would avoid the overlap in functionality issue.
I am busy with other work today so will have to return to this topic tomorrow.
Robert.
___
osg-users mailing list
osg-users@lists.openscenegraph.org
http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org
