diff --git a/CMakeLists.txt b/CMakeLists.txt index d0a2e57dd5..6956fd22c3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -108,8 +108,10 @@ set(PLATFORM_QT_GL OpenGL) if (USE_GLES) add_definitions(-DUSE_GLES) + add_definitions(-DGPU_POINTER_STORAGE_SHARED) set(PLATFORM_GL_BACKEND gpu-gl-common gpu-gles) else() + add_definitions(-DGPU_POINTER_STORAGE_RAW) set(PLATFORM_GL_BACKEND gpu-gl-common gpu-gl) endif() diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index af9d793fd0..978ab313ed 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -3962,7 +3962,7 @@ void Application::keyPressEvent(QKeyEvent* event) { break; case Qt::Key_G: - if (isShifted && isMeta) { + if (isShifted && isMeta && Menu::getInstance() && Menu::getInstance()->getMenu("Developer")->isVisible()) { static const QString HIFI_FRAMES_FOLDER_VAR = "HIFI_FRAMES_FOLDER"; static const QString GPU_FRAME_FOLDER = QProcessEnvironment::systemEnvironment().contains(HIFI_FRAMES_FOLDER_VAR) ? QProcessEnvironment::systemEnvironment().value(HIFI_FRAMES_FOLDER_VAR) diff --git a/libraries/gpu-gl-common/src/gpu/gl/GLBackend.h b/libraries/gpu-gl-common/src/gpu/gl/GLBackend.h index ed94cbb64d..b5a279a54c 100644 --- a/libraries/gpu-gl-common/src/gpu/gl/GLBackend.h +++ b/libraries/gpu-gl-common/src/gpu/gl/GLBackend.h @@ -54,28 +54,6 @@ #define GPU_STEREO_CAMERA_BUFFER #endif -// -// GL Backend pointer storage mechanism -// One of the following three defines must be defined. -// GPU_POINTER_STORAGE_SHARED - -// The platonic ideal, use references to smart pointers. -// However, this produces artifacts because there are too many places in the code right now that -// create temporary values (undesirable smart pointer duplications) and then those temp variables -// get passed on and have their reference taken, and then invalidated -// GPU_POINTER_STORAGE_REF - -// Raw pointer manipulation. Seems more dangerous than the reference wrappers, -// but in practice, the danger of grabbing a reference to a temporary variable -// is causing issues -// GPU_POINTER_STORAGE_RAW - -#if defined(USE_GLES) -#define GPU_POINTER_STORAGE_SHARED -#else -#define GPU_POINTER_STORAGE_RAW -#endif - namespace gpu { namespace gl { class GLBackend : public Backend, public std::enable_shared_from_this { diff --git a/libraries/gpu-gl-common/src/gpu/gl/GLState.cpp b/libraries/gpu-gl-common/src/gpu/gl/GLState.cpp index 4a27f1ec1f..89ba80031b 100644 --- a/libraries/gpu-gl-common/src/gpu/gl/GLState.cpp +++ b/libraries/gpu-gl-common/src/gpu/gl/GLState.cpp @@ -44,7 +44,7 @@ const GLState::Commands makeResetStateCommands() { // and we have a 50/50 chance that State::DEFAULT is not yet initialized. // Since State::DEFAULT = State::Data() it is much easier to not use the actual State::DEFAULT // but another State::Data object with a default initialization. - const State::Data& DEFAULT = State::DEFAULT; + const State::Data DEFAULT = State::Data(); auto depthBiasCommand = std::make_shared(&GLBackend::do_setStateDepthBias, Vec2(DEFAULT.depthBias, DEFAULT.depthBiasSlopeScale)); diff --git a/libraries/gpu/src/gpu/Context.h b/libraries/gpu/src/gpu/Context.h index 361821f7bd..b080b0ceac 100644 --- a/libraries/gpu/src/gpu/Context.h +++ b/libraries/gpu/src/gpu/Context.h @@ -24,157 +24,12 @@ #include "Pipeline.h" #include "Framebuffer.h" #include "Frame.h" +#include "PointerStorage.h" class QImage; namespace gpu { -// -// GL Backend pointer storage mechanism -// One of the following three defines must be defined. -// GPU_POINTER_STORAGE_SHARED - -// The platonic ideal, use references to smart pointers. -// However, this produces artifacts because there are too many places in the code right now that -// create temporary values (undesirable smart pointer duplications) and then those temp variables -// get passed on and have their reference taken, and then invalidated -// GPU_POINTER_STORAGE_REF - -// Raw pointer manipulation. Seems more dangerous than the reference wrappers, -// but in practice, the danger of grabbing a reference to a temporary variable -// is causing issues -// GPU_POINTER_STORAGE_RAW - -#if defined(USE_GLES) -#define GPU_POINTER_STORAGE_SHARED -#else -#define GPU_POINTER_STORAGE_RAW -#endif - -#if defined(GPU_POINTER_STORAGE_SHARED) -template -static inline bool compare(const std::shared_ptr& a, const std::shared_ptr& b) { - return a == b; -} - -template -static inline T* acquire(const std::shared_ptr& pointer) { - return pointer.get(); -} - -template -static inline void reset(std::shared_ptr& pointer) { - return pointer.reset(); -} - -template -static inline bool valid(const std::shared_ptr& pointer) { - return pointer.operator bool(); -} - -template -static inline void assign(std::shared_ptr& pointer, const std::shared_ptr& source) { - pointer = source; -} - -using BufferReference = BufferPointer; -using TextureReference = TexturePointer; -using FramebufferReference = FramebufferPointer; -using FormatReference = Stream::FormatPointer; -using PipelineReference = PipelinePointer; - -#define GPU_REFERENCE_INIT_VALUE nullptr - -#elif defined(GPU_POINTER_STORAGE_REF) - -template -class PointerReferenceWrapper : public std::reference_wrapper> { - using Parent = std::reference_wrapper>; - -public: - using Pointer = std::shared_ptr; - PointerReferenceWrapper() : Parent(EMPTY()) {} - PointerReferenceWrapper(const Pointer& pointer) : Parent(pointer) {} - void clear() { *this = EMPTY(); } - -private: - static const Pointer& EMPTY() { - static const Pointer EMPTY_VALUE; - return EMPTY_VALUE; - }; -}; - -template -static bool compare(const PointerReferenceWrapper& reference, const std::shared_ptr& pointer) { - return reference.get() == pointer; -} - -template -static inline T* acquire(const PointerReferenceWrapper& reference) { - return reference.get().get(); -} - -template -static void assign(PointerReferenceWrapper& reference, const std::shared_ptr& pointer) { - reference = pointer; -} - -template -static bool valid(const PointerReferenceWrapper& reference) { - return reference.get().operator bool(); -} - -template -static inline void reset(PointerReferenceWrapper& reference) { - return reference.clear(); -} - -using BufferReference = PointerReferenceWrapper; -using TextureReference = PointerReferenceWrapper; -using FramebufferReference = PointerReferenceWrapper; -using FormatReference = PointerReferenceWrapper; -using PipelineReference = PointerReferenceWrapper; - -#define GPU_REFERENCE_INIT_VALUE - -#elif defined(GPU_POINTER_STORAGE_RAW) - -template -static bool compare(const T* const& rawPointer, const std::shared_ptr& pointer) { - return rawPointer == pointer.get(); -} - -template -static inline T* acquire(T* const& rawPointer) { - return rawPointer; -} - -template -static inline bool valid(const T* const& rawPointer) { - return rawPointer; -} - -template -static inline void reset(T*& rawPointer) { - rawPointer = nullptr; -} - -template -static inline void assign(T*& rawPointer, const std::shared_ptr& pointer) { - rawPointer = pointer.get(); -} - -using BufferReference = Buffer*; -using TextureReference = Texture*; -using FramebufferReference = Framebuffer*; -using FormatReference = Stream::Format*; -using PipelineReference = Pipeline*; - -#define GPU_REFERENCE_INIT_VALUE nullptr - -#endif - - struct ContextStats { public: int _ISNumFormatChanges = 0; diff --git a/libraries/gpu/src/gpu/PointerStorage.h b/libraries/gpu/src/gpu/PointerStorage.h new file mode 100644 index 0000000000..3854205efb --- /dev/null +++ b/libraries/gpu/src/gpu/PointerStorage.h @@ -0,0 +1,159 @@ +// +// Created by Bradley Austin Davis on 2019/01/25 +// Copyright 2013-2019 High Fidelity, Inc. +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// +#pragma once +#ifndef hifi_gpu_PointerStorage_h +#define hifi_gpu_PointerStorage_h + +namespace gpu { + +// +// GL Backend pointer storage mechanism +// One of the following three defines must be defined. +// GPU_POINTER_STORAGE_SHARED + +// The platonic ideal, use references to smart pointers. +// However, this produces artifacts because there are too many places in the code right now that +// create temporary values (undesirable smart pointer duplications) and then those temp variables +// get passed on and have their reference taken, and then invalidated +// GPU_POINTER_STORAGE_REF + +// Raw pointer manipulation. Seems more dangerous than the reference wrappers, +// but in practice, the danger of grabbing a reference to a temporary variable +// is causing issues +// GPU_POINTER_STORAGE_RAW + +#if defined(GPU_POINTER_STORAGE_SHARED) +template +static inline bool compare(const std::shared_ptr& a, const std::shared_ptr& b) { + return a == b; +} + +template +static inline T* acquire(const std::shared_ptr& pointer) { + return pointer.get(); +} + +template +static inline void reset(std::shared_ptr& pointer) { + return pointer.reset(); +} + +template +static inline bool valid(const std::shared_ptr& pointer) { + return pointer.operator bool(); +} + +template +static inline void assign(std::shared_ptr& pointer, const std::shared_ptr& source) { + pointer = source; +} + +using BufferReference = BufferPointer; +using TextureReference = TexturePointer; +using FramebufferReference = FramebufferPointer; +using FormatReference = Stream::FormatPointer; +using PipelineReference = PipelinePointer; + +#define GPU_REFERENCE_INIT_VALUE nullptr + +#elif defined(GPU_POINTER_STORAGE_REF) + +template +class PointerReferenceWrapper : public std::reference_wrapper> { + using Parent = std::reference_wrapper>; + +public: + using Pointer = std::shared_ptr; + PointerReferenceWrapper() : Parent(EMPTY()) {} + PointerReferenceWrapper(const Pointer& pointer) : Parent(pointer) {} + void clear() { *this = EMPTY(); } + +private: + static const Pointer& EMPTY() { + static const Pointer EMPTY_VALUE; + return EMPTY_VALUE; + }; +}; + +template +static bool compare(const PointerReferenceWrapper& reference, const std::shared_ptr& pointer) { + return reference.get() == pointer; +} + +template +static inline T* acquire(const PointerReferenceWrapper& reference) { + return reference.get().get(); +} + +template +static void assign(PointerReferenceWrapper& reference, const std::shared_ptr& pointer) { + reference = pointer; +} + +template +static bool valid(const PointerReferenceWrapper& reference) { + return reference.get().operator bool(); +} + +template +static inline void reset(PointerReferenceWrapper& reference) { + return reference.clear(); +} + +using BufferReference = PointerReferenceWrapper; +using TextureReference = PointerReferenceWrapper; +using FramebufferReference = PointerReferenceWrapper; +using FormatReference = PointerReferenceWrapper; +using PipelineReference = PointerReferenceWrapper; + +#define GPU_REFERENCE_INIT_VALUE + +#elif defined(GPU_POINTER_STORAGE_RAW) + +template +static bool compare(const T* const& rawPointer, const std::shared_ptr& pointer) { + return rawPointer == pointer.get(); +} + +template +static inline T* acquire(T* const& rawPointer) { + return rawPointer; +} + +template +static inline bool valid(const T* const& rawPointer) { + return rawPointer; +} + +template +static inline void reset(T*& rawPointer) { + rawPointer = nullptr; +} + +template +static inline void assign(T*& rawPointer, const std::shared_ptr& pointer) { + rawPointer = pointer.get(); +} + +using BufferReference = Buffer*; +using TextureReference = Texture*; +using FramebufferReference = Framebuffer*; +using FormatReference = Stream::Format*; +using PipelineReference = Pipeline*; + +#define GPU_REFERENCE_INIT_VALUE nullptr + +#else + +#error "No GPU pointer storage scheme defined" + +#endif + +} + +#endif // hifi_gpu_PointerStorage_h \ No newline at end of file diff --git a/libraries/gpu/src/gpu/State.cpp b/libraries/gpu/src/gpu/State.cpp index c62d2060df..fab0438f75 100755 --- a/libraries/gpu/src/gpu/State.cpp +++ b/libraries/gpu/src/gpu/State.cpp @@ -105,7 +105,6 @@ std::string State::getKey() const { key += ":" + hex(_values.stencilTestBack.getRaw()); key += ":" + hex(_values.blendFunction.getRaw()); key += ":" + hex(_values.sampleMask); - key += ":" + hex(_values.sampleMask); // fillMode, cullMode, colorMaskWrite and the flags consume 32 bits alltogether static_assert(0 == offsetof(State::Data, fillMode) % 4, "Validate fillMode offset"); key += ":" + hex(*(int*)&_values.fillMode); diff --git a/libraries/gpu/src/gpu/State.h b/libraries/gpu/src/gpu/State.h index 2b7a661798..abe0cd7731 100755 --- a/libraries/gpu/src/gpu/State.h +++ b/libraries/gpu/src/gpu/State.h @@ -262,7 +262,7 @@ public: static_assert(sizeof(Flags) == sizeof(uint8), "Flags size check"); - // The Data class is the full explicit description of the State class fields value. + // The Data class is the full explicit description of the State class fields value. // Useful for having one const static called Default for reference or for the gpu::Backend to keep track of the current value class Data { public: @@ -273,13 +273,11 @@ public: StencilActivation stencilActivation; StencilTest stencilTestFront; StencilTest stencilTestBack; - BlendFunction blendFunction; - uint32 sampleMask = 0xFFFFFFFF; - - FillMode fillMode = FILL_FACE; - CullMode cullMode = CULL_NONE; - ColorMask colorWriteMask = WRITE_ALL; + BlendFunction blendFunction; + FillMode fillMode{ FILL_FACE }; + CullMode cullMode{ CULL_NONE }; + ColorMask colorWriteMask{ WRITE_ALL }; Flags flags; }; @@ -290,15 +288,14 @@ public: static_assert(offsetof(Data, stencilActivation) == 12, "Data offsets"); static_assert(offsetof(Data, stencilTestFront) == 16, "Data offsets"); static_assert(offsetof(Data, stencilTestBack) == 20, "Data offsets"); - static_assert(offsetof(Data, blendFunction) == 24, "Data offsets"); - static_assert(offsetof(Data, sampleMask) == 28, "Data offsets"); + static_assert(offsetof(Data, sampleMask) == 24, "Data offsets"); + static_assert(offsetof(Data, blendFunction) == 28, "Data offsets"); static_assert(offsetof(Data, fillMode) == 32, "Data offsets"); static_assert(offsetof(Data, cullMode) == 33, "Data offsets"); static_assert(offsetof(Data, colorWriteMask) == 34, "Data offsets"); static_assert(offsetof(Data, flags) == 35, "Data offsets"); static_assert(sizeof(Data) == 36, "Data Size Check"); - std::string getKey() const; // The unique default values for all the fields @@ -310,7 +307,7 @@ public: CullMode getCullMode() const { return _values.cullMode; } const Flags& getFlags() const { return _values.flags; } - + void setFrontFaceClockwise(bool isClockwise) { SET_FIELD(FRONT_FACE_CLOCKWISE, flags.frontFaceClockwise, isClockwise); } bool isFrontFaceClockwise() const { return _values.flags.frontFaceClockwise; }