Skip to content

Commit

Permalink
Typed[Object]Plug : Inline getValue() call
Browse files Browse the repository at this point in the history
This also means declaring AtomicFormatPlug's specialisations publicly so that we don't get the publicly visible default `getValue()` implementation being compiled instead. And the part I don't understand : we need to link `libGafferImage` to far more libraries now, to avoid undefined symbols related to AtomicFormatPlug with MSVC. It seems that the mere existence of the inlined `getValue()` - even though it is invalidated by the specialization - is enough to make MSVC greedily do a bunch of implicit template instantiations that it wasn't doing before, and which ultimately depend on `GafferImage::Format`.
  • Loading branch information
johnhaddon committed Jul 11, 2023
1 parent 4ac716f commit c7e4e53
Show file tree
Hide file tree
Showing 8 changed files with 115 additions and 16 deletions.
8 changes: 4 additions & 4 deletions SConstruct
Original file line number Diff line number Diff line change
Expand Up @@ -1093,7 +1093,7 @@ libraries = {

"GafferSceneTest" : {
"envAppends" : {
"LIBS" : [ "Gaffer", "GafferDispatch", "GafferScene", "IECoreScene$CORTEX_LIB_SUFFIX" ],
"LIBS" : [ "Gaffer", "GafferDispatch", "GafferScene", "GafferImage", "IECoreScene$CORTEX_LIB_SUFFIX" ],
},
"pythonEnvAppends" : {
"LIBS" : [ "Gaffer", "GafferDispatch", "GafferBindings", "GafferScene", "GafferSceneTest" ],
Expand All @@ -1106,7 +1106,7 @@ libraries = {
"LIBS" : [ "Gaffer", "GafferUI", "GafferImage", "GafferImageUI", "GafferScene", "Iex$IMATH_LIB_SUFFIX", "IECoreGL$CORTEX_LIB_SUFFIX", "IECoreImage$CORTEX_LIB_SUFFIX", "IECoreScene$CORTEX_LIB_SUFFIX" ],
},
"pythonEnvAppends" : {
"LIBS" : [ "IECoreGL$CORTEX_LIB_SUFFIX", "GafferBindings", "GafferScene", "GafferUI", "GafferImageUI", "GafferSceneUI", "IECoreScene$CORTEX_LIB_SUFFIX" ],
"LIBS" : [ "IECoreGL$CORTEX_LIB_SUFFIX", "GafferBindings", "GafferScene", "GafferImage", "GafferUI", "GafferImageUI", "GafferSceneUI", "IECoreScene$CORTEX_LIB_SUFFIX" ],
},
},

Expand Down Expand Up @@ -1246,7 +1246,7 @@ libraries = {

"GafferOSLUI" : {
"envAppends" : {
"LIBS" : [ "Gaffer", "GafferUI", "GafferOSL" ],
"LIBS" : [ "Gaffer", "GafferImage", "GafferUI", "GafferOSL" ],
},
"pythonEnvAppends" : {
"LIBS" : [ "IECoreGL$CORTEX_LIB_SUFFIX", "GafferBindings", "GafferScene", "GafferUI", "GafferImageUI", "GafferOSLUI" ],
Expand Down Expand Up @@ -1351,7 +1351,7 @@ libraries = {

"GafferUSD" : {
"envAppends" : {
"LIBS" : [ "Gaffer", "GafferDispatch", "GafferScene", "IECoreScene$CORTEX_LIB_SUFFIX" ] + [ "${USD_LIB_PREFIX}" + x for x in ( [ "sdf", "arch", "tf", "vt" ] if not env["USD_MONOLITHIC"] else [ "usd_ms" ] ) ],
"LIBS" : [ "Gaffer", "GafferDispatch", "GafferScene", "GafferImage", "IECoreScene$CORTEX_LIB_SUFFIX" ] + [ "${USD_LIB_PREFIX}" + x for x in ( [ "sdf", "arch", "tf", "vt" ] if not env["USD_MONOLITHIC"] else [ "usd_ms" ] ) ],
# USD includes "at least one deprecated or antiquated header", so we
# have to drop our usual strict warning levels.
"CXXFLAGS" : [ "-Wno-deprecated" if env["PLATFORM"] != "win32" else "/wd4996" ],
Expand Down
2 changes: 2 additions & 0 deletions include/Gaffer/TypedObjectPlug.h
Original file line number Diff line number Diff line change
Expand Up @@ -181,3 +181,5 @@ IE_CORE_DECLAREPTR( AtomicCompoundDataPlug );
IE_CORE_DECLAREPTR( PathMatcherDataPlug );

} // namespace Gaffer

#include "TypedObjectPlug.inl"
48 changes: 48 additions & 0 deletions include/Gaffer/TypedObjectPlug.inl
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
//////////////////////////////////////////////////////////////////////////
//
// Copyright (c) 2023, Cinesite VFX Ltd. 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 John Haddon 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.
//
//////////////////////////////////////////////////////////////////////////

#pragma once

namespace Gaffer
{

template<class T>
inline typename TypedObjectPlug<T>::ConstValuePtr TypedObjectPlug<T>::getValue( const IECore::MurmurHash *precomputedHash ) const
{
return getObjectValue<ValueType>( precomputedHash );
}

} // namespace Gaffer
6 changes: 0 additions & 6 deletions include/Gaffer/TypedObjectPlugImplementation.h
Original file line number Diff line number Diff line change
Expand Up @@ -95,12 +95,6 @@ void TypedObjectPlug<T>::setValue( ConstValuePtr value )
setObjectValue( value );
}

template<class T>
typename TypedObjectPlug<T>::ConstValuePtr TypedObjectPlug<T>::getValue( const IECore::MurmurHash *precomputedHash ) const
{
return getObjectValue<ValueType>( precomputedHash );
}

template<class T>
void TypedObjectPlug<T>::setFrom( const ValuePlug *other )
{
Expand Down
2 changes: 2 additions & 0 deletions include/Gaffer/TypedPlug.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,3 +107,5 @@ IE_CORE_DECLAREPTR( AtomicBox3fPlug );
IE_CORE_DECLAREPTR( AtomicBox2iPlug );

} // namespace Gaffer

#include "Gaffer/TypedPlug.inl"
48 changes: 48 additions & 0 deletions include/Gaffer/TypedPlug.inl
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
//////////////////////////////////////////////////////////////////////////
//
// Copyright (c) 2023, Cinesite VFX Ltd. 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 John Haddon 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.
//
//////////////////////////////////////////////////////////////////////////

#pragma once

namespace Gaffer
{

template<typename T>
inline T TypedPlug<T>::getValue( const IECore::MurmurHash *precomputedHash ) const
{
return getObjectValue<DataType>( precomputedHash )->readable();
}

} // namespace Gaffer
6 changes: 0 additions & 6 deletions include/Gaffer/TypedPlugImplementation.h
Original file line number Diff line number Diff line change
Expand Up @@ -95,12 +95,6 @@ void TypedPlug<T>::setValue( const T &value )
setObjectValue( new DataType( value ) );
}

template<class T>
T TypedPlug<T>::getValue( const IECore::MurmurHash *precomputedHash ) const
{
return getObjectValue<DataType>( precomputedHash )->readable();
}

template<class T>
void TypedPlug<T>::setFrom( const ValuePlug *other )
{
Expand Down
11 changes: 11 additions & 0 deletions include/GafferImage/AtomicFormatPlug.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,3 +49,14 @@ using AtomicFormatPlug = Gaffer::TypedPlug<GafferImage::Format>;
IE_CORE_DECLAREPTR( AtomicFormatPlug );

} // namespace GafferImage

namespace Gaffer
{

template<>
GafferImage::Format GafferImage::AtomicFormatPlug::getValue( const IECore::MurmurHash *precomputedHash ) const;

template<>
IECore::MurmurHash GafferImage::AtomicFormatPlug::hash() const;

} // namespace Gaffer

0 comments on commit c7e4e53

Please sign in to comment.