AttribNodeRegistry, ordering in BamReader::complete_pointers()

panda/src/gobj/geomVertexArrayData.cxx

@@ -64,7 +64,6 @@ ALLOC_DELETED_CHAIN_DEF(GeomVertexArrayDataHandle);
 GeomVertexArrayData::
 GeomVertexArrayData() : SimpleLruPage(0) {
   _contexts = NULL;
-  _endian_reversed = false;
 
   // Can't put it in the LRU until it has been read in and made valid.
 }
@@ -97,7 +96,6 @@ GeomVertexArrayData(const GeomVertexArrayFormat *array_format,
   CLOSE_ITERATE_ALL_STAGES(_cycler);
 
   _contexts = NULL;
-  _endian_reversed = false;
 
   set_lru_size(0);
   nassertv(_array_format->is_registered());
@@ -116,7 +114,6 @@ GeomVertexArrayData(const GeomVertexArrayData &copy) :
   _cycler(copy._cycler)
 {
   _contexts = NULL;
-  _endian_reversed = false;
 
   copy.mark_used_lru();
 
@@ -511,11 +508,14 @@ finalize(BamReader *manager) {
   manager->change_pointer(_array_format, new_array_format);
   _array_format = new_array_format;
 
-  if (_endian_reversed) {
-    // Now is the time to endian-reverse the data.
-    VertexDataBuffer new_buffer(cdata->_buffer.get_size());
-    reverse_data_endianness(new_buffer.get_write_pointer(), cdata->_buffer.get_read_pointer(true), cdata->_buffer.get_size());
-    cdata->_buffer.swap(new_buffer);
+  PT(BamAuxData) aux_data = (BamAuxData *)manager->get_aux_data(this, "");
+  if (aux_data != (BamAuxData *)NULL) {
+    if (aux_data->_endian_reversed) {
+      // Now is the time to endian-reverse the data.
+      VertexDataBuffer new_buffer(cdata->_buffer.get_size());
+      reverse_data_endianness(new_buffer.get_write_pointer(), cdata->_buffer.get_read_pointer(true), cdata->_buffer.get_size());
+      cdata->_buffer.swap(new_buffer);
+    }
   }
 
   set_lru_size(cdata->_buffer.get_size());
@@ -632,13 +632,15 @@ fillin(DatagramIterator &scan, BamReader *manager, void *extra_data) {
     scan.skip_bytes(size);
   }
 
+  bool endian_reversed = false;
+
   if (manager->get_file_endian() != BE_native) {
     // For non-native endian files, we have to convert the data.  
 
     if (array_data->_array_format == (GeomVertexArrayFormat *)NULL) {
       // But we can't do that until we've completed the _array_format
       // pointer, which tells us how to convert it.
-      array_data->_endian_reversed = true;
+      endian_reversed = true;
     } else {
       // Since we have the _array_format pointer now, we can reverse
       // it immediately (and we should, to support threaded CData
@@ -649,6 +651,12 @@ fillin(DatagramIterator &scan, BamReader *manager, void *extra_data) {
     }
   }
 
+  if (endian_reversed) {
+    PT(BamAuxData) aux_data = new BamAuxData;
+    aux_data->_endian_reversed = endian_reversed;
+    manager->set_aux_data(array_data, "", aux_data);
+  }
+
   array_data->set_lru_size(_buffer.get_size());
 
   _modified = Geom::get_next_modified();

panda/src/gobj/geomVertexArrayData.h

@@ -36,6 +36,7 @@
 #include "simpleLru.h"
 #include "vertexDataBuffer.h"
 #include "config_gobj.h"
+#include "bamReader.h"
 
 class PreparedGraphicsObjects;
 class VertexBufferContext;
@@ -135,9 +136,13 @@ private:
   typedef pmap<PreparedGraphicsObjects *, VertexBufferContext *> Contexts;
   Contexts *_contexts;
 
-  // This is only used when reading from a bam file.  It is set true
-  // to indicate the data must be endian-reversed in finalize().
-  bool _endian_reversed;
+  // This data is only needed when reading from a bam file.
+  class BamAuxData : public BamReader::AuxData {
+  public:
+    // set true to indicate the data must be endian-reversed in
+    // finalize().
+    bool _endian_reversed;
+  };
 
   // This is the data that must be cycled between pipeline stages.
   class EXPCL_PANDA CData : public CycleData {

panda/src/pgraph/Sources.pp

@@ -17,6 +17,7 @@
     alphaTestAttrib.I alphaTestAttrib.h \  
     ambientLight.I ambientLight.h \
     antialiasAttrib.I antialiasAttrib.h \
+    attribNodeRegistry.I attribNodeRegistry.h \
     attribSlots.h attribSlots.I \
     audioVolumeAttrib.I audioVolumeAttrib.h \
     auxSceneData.I auxSceneData.h \
@@ -127,6 +128,7 @@
     alphaTestAttrib.cxx \  
     ambientLight.cxx \
     antialiasAttrib.cxx \
+    attribNodeRegistry.cxx \
     attribSlots.cxx \
     audioVolumeAttrib.cxx \
     auxSceneData.cxx \
@@ -232,6 +234,7 @@
     alphaTestAttrib.I alphaTestAttrib.h \  
     ambientLight.I ambientLight.h \
     antialiasAttrib.I antialiasAttrib.h \
+    attribNodeRegistry.I attribNodeRegistry.h \
     attribSlots.h attribSlots.I \
     audioVolumeAttrib.I audioVolumeAttrib.h \
     auxSceneData.I auxSceneData.h \

panda/src/pgraph/attribNodeRegistry.I

@@ -0,0 +1,69 @@
+// Filename: attribNodeRegistry.I
+// Created by:  drose (07Jul07)
+//
+////////////////////////////////////////////////////////////////////
+//
+// PANDA 3D SOFTWARE
+// Copyright (c) 2001 - 2004, Disney Enterprises, Inc.  All rights reserved
+//
+// All use of this software is subject to the terms of the Panda 3d
+// Software license.  You should have received a copy of this license
+// along with this source code; you will also find a current copy of
+// the license at http://etc.cmu.edu/panda3d/docs/license/ .
+//
+// To contact the maintainers of this program write to
+// [email protected] .
+//
+////////////////////////////////////////////////////////////////////
+
+
+////////////////////////////////////////////////////////////////////
+//     Function: AttribNodeRegistry::get_global_ptr
+//       Access: Published, Static
+//  Description: 
+////////////////////////////////////////////////////////////////////
+INLINE AttribNodeRegistry *AttribNodeRegistry::
+get_global_ptr() {
+  if (_global_ptr == (AttribNodeRegistry *)NULL) {
+    make_global_ptr();
+  }
+  return _global_ptr;
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: AttribNodeRegistry::Entry::Constructor
+//       Access: Public
+//  Description: 
+////////////////////////////////////////////////////////////////////
+INLINE AttribNodeRegistry::Entry::
+Entry(const NodePath &node) :
+  _type(node.node()->get_type()),
+  _name(node.get_name()),
+  _node(node)
+{
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: AttribNodeRegistry::Entry::Constructor
+//       Access: Public
+//  Description: 
+////////////////////////////////////////////////////////////////////
+INLINE AttribNodeRegistry::Entry::
+Entry(TypeHandle type, const string &name) :
+  _type(type),
+  _name(name)
+{
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: AttribNodeRegistry::Entry::operator <
+//       Access: Public
+//  Description: 
+////////////////////////////////////////////////////////////////////
+INLINE bool AttribNodeRegistry::Entry::
+operator < (const Entry &other) const {
+  if (_type != other._type) {
+    return _type < other._type;
+  }
+  return _name < other._name;
+}

panda/src/pgraph/attribNodeRegistry.cxx

@@ -0,0 +1,275 @@
+// Filename: attribNodeRegistry.cxx
+// Created by:  drose (07Jul07)
+//
+////////////////////////////////////////////////////////////////////
+//
+// PANDA 3D SOFTWARE
+// Copyright (c) 2001 - 2004, Disney Enterprises, Inc.  All rights reserved
+//
+// All use of this software is subject to the terms of the Panda 3d
+// Software license.  You should have received a copy of this license
+// along with this source code; you will also find a current copy of
+// the license at http://etc.cmu.edu/panda3d/docs/license/ .
+//
+// To contact the maintainers of this program write to
+// [email protected] .
+//
+////////////////////////////////////////////////////////////////////
+
+#include "attribNodeRegistry.h"
+#include "mutexHolder.h"
+
+AttribNodeRegistry * TVOLATILE AttribNodeRegistry::_global_ptr;
+
+////////////////////////////////////////////////////////////////////
+//     Function: AttribNodeRegistry::Constructor
+//       Access: Protected
+//  Description: 
+////////////////////////////////////////////////////////////////////
+AttribNodeRegistry::
+AttribNodeRegistry() {
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: AttribNodeRegistry::add_node
+//       Access: Published
+//  Description: Adds the indicated NodePath to the registry.  The
+//               name and type of the node are noted at the time of
+//               this call; if the name changes later, it will not
+//               update the registry index.
+//
+//               The NodePath must reference some kind of an attribute
+//               node, such as a LightNode or a PlaneNode.  When bam
+//               files that reference an attribute node of the same
+//               type and the same name are loaded, they will quietly
+//               be redirected to reference this NodePath.
+////////////////////////////////////////////////////////////////////
+void AttribNodeRegistry::
+add_node(const NodePath &attrib_node) {
+  nassertv(!attrib_node.is_empty());
+  MutexHolder holder(_lock);
+  _entries.insert(Entry(attrib_node));
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: AttribNodeRegistry::remove_node
+//       Access: Published
+//  Description: Removes the indicated NodePath from the registry.
+//               The name of the node must not have changed since the
+//               matching call to add_node(), or it will not be
+//               successfully removed.
+//
+//               Returns true if the NodePath is found and removed,
+//               false if it is not found (for instance, because the
+//               name has changed).
+////////////////////////////////////////////////////////////////////
+bool AttribNodeRegistry::
+remove_node(const NodePath &attrib_node) {
+  nassertr(!attrib_node.is_empty(), false);
+  MutexHolder holder(_lock);
+  Entries::iterator ei = _entries.find(Entry(attrib_node));
+  if (ei != _entries.end()) {
+    _entries.erase(ei);
+    return true;
+  }
+  return false;
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: AttribNodeRegistry::lookup_node
+//       Access: Published
+//  Description: Looks up the indicated NodePath in the registry.  If
+//               there is a node already in the registry with the
+//               matching name and type, returns that NodePath
+//               instead; otherwise, returns the original NodePath.
+////////////////////////////////////////////////////////////////////
+NodePath AttribNodeRegistry::
+lookup_node(const NodePath &orig_node) const {
+  nassertr(!orig_node.is_empty(), orig_node);
+
+  MutexHolder holder(_lock);
+  Entries::const_iterator ei = _entries.find(Entry(orig_node));
+  if (ei != _entries.end()) {
+    return (*ei)._node;
+  }
+  return orig_node;
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: AttribNodeRegistry::get_num_nodes
+//       Access: Published
+//  Description: Returns the total number of nodes in the registry.
+////////////////////////////////////////////////////////////////////
+int AttribNodeRegistry::
+get_num_nodes() const {
+  MutexHolder holder(_lock);
+  return _entries.size();
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: AttribNodeRegistry::get_node
+//       Access: Published
+//  Description: Returns the nth NodePath recorded in the registry.
+////////////////////////////////////////////////////////////////////
+NodePath AttribNodeRegistry::
+get_node(int n) const {
+  MutexHolder holder(_lock);
+  nassertr(n >= 0 && n < (int)_entries.size(), NodePath());
+  return _entries[n]._node;
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: AttribNodeRegistry::get_node_type
+//       Access: Published
+//  Description: Returns the type of the nth node, as recorded in the
+//               registry.
+////////////////////////////////////////////////////////////////////
+TypeHandle AttribNodeRegistry::
+get_node_type(int n) const {
+  MutexHolder holder(_lock);
+  nassertr(n >= 0 && n < (int)_entries.size(), TypeHandle::none());
+  return _entries[n]._type;
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: AttribNodeRegistry::get_node_name
+//       Access: Published
+//  Description: Returns the name of the nth node, as recorded in the
+//               registry.  This will be the node name as it was at
+//               the time the node was recorded; if the node has
+//               changed names since then, this will still return the
+//               original name.
+////////////////////////////////////////////////////////////////////
+string AttribNodeRegistry::
+get_node_name(int n) const {
+  MutexHolder holder(_lock);
+  nassertr(n >= 0 && n < (int)_entries.size(), string());
+  return _entries[n]._name;
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: AttribNodeRegistry::find_node
+//       Access: Published
+//  Description: Returns the index number of the indicated NodePath in
+//               the registry (assuming its name hasn't changed since
+//               it was recorded in the registry), or -1 if the
+//               NodePath cannot be found (for instance, because its
+//               name has changed).
+////////////////////////////////////////////////////////////////////
+int AttribNodeRegistry::
+find_node(const NodePath &attrib_node) const {
+  nassertr(!attrib_node.is_empty(), -1);
+  MutexHolder holder(_lock);
+  Entries::const_iterator ei = _entries.find(Entry(attrib_node));
+  if (ei != _entries.end()) {
+    return ei - _entries.begin();
+  }
+  return -1;
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: AttribNodeRegistry::find_node
+//       Access: Published
+//  Description: Returns the index number of the node with the
+//               indicated type and name in the registry, or -1 if
+//               there is no such node in the registry.
+////////////////////////////////////////////////////////////////////
+int AttribNodeRegistry::
+find_node(TypeHandle type, const string &name) const {
+  MutexHolder holder(_lock);
+  Entries::const_iterator ei = _entries.find(Entry(type, name));
+  if (ei != _entries.end()) {
+    return ei - _entries.begin();
+  }
+  return -1;
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: AttribNodeRegistry::remove_node
+//       Access: Published
+//  Description: Removes the nth node from the registry.
+////////////////////////////////////////////////////////////////////
+void AttribNodeRegistry::
+remove_node(int n) {
+  MutexHolder holder(_lock);
+  nassertv(n >= 0 && n < (int)_entries.size());
+  _entries.erase(_entries.begin() + n);
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: AttribNodeRegistry::clear
+//       Access: Published
+//  Description: Removes all nodes from the registry.
+////////////////////////////////////////////////////////////////////
+void AttribNodeRegistry::
+clear() {
+  MutexHolder holder(_lock);
+  _entries.clear();
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: AttribNodeRegistry::output
+//       Access: Published
+//  Description: 
+////////////////////////////////////////////////////////////////////
+void AttribNodeRegistry::
+output(ostream &out) const {
+  MutexHolder holder(_lock);
+
+  typedef pmap<TypeHandle, int> Counts;
+  Counts counts;
+
+  Entries::const_iterator ei;
+  for (ei = _entries.begin(); ei != _entries.end(); ++ei) {
+    TypeHandle type = (*ei)._type;
+    Counts::iterator ci = counts.insert(Counts::value_type(type, 0)).first;
+    ++((*ci).second);
+  }
+
+  out << _entries.size() << " entries";
+
+  if (!counts.empty()) {
+    Counts::iterator ci = counts.begin();
+    out << " (" << (*ci).first << ":" << (*ci).second;
+    ++ci;
+    while (ci != counts.end()) {
+      out << ", " << (*ci).first << ":" << (*ci).second;
+      ++ci;
+    }
+    out << ")";
+  }
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: AttribNodeRegistry::write
+//       Access: Published
+//  Description: 
+////////////////////////////////////////////////////////////////////
+void AttribNodeRegistry::
+write(ostream &out) const {
+  MutexHolder holder(_lock);
+
+  Entries::const_iterator ei;
+  for (ei = _entries.begin(); ei != _entries.end(); ++ei) {
+    const Entry &entry = (*ei);
+    out << entry._type << ", \"" << entry._name << "\": " << entry._node
+        << "\n";
+  }
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: AttribNodeRegistry::make_global_ptr
+//       Access: Private, Static
+//  Description: 
+////////////////////////////////////////////////////////////////////
+void AttribNodeRegistry::
+make_global_ptr() {
+  AttribNodeRegistry *ptr = new AttribNodeRegistry;
+  void *result = AtomicAdjust::compare_and_exchange_ptr
+    ((void * TVOLATILE &)_global_ptr, (void *)NULL, (void *)ptr);
+  if (result != NULL) {
+    // Someone else got there first.
+    delete ptr;
+  }
+  assert(_global_ptr != (AttribNodeRegistry *)NULL);
+}

panda/src/pgraph/attribNodeRegistry.h

@@ -0,0 +1,91 @@
+// Filename: attribNodeRegistry.h
+// Created by:  drose (07Jul07)
+//
+////////////////////////////////////////////////////////////////////
+//
+// PANDA 3D SOFTWARE
+// Copyright (c) 2001 - 2004, Disney Enterprises, Inc.  All rights reserved
+//
+// All use of this software is subject to the terms of the Panda 3d
+// Software license.  You should have received a copy of this license
+// along with this source code; you will also find a current copy of
+// the license at http://etc.cmu.edu/panda3d/docs/license/ .
+//
+// To contact the maintainers of this program write to
+// [email protected] .
+//
+////////////////////////////////////////////////////////////////////
+
+#ifndef ATTRIBNODEREGISTRY_H
+#define ATTRIBNODEREGISTRY_H
+
+#include "pandabase.h"
+#include "nodePath.h"
+#include "ordered_vector.h"
+#include "pmutex.h"
+
+////////////////////////////////////////////////////////////////////
+//       Class : AttribNodeRegistry
+// Description : This global object records NodePaths that are
+//               referenced by scene graph attribs, such as
+//               ClipPlaneAttribs and LightAttribs.
+//
+//               Its primary purpose is to unify attribs that are
+//               loaded in from bam files.  Attrib nodes are
+//               identified by name and type; when a bam file that
+//               contains references to some attrib nodes is loaded,
+//               those nodes are first looked up here in the
+//               AttribNodeRegistry.  If there is a match (by name and
+//               node type), the identified node is used instead of
+//               the node referenced within the bam file itself.
+////////////////////////////////////////////////////////////////////
+class EXPCL_PANDA AttribNodeRegistry {
+protected:
+  AttribNodeRegistry();
+
+PUBLISHED:
+  void add_node(const NodePath &attrib_node);
+  bool remove_node(const NodePath &attrib_node);
+  NodePath lookup_node(const NodePath &orig_node) const;
+
+  int get_num_nodes() const;
+  NodePath get_node(int n) const;
+  TypeHandle get_node_type(int n) const;
+  string get_node_name(int n) const;
+
+  int find_node(const NodePath &attrib_node) const;
+  int find_node(TypeHandle type, const string &name) const;
+  void remove_node(int n);
+  void clear();
+
+  void output(ostream &out) const;
+  void write(ostream &out) const;
+
+  INLINE static AttribNodeRegistry *get_global_ptr();
+
+private:
+  static void make_global_ptr();
+
+  class Entry {
+  public:
+    INLINE Entry(const NodePath &node);
+    INLINE Entry(TypeHandle type, const string &name);
+    INLINE bool operator < (const Entry &other) const;
+
+    TypeHandle _type;
+    string _name;
+    NodePath _node;
+  };
+
+  typedef ov_set<Entry> Entries;
+  Entries _entries;
+
+  Mutex _lock;
+
+  static AttribNodeRegistry * TVOLATILE _global_ptr;
+};
+
+#include "attribNodeRegistry.I"
+
+#endif
+

panda/src/pgraph/clipPlaneAttrib.cxx

@@ -25,6 +25,7 @@
 #include "datagram.h"
 #include "datagramIterator.h"
 #include "config_pgraph.h"
+#include "attribNodeRegistry.h"
 
 CPT(RenderAttrib) ClipPlaneAttrib::_empty_attrib;
 CPT(RenderAttrib) ClipPlaneAttrib::_all_off_attrib;
@@ -937,7 +938,9 @@ write_datagram(BamWriter *manager, Datagram &dg) {
   for (fi = _off_planes.begin(); fi != _off_planes.end(); ++fi) {
     NodePath plane = (*fi);
 
-    // Whoops, we don't have a way to write out a NodePath right now.
+    // Since we can't write out a NodePath, we write out just the
+    // plain PandaNode.  The user can use the AttribNodeRegistry on
+    // re-read if there is any ambiguity that needs to be resolved.
     manager->write_pointer(dg, plane.node());
   }
 
@@ -961,28 +964,46 @@ write_datagram(BamWriter *manager, Datagram &dg) {
 int ClipPlaneAttrib::
 complete_pointers(TypedWritable **p_list, BamReader *manager) {
   int pi = RenderAttrib::complete_pointers(p_list, manager);
+  AttribNodeRegistry *areg = AttribNodeRegistry::get_global_ptr();
 
   Planes::iterator ci = _off_planes.begin();
   while (ci != _off_planes.end()) {
     PandaNode *node;
     DCAST_INTO_R(node, p_list[pi++], pi);
     NodePath np(node);
-    (*ci) = np;
+    (*ci) = areg->lookup_node(np);
     ++ci;
   }
+  _off_planes.sort();
 
   ci = _on_planes.begin();
   while (ci != _on_planes.end()) {
     PandaNode *node;
     DCAST_INTO_R(node, p_list[pi++], pi);
     NodePath np(node);
-    (*ci) = np;
+    (*ci) = areg->lookup_node(np);
     ++ci;
   }
+  _on_planes.sort();
 
   return pi;
 }
 
+////////////////////////////////////////////////////////////////////
+//     Function: ClipPlaneAttrib::require_fully_complete
+//       Access: Public, Virtual
+//  Description: Some objects require all of their nested pointers to
+//               have been completed before the objects themselves can
+//               be completed.  If this is the case, override this
+//               method to return true, and be careful with circular
+//               references (which would make the object unreadable
+//               from a bam file).
+////////////////////////////////////////////////////////////////////
+bool ClipPlaneAttrib::
+require_fully_complete() const {
+  return true;
+}
+
 ////////////////////////////////////////////////////////////////////
 //     Function: ClipPlaneAttrib::make_from_bam
 //       Access: Protected, Static
@@ -1026,19 +1047,19 @@ fillin(DatagramIterator &scan, BamReader *manager) {
 
   int num_off_planes = scan.get_uint16();
     
-  // Push back a NULL pointer for each off Plane for now, until
-  // we get the actual list of pointers later in complete_pointers().
+  // Push back an empty NodePath for each off Plane for now, until we
+  // get the actual list of pointers later in complete_pointers().
   _off_planes.reserve(num_off_planes);
   int i;
   for (i = 0; i < num_off_planes; i++) {
     manager->read_pointer(scan);
-    _off_planes.push_back(NULL);
+    _off_planes.push_back(NodePath());
   }
     
   int num_on_planes = scan.get_uint16();
   _on_planes.reserve(num_on_planes);
   for (i = 0; i < num_on_planes; i++) {
     manager->read_pointer(scan);
-    _on_planes.push_back(NULL);
+    _on_planes.push_back(NodePath());
   }
 }

panda/src/pgraph/clipPlaneAttrib.h

@@ -127,6 +127,7 @@ public:
   static void register_with_read_factory();
   virtual void write_datagram(BamWriter *manager, Datagram &dg);
   virtual int complete_pointers(TypedWritable **plist, BamReader *manager);
+  virtual bool require_fully_complete() const;
 
 protected:
   static TypedWritable *make_from_bam(const FactoryParams &params);

panda/src/pgraph/lightAttrib.cxx

@@ -27,6 +27,7 @@
 #include "datagram.h"
 #include "datagramIterator.h"
 #include "config_pgraph.h"
+#include "attribNodeRegistry.h"
 
 CPT(RenderAttrib) LightAttrib::_empty_attrib;
 CPT(RenderAttrib) LightAttrib::_all_off_attrib;
@@ -916,7 +917,9 @@ write_datagram(BamWriter *manager, Datagram &dg) {
   for (fi = _off_lights.begin(); fi != _off_lights.end(); ++fi) {
     NodePath light = (*fi);
 
-    // Whoops, we don't have a way to write out a NodePath right now.
+    // Since we can't write out a NodePath, we write out just the
+    // plain PandaNode.  The user can use the AttribNodeRegistry on
+    // re-read if there is any ambiguity that needs to be resolved.
     manager->write_pointer(dg, light.node());
   }
 
@@ -940,28 +943,46 @@ write_datagram(BamWriter *manager, Datagram &dg) {
 int LightAttrib::
 complete_pointers(TypedWritable **p_list, BamReader *manager) {
   int pi = RenderAttrib::complete_pointers(p_list, manager);
+  AttribNodeRegistry *areg = AttribNodeRegistry::get_global_ptr();
 
   Lights::iterator ci = _off_lights.begin();
   while (ci != _off_lights.end()) {
     PandaNode *node;
     DCAST_INTO_R(node, p_list[pi++], pi);
     NodePath np(node);
-    (*ci) = np;
+    (*ci) = areg->lookup_node(np);
     ++ci;
   }
+  _off_lights.sort();
 
   ci = _on_lights.begin();
   while (ci != _on_lights.end()) {
     PandaNode *node;
     DCAST_INTO_R(node, p_list[pi++], pi);
     NodePath np(node);
-    (*ci) = np;
+    (*ci) = areg->lookup_node(np);
     ++ci;
   }
+  _on_lights.sort();
 
   return pi;
 }
 
+////////////////////////////////////////////////////////////////////
+//     Function: LightAttrib::require_fully_complete
+//       Access: Public, Virtual
+//  Description: Some objects require all of their nested pointers to
+//               have been completed before the objects themselves can
+//               be completed.  If this is the case, override this
+//               method to return true, and be careful with circular
+//               references (which would make the object unreadable
+//               from a bam file).
+////////////////////////////////////////////////////////////////////
+bool LightAttrib::
+require_fully_complete() const {
+  return true;
+}
+
 ////////////////////////////////////////////////////////////////////
 //     Function: LightAttrib::make_from_bam
 //       Access: Protected, Static
@@ -1005,19 +1026,19 @@ fillin(DatagramIterator &scan, BamReader *manager) {
 
   int num_off_lights = scan.get_uint16();
     
-  // Push back a NULL pointer for each off Light for now, until
-  // we get the actual list of pointers later in complete_pointers().
+  // Push back an empty NodePath for each off Light for now, until we
+  // get the actual list of pointers later in complete_pointers().
   _off_lights.reserve(num_off_lights);
   int i;
   for (i = 0; i < num_off_lights; i++) {
     manager->read_pointer(scan);
-    _off_lights.push_back(NULL);
+    _off_lights.push_back(NodePath());
   }
     
   int num_on_lights = scan.get_uint16();
   _on_lights.reserve(num_on_lights);
   for (i = 0; i < num_on_lights; i++) {
     manager->read_pointer(scan);
-    _on_lights.push_back(NULL);
+    _on_lights.push_back(NodePath());
   }
 }

panda/src/pgraph/lightAttrib.h

@@ -125,6 +125,7 @@ public:
   static void register_with_read_factory();
   virtual void write_datagram(BamWriter *manager, Datagram &dg);
   virtual int complete_pointers(TypedWritable **plist, BamReader *manager);
+  virtual bool require_fully_complete() const;
 
 protected:
   static TypedWritable *make_from_bam(const FactoryParams &params);

panda/src/pgraph/pgraph_composite1.cxx

@@ -1,6 +1,7 @@
 #include "accumulatedAttribs.cxx"
 #include "ambientLight.cxx"
 #include "antialiasAttrib.cxx"
+#include "attribNodeRegistry.cxx"
 #include "audioVolumeAttrib.cxx"
 #include "auxSceneData.cxx"
 #include "attribSlots.cxx"

panda/src/putil/bamReader.I

@@ -173,6 +173,15 @@ get_datagram(Datagram &datagram) {
   return _source->get_datagram(datagram);
 }
 
+////////////////////////////////////////////////////////////////////
+//     Function: BamReader::AuxData::Constructor
+//       Access: Public
+//  Description: 
+////////////////////////////////////////////////////////////////////
+INLINE BamReader::AuxData::
+AuxData() {
+}
+
 ////////////////////////////////////////////////////////////////////
 //     Function: parse_params
 //       Access: Private, Static

panda/src/putil/bamReader.cxx

@@ -135,20 +135,44 @@ init() {
 ////////////////////////////////////////////////////////////////////
 //     Function: BamReader::set_aux_data
 //       Access: Public
-//  Description: Associates an arbitrary pointer to the bam reader
-//               with the indicated name.  The name is an arbitrary
-//               user-defined key to access the data later.  This data
-//               is typically queried by objects reading themselves
-//               from the bam file; this is intended to provide some
-//               context information to objects in the bam file.  Set
-//               the aux data to NULL to remove it.
+//  Description: Associates an arbitrary block of data with the
+//               indicated object (or NULL), and the indicated name.
+//
+//               This is intended to provide a place for temporary
+//               storage for objects reading themselves from the bam
+//               file.  To use it, inherit from BamReader::AuxData and
+//               store whatever data you like there.  Then associate
+//               your AuxData with the object as it is being read with
+//               set_aux_data().  You may later set the aux data to
+//               NULL to remove it; or it will automatically be
+//               removed (and deleted) after finalize() is called for
+//               the object in question.
+//
+//               If the TypedWritable pointer is NULL, the the aux
+//               data is stored globally for the BamReader in general.
+//               This pointer is available to any bam objects, and
+//               will not be automatically removed until the BamReader
+//               itself destructs.
+//
+//               In either case, the name is just an arbitrary
+//               user-defined key.  If there is already a data pointer
+//               stored for the obj/name pair, that data pointer will
+//               be replaced (and deleted).
 ////////////////////////////////////////////////////////////////////
 void BamReader::
-set_aux_data(const string &name, void *data) {
+set_aux_data(TypedWritable *obj, const string &name, BamReader::AuxData *data) {
   if (data == (void *)NULL) {
-    _aux_data.erase(name);
+    AuxDataTable::iterator ti = _aux_data.find(obj);
+    if (ti != _aux_data.end()) {
+      AuxDataNames &names = (*ti).second;
+      names.erase(name);
+      if (names.empty()) {
+        _aux_data.erase(ti);
+      }
+    }
+
   } else {
-    _aux_data[name] = data;
+    _aux_data[obj][name] = data;
   }
 }
 
@@ -157,15 +181,19 @@ set_aux_data(const string &name, void *data) {
 //       Access: Public
 //  Description: Returns the pointer previously associated with the
 //               bam reader by a previous call to set_aux_data(), or
-//               NULL if the data with the indicated key has not been
-//               set.
+//               NULL if data with the indicated key has not been set.
 ////////////////////////////////////////////////////////////////////
-void *BamReader::
-get_aux_data(const string &name) const {
-  AuxData::const_iterator di = _aux_data.find(name);
-  if (di != _aux_data.end()) {
-    return (*di).second;
+BamReader::AuxData *BamReader::
+get_aux_data(TypedWritable *obj, const string &name) const {
+  AuxDataTable::const_iterator ti = _aux_data.find(obj);
+  if (ti != _aux_data.end()) {
+    const AuxDataNames &names = (*ti).second;
+    AuxDataNames::const_iterator ni = names.find(name);
+    if (ni != names.end()) {
+      return (*ni).second;
+    }
   }
+
   return NULL;
 }
 
@@ -272,8 +300,31 @@ resolve() {
   do {
     all_completed = true;
     any_completed_this_pass = false;
-
-    // Walk through all the objects that still have outstanding pointers.
+    
+    // First do the PipelineCycler objects.    
+    CyclerPointers::iterator ci;
+    ci = _cycler_pointers.begin();
+    while (ci != _cycler_pointers.end()) {
+      PipelineCyclerBase *cycler = (*ci).first;
+      const vector_int &pointer_ids = (*ci).second;
+      
+      if (resolve_cycler_pointers(cycler, pointer_ids)) {
+        // Now remove this cycler from the list of things that need
+        // completion.  We have to be a bit careful when deleting things
+        // from the STL container while we are traversing it.
+        CyclerPointers::iterator old = ci;
+        ++ci;
+        _cycler_pointers.erase(old);
+        any_completed_this_pass = true;
+        
+      } else {
+        // Couldn't complete this cycler yet; it'll wait for next time.
+        ++ci;
+        all_completed = false;
+      }
+    }
+    
+    // Now do the main objects.
     ObjectPointers::iterator oi;
     oi = _object_pointers.begin();
     while (oi != _object_pointers.end()) {
@@ -320,31 +371,8 @@ resolve() {
         all_completed = false;
       }
     }
-  } while (!all_completed && any_completed_this_pass);
 
-  // Also do the PipelineCycler objects.  We only need to try these
-  // once, since they don't depend on each other.
-
-  CyclerPointers::iterator ci;
-  ci = _cycler_pointers.begin();
-  while (ci != _cycler_pointers.end()) {
-    PipelineCyclerBase *cycler = (*ci).first;
-    const vector_int &pointer_ids = (*ci).second;
-    
-    if (resolve_cycler_pointers(cycler, pointer_ids)) {
-      // Now remove this cycler from the list of things that need
-      // completion.  We have to be a bit careful when deleting things
-      // from the STL container while we are traversing it.
-      CyclerPointers::iterator old = ci;
-      ++ci;
-      _cycler_pointers.erase(old);
-      
-    } else {
-      // Couldn't complete this cycler yet; it'll wait for next time.
-      ++ci;
-      all_completed = false;
-    }
-  }
+  } while (!all_completed && any_completed_this_pass);
 
   if (all_completed) {
     finalize();
@@ -396,6 +424,12 @@ change_pointer(const TypedWritable *orig_pointer, const TypedWritable *new_point
     return false;
   }
 
+  if (bam_cat.is_spam()) {
+    bam_cat.spam()
+      << "change_pointer(" << (void *)orig_pointer << ", " 
+      << (void *)new_pointer << ") (" << new_pointer->get_type() << ")\n";
+  }
+
   const vector_int &old_refs = (*ci).second;
   vector_int &new_refs = _created_objs_by_pointer[new_pointer];
 
@@ -638,10 +672,14 @@ read_cdata(DatagramIterator &scan, PipelineCyclerBase &cycler,
 ////////////////////////////////////////////////////////////////////
 void BamReader::
 register_finalize(TypedWritable *whom) {
-  if (whom == TypedWritable::Null) {
-    bam_cat.error() << "Can't register a null pointer to finalize!" << endl;
-    return;
+  nassertv(whom != (TypedWritable *)NULL);
+
+  if (bam_cat.is_spam()) {
+    bam_cat.spam()
+      << "register_finalize(" << (void *)whom << ") (" << whom->get_type()
+      << ")\n";
   }
+
   _finalize_list.insert(whom);
 }
 
@@ -698,6 +736,11 @@ finalize_now(TypedWritable *whom) {
   Finalize::iterator fi = _finalize_list.find(whom);
   if (fi != _finalize_list.end()) {
     _finalize_list.erase(fi);
+    if (bam_cat.is_spam()) {
+      bam_cat.spam()
+        << "finalizing " << (void *)whom << " (" << whom->get_type()
+        << ")\n";
+    }
     whom->finalize(this);
   }
 }
@@ -994,7 +1037,7 @@ p_read_object() {
     } else {
       if (bam_cat.is_spam()) {
         bam_cat.spam()
-          << "Read a " << object->get_type() << "\n";
+          << "Read a " << object->get_type() << ": " << (void *)object << "\n";
       }
     }
   }
@@ -1018,12 +1061,17 @@ resolve_object_pointers(TypedWritable *object, const vector_int &pointer_ids) {
   // given object until we have *all* outstanding pointers for
   // that object.
   bool is_complete = true;
+
+  // Some objects further require all of their nested objects to have
+  // been completed (i.e. complete_pointers has been called on each
+  // nested object) before they can themselves be completed.
+  bool require_fully_complete = object->require_fully_complete();
+
   vector_typedWritable references;
-  
+
   vector_int::const_iterator pi;
   for (pi = pointer_ids.begin(); pi != pointer_ids.end() && is_complete; ++pi) {
     int child_id = (*pi);
-    
     if (child_id == 0) {
       // A NULL pointer is a NULL pointer.
       references.push_back((TypedWritable *)NULL);
@@ -1042,8 +1090,15 @@ resolve_object_pointers(TypedWritable *object, const vector_int &pointer_ids) {
           is_complete = false;
 
         } else {
-          // Yes, it's ready.
-          references.push_back(child_obj._ptr);
+          if (require_fully_complete && 
+              _object_pointers.find(child_id) != _object_pointers.end()) {
+            // It's not yet complete itself.
+            is_complete = false;
+            
+          } else {
+            // Yes, it's ready.
+            references.push_back(child_obj._ptr);
+          }
         }
       }
     }
@@ -1051,6 +1106,14 @@ resolve_object_pointers(TypedWritable *object, const vector_int &pointer_ids) {
       
   if (is_complete) {
     // Okay, here's the complete list of pointers for you!
+    nassertr(references.size() == pointer_ids.size(), false);
+
+    if (bam_cat.is_spam()) {
+      bam_cat.spam()
+        << "complete_pointers for " << (void *)object
+        << " (" << object->get_type() << "), " << references.size()
+        << " pointers.\n";
+    }
     int num_completed = object->complete_pointers(&references[0], this);
     if (num_completed != (int)references.size()) {
       bam_cat.warning()
@@ -1058,6 +1121,13 @@ resolve_object_pointers(TypedWritable *object, const vector_int &pointer_ids) {
         << " of " << references.size() << " pointers.\n";
     }
     return true;
+
+  } else {
+    if (bam_cat.is_spam()) {
+      bam_cat.spam()
+        << "not ready: complete_pointers for " << (void *)object
+        << " (" << object->get_type() << ")\n";
+    }
   }
 
   return false;
@@ -1115,6 +1185,11 @@ resolve_cycler_pointers(PipelineCyclerBase *cycler,
   if (is_complete) {
     // Okay, here's the complete list of pointers for you!
     CycleData *cdata = cycler->write(Thread::get_current_thread());
+    if (bam_cat.is_spam()) {
+      bam_cat.spam()
+        << "complete_pointers for CycleData object " << (void *)cdata
+        << "\n";
+    }
     int num_completed = cdata->complete_pointers(&references[0], this);
     cycler->release_write(cdata);
     if (num_completed != (int)references.size()) {
@@ -1147,8 +1222,42 @@ finalize() {
     TypedWritable *object = (*fi);
     nassertv(object != (TypedWritable *)NULL);
     _finalize_list.erase(fi);
+    if (bam_cat.is_spam()) {
+      bam_cat.spam()
+        << "finalizing " << (void *)object << " (" << object->get_type()
+        << ")\n";
+    }
     object->finalize(this);
-
+    _aux_data.erase(object);
     fi = _finalize_list.begin();
   }
+
+  // Now clear the aux data of all objects, except the NULL object.
+  if (!_aux_data.empty()) {
+    AuxDataTable::iterator ti = _aux_data.find((TypedWritable *)NULL);
+
+    if (ti != _aux_data.end()) {
+      if (_aux_data.size() > 1) {
+        // Move the NULL data to the new table; remove the rest.
+        AuxDataTable new_aux_data;
+        AuxDataTable::iterator nti = 
+          new_aux_data.insert(AuxDataTable::value_type(NULL, AuxDataNames())).first;
+        (*nti).second.swap((*ti).second);
+        _aux_data.swap(new_aux_data);
+      }
+    } else {
+      // There's no NULL data; clear the whole table.
+      _aux_data.clear();
+    }
+  }
 }
+
+////////////////////////////////////////////////////////////////////
+//     Function: BamReader::AuxData::Destructor
+//       Access: Public, Virtual
+//  Description: 
+////////////////////////////////////////////////////////////////////
+BamReader::AuxData::
+~AuxData() {
+}
+

panda/src/putil/bamReader.h

@@ -98,8 +98,10 @@ public:
 
   bool init();
 
-  void set_aux_data(const string &name, void *data);
-  void *get_aux_data(const string &name) const;
+  class AuxData;
+  void set_aux_data(TypedWritable *obj, const string &name, AuxData *data);
+  AuxData *get_aux_data(TypedWritable *obj, const string &name) const;
+
   INLINE const Filename &get_filename() const;
   
   TypedWritable *read_object();
@@ -115,10 +117,6 @@ public:
   INLINE int get_current_major_ver() const;
   INLINE int get_current_minor_ver() const;
 
-  // This special TypeHandle is written to the bam file to indicate an
-  // object id is no longer needed.
-  static TypeHandle _remove_flag;
-
 public:
   // Functions to support classes that read themselves from the Bam.
 
@@ -161,6 +159,20 @@ private:
 
   INLINE bool get_datagram(Datagram &datagram);
 
+public:
+  // This special TypeHandle is written to the bam file to indicate an
+  // object id is no longer needed.
+  static TypeHandle _remove_flag;
+
+  // Inherit from this class to piggyback additional temporary data on
+  // the bamReader (via set_aux_data() and get_aux_data()) for any
+  // particular objects during the bam reading process.
+  class AuxData : public ReferenceCount {
+  public:
+    INLINE AuxData();
+    virtual ~AuxData();
+  };
+
 private:
   static WritableFactory *_factory;
 
@@ -234,8 +246,9 @@ private:
   static NewTypes _new_types;
 
   // This is used in support of set_aux_data() and get_aux_data().
-  typedef phash_map<string, void *, string_hash> AuxData;
-  AuxData _aux_data;
+  typedef pmap<string, PT(AuxData)> AuxDataNames;
+  typedef phash_map<TypedWritable *, AuxDataNames, pointer_hash> AuxDataTable;
+  AuxDataTable _aux_data;
 
   int _file_major, _file_minor;
   BamEndian _file_endian;

panda/src/putil/typedWritable.cxx

@@ -85,7 +85,21 @@ int TypedWritable::
 complete_pointers(TypedWritable **, BamReader *) {
   return 0;
 }
-          
+
+////////////////////////////////////////////////////////////////////
+//     Function: TypedWritable::require_fully_complete
+//       Access: Public, Virtual
+//  Description: Some objects require all of their nested pointers to
+//               have been completed before the objects themselves can
+//               be completed.  If this is the case, override this
+//               method to return true, and be careful with circular
+//               references (which would make the object unreadable
+//               from a bam file).
+////////////////////////////////////////////////////////////////////
+bool TypedWritable::
+require_fully_complete() const {
+  return false;
+}
 
 ////////////////////////////////////////////////////////////////////
 //     Function: TypedWritable::finalize

panda/src/putil/typedWritable.h

@@ -50,6 +50,7 @@ public:
   virtual void write_datagram(BamWriter *, Datagram &);
 
   virtual int complete_pointers(TypedWritable **p_list, BamReader *manager);
+  virtual bool require_fully_complete() const;
 
   virtual void finalize(BamReader *manager);