Replace Spline With Ramp For Shader Parameters #1499
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,153 @@ | ||
| ////////////////////////////////////////////////////////////////////////// | ||
| // | ||
| // Copyright (c) 2025, Image Engine Design Inc. All rights reserved. | ||
| // | ||
| // Redistribution and use in source and binary forms, with or without | ||
| // modification, are permitted provided that the following conditions are | ||
| // met: | ||
| // | ||
| // * Redistributions of source code must retain the above copyright | ||
| // notice, this list of conditions and the following disclaimer. | ||
| // | ||
| // * Redistributions in binary form must reproduce the above copyright | ||
| // notice, this list of conditions and the following disclaimer in the | ||
| // documentation and/or other materials provided with the distribution. | ||
| // | ||
| // * Neither the name of Image Engine Design nor the names of any | ||
| // other contributors to this software may be used to endorse or | ||
| // promote products derived from this software without specific prior | ||
| // written permission. | ||
| // | ||
| // THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS | ||
| // IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, | ||
| // THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR | ||
| // PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR | ||
| // CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, | ||
| // EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, | ||
| // PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR | ||
| // PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF | ||
| // LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING | ||
| // NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS | ||
| // SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. | ||
| // | ||
| ////////////////////////////////////////////////////////////////////////// | ||
|
|
||
| #ifndef IECORE_RAMP_H | ||
| #define IECORE_RAMP_H | ||
|
|
||
| #include "IECore/Export.h" | ||
| #include "IECore/Spline.h" | ||
| #include "IECore/MessageHandler.h" | ||
|
|
||
| IECORE_PUSH_DEFAULT_VISIBILITY | ||
| #include "Imath/ImathColor.h" | ||
| IECORE_POP_DEFAULT_VISIBILITY | ||
|
|
||
| #include <map> | ||
|
|
||
| namespace IECore | ||
| { | ||
|
|
||
| // This lives outside the class because we don't want multiple incompatible templated versions of | ||
| // the same enum floating around | ||
| enum class RampInterpolation | ||
| { | ||
| Linear = 0, | ||
| CatmullRom = 1, | ||
| BSpline = 2, | ||
| MonotoneCubic = 3, | ||
| Constant = 4, | ||
| }; | ||
|
|
||
| /// A Ramp represents a spline-like curve as it is represented in a simple UI: with a set of independent | ||
| /// control points, and an interpolation type selected from RampInterpolation. | ||
|
Comment on lines
+62
to
+63
Member
There was a problem hiding this comment. I think it's worth mentioning that another motivation is removing the need to store duplicate endpoints to ensure interpolation to the final values. |
||
| /// | ||
| /// Rather than storing the lower level IECore::Spline*, we now store this Ramp type in shader networks, | ||
| /// and only convert to the lower level class with the evaluator() function when evaluation is needed. | ||
| template<typename X, typename Y> | ||
| class IECORE_EXPORT Ramp | ||
| { | ||
|
|
||
| public : | ||
|
Contributor
Author
There was a problem hiding this comment. Oh, right, I'd copied this from a Gaffer struct, and I guess I forgot to change the identation. I don't really remember why I switched it to a class, but I'm guessing it had private members at some point during the refactor - maybe it makes sense to keep it as a class in case it gets private members again at some point in the future? I'm also not quite sure why our indentation is different for structs, but that's a topic for another time, I'll just make this indentation consistent for now.
Member
There was a problem hiding this comment. I don't think our indentation is different for structs, at least not intentionally. For another time for sure, but I think maybe what I would like the most is to change the way we do things like so : |
||
|
|
||
| typedef X XType; | ||
| typedef Y YType; | ||
|
|
||
| typedef CubicBasis<XType> Basis; | ||
| typedef std::multimap<X, Y> PointContainer; | ||
| typedef typename PointContainer::value_type Point; | ||
|
|
||
|
Comment on lines
+73
to
+79
Member
There was a problem hiding this comment.
|
||
|
|
||
|
|
||
|
Comment on lines
+79
to
+81
Member
There was a problem hiding this comment. Vertical space is at a premium on most monitors, and three blank lines doesn't really convey anything more than one blank line would. |
||
| Ramp() : interpolation( RampInterpolation::CatmullRom ) | ||
| { | ||
| } | ||
|
|
||
| Ramp( const PointContainer &p, RampInterpolation i ) | ||
| : points( p ), interpolation( i ) | ||
| { | ||
| } | ||
|
|
||
| PointContainer points; | ||
| RampInterpolation interpolation; | ||
|
|
||
|
|
||
| // Convert to Cortex Spline | ||
| // In the future, IECore::Spline may be replaced with IECore::SplineEvaluator, and this | ||
| // function would be the only way to setup one. | ||
| IECore::Spline<X, Y> evaluator() const; | ||
|
|
||
| // In order to write shader parameters to renderers or USD, we need to convert Ramps to a convention | ||
| // that can be stored in raw OSL. Based on the arguments to the OSL spline() function, we've come up | ||
| // with a convention for storing a string basis and separate position and value arrays, where the | ||
| // the position and value arrays contain any duplicated end points necessary for the resulting OSL | ||
| // spline to cover the full range of the ramp. | ||
|
Comment on lines
+100
to
+104
Member
There was a problem hiding this comment. "We've come up with a convention" seems a bit misleading - the only convention here is the one defined by OSL's |
||
| // | ||
| // These functions convert to and from this convention ( and can also be used for converting from other | ||
| // similar conventions with some pre-processing ). | ||
| // | ||
| // NOTE: Some aspects of the convention we've picked are not actually very universal. We haven't | ||
|
Member
There was a problem hiding this comment. Again, the convention isn't ours - it's the one defined by OSL. It's worth documenting that some renderers do their own endpoint duplication for some reason, but OSL is the common standard here, and I don't want to imply that we're the ones deviating from it. |
||
| // found any shader's in the wild that actually expect duplicated end point ( aside from Gaffer's | ||
| // shader library ). Other shader authors seem to be implementing their own spline functions that don't | ||
| // require duplicated endpoints ( 3delight ), or put code to perform the duplication in their shader | ||
| // before calling spline ( PRMan ). We've stuck to duplicating the end points because unnecessarily | ||
| // duplicating endpoints doesn't cause any problems other than being a minor waste of performance. | ||
| // It seems like there's an argument that the best approach would be to put the endpoint duplication | ||
| // inside the shader like PRMan, but this is working fine for now. | ||
|
Comment on lines
+115
to
+116
Member
There was a problem hiding this comment. What would the argument be? Surely it's always going to be cheaper to do the duplication once outside the shader than it is to do it per sample inside the renderer? |
||
| void fromOSL( const std::string &basis, const std::vector<X> &positions, const std::vector<Y> &values, const std::string &identifier ); | ||
| void toOSL( std::string &basis, std::vector<X> &positions, std::vector<Y> &values ) const; | ||
|
|
||
| // If there are connections to a Ramp, and we convert it to the OSL convention with duplicated endpoints, | ||
| // then the connections will need to be offset to match - this defines the offset. | ||
| int oslAdaptorOffset() const; | ||
|
Comment on lines
+120
to
+122
Member
There was a problem hiding this comment. I recall we spent a little while debating if the OSL conversions should be provided by members of this class or by ShaderNetworkAlgo, and we went for this one because it had the monotone->bezier code in it already. But this |
||
|
|
||
| // In Cortex 10.6 and earlier, shader parameters were represented uing IECore::Spline*Data instead of | ||
| // IECore::Ramp*Data. This is used in converting SCC files to the new standard. | ||
| // \todo : This can probably be removed in the next major version - we're not actually aware of any | ||
| // significant users of Cortex who both use SCC files, and cache shaders, so this compatibility shim | ||
| // is only needed theoretically. | ||
| void fromDeprecatedSpline( const IECore::Spline<X, Y> &deprecated ); | ||
|
Comment on lines
+124
to
+129
Member
There was a problem hiding this comment. Likewise, this feels like it clutters up an otherwise very clean class, and I wonder if it wouldn't be better in ShaderNetworkAlgo or hidden in SceneCache (assuming we can achieve that without excessive contortion elsewhere). |
||
|
|
||
| bool operator==( const Ramp<X,Y> &rhs ) const; | ||
| bool operator!=( const Ramp<X,Y> &rhs ) const; | ||
|
|
||
| }; | ||
|
|
||
| using Rampff = Ramp<float, float>; | ||
| using RampfColor3f = Ramp<float, Imath::Color3f>; | ||
| using RampfColor4f = Ramp<float, Imath::Color4f>; | ||
|
|
||
| template<typename X, typename Y> | ||
| inline void murmurHashAppend( IECore::MurmurHash &h, const Ramp<X,Y> &data ) | ||
| { | ||
| h.append( data.interpolation ); | ||
| for ( auto &p : data.points ) | ||
| { | ||
| h.append( p.first ); | ||
| h.append( p.second ); | ||
| } | ||
| } | ||
|
|
||
| } // namespace IECore | ||
|
|
||
| #endif // IECORE_RAMP_H | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| ////////////////////////////////////////////////////////////////////////// | ||
| // | ||
| // Copyright (c) 2025, Image Engine Design Inc. All rights reserved. | ||
| // | ||
| // Redistribution and use in source and binary forms, with or without | ||
| // modification, are permitted provided that the following conditions are | ||
| // met: | ||
| // | ||
| // * Redistributions of source code must retain the above copyright | ||
| // notice, this list of conditions and the following disclaimer. | ||
| // | ||
| // * Redistributions in binary form must reproduce the above copyright | ||
| // notice, this list of conditions and the following disclaimer in the | ||
| // documentation and/or other materials provided with the distribution. | ||
| // | ||
| // * Neither the name of Image Engine Design nor the names of any | ||
| // other contributors to this software may be used to endorse or | ||
| // promote products derived from this software without specific prior | ||
| // written permission. | ||
| // | ||
| // THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS | ||
| // IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, | ||
| // THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR | ||
| // PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR | ||
| // CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, | ||
| // EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, | ||
| // PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR | ||
| // PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF | ||
| // LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING | ||
| // NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS | ||
| // SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. | ||
| // | ||
| ////////////////////////////////////////////////////////////////////////// | ||
|
|
||
| #ifndef IECORE_RAMPDATA_H | ||
| #define IECORE_RAMPDATA_H | ||
|
|
||
| #include "IECore/Ramp.h" | ||
| #include "IECore/TypedData.h" | ||
|
|
||
| namespace IECore | ||
| { | ||
|
|
||
| // Ramp data types. | ||
|
|
||
| IECORE_DECLARE_TYPEDDATA( RampffData, Rampff, void, SharedDataHolder ) | ||
| IECORE_DECLARE_TYPEDDATA( RampfColor3fData, RampfColor3f, void, SharedDataHolder ) | ||
| IECORE_DECLARE_TYPEDDATA( RampfColor4fData, RampfColor4f, void, SharedDataHolder ) | ||
|
|
||
| } | ||
|
|
||
| #endif // IECORE_RAMPDATA_H |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| ////////////////////////////////////////////////////////////////////////// | ||
| // | ||
| // Copyright (c) 2025, Image Engine Design Inc. All rights reserved. | ||
| // | ||
| // Redistribution and use in source and binary forms, with or without | ||
| // modification, are permitted provided that the following conditions are | ||
| // met: | ||
| // | ||
| // * Redistributions of source code must retain the above copyright | ||
| // notice, this list of conditions and the following disclaimer. | ||
| // | ||
| // * Redistributions in binary form must reproduce the above copyright | ||
| // notice, this list of conditions and the following disclaimer in the | ||
| // documentation and/or other materials provided with the distribution. | ||
| // | ||
| // * Neither the name of Image Engine Design nor the names of any | ||
| // other contributors to this software may be used to endorse or | ||
| // promote products derived from this software without specific prior | ||
| // written permission. | ||
| // | ||
| // THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS | ||
| // IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, | ||
| // THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR | ||
| // PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR | ||
| // CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, | ||
| // EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, | ||
| // PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR | ||
| // PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF | ||
| // LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING | ||
| // NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS | ||
| // SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. | ||
| // | ||
| ////////////////////////////////////////////////////////////////////////// | ||
|
|
||
| #ifndef IECOREPYTHON_RAMPBINDING_H | ||
| #define IECOREPYTHON_RAMPBINDING_H | ||
|
|
||
| #include "IECorePython/Export.h" | ||
|
|
||
| namespace IECorePython | ||
| { | ||
|
|
||
| IECOREPYTHON_API void bindRamp(); | ||
|
|
||
| } | ||
|
|
||
| #endif // IECOREPYTHON_RAMPBINDING_H |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd be open to omitting this -
despatchTypedData()is deprecated so it might even be considered counter-productive to support new types in it.