[Bf-blender-cvs] [2d653364086] master: Cleanup: C++ code style for Ghost-XR

Julian Eisel noreply at git.blender.org
Fri Aug 14 12:50:12 CEST 2020


Commit: 2d653364086d62cc9b503724c962cc466ad3e4b4
Author: Julian Eisel
Date:   Fri Aug 14 12:37:53 2020 +0200
Branches: master
https://developer.blender.org/rB2d653364086d62cc9b503724c962cc466ad3e4b4

Cleanup: C++ code style for Ghost-XR

* Avoid deep copy of vectors (technically more than a cleanup).
* Use `std::make_unique` for allocating unique pointers, rather than
  manual `new`.
* Use `std::optional` for optional by-value return values, rather than
  C-style `bool` to indicate success + return-argument.
* Use references rather than pointers for non-optional arguments.
* Avoid manual `new`/`delete`. Use `std::unique_ptr` for local scope
  bound lifetime.
* Use C++ `nullptr` rather than C's `NULL`.
* Remove unnecessary friend declaration.

These changes are generally considered good practise and move us more to
a "modern C++" style. We can still go much further of course.
See https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines.

===================================================================

M	intern/ghost/intern/GHOST_IXrGraphicsBinding.h
M	intern/ghost/intern/GHOST_Xr.cpp
M	intern/ghost/intern/GHOST_XrContext.cpp
M	intern/ghost/intern/GHOST_XrContext.h
M	intern/ghost/intern/GHOST_XrEvent.cpp
M	intern/ghost/intern/GHOST_XrGraphicsBinding.cpp
M	intern/ghost/intern/GHOST_XrSession.cpp
M	intern/ghost/intern/GHOST_XrSession.h
M	intern/ghost/intern/GHOST_XrSwapchain.cpp

===================================================================

diff --git a/intern/ghost/intern/GHOST_IXrGraphicsBinding.h b/intern/ghost/intern/GHOST_IXrGraphicsBinding.h
index de8bf9f1a5a..24b42c08e8a 100644
--- a/intern/ghost/intern/GHOST_IXrGraphicsBinding.h
+++ b/intern/ghost/intern/GHOST_IXrGraphicsBinding.h
@@ -21,15 +21,13 @@
 #pragma once
 
 #include <memory>
+#include <optional>
 #include <string>
 #include <vector>
 
 #include "GHOST_Xr_openxr_includes.h"
 
 class GHOST_IXrGraphicsBinding {
-  friend std::unique_ptr<GHOST_IXrGraphicsBinding> GHOST_XrGraphicsBindingCreateFromType(
-      GHOST_TXrGraphicsBinding type);
-
  public:
   union {
 #if defined(WITH_GHOST_X11)
@@ -49,18 +47,17 @@ class GHOST_IXrGraphicsBinding {
    * \param r_requirement_info Return argument to retrieve an informal string on the requirements
    *                           to be met. Useful for error/debug messages.
    */
-  virtual bool checkVersionRequirements(class GHOST_Context *ghost_ctx,
+  virtual bool checkVersionRequirements(class GHOST_Context &ghost_ctx,
                                         XrInstance instance,
                                         XrSystemId system_id,
                                         std::string *r_requirement_info) const = 0;
-  virtual void initFromGhostContext(class GHOST_Context *ghost_ctx) = 0;
-  virtual bool chooseSwapchainFormat(const std::vector<int64_t> &runtime_formats,
-                                     int64_t &r_result,
-                                     bool &r_is_rgb_format) const = 0;
+  virtual void initFromGhostContext(class GHOST_Context &ghost_ctx) = 0;
+  virtual std::optional<int64_t> chooseSwapchainFormat(const std::vector<int64_t> &runtime_formats,
+                                                       bool &r_is_rgb_format) const = 0;
   virtual std::vector<XrSwapchainImageBaseHeader *> createSwapchainImages(
       uint32_t image_count) = 0;
-  virtual void submitToSwapchainImage(XrSwapchainImageBaseHeader *swapchain_image,
-                                      const GHOST_XrDrawViewInfo *draw_info) = 0;
+  virtual void submitToSwapchainImage(XrSwapchainImageBaseHeader &swapchain_image,
+                                      const GHOST_XrDrawViewInfo &draw_info) = 0;
   virtual bool needsUpsideDownDrawing(GHOST_Context &ghost_ctx) const = 0;
 
  protected:
@@ -69,4 +66,4 @@ class GHOST_IXrGraphicsBinding {
 };
 
 std::unique_ptr<GHOST_IXrGraphicsBinding> GHOST_XrGraphicsBindingCreateFromType(
-    GHOST_TXrGraphicsBinding type, GHOST_Context *ghost_ctx);
+    GHOST_TXrGraphicsBinding type, GHOST_Context &ghost_ctx);
diff --git a/intern/ghost/intern/GHOST_Xr.cpp b/intern/ghost/intern/GHOST_Xr.cpp
index dc63aac217c..2c94aaab430 100644
--- a/intern/ghost/intern/GHOST_Xr.cpp
+++ b/intern/ghost/intern/GHOST_Xr.cpp
@@ -31,7 +31,7 @@
 
 GHOST_XrContextHandle GHOST_XrContextCreate(const GHOST_XrContextCreateInfo *create_info)
 {
-  GHOST_XrContext *xr_context = new GHOST_XrContext(create_info);
+  auto xr_context = std::make_unique<GHOST_XrContext>(create_info);
 
   /* TODO GHOST_XrContext's should probably be owned by the GHOST_System, which will handle context
    * creation and destruction. Try-catch logic can be moved to C-API then. */
@@ -40,12 +40,11 @@ GHOST_XrContextHandle GHOST_XrContextCreate(const GHOST_XrContextCreateInfo *cre
   }
   catch (GHOST_XrException &e) {
     xr_context->dispatchErrorMessage(&e);
-    delete xr_context;
-
     return nullptr;
   }
 
-  return (GHOST_XrContextHandle)xr_context;
+  /* Give ownership to the caller. */
+  return (GHOST_XrContextHandle)xr_context.release();
 }
 
 void GHOST_XrContextDestroy(GHOST_XrContextHandle xr_contexthandle)
diff --git a/intern/ghost/intern/GHOST_XrContext.cpp b/intern/ghost/intern/GHOST_XrContext.cpp
index 16687c34679..11ff6fb7d97 100644
--- a/intern/ghost/intern/GHOST_XrContext.cpp
+++ b/intern/ghost/intern/GHOST_XrContext.cpp
@@ -58,7 +58,7 @@ void *GHOST_XrContext::s_error_handler_customdata = nullptr;
  * \{ */
 
 GHOST_XrContext::GHOST_XrContext(const GHOST_XrContextCreateInfo *create_info)
-    : m_oxr(new OpenXRInstanceData()),
+    : m_oxr(std::make_unique<OpenXRInstanceData>()),
       m_debug(create_info->context_flag & GHOST_kXrContextDebug),
       m_debug_time(create_info->context_flag & GHOST_kXrContextDebugTime)
 {
@@ -327,7 +327,7 @@ void GHOST_XrContext::initApiLayers()
   }
 }
 
-static bool openxr_layer_is_available(const std::vector<XrApiLayerProperties> layers_info,
+static bool openxr_layer_is_available(const std::vector<XrApiLayerProperties> &layers_info,
                                       const std::string &layer_name)
 {
   for (const XrApiLayerProperties &layer_info : layers_info) {
@@ -339,8 +339,8 @@ static bool openxr_layer_is_available(const std::vector<XrApiLayerProperties> la
   return false;
 }
 
-static bool openxr_extension_is_available(const std::vector<XrExtensionProperties> extensions_info,
-                                          const std::string &extension_name)
+static bool openxr_extension_is_available(
+    const std::vector<XrExtensionProperties> &extensions_info, const std::string &extension_name)
 {
   for (const XrExtensionProperties &ext_info : extensions_info) {
     if (ext_info.extensionName == extension_name) {
@@ -459,7 +459,7 @@ void GHOST_XrContext::startSession(const GHOST_XrSessionBeginInfo *begin_info)
   m_custom_funcs.session_exit_customdata = begin_info->exit_customdata;
 
   if (m_session == nullptr) {
-    m_session = std::unique_ptr<GHOST_XrSession>(new GHOST_XrSession(this));
+    m_session = std::make_unique<GHOST_XrSession>(*this);
   }
   m_session->start(begin_info);
 }
@@ -489,7 +489,7 @@ void GHOST_XrContext::drawSessionViews(void *draw_customdata)
 /**
  * Delegates event to session, allowing context to destruct the session if needed.
  */
-void GHOST_XrContext::handleSessionStateChange(const XrEventDataSessionStateChanged *lifecycle)
+void GHOST_XrContext::handleSessionStateChange(const XrEventDataSessionStateChanged &lifecycle)
 {
   if (m_session &&
       m_session->handleStateChangeEvent(lifecycle) == GHOST_XrSession::SESSION_DESTROY) {
diff --git a/intern/ghost/intern/GHOST_XrContext.h b/intern/ghost/intern/GHOST_XrContext.h
index d2edb40c080..5e93d5a0b9e 100644
--- a/intern/ghost/intern/GHOST_XrContext.h
+++ b/intern/ghost/intern/GHOST_XrContext.h
@@ -80,7 +80,7 @@ class GHOST_XrContext : public GHOST_IXrContext {
   void setDrawViewFunc(GHOST_XrDrawViewFn draw_view_fn) override;
   bool needsUpsideDownDrawing() const override;
 
-  void handleSessionStateChange(const XrEventDataSessionStateChanged *lifecycle);
+  void handleSessionStateChange(const XrEventDataSessionStateChanged &lifecycle);
 
   GHOST_TXrOpenXRRuntimeID getOpenXRRuntimeID() const;
   const GHOST_XrCustomFuncs &getCustomFuncs() const;
diff --git a/intern/ghost/intern/GHOST_XrEvent.cpp b/intern/ghost/intern/GHOST_XrEvent.cpp
index 30005055f9b..992f89fadeb 100644
--- a/intern/ghost/intern/GHOST_XrEvent.cpp
+++ b/intern/ghost/intern/GHOST_XrEvent.cpp
@@ -35,25 +35,25 @@ static bool GHOST_XrEventPollNext(XrInstance instance, XrEventDataBuffer &r_even
 
 GHOST_TSuccess GHOST_XrEventsHandle(GHOST_XrContextHandle xr_contexthandle)
 {
-  GHOST_XrContext *xr_context = (GHOST_XrContext *)xr_contexthandle;
-  XrEventDataBuffer event_buffer; /* Structure big enough to hold all possible events. */
-
-  if (xr_context == NULL) {
+  if (xr_contexthandle == nullptr) {
     return GHOST_kFailure;
   }
 
-  while (GHOST_XrEventPollNext(xr_context->getInstance(), event_buffer)) {
+  GHOST_XrContext &xr_context = *(GHOST_XrContext *)xr_contexthandle;
+  XrEventDataBuffer event_buffer; /* Structure big enough to hold all possible events. */
+
+  while (GHOST_XrEventPollNext(xr_context.getInstance(), event_buffer)) {
     XrEventDataBaseHeader *event = (XrEventDataBaseHeader *)&event_buffer;
 
     switch (event->type) {
       case XR_TYPE_EVENT_DATA_SESSION_STATE_CHANGED:
-        xr_context->handleSessionStateChange((XrEventDataSessionStateChanged *)event);
+        xr_context.handleSessionStateChange((XrEventDataSessionStateChanged &)*event);
         return GHOST_kSuccess;
       case XR_TYPE_EVENT_DATA_INSTANCE_LOSS_PENDING:
         GHOST_XrContextDestroy(xr_contexthandle);
         return GHOST_kSuccess;
       default:
-        if (xr_context->isDebugMode()) {
+        if (xr_context.isDebugMode()) {
           printf("Unhandled event: %i\n", event->type);
         }
         return GHOST_kFailure;
diff --git a/intern/ghost/intern/GHOST_XrGraphicsBinding.cpp b/intern/ghost/intern/GHOST_XrGraphicsBinding.cpp
index 7d7405a974d..39b7f0776e1 100644
--- a/intern/ghost/intern/GHOST_XrGraphicsBinding.cpp
+++ b/intern/ghost/intern/GHOST_XrGraphicsBinding.cpp
@@ -34,12 +34,11 @@
 
 #include "GHOST_IXrGraphicsBinding.h"
 
-static bool choose_swapchain_format_from_candidates(std::vector<int64_t> gpu_binding_formats,
-                                                    std::vector<int64_t> runtime_formats,
-                                                    int64_t &r_result)
+static std::optional<int64_t> choose_swapchain_format_from_candidates(
+    const std::vector<int64_t> &gpu_binding_formats, const std::vector<int64_t> &runtime_formats)
 {
   if (gpu_binding_formats.empty()) {
-    return false;
+    return std::nullopt;
   }
 
   auto res = std::find_first_of(gpu_binding_formats.begin(),
@@ -47,11 +46,10 @@ static bool choose_swapchain_format_from_candidates(std::vector<int64_t> gpu_bin
                                 runtime_formats.begin(),
                                 runtime_formats.end());
   if (res == gpu_binding_formats.end()) {
-    return false;
+    return std::nullopt;
   }
 
-  r_result = *res;
-  return true;
+  return *res;
 }
 
 class GHOST_XrGraphicsBindingOpenGL : public GHOST_IXrGraphicsBinding {
@@ -63,21 +61,21 @@ class GHOST_XrGraphicsBindingOpenGL : public GHOST_IXrGraphicsBinding {
     }
   }
 
-  bool checkVersionRequirements(GHOST_Context *ghost_ctx,
+  bool checkVersionRequirements(GHOST_Context &ghost_ctx,
                                 XrInstance instance,
                                 XrSystemId system_id,
                                 std::string *r_requirement_info) const override
   {
 #if

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list