chore(⚠️): Fix some clang warnings (#2798)

Shopify · Dec 9, 2024 · 21eb223 · 21eb223
1 parent 4073bb4
commit 21eb223
Show file tree

Hide file tree

Showing 16 changed files with 52 additions and 46 deletions.
diff --git a/externals/depot_tools b/externals/depot_tools
diff --git a/packages/skia/android/cpp/jni/include/JniSkiaBaseView.h b/packages/skia/android/cpp/jni/include/JniSkiaBaseView.h
@@ -20,9 +20,9 @@ class JniSkiaBaseView {
 public:
   JniSkiaBaseView(jni::alias_ref<JniSkiaManager::javaobject> skiaManager,
                   std::shared_ptr<RNSkBaseAndroidView> skiaView)
-      : _manager(skiaManager->cthis()), _skiaAndroidView(skiaView) {}
+      : _manager(skiaManager->cthis()), _skiaAndroidView(std::move(skiaView)) {}
 
-  ~JniSkiaBaseView() {}
+  ~JniSkiaBaseView() = default;
 
   std::shared_ptr<RNSkManager> getSkiaManager() {
     return _manager->getSkiaManager();

diff --git a/packages/skia/android/cpp/rnskia-android/GrAHardwareBufferUtils.cpp b/packages/skia/android/cpp/rnskia-android/GrAHardwareBufferUtils.cpp
@@ -41,7 +41,7 @@ GrBackendFormat GetGLBackendFormat(GrDirectContext *dContext,
                                    bool requireKnownFormat) {
   GrBackendApi backend = dContext->backend();
   if (backend != GrBackendApi::kOpenGL) {
-    return GrBackendFormat();
+    return {};
   }
   switch (bufferFormat) {
   // TODO: find out if we can detect, which graphic buffers support
@@ -63,7 +63,7 @@ GrBackendFormat GetGLBackendFormat(GrDirectContext *dContext,
 #endif
   default:
     if (requireKnownFormat) {
-      return GrBackendFormat();
+      return {};
     } else {
       return GrBackendFormats::MakeGL(GR_GL_RGBA8, GR_GL_TEXTURE_EXTERNAL);
     }
@@ -110,12 +110,12 @@ void GLTextureHelper::rebind(GrDirectContext *dContext) {
 }
 
 void delete_gl_texture(void *context) {
-  GLTextureHelper *cleanupHelper = static_cast<GLTextureHelper *>(context);
+  auto cleanupHelper = static_cast<GLTextureHelper *>(context);
   delete cleanupHelper;
 }
 
 void update_gl_texture(void *context, GrDirectContext *dContext) {
-  GLTextureHelper *cleanupHelper = static_cast<GLTextureHelper *>(context);
+  auto cleanupHelper = static_cast<GLTextureHelper *>(context);
   cleanupHelper->rebind(dContext);
 }
 
@@ -127,13 +127,13 @@ static GrBackendTexture make_gl_backend_texture(
   while (GL_NO_ERROR != glGetError()) {
   } // clear GL errors
 
-  EGLGetNativeClientBufferANDROIDProc eglGetNativeClientBufferANDROID =
+  auto eglGetNativeClientBufferANDROID =
       (EGLGetNativeClientBufferANDROIDProc)eglGetProcAddress(
           "eglGetNativeClientBufferANDROID");
   if (!eglGetNativeClientBufferANDROID) {
     RNSkLogger::logToConsole(
         "Failed to get the eglGetNativeClientBufferAndroid proc");
-    return GrBackendTexture();
+    return {};
   }
 
   EGLClientBuffer clientBuffer =
@@ -149,14 +149,14 @@ static GrBackendTexture make_gl_backend_texture(
   if (EGL_NO_IMAGE_KHR == image) {
     SkDebugf("Could not create EGL image, err = (%#x)",
              static_cast<int>(eglGetError()));
-    return GrBackendTexture();
+    return {};
   }
 
   GrGLuint texID;
   glGenTextures(1, &texID);
   if (!texID) {
     eglDestroyImageKHR(display, image);
-    return GrBackendTexture();
+    return {};
   }
 
   GrGLuint target = isRenderable ? GR_GL_TEXTURE_2D : GR_GL_TEXTURE_EXTERNAL;
@@ -167,15 +167,15 @@ static GrBackendTexture make_gl_backend_texture(
     SkDebugf("glBindTexture failed (%#x)", static_cast<int>(status));
     glDeleteTextures(1, &texID);
     eglDestroyImageKHR(display, image);
-    return GrBackendTexture();
+    return {};
   }
   glEGLImageTargetTexture2DOES(target, image);
   if ((status = glGetError()) != GL_NO_ERROR) {
     SkDebugf("glEGLImageTargetTexture2DOES failed (%#x)",
              static_cast<int>(status));
     glDeleteTextures(1, &texID);
     eglDestroyImageKHR(display, image);
-    return GrBackendTexture();
+    return {};
   }
   dContext->resetContext(kTextureBinding_GrGLBackendState);
 
@@ -223,16 +223,16 @@ MakeGLBackendTexture(GrDirectContext *dContext, AHardwareBuffer *hardwareBuffer,
                      bool isProtectedContent,
                      const GrBackendFormat &backendFormat, bool isRenderable) {
   SkASSERT(dContext);
-  if (!dContext || dContext->abandoned()) {
-    return GrBackendTexture();
+  if (dContext->abandoned()) {
+    return {};
   }
 
   if (GrBackendApi::kOpenGL != dContext->backend()) {
-    return GrBackendTexture();
+    return {};
   }
 
   if (isProtectedContent && !can_import_protected_content(dContext)) {
-    return GrBackendTexture();
+    return {};
   }
 
   return make_gl_backend_texture(

diff --git a/packages/skia/android/cpp/rnskia-android/MainThreadDispatcher.h b/packages/skia/android/cpp/rnskia-android/MainThreadDispatcher.h
@@ -6,8 +6,8 @@
 
 class MainThreadDispatcher {
 private:
-  ALooper *mainLooper;
-  int messagePipe[2];
+  ALooper *mainLooper = nullptr;
+  int messagePipe[2] = { -1, -1 };
   std::queue<std::function<void()>> taskQueue;
   std::mutex queueMutex;
 

diff --git a/packages/skia/android/cpp/rnskia-android/OpenGLContext.h b/packages/skia/android/cpp/rnskia-android/OpenGLContext.h
@@ -117,17 +117,23 @@ class OpenGLContext {
     // GR_GL_TEXTURE_2D
     case AHARDWAREBUFFER_FORMAT_R8G8B8A8_UNORM:
       format = GrBackendFormats::MakeGL(GR_GL_RGBA8, GR_GL_TEXTURE_EXTERNAL);
+      break;
     case AHARDWAREBUFFER_FORMAT_R16G16B16A16_FLOAT:
       format = GrBackendFormats::MakeGL(GR_GL_RGBA16F, GR_GL_TEXTURE_EXTERNAL);
+      break;
     case AHARDWAREBUFFER_FORMAT_R5G6B5_UNORM:
-      format = GrBackendFormats::MakeGL(GR_GL_RGB565, GR_GL_TEXTURE_EXTERNAL);
+        GrBackendFormats::MakeGL(GR_GL_RGB565, GR_GL_TEXTURE_EXTERNAL);
+        break;
     case AHARDWAREBUFFER_FORMAT_R10G10B10A2_UNORM:
       format = GrBackendFormats::MakeGL(GR_GL_RGB10_A2, GR_GL_TEXTURE_EXTERNAL);
+      break;
     case AHARDWAREBUFFER_FORMAT_R8G8B8_UNORM:
       format = GrBackendFormats::MakeGL(GR_GL_RGB8, GR_GL_TEXTURE_EXTERNAL);
+      break;
 #if __ANDROID_API__ >= 33
     case AHARDWAREBUFFER_FORMAT_R8_UNORM:
       format = GrBackendFormats::MakeGL(GR_GL_R8, GR_GL_TEXTURE_EXTERNAL);
+      break;
 #endif
     default:
       if (requireKnownFormat) {
@@ -136,10 +142,11 @@ class OpenGLContext {
         format = GrBackendFormats::MakeGL(GR_GL_RGBA8, GR_GL_TEXTURE_EXTERNAL);
       }
     }
-
+    auto width = static_cast<int>(description.width);
+    auto height = static_cast<int>(description.height);
     auto backendTex = MakeGLBackendTexture(
         _directContext.get(), const_cast<AHardwareBuffer *>(hardwareBuffer),
-        description.width, description.height, &deleteImageProc,
+        width, height, &deleteImageProc,
         &updateImageProc, &deleteImageCtx, false, format, false);
     if (!backendTex.isValid()) {
       RNSkLogger::logToConsole(
@@ -159,8 +166,7 @@ class OpenGLContext {
   }
 
   // TODO: remove width, height
-  std::unique_ptr<WindowContext> MakeWindow(ANativeWindow *window, int width,
-                                            int height) {
+  std::unique_ptr<WindowContext> MakeWindow(ANativeWindow *window) {
     auto display = OpenGLSharedContext::getInstance().getDisplay();
     return std::make_unique<OpenGLWindowContext>(
         _directContext.get(), display, _glContext.get(), window,

diff --git a/packages/skia/android/cpp/rnskia-android/OpenGLWindowContext.h b/packages/skia/android/cpp/rnskia-android/OpenGLWindowContext.h
@@ -42,7 +42,7 @@ class OpenGLWindowContext : public WindowContext {
     _glSurface = display->makeWindowSurface(config, _window);
   }
 
-  ~OpenGLWindowContext() {
+  ~OpenGLWindowContext() override {
     _skSurface = nullptr;
     _glSurface = nullptr;
     ANativeWindow_release(_window);

diff --git a/packages/skia/android/cpp/rnskia-android/RNSkAndroidPlatformContext.h b/packages/skia/android/cpp/rnskia-android/RNSkAndroidPlatformContext.h
@@ -33,11 +33,11 @@ class RNSkAndroidPlatformContext : public RNSkPlatformContext {
   RNSkAndroidPlatformContext(
       JniPlatformContext *jniPlatformContext,
       std::shared_ptr<facebook::react::CallInvoker> jsCallInvoker)
-      : RNSkPlatformContext(jsCallInvoker,
+      : RNSkPlatformContext(std::move(jsCallInvoker),
                             jniPlatformContext->getPixelDensity()),
         _jniPlatformContext(jniPlatformContext) {}
 
-  ~RNSkAndroidPlatformContext() {}
+  ~RNSkAndroidPlatformContext() override = default;
 
   void performStreamOperation(
       const std::string &sourceUri,
@@ -63,7 +63,7 @@ class RNSkAndroidPlatformContext : public RNSkPlatformContext {
     return DawnContext::getInstance().MakeWindow(surface, width, height);
 #else
     auto aWindow = reinterpret_cast<ANativeWindow *>(surface);
-    return OpenGLContext::getInstance().MakeWindow(aWindow, width, height);
+    return OpenGLContext::getInstance().MakeWindow(aWindow);
 #endif
   }
 
@@ -87,7 +87,7 @@ class RNSkAndroidPlatformContext : public RNSkPlatformContext {
 
     OpenGLContext::getInstance().makeCurrent();
     if (glIsTexture(textureInfo.fID) == GL_FALSE) {
-      throw new std::runtime_error("Invalid textureInfo");
+      throw std::runtime_error("Invalid textureInfo");
     }
 
     GrBackendTexture backendTexture = GrBackendTextures::MakeGL(
@@ -107,7 +107,7 @@ class RNSkAndroidPlatformContext : public RNSkPlatformContext {
 
   void releaseNativeBuffer(uint64_t pointer) override {
 #if __ANDROID_API__ >= 26
-    AHardwareBuffer *buffer = reinterpret_cast<AHardwareBuffer *>(pointer);
+    auto *buffer = reinterpret_cast<AHardwareBuffer *>(pointer);
     AHardwareBuffer_release(buffer);
 #endif
   }
@@ -185,7 +185,7 @@ class RNSkAndroidPlatformContext : public RNSkPlatformContext {
     return getTextureInfo(texture);
   }
 
-  static const TextureInfo getTextureInfo(const GrBackendTexture &texture) {
+  static TextureInfo getTextureInfo(const GrBackendTexture &texture) {
     if (!texture.isValid()) {
       throw std::runtime_error("invalid backend texture");
     }

diff --git a/packages/skia/android/cpp/rnskia-android/RNSkAndroidVideo.cpp b/packages/skia/android/cpp/rnskia-android/RNSkAndroidVideo.cpp
@@ -35,7 +35,7 @@ RNSkAndroidVideo::RNSkAndroidVideo(jni::global_ref<jobject> jniVideo)
 #endif
 }
 
-RNSkAndroidVideo::~RNSkAndroidVideo() {}
+RNSkAndroidVideo::~RNSkAndroidVideo() = default;
 
 sk_sp<SkImage> RNSkAndroidVideo::nextImage(double *timeStamp) {
 #if __ANDROID_API__ >= 26

diff --git a/packages/skia/android/cpp/rnskia-android/RNSkAndroidVideo.h b/packages/skia/android/cpp/rnskia-android/RNSkAndroidVideo.h
@@ -26,7 +26,7 @@ class RNSkAndroidVideo : public RNSkVideo {
 
 public:
   explicit RNSkAndroidVideo(jni::global_ref<jobject> jniVideo);
-  ~RNSkAndroidVideo();
+  ~RNSkAndroidVideo() override;
   sk_sp<SkImage> nextImage(double *timeStamp = nullptr) override;
   double duration() override;
   double framerate() override;

diff --git a/packages/skia/android/cpp/rnskia-android/RNSkOpenGLCanvasProvider.cpp b/packages/skia/android/cpp/rnskia-android/RNSkOpenGLCanvasProvider.cpp
@@ -26,9 +26,9 @@ namespace RNSkia {
 RNSkOpenGLCanvasProvider::RNSkOpenGLCanvasProvider(
     std::function<void()> requestRedraw,
     std::shared_ptr<RNSkia::RNSkPlatformContext> platformContext)
-    : RNSkCanvasProvider(requestRedraw), _platformContext(platformContext) {}
+    : RNSkCanvasProvider(std::move(requestRedraw)), _platformContext(std::move(platformContext)) {}
 
-RNSkOpenGLCanvasProvider::~RNSkOpenGLCanvasProvider() {}
+RNSkOpenGLCanvasProvider::~RNSkOpenGLCanvasProvider() = default;
 
 float RNSkOpenGLCanvasProvider::getScaledWidth() {
   if (_surfaceHolder) {
@@ -110,7 +110,7 @@ void RNSkOpenGLCanvasProvider::surfaceAvailable(jobject jSurfaceTexture,
   _surfaceHolder = DawnContext::getInstance().MakeWindow(window, width, height);
 #else
   _surfaceHolder =
-      OpenGLContext::getInstance().MakeWindow(window, width, height);
+      OpenGLContext::getInstance().MakeWindow(window);
 #endif
 
   // Post redraw request to ensure we paint in the next draw cycle.

diff --git a/packages/skia/cpp/api/JsiSkApi.h b/packages/skia/cpp/api/JsiSkApi.h
@@ -64,7 +64,7 @@ class JsiSkApi : public JsiSkHostObject {
    * and provide functions for accessing and creating the Skia wrapper objects
    * @param context Platform context
    */
-  JsiSkApi(jsi::Runtime &runtime, std::shared_ptr<RNSkPlatformContext> context)
+  explicit JsiSkApi(const std::shared_ptr<RNSkPlatformContext> &context)
       : JsiSkHostObject(context) {
     // We create the system font manager eagerly since it has proven to be too
     // slow to do it on demand

diff --git a/packages/skia/cpp/api/JsiSkCanvas.h b/packages/skia/cpp/api/JsiSkCanvas.h
@@ -83,7 +83,7 @@ class JsiSkCanvas : public JsiSkHostObject {
     auto src = JsiSkRect::fromValue(runtime, arguments[1]);
     auto dest = JsiSkRect::fromValue(runtime, arguments[2]);
     auto paint = JsiSkPaint::fromValue(runtime, arguments[3]);
-    auto fastSample = count < 5 ? false : arguments[4].getBool();
+    auto fastSample = count >= 5 && arguments[4].getBool();
     _canvas->drawImageRect(image, *src, *dest, SkSamplingOptions(), paint.get(),
                            fastSample ? SkCanvas::kFast_SrcRectConstraint
                                       : SkCanvas::kStrict_SrcRectConstraint);

diff --git a/packages/skia/cpp/api/JsiSkColor.h b/packages/skia/cpp/api/JsiSkColor.h
@@ -24,7 +24,7 @@ class JsiSkColor : public RNJsi::JsiHostObject {
 public:
   JsiSkColor() : JsiHostObject() {}
 
-  ~JsiSkColor() {}
+  ~JsiSkColor() override = default;
 
   static jsi::Object toValue(jsi::Runtime &runtime, SkColor color) {
     auto result = runtime.global()
@@ -63,7 +63,7 @@ class JsiSkColor : public RNJsi::JsiHostObject {
    * @return A function for creating a new host object wrapper for the SkColor
    * class
    */
-  static const jsi::HostFunctionType createCtor() {
+  static jsi::HostFunctionType createCtor() {
     return JSI_HOST_FUNCTION_LAMBDA {
       if (arguments[0].isNumber()) {
         return JsiSkColor::toValue(runtime, arguments[0].getNumber());

diff --git a/packages/skia/cpp/api/third_party/CSSColorParser.h b/packages/skia/cpp/api/third_party/CSSColorParser.h
@@ -34,7 +34,7 @@
 namespace CSSColorParser {
 
 struct Color {
-  inline Color() {}
+  inline Color() = default;
   inline Color(unsigned char r_, unsigned char g_, unsigned char b_, float a_)
       : r(r_), g(g_), b(b_), a(a_ > 1   ? 1
                                : a_ < 0 ? 0

diff --git a/packages/skia/cpp/api/third_party/base64.cpp b/packages/skia/cpp/api/third_party/base64.cpp
@@ -26,8 +26,8 @@ namespace RNSkia {
 
 Base64::Error Base64::Decode(const void *srcv, size_t srcLength, void *dstv,
                              size_t *dstLength) {
-  const unsigned char *src = static_cast<const unsigned char *>(srcv);
-  unsigned char *dst = static_cast<unsigned char *>(dstv);
+  auto *src = static_cast<const unsigned char *>(srcv);
+  auto *dst = static_cast<unsigned char *>(dstv);
 
   int i = 0;
   bool padTwo = false;
@@ -112,8 +112,8 @@ Base64::Error Base64::Decode(const void *srcv, size_t srcLength, void *dstv,
 }
 
 size_t Base64::Encode(const void *srcv, size_t length, void *dstv) {
-  const unsigned char *src = static_cast<const unsigned char *>(srcv);
-  unsigned char *dst = static_cast<unsigned char *>(dstv);
+  auto *src = static_cast<const unsigned char *>(srcv);
+  auto *dst = static_cast<unsigned char *>(dstv);
 
   const char *encode = kDefaultEncode;
   if (dst) {

diff --git a/packages/skia/cpp/rnskia/RNSkManager.cpp b/packages/skia/cpp/rnskia/RNSkManager.cpp
@@ -69,7 +69,7 @@ void RNSkManager::installBindings() {
   // Create the API objects and install it on the global object in the
   // provided runtime.
 
-  auto skiaApi = std::make_shared<JsiSkApi>(*_jsRuntime, _platformContext);
+  auto skiaApi = std::make_shared<JsiSkApi>(_platformContext);
   _jsRuntime->global().setProperty(
       *_jsRuntime, "SkiaApi",
       jsi::Object::createFromHostObject(*_jsRuntime, std::move(skiaApi)));