On Thu, 2010-07-08 at 22:38 +0200, Wojciech Lewandowski wrote: > Hi Guys, > > Just to make it clear. Solution present in StandardShadowMap works on all > shader sources even those substituted by user.
The function in question here, searchAndReplaceShaderSource, works on all shader sources, but requires explicitly matching ALL of the string and then replacing it. > Your solution seems to be > custom tuned to ShadowMap shaders. yes, however the function we added, searchTwoAndReplaceShaderSource, is not specific to ShadowMap and is in essence a variation of searchAndReplaceShaderSource. It also works on all shader sources, regardless of who originated them. The difference we take two strings that build the context of the place that should replaced. When used as envisioned, the actual number in between the two bits of context is irrelevant. Both sides of the context and the number (or any other string) that is between them is replaced. > I don't think using it for > StandardShadowMap would be appropriate. Why not? It does the same work, though slightly more robustly. > I for example use LispSM with my own > shaders and do search and replace of shadow units on them. > One needs to remeber that there might be more than few texture coord units > used in the shaders. Of course. That was part of the issue with using the other function. As I mentioned before that is part of why we wrote something different. (as an aside, I also was considering the case the brought me to this change in the first place, getting shadows working with the shaders used in osgCal) > Its really impossible to selectively replace proper > coords without actual knowledge which stages are used for shadows and which > are not. I agree partially. The coder does have to know what they are doing, i.e. what portions of the code produce the effect to be changed. However, they don't have to know everything, at least not the currently used shader unit. > Which means I don't believe you could make it invariant of initial > shadow map unit used in shader sources in general case. As we use context on both sides of the value and ignore the value in determining the position to change, the method is invariant to the initial shadow map unit. Of course, this assumes that the caller correctly builds their context strings. I have just reverified that the function does work as expected, using a series of changes to the texture unit (4, 17, 1). All of that being said, there is a way to use the same function as available in StandardShadowMap, but in involves more in depth changes to the ShadowMap/SoftShadowMap classes. Basically, because of the way the no-base-texture vs. base-texture versions of the shaders are selected, we have to use our new, more complex method. If we add a separate method to select which shader to use (which btw. the method to do this is currently undocumented, non-obvious, and non-robust, as it is dependent on the order people call functions), then, when we are careful, we always know what unit is the previous unit and we can replace it cleanly with the simpler function. Finally, I just figure out that the no-base-texture case does not currently work at all in the svn version! When I "select" it, and, then, change the texture unit to something other than 0 (using our function) it does work. The "fake texture" that is generated causes it not to work on texture unit 0. Maybe this means the best thing is to actually clean up the ShadowMap/SoftShadowMap first. Unfortunately, I don't currently have time to undertake this. Cheers, -Kris > > Cheers, > Wojtek Lewandowski > > -------------------------------------------------- > From: "Kristopher J. Blom" <[email protected]> > Sent: Thursday, July 08, 2010 5:47 PM > To: "OpenSceneGraph Submissions" <[email protected]> > Subject: Re: [osg-submissions] ShadowMap TextureUnit fix > > > > > The function is not duplicated, but adapted for the needs of that case. > > > > Initially, I discarded doing as you suggested because of the large > > differences in the implementations of the two. Having reevaluated it > > just now, I think it would be possible to bring the new function up a > > level and use it instead of the one currently in StandardShadowMap. > > > > The new function similarly uses context before and after the value, but > > is also invariant to the number that is presently there (one reason we > > changed the function). However, it is also more expensive to use. Since > > one wouldn't expect to be calling the functions other than during setup, > > that might not be an issue. > > > > -Kris > > > > > > On Thu, 2010-07-08 at 11:16 -0400, Jean-Sébastien Guay wrote: > >> Hi Kristopher, > >> > >> > To do this, it > >> > introduces a protected (private) function that does the actual search > >> > and replace. The implementation is based on the one found in > >> > StandardShadowMap. > >> > >> I didn't look at the implementation, but did you duplicate the functions > >> from StandardShadowMap, or did you move them up the class hierarchy > >> (into ShadowTechnique for example, even perhaps static if possible) so > >> both StandardShadowMap and {Soft}ShadowMap can use them? I would prefer > >> the latter of course... > >> > >> Just wondering, good change in any case. > >> > >> J-S > > > > _______________________________________________ > > 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
