Project import generated by Copybara.

GitOrigin-RevId: d0039a576e2db9c0fcefffd26a527df74cbe145b

mediapipe/gpu/gl_context.cc

@@ -274,6 +274,11 @@ bool GlContext::HasGlExtension(absl::string_view extension) const {
   }
 
   return Run([this]() -> ::mediapipe::Status {
+    // Clear any GL errors at this point: as this is a fresh context
+    // there shouldn't be any, but if we adopted an existing context (e.g. in
+    // some Emscripten cases), there might be some existing tripped error.
+    ForceClearExistingGlErrors();
+
     absl::string_view version_string(
         reinterpret_cast<const char*>(glGetString(GL_VERSION)));
 
@@ -769,10 +774,18 @@ bool GlContext::SyncTokenIsReady(const std::shared_ptr<GlSyncPoint>& token) {
   return token->IsReady();
 }
 
-bool GlContext::CheckForGlErrors() {
+void GlContext::ForceClearExistingGlErrors() {
+  LogUncheckedGlErrors(CheckForGlErrors(/*force=*/true));
+}
+
+bool GlContext::CheckForGlErrors() { return CheckForGlErrors(false); }
+
+bool GlContext::CheckForGlErrors(bool force) {
 #if UNSAFE_EMSCRIPTEN_SKIP_GL_ERROR_HANDLING
-  LOG_FIRST_N(WARNING, 1) << "MediaPipe OpenGL error checking is disabled";
-  return false;
+  if (!force) {
+    LOG_FIRST_N(WARNING, 1) << "MediaPipe OpenGL error checking is disabled";
+    return false;
+  }
 #endif
 
   if (!HasContext()) return false;

mediapipe/gpu/gl_context.h

@@ -348,7 +348,20 @@ class GlContext : public std::enable_shared_from_this<GlContext> {
   void DestroyContext();
 
   bool HasContext() const;
+
+  // This function clears out any tripped gl Errors and just logs them. This
+  // is used by code that needs to check glGetError() to know if it succeeded,
+  // but can't rely on the existing state to be 'clean'.
+  void ForceClearExistingGlErrors();
+
+  // Returns true if there were any GL errors. Note that this may be a no-op
+  // for performance reasons in some contexts (specifically Emscripten opt).
   bool CheckForGlErrors();
+
+  // Same as `CheckForGLErrors()` but with the option of forcing the check
+  // even if we would otherwise skip for performance reasons.
+  bool CheckForGlErrors(bool force);
+
   void LogUncheckedGlErrors(bool had_gl_errors);
   ::mediapipe::Status GetGlExtensions();
   ::mediapipe::Status GetGlExtensionsCompat();

mediapipe/gpu/gl_simple_calculator.h

@@ -36,9 +36,9 @@ namespace mediapipe {
 // - GlRender(), which is called for each frame.
 // - A destructor, to destroy the objects created in GlSetup.
 // Note that when GlSetup and GlRender are called, the GL context has already
-// been set, but in the destructor it has not. The destructor should have a
-// local variable set to ContextAutoSetter() to make sure it is doing the
-// destruction in the right GL context.
+// been set, but in the destructor it has not. The destructor should use the
+// RunInGlContext() helper to make sure it is doing the destruction in the right
+// GL context.
 //
 // Additionally, you can define a GlBind() method, which will be called to
 // enable shader programs, bind any additional textures you may need, etc.

mediapipe/gpu/gl_texture_buffer.cc

@@ -87,11 +87,10 @@ bool GlTextureBuffer::CreateInternal(const void* data) {
 }
 
 void GlTextureBuffer::Reuse() {
-  WaitForConsumersOnGpu();
-  // TODO: should we just do this inside WaitForConsumersOnGpu?
-  // if we do that, WaitForConsumersOnGpu can be called only once.
+  absl::MutexLock lock(&consumer_sync_mutex_);
+  consumer_multi_sync_->WaitOnGpu();
+  // Reset the sync points.
   consumer_multi_sync_ = absl::make_unique<GlMultiSyncPoint>();
-  // Reset the token.
   producer_sync_ = nullptr;
 }
 
@@ -102,11 +101,15 @@ void GlTextureBuffer::Updated(std::shared_ptr<GlSyncPoint> prod_token) {
 }
 
 void GlTextureBuffer::DidRead(std::shared_ptr<GlSyncPoint> cons_token) {
+  absl::MutexLock lock(&consumer_sync_mutex_);
   consumer_multi_sync_->Add(std::move(cons_token));
 }
 
 GlTextureBuffer::~GlTextureBuffer() {
   if (deletion_callback_) {
+    // Note: at this point there are no more consumers that could be added
+    // to the consumer_multi_sync_, so it no longer needs to be protected
+    // by out mutex when we hand it to the deletion callback.
     deletion_callback_(std::move(consumer_multi_sync_));
   }
 }
@@ -129,10 +132,17 @@ void GlTextureBuffer::WaitOnGpu() {
   }
 }
 
-void GlTextureBuffer::WaitForConsumers() { consumer_multi_sync_->Wait(); }
+void GlTextureBuffer::WaitForConsumers() {
+  absl::MutexLock lock(&consumer_sync_mutex_);
+  consumer_multi_sync_->Wait();
+}
 
 void GlTextureBuffer::WaitForConsumersOnGpu() {
+  absl::MutexLock lock(&consumer_sync_mutex_);
   consumer_multi_sync_->WaitOnGpu();
+  // TODO: should we clear the consumer_multi_sync_ here?
+  // It would mean that WaitForConsumersOnGpu can be called only once, or more
+  // precisely, on only one GL context.
 }
 
 }  // namespace mediapipe

mediapipe/gpu/gl_texture_buffer.h

@@ -121,15 +121,16 @@ class GlTextureBuffer {
   friend class GlCalculatorHelperImpl;
 
   GLuint name_ = 0;
-  int width_ = 0;
-  int height_ = 0;
-  GpuBufferFormat format_ = GpuBufferFormat::kUnknown;
-  GLenum target_ = GL_TEXTURE_2D;
+  const int width_ = 0;
+  const int height_ = 0;
+  const GpuBufferFormat format_ = GpuBufferFormat::kUnknown;
+  const GLenum target_ = GL_TEXTURE_2D;
   // Token tracking changes to this texture. Used by WaitUntilComplete.
   std::shared_ptr<GlSyncPoint> producer_sync_;
+  absl::Mutex consumer_sync_mutex_;
   // Tokens tracking the point when consumers finished using this texture.
-  std::unique_ptr<GlMultiSyncPoint> consumer_multi_sync_ =
-      absl::make_unique<GlMultiSyncPoint>();
+  std::unique_ptr<GlMultiSyncPoint> consumer_multi_sync_ ABSL_GUARDED_BY(
+      consumer_sync_mutex_) = absl::make_unique<GlMultiSyncPoint>();
   DeletionCallback deletion_callback_;
 };