shaderpipeline: Fix issues with memory cache timestamp validation

panda/src/gobj/shader.cxx

@@ -669,7 +669,7 @@ check_modified() const {
   for (const LinkedModule &linked_module : _modules) {
     const ShaderModule *module = linked_module._module.get_read_pointer();
 
-    if (module->_record != nullptr && !module->_record->dependents_unchanged()) {
+    if (module->check_source_modified()) {
       return true;
     }
   }

panda/src/gobj/shaderCompiler.cxx

@@ -98,8 +98,9 @@ compile_now(Stage stage, const Filename &fn, const CompilerOptions &options,
 
   if (module != nullptr) {
     module->set_source_filename(fullpath);
+    module->add_source_file(vf);
 
-    if (record2 != nullptr && module != nullptr) {
+    if (record2 != nullptr) {
       // Update the compiled shader module cache.
       record2->set_data(module, module);
       cache->store(record2);

panda/src/gobj/shaderModule.cxx

@@ -12,6 +12,7 @@
  */
 
 #include "shaderModule.h"
+#include "virtualFile.h"
 
 TypeHandle ShaderModule::_type_handle;
 
@@ -46,6 +47,45 @@ ShaderModule::
 ~ShaderModule() {
 }
 
+/**
+ * Adds the given file to the list of source files that this module was
+ * compiled from.  This is used by check_source_modified() to detect if any
+ * of the source files have changed since the module was compiled.
+ */
+void ShaderModule::
+add_source_file(const VirtualFile *file) {
+  SourceFile sf;
+  sf._pathname = file->get_filename();
+  sf._timestamp = file->get_timestamp();
+  sf._size = file->get_file_size();
+  _source_files.push_back(std::move(sf));
+}
+
+/**
+ * Returns true if any of the source files that this module was compiled from
+ * have been modified since the module was compiled.
+ */
+bool ShaderModule::
+check_source_modified() const {
+  VirtualFileSystem *vfs = VirtualFileSystem::get_global_ptr();
+
+  for (const SourceFile &sf : _source_files) {
+    PT(VirtualFile) file = vfs->get_file(sf._pathname);
+    if (file == nullptr) {
+      // File no longer exists.
+      if (sf._timestamp != 0) {
+        return true;
+      }
+    } else {
+      if (file->get_timestamp() != sf._timestamp ||
+          file->get_file_size() != sf._size) {
+        return true;
+      }
+    }
+  }
+  return false;
+}
+
 /**
  * Links the stage with the given previous stage, by matching up its inputs with
  * the outputs of the previous stage.  Rather than reassigning the locations

panda/src/gobj/shaderModule.h

@@ -14,12 +14,13 @@
 #ifndef SHADERMODULE_H
 #define SHADERMODULE_H
 
-#include "bamCacheRecord.h"
 #include "copyOnWriteObject.h"
 #include "internalName.h"
 #include "shaderEnums.h"
 #include "shaderType.h"
 
+class VirtualFile;
+
 /**
  * Represents a single shader module in some intermediate representation for
  * passing to the driver.  This could contain compiled bytecode, or in some
@@ -64,6 +65,9 @@ public:
   ShaderModule(Stage stage);
   virtual ~ShaderModule();
 
+  void add_source_file(const VirtualFile *file);
+  bool check_source_modified() const;
+
   INLINE Stage get_stage() const;
   INLINE uint64_t get_used_capabilities() const;
 
@@ -125,11 +129,16 @@ protected:
   void fillin(DatagramIterator &scan, BamReader *manager) override;
 
 protected:
+  struct SourceFile {
+    Filename _pathname;
+    time_t _timestamp;
+    std::streamsize _size;
+  };
+  typedef pvector<SourceFile> SourceFiles;
+
   Stage _stage;
-  PT(BamCacheRecord) _record;
-  //std::pvector<Filename> _source_files;
   Filename _source_filename;
-  //time_t _source_modified = 0;
+  SourceFiles _source_files;
   uint64_t _used_caps = 0;
 
   typedef pvector<Variable> Variables;

panda/src/shaderpipeline/shaderCompilerGlslPreProc.cxx

@@ -132,7 +132,9 @@ compile_now(Stage stage, std::istream &in, const Filename &fullpath,
   PT(ShaderModuleGlsl) module = new ShaderModuleGlsl(stage, std::move(code), state.version);
   module->_included_files = std::move(state.included_files);
   module->_used_caps |= state.required_caps;
-  module->_record = record;
+  for (const PT(VirtualFile) &vf : state.source_files) {
+    module->add_source_file(vf);
+  }
   return module;
 }
 
@@ -552,8 +554,7 @@ r_preprocess_include(State &state, const std::string &fn,
   if (record != nullptr) {
     record->add_dependent_file(vf);
   }
-  //module->_source_modified = std::max(module->_source_modified, vf->get_timestamp());
-  //module->_source_files.push_back(full_fn);
+  state.source_files.push_back(vf);
 
   // We give each file an unique index.  This is so that we can identify a
   // particular shader in the error output.  We offset them by 2048 so that

panda/src/shaderpipeline/shaderCompilerGlslPreProc.h

@@ -17,6 +17,7 @@
 #include "pandabase.h"
 
 #include "shaderCompiler.h"
+#include "virtualFile.h"
 
 class ShaderModuleGlsl;
 
@@ -40,6 +41,7 @@ private:
     std::ostringstream code;
     std::set<Filename> once_files;
     pvector<Filename> included_files;
+    pvector<PT(VirtualFile)> source_files;
     uint64_t required_caps = 0;
     int cond_nesting = 0;
     int version = 0;

panda/src/shaderpipeline/shaderCompilerGlslang.cxx

@@ -60,6 +60,7 @@ public:
     if (_record != nullptr) {
       _record->add_dependent_file(vf);
     }
+    _source_files.push_back(vf);
 
     return new IncludeResult(vf->get_filename(), (const char *)data->data(), data->size(), data);
   }
@@ -90,6 +91,7 @@ public:
     if (_record != nullptr) {
       _record->add_dependent_file(vf);
     }
+    _source_files.push_back(vf);
 
     return new IncludeResult(vf->get_filename(), (const char *)data->data(), data->size(), data);
   }
@@ -101,6 +103,8 @@ public:
     }
   }
 
+  pvector<PT(VirtualFile)> _source_files;
+
 private:
   BamCacheRecord *_record = nullptr;
 };
@@ -424,7 +428,11 @@ compile_now(ShaderModule::Stage stage, std::istream &in,
     optimized = stream;
   }
 
-  return new ShaderModuleSpirV(stage, std::move(optimized), options, record);
+  PT(ShaderModuleSpirV) module = new ShaderModuleSpirV(stage, std::move(optimized), options);
+  for (const PT(VirtualFile) &vf : includer._source_files) {
+    module->add_source_file(vf);
+  }
+  return module;
 }
 
 /**

panda/src/shaderpipeline/shaderModuleSpirV.cxx

@@ -38,14 +38,13 @@ TypeHandle ShaderModuleSpirV::_type_handle;
  * - Strips debugging information from the module.
  */
 ShaderModuleSpirV::
-ShaderModuleSpirV(Stage stage, std::vector<uint32_t> words, const CompilerOptions &options, BamCacheRecord *record) :
+ShaderModuleSpirV(Stage stage, std::vector<uint32_t> words, const CompilerOptions &options) :
   ShaderModule(stage),
   _instructions(std::move(words))
 {
   if (!_instructions.validate_header()) {
     return;
   }
-  _record = record;
 
   // Check for caps and sanity.
   for (InstructionIterator it = _instructions.begin(); it != _instructions.begin_annotations(); ++it) {

panda/src/shaderpipeline/shaderModuleSpirV.h

@@ -38,8 +38,7 @@ private:
 
 public:
   ShaderModuleSpirV(Stage stage, std::vector<uint32_t> words,
-                    const CompilerOptions &options = CompilerOptions(),
-                    BamCacheRecord *record = nullptr);
+                    const CompilerOptions &options = CompilerOptions());
   virtual ~ShaderModuleSpirV();
 
   virtual PT(CopyOnWriteObject) make_cow_copy() override;

tests/gobj/test_shader.py

@@ -23,7 +23,6 @@ def ramdir():
     vfs.unmount(mount)
 
 
[email protected](sys.platform == "win32", reason="unknown issue")
 def test_shader_load_multi(vfs, ramdir):
     # Try non-existent first.
     shad0 = Shader.load(Shader.SL_GLSL,
@@ -56,7 +55,6 @@ def test_shader_load_multi(vfs, ramdir):
     assert shad2.this != shad1.this
 
 
[email protected](sys.platform == "win32", reason="unknown issue")
 def test_shader_load_compute(vfs, ramdir):
     # Try non-existent first.
     shad0 = Shader.load_compute(Shader.SL_GLSL, "/nonexistent.glsl")