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. @@ _______________________________________________ Bf-blender-cvs mailing list [email protected] https://lists.blender.org/mailman/listinfo/bf-blender-cvs
