Hi Robert, 2013/5/28 Robert Osfield <[email protected]> > > > In the future I'm inclined to evolve osgShadow towards being based on > ViewDepedentShadowMap as it's base rather than the current collection > of curious and not so useful shadow implementations. This evolution > will really depend upon how ViewDependentShadowMap pans out, but for > now that's the direction I'm optimitic about heading. This will be > something for OSG-3.4 rather than OSG-3.2 though, so for now focus has > to be on getting ViewDependentShadowMap the best it can be - which > means something based upon your latest revisions. > > In my own opinion, VDSM should be a replacement of all other techniques except those with completely different theories (e.g., shadow volume). Future developments and new algorithms may also work based on the VDSM class. That's why I add a series of callbacks to let future developers control every step of a shadow mapping pipeline.
> ViewDepdentShadowMapUtils is really just a collection of Utils, that > just happen to only be used by ViewDependentShadowMap right now. So.. > I'm inclined to just simplify the header and .cpp name. to Utils or > something else as generic. > > It's OK to me. I put those classes to an independent file just because the original ViewDependShadowMap source file is too large and hard for reading. Some of these util code may also be of help for users who are interested in creating their own shadow map classes. > The name change of ShadowSettings::setMaximumShadowMapDistance() to > ShadowSettings::setMaximumShadowMapNearFarDistance() suggests a change > in this parameters role, but I'm not yet clear how that could involved > Near and Far Distance as both Near and Far can't ever be the same > distance, this is also a change in the API that really has to be > justified, and in this instance if the name really has to change the I > can't see any reason to have Near in the name as it's misleading. > > Maybe the method name could be changed to setMaximumShadowMapFarDistanceRelatively(). This is to limit the far distance not to exceed near value + relativeDistance. > The removal of ShadowSettings::setShadowMapProjectionHint() and > setMultipleShadowMapHint(), replacing them with be bit mask > setShadowMapTechniqueHints() both breaks the API and is limiting for > no gain of functionality. While both the ShadowMapProjectionHint and > MultipleShadowMapHint currently only have two options each I do not > envisage this to be the case going forward. There are lots of > different ways to multiple shadow maps - parallel split is only one, > and even the projection side has papers that use custom projections, > given this it's a backwards step to force a bit mask that has no > capability of handling the introduction of new enum's and back end > functionality that they map to. My suggestion would be to revert this > to the original method names. > > I agree that the original method names should be kept. The new setShadowMapTechniqueHints() doesn't provide more information at present. And for future newly added shadow techniques like VSM, ESM and so on, it is better to directly derive from the VDSM class than to add new hints for setShadowMapTechniqueHints(). > Implementation wise I'm still digesting the changes. I'll let you > comment on the above before I dive back in for another pass. > Thanks Robert. Will be catching up with you for the updating of the VDSM code. Wang Rui
_______________________________________________ osg-submissions mailing list [email protected] http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org
