Hi Robert, hi Erik,

On 20/03/12 22:19 , Robert Osfield wrote:
> On 19 March 2012 22:20, Erik den Dekker <[email protected]> wrote:
>> using clang I received several warnings like:
>>
>> [ 8%] Building CXX object src/osg/CMakeFiles/osg.dir/State.cpp.o
>> /Users/erik/Projects/OpenSceneGraph/OpenSceneGraph-SVN/src/osg/State.cpp:1166:57:
>> warning: use of logical '||' with constant operand
>> [-Wconstant-logical-operand]
>> _isVertexBufferObjectSupported = OSG_GLES2_FEATURES || OSG_GL3_FEATURES ||
>> osg::isGLExtensionSupported(_contextID,"GL_ARB_vertex_buffer_object");
>> ^ ~~~~~~~~~~~~~~~~
>> /Users/erik/Projects/OpenSceneGraph/OpenSceneGraph-SVN/src/osg/State.cpp:1166:57:
>> note: use '|' for a bitwise operation
>> _isVertexBufferObjectSupported = OSG_GLES2_FEATURES || OSG_GL3_FEATURES ||
>> osg::isGLExtensionSupported(_contextID,"GL_ARB_vertex_buffer_object");
>> ^~
>> |
>> 1 warning generated.
> 
> The clang warning you've "fixed" is pretty dumb and the changes are
> extensive and for me are less readable.  Coding wise there are
> completely pointless, the original code is in no way ambiguous, so I'm
> rejecting the submission.  If you want to fix the warning then the
> most appropriate thing to do would be to disable the stupid warning.

While I'm would rarely dismiss a warning as 'dumb' (they're always helpful to 
some degree)
I do agree that it is a bit cryptic in this context (but very much helpful in 
the
dxfEntity case).

How about replacing this:

_isVertexBufferObjectSupported = OSG_GLES2_FEATURES || OSG_GL3_FEATURES ||
osg::isGLExtensionSupported(_contextID,"GL_ARB_vertex_buffer_object");

with either this:

_isVertexBufferObjectSupported = OSG_GLES2_FEATURES == 1 || OSG_GL3_FEATURES == 
1 ||
osg::isGLExtensionSupported(_contextID,"GL_ARB_vertex_buffer_object");

or that:

_isVertexBufferObjectSupported =
osg::isGLExtensionSupported(_contextID,"GL_ARB_vertex_buffer_object") ||
OSG_GLES2_FEATURES || OSG_GL3_FEATURES;

Shuts up the compiler, (mostly) keeps the readability, and doesn't introduce a 
const to
shadow the #define.

Cheers,
/ulrich
_______________________________________________
osg-submissions mailing list
[email protected]
http://lists.openscenegraph.org/listinfo.cgi/osg-submissions-openscenegraph.org

Reply via email to