pgraph: use bit flags instead of incremental enum for ShaderAttrib flags

The flags in the enum are actual bit flags instead of indices. This allows for setting/clearing a mask of flags in one go, rather than having to set each flag in its own function call. Existing code shouldn't be affected. Closes #688
panda3d · Dec 11, 2019 · 48ff3ae · 48ff3ae
1 parent 636c3d0
commit 48ff3ae
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 13 deletions.
diff --git a/panda/src/pgraph/shaderAttrib.I b/panda/src/pgraph/shaderAttrib.I
@@ -93,8 +93,8 @@ get_instance_count() const {
  *
  */
 INLINE bool ShaderAttrib::
-get_flag(int index) const {
-  return (_flags & (1<<index)) ? true:false;
+get_flag(int flag) const {
+  return (_flags & flag) != 0;
 }
 
 /**

diff --git a/panda/src/pgraph/shaderAttrib.cxx b/panda/src/pgraph/shaderAttrib.cxx
@@ -172,13 +172,12 @@ clear_shader() const {
 CPT(RenderAttrib) ShaderAttrib::
 set_flag(int flag, bool value) const {
   ShaderAttrib *result = new ShaderAttrib(*this);
-  int bit = 1<<flag;
   if (value) {
-    result->_flags |= bit;
+    result->_flags |= flag;
   } else {
-    result->_flags &= ~bit;
+    result->_flags &= ~flag;
   }
-  result->_has_flags |= bit;
+  result->_has_flags |= flag;
   return return_new(result);
 }
 
@@ -188,9 +187,8 @@ set_flag(int flag, bool value) const {
 CPT(RenderAttrib) ShaderAttrib::
 clear_flag(int flag) const {
   ShaderAttrib *result = new ShaderAttrib(*this);
-  int bit = 1<<flag;
-  result->_flags &= ~bit;
-  result->_has_flags &= ~bit;
+  result->_flags &= ~flag;
+  result->_has_flags &= ~flag;
   return return_new(result);
 }
 

diff --git a/panda/src/pgraph/shaderAttrib.h b/panda/src/pgraph/shaderAttrib.h
@@ -47,10 +47,10 @@ class EXPCL_PANDA_PGRAPH ShaderAttrib: public RenderAttrib {
   static CPT(RenderAttrib) make_default();
 
   enum {
-    F_disable_alpha_write = 0,  // Suppress writes to color buffer alpha channel.
-    F_subsume_alpha_test  = 1,  // Shader promises to subsume the alpha test using TEXKILL
-    F_hardware_skinning   = 2,  // Shader needs pre-animated vertices
-    F_shader_point_size   = 3,  // Shader provides point size, not RenderModeAttrib
+    F_disable_alpha_write = 1 << 0,  // Suppress writes to color buffer alpha channel.
+    F_subsume_alpha_test  = 1 << 1,  // Shader promises to subsume the alpha test using TEXKILL
+    F_hardware_skinning   = 1 << 2,  // Shader needs pre-animated vertices
+    F_shader_point_size   = 1 << 3,  // Shader provides point size, not RenderModeAttrib
   };
 
   INLINE bool               has_shader() const;

diff --git a/tests/pgraph/test_shaderattrib.py b/tests/pgraph/test_shaderattrib.py
@@ -0,0 +1,42 @@
+from panda3d import core
+
+
+def test_shaderattrib_flags():
+    # Ensure the old single-flag behavior still works
+
+    shattr = core.ShaderAttrib.make()
+
+    # Make sure we have the flag
+    shattr = shattr.set_flag(core.ShaderAttrib.F_hardware_skinning, True)
+    assert shattr.get_flag(core.ShaderAttrib.F_hardware_skinning)
+
+    # Make sure we don't have a flag that we didn't set
+    assert not shattr.get_flag(core.ShaderAttrib.F_subsume_alpha_test)
+
+    # Clear it, we should not longer have the flag
+    shattr = shattr.clear_flag(core.ShaderAttrib.F_hardware_skinning)
+    assert not shattr.get_flag(core.ShaderAttrib.F_hardware_skinning)
+
+    # Set a flag to false, we shouldn't have it
+    shattr = shattr.set_flag(core.ShaderAttrib.F_hardware_skinning, False)
+    assert not shattr.get_flag(core.ShaderAttrib.F_hardware_skinning)
+
+    # Ensure the new behavior works
+    shattr = core.ShaderAttrib.make()
+
+    # Make sure we have the flags
+    shattr = shattr.set_flag(core.ShaderAttrib.F_hardware_skinning | core.ShaderAttrib.F_subsume_alpha_test, True)
+    assert shattr.get_flag(core.ShaderAttrib.F_hardware_skinning | core.ShaderAttrib.F_subsume_alpha_test)
+
+    # Make sure we don't have a flag that we didn't set
+    assert not shattr.get_flag(core.ShaderAttrib.F_shader_point_size)
+    # ...group of flags we didn't set
+    assert not shattr.get_flag(core.ShaderAttrib.F_disable_alpha_write | core.ShaderAttrib.F_shader_point_size)
+
+    # Make sure they clear correctly
+    shattr = shattr.clear_flag(core.ShaderAttrib.F_hardware_skinning | core.ShaderAttrib.F_subsume_alpha_test)
+    assert not shattr.get_flag(core.ShaderAttrib.F_hardware_skinning | core.ShaderAttrib.F_subsume_alpha_test)
+
+    # Set group to false
+    shattr = shattr.set_flag(core.ShaderAttrib.F_hardware_skinning | core.ShaderAttrib.F_subsume_alpha_test, False)
+    assert not shattr.get_flag(core.ShaderAttrib.F_hardware_skinning | core.ShaderAttrib.F_subsume_alpha_test)