Merge pull request #15862 from annabrewer/display-simplification

BUGZ-838, BUGZ-850, BUGZ-853: Muted sign displays backwards in second person view, Fix lossy/duplicate color format conversions, Adjust wearables screen is flipped

libraries/display-plugins/src/display-plugins/Basic2DWindowOpenGLDisplayPlugin.cpp

@@ -113,14 +113,8 @@ gpu::PipelinePointer Basic2DWindowOpenGLDisplayPlugin::getRenderTexturePipeline(
 #if defined(Q_OS_ANDROID)
     return _linearToSRGBPipeline;
 #else
-
-#ifndef USE_GLES
-    return _SRGBToLinearPipeline;
-#else
     return _drawTexturePipeline;
 #endif
-
-#endif
 }
 
 void Basic2DWindowOpenGLDisplayPlugin::compositeExtra() {

libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.cpp

@@ -392,13 +392,11 @@ void OpenGLDisplayPlugin::customizeContext() {
 
         _drawTexturePipeline = gpu::Pipeline::create(gpu::Shader::createProgram(DrawTexture), scissorState);
 
-        _linearToSRGBPipeline = gpu::Pipeline::create(gpu::Shader::createProgram(DrawTextureGammaLinearToSRGB), scissorState);
+        _linearToSRGBPipeline = gpu::Pipeline::create(gpu::Shader::createProgram(DrawTextureLinearToSRGB), scissorState);
 
-        _SRGBToLinearPipeline = gpu::Pipeline::create(gpu::Shader::createProgram(DrawTextureGammaSRGBToLinear), scissorState);
+        _SRGBToLinearPipeline = gpu::Pipeline::create(gpu::Shader::createProgram(DrawTextureSRGBToLinear), scissorState);
 
-        _hudPipeline = gpu::Pipeline::create(gpu::Shader::createProgram(DrawTexture), blendState);
+        _hudPipeline = gpu::Pipeline::create(gpu::Shader::createProgram(DrawTextureSRGBToLinear), blendState);
-
-        _mirrorHUDPipeline = gpu::Pipeline::create(gpu::Shader::createProgram(DrawTextureMirroredX), blendState);
 
         _cursorPipeline = gpu::Pipeline::create(gpu::Shader::createProgram(DrawTransformedTexture), blendState);
     }
@@ -413,7 +411,6 @@ void OpenGLDisplayPlugin::uncustomizeContext() {
     _SRGBToLinearPipeline.reset();
     _cursorPipeline.reset();
     _hudPipeline.reset();
-    _mirrorHUDPipeline.reset();
     _compositeFramebuffer.reset();
 
     withPresentThreadLock([&] {
@@ -582,20 +579,18 @@ void OpenGLDisplayPlugin::updateFrameData() {
     });
 }
 
-std::function<void(gpu::Batch&, const gpu::TexturePointer&, bool mirror)> OpenGLDisplayPlugin::getHUDOperator() {
+std::function<void(gpu::Batch&, const gpu::TexturePointer&)> OpenGLDisplayPlugin::getHUDOperator() {
     auto hudPipeline = _hudPipeline;
-    auto hudMirrorPipeline = _mirrorHUDPipeline;
     auto hudStereo = isStereo();
     auto hudCompositeFramebufferSize = _compositeFramebuffer->getSize();
     std::array<glm::ivec4, 2> hudEyeViewports;
     for_each_eye([&](Eye eye) {
         hudEyeViewports[eye] = eyeViewport(eye);
     });
-    return [=](gpu::Batch& batch, const gpu::TexturePointer& hudTexture, bool mirror) {
+    return [=](gpu::Batch& batch, const gpu::TexturePointer& hudTexture) {
-        auto pipeline = mirror ? hudMirrorPipeline : hudPipeline;
+        if (hudPipeline && hudTexture) {
-        if (pipeline && hudTexture) {
             batch.enableStereo(false);
-            batch.setPipeline(pipeline);
+            batch.setPipeline(hudPipeline);
             batch.setResourceTexture(0, hudTexture);
             if (hudStereo) {
                 for_each_eye([&](Eye eye) {
@@ -642,7 +637,7 @@ void OpenGLDisplayPlugin::compositeScene() {
         batch.setStateScissorRect(ivec4(uvec2(), _compositeFramebuffer->getSize()));
         batch.resetViewTransform();
         batch.setProjectionTransform(mat4());
-        batch.setPipeline(getCompositeScenePipeline());
+        batch.setPipeline(_drawTexturePipeline);
         batch.setResourceTexture(0, _currentFrame->framebuffer->getRenderBuffer(0));
         batch.draw(gpu::TRIANGLE_STRIP, 4);
     });
@@ -905,7 +900,7 @@ OpenGLDisplayPlugin::~OpenGLDisplayPlugin() {
 void OpenGLDisplayPlugin::updateCompositeFramebuffer() {
     auto renderSize = glm::uvec2(getRecommendedRenderSize());
     if (!_compositeFramebuffer || _compositeFramebuffer->getSize() != renderSize) {
-        _compositeFramebuffer = gpu::FramebufferPointer(gpu::Framebuffer::create("OpenGLDisplayPlugin::composite", gpu::Element::COLOR_RGBA_32, renderSize.x, renderSize.y));
+        _compositeFramebuffer = gpu::FramebufferPointer(gpu::Framebuffer::create("OpenGLDisplayPlugin::composite", gpu::Element::COLOR_SRGBA_32, renderSize.x, renderSize.y));
     }
 }
 
@@ -964,7 +959,3 @@ gpu::PipelinePointer OpenGLDisplayPlugin::getRenderTexturePipeline() {
     return _drawTexturePipeline;
 }
 
-gpu::PipelinePointer OpenGLDisplayPlugin::getCompositeScenePipeline() {
-    return _drawTexturePipeline;
-}
-

libraries/display-plugins/src/display-plugins/OpenGLDisplayPlugin.h

@@ -80,7 +80,7 @@ public:
     // Three threads, one for rendering, one for texture transfers, one reserved for the GL driver
     int getRequiredThreadCount() const override { return 3; }
 
-    virtual std::function<void(gpu::Batch&, const gpu::TexturePointer&, bool mirror)> getHUDOperator() override;
+    virtual std::function<void(gpu::Batch&, const gpu::TexturePointer&)> getHUDOperator() override;
     void copyTextureToQuickFramebuffer(NetworkTexturePointer source,
                                        QOpenGLFramebufferObject* target,
                                        GLsync* fenceSync) override;
@@ -162,7 +162,6 @@ protected:
     float _compositeHUDAlpha{ 1.0f };
 
     virtual gpu::PipelinePointer getRenderTexturePipeline();
-    virtual gpu::PipelinePointer getCompositeScenePipeline();
 
     struct CursorData {
         QImage image;

libraries/display-plugins/src/display-plugins/hmd/HmdDisplayPlugin.cpp

@@ -174,10 +174,6 @@ float HmdDisplayPlugin::getLeftCenterPixel() const {
     return leftCenterPixel;
 }
 
-gpu::PipelinePointer HmdDisplayPlugin::getRenderTexturePipeline() {
-    return _SRGBToLinearPipeline;
-}
-
 void HmdDisplayPlugin::internalPresent() {
     PROFILE_RANGE_EX(render, __FUNCTION__, 0xff00ff00, (uint64_t)presentCount())
 
@@ -417,7 +413,7 @@ void HmdDisplayPlugin::HUDRenderer::build() {
     pipeline = gpu::Pipeline::create(program, state);
 }
 
-std::function<void(gpu::Batch&, const gpu::TexturePointer&, bool mirror)> HmdDisplayPlugin::HUDRenderer::render() {
+std::function<void(gpu::Batch&, const gpu::TexturePointer&)> HmdDisplayPlugin::HUDRenderer::render() {
     auto hudPipeline = pipeline;
     auto hudFormat = format;
     auto hudVertices = vertices;
@@ -425,7 +421,7 @@ std::function<void(gpu::Batch&, const gpu::TexturePointer&, bool mirror)> HmdDis
     auto hudUniformBuffer = uniformsBuffer;
     auto hudUniforms = uniforms;
     auto hudIndexCount = indexCount;
-    return [=](gpu::Batch& batch, const gpu::TexturePointer& hudTexture, bool mirror) {
+    return [=](gpu::Batch& batch, const gpu::TexturePointer& hudTexture) {
         if (hudPipeline && hudTexture) {
             batch.setPipeline(hudPipeline);
 
@@ -440,9 +436,6 @@ std::function<void(gpu::Batch&, const gpu::TexturePointer&, bool mirror)> HmdDis
 
             auto compositorHelper = DependencyManager::get<CompositorHelper>();
             glm::mat4 modelTransform = compositorHelper->getUiTransform();
-            if (mirror) {
-                modelTransform = glm::scale(modelTransform, glm::vec3(-1, 1, 1));
-            }
             batch.setModelTransform(modelTransform);
             batch.setResourceTexture(0, hudTexture);
 
@@ -475,7 +468,7 @@ void HmdDisplayPlugin::compositePointer() {
     });
 }
 
-std::function<void(gpu::Batch&, const gpu::TexturePointer&, bool mirror)> HmdDisplayPlugin::getHUDOperator() {
+std::function<void(gpu::Batch&, const gpu::TexturePointer&)> HmdDisplayPlugin::getHUDOperator() {
     return _hudRenderer.render();
 }
 

libraries/display-plugins/src/display-plugins/hmd/HmdDisplayPlugin.h

@@ -48,11 +48,9 @@ public:
 
     void pluginUpdate() override {};
 
-    std::function<void(gpu::Batch&, const gpu::TexturePointer&, bool mirror)> getHUDOperator() override;
+    std::function<void(gpu::Batch&, const gpu::TexturePointer&)> getHUDOperator() override;
     virtual StencilMaskMode getStencilMaskMode() const override { return StencilMaskMode::PAINT; }
 
-    virtual gpu::PipelinePointer getRenderTexturePipeline() override;
-
 signals:
     void hmdMountedChanged();
     void hmdVisibleChanged(bool visible);
@@ -125,6 +123,6 @@ private:
         static const int VERTEX_STRIDE { sizeof(Vertex) };
 
         void build();
-        std::function<void(gpu::Batch&, const gpu::TexturePointer&, bool mirror)> render();
+        std::function<void(gpu::Batch&, const gpu::TexturePointer&)> render();
     } _hudRenderer;
 };

libraries/gpu/src/gpu/DrawTextureLinearToSRGB.slf

@@ -2,7 +2,7 @@
 <$VERSION_HEADER$>
 //  Generated on <$_SCRIBE_DATE$>
 //
-//  DrawTextureGammaLinearToSRGB.frag
+//  DrawTextureLinearToSRGB.frag
 //
 //  Draw texture 0 fetched at texcoord.xy, and apply linear to sRGB color space conversion
 //

libraries/gpu/src/gpu/DrawTextureMirroredX.slf

@@ -13,6 +13,7 @@
 //  See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
 //
 
+<@include gpu/Color.slh@>
 
 LAYOUT(binding=0) uniform sampler2D colorMap;
 
@@ -20,5 +21,5 @@ layout(location=0) in vec2 varTexCoord0;
 layout(location=0) out vec4 outFragColor;
 
 void main(void) {
-    outFragColor = texture(colorMap, vec2(1.0 - varTexCoord0.x, varTexCoord0.y));
+    outFragColor =  vec4(texture(colorMap, vec2(1.0 - varTexCoord0.x, varTexCoord0.y)).xyz, 1.0);
 }

libraries/gpu/src/gpu/DrawTextureMirroredX.slp

@@ -1 +1 @@
-VERTEX DrawUnitQuadTexcoord
+VERTEX DrawTransformUnitQuad

libraries/gpu/src/gpu/DrawTextureOpaque.slf

@@ -14,8 +14,6 @@
 //  See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
 //
 
-<@include gpu/ShaderConstants.h@>
-
 LAYOUT(binding=0) uniform sampler2D colorMap;
 
 layout(location=0) in vec2 varTexCoord0;

libraries/gpu/src/gpu/DrawTextureSRGBToLinear.slf

@@ -2,7 +2,7 @@
 <$VERSION_HEADER$>
 //  Generated on <$_SCRIBE_DATE$>
 //
-//  DrawTextureGammaSRGBToLinear.frag
+//  DrawTextureSRGBToLinear.frag
 //
 //  Draw texture 0 fetched at texcoord.xy, and apply sRGB to Linear color space conversion
 //

libraries/plugins/src/plugins/DisplayPlugin.h

@@ -210,7 +210,7 @@ public:
     // for updating plugin-related commands. Mimics the input plugin.
     virtual void pluginUpdate() = 0;
 
-    virtual std::function<void(gpu::Batch&, const gpu::TexturePointer&, bool mirror)> getHUDOperator() { return nullptr; }
+    virtual std::function<void(gpu::Batch&, const gpu::TexturePointer&)> getHUDOperator() { return nullptr; }
     virtual StencilMaskMode getStencilMaskMode() const { return StencilMaskMode::NONE; }
     using StencilMaskMeshOperator = std::function<void(gpu::Batch&)>;
     virtual StencilMaskMeshOperator getStencilMaskMeshOperator() { return nullptr; }

libraries/render-utils/src/RenderCommonTask.cpp

@@ -128,52 +128,11 @@ void Blit::run(const RenderContextPointer& renderContext, const gpu::Framebuffer
     gpu::doInBatch("Blit", renderArgs->_context, [&](gpu::Batch& batch) {
         batch.setFramebuffer(blitFbo);
 
-        if (renderArgs->_renderMode == RenderArgs::MIRROR_RENDER_MODE) {
+        gpu::Vec4i rect;
-            if (renderArgs->isStereo()) {
+        rect.z = width;
-                gpu::Vec4i srcRectLeft;
+        rect.w = height;
-                srcRectLeft.z = width / 2;
-                srcRectLeft.w = height;
 
-                gpu::Vec4i srcRectRight;
+        batch.blit(primaryFbo, rect, blitFbo, rect);
-                srcRectRight.x = width / 2;
-                srcRectRight.z = width;
-                srcRectRight.w = height;
-
-                gpu::Vec4i destRectLeft;
-                destRectLeft.x = srcRectLeft.z;
-                destRectLeft.z = srcRectLeft.x;
-                destRectLeft.y = srcRectLeft.y;
-                destRectLeft.w = srcRectLeft.w;
-
-                gpu::Vec4i destRectRight;
-                destRectRight.x = srcRectRight.z;
-                destRectRight.z = srcRectRight.x;
-                destRectRight.y = srcRectRight.y;
-                destRectRight.w = srcRectRight.w;
-
-                // Blit left to right and right to left in stereo
-                batch.blit(primaryFbo, srcRectRight, blitFbo, destRectLeft);
-                batch.blit(primaryFbo, srcRectLeft, blitFbo, destRectRight);
-            } else {
-                gpu::Vec4i srcRect;
-                srcRect.z = width;
-                srcRect.w = height;
-
-                gpu::Vec4i destRect;
-                destRect.x = width;
-                destRect.y = 0;
-                destRect.z = 0;
-                destRect.w = height;
-
-                batch.blit(primaryFbo, srcRect, blitFbo, destRect);
-            }
-        } else {
-            gpu::Vec4i rect;
-            rect.z = width;
-            rect.w = height;
-
-            batch.blit(primaryFbo, rect, blitFbo, rect);
-        }
     });
 }
 

libraries/render-utils/src/RenderForwardTask.cpp

@@ -144,9 +144,9 @@ void RenderForwardTask::build(JobModel& task, const render::Varying& input, rend
 
     // Just resolve the msaa
     const auto resolveInputs = ResolveFramebuffer::Inputs(scaledPrimaryFramebuffer, static_cast<gpu::FramebufferPointer>(nullptr)).asVarying();
-     const auto resolvedFramebuffer = task.addJob<ResolveFramebuffer>("Resolve", resolveInputs);
+    const auto resolvedFramebuffer = task.addJob<ResolveFramebuffer>("Resolve", resolveInputs);
 
-     const auto toneMappedBuffer = resolvedFramebuffer;
+    const auto toneMappedBuffer = resolvedFramebuffer;
 #else
     const auto newResolvedFramebuffer = task.addJob<NewOrDefaultFramebuffer>("MakeResolvingFramebuffer");
 

libraries/render-utils/src/RenderHUDLayerTask.cpp

@@ -34,7 +34,7 @@ void CompositeHUD::run(const RenderContextPointer& renderContext, const gpu::Fra
             batch.setFramebuffer(inputs);
         }
         if (renderContext->args->_hudOperator) {
-            renderContext->args->_hudOperator(batch, renderContext->args->_hudTexture, renderContext->args->_renderMode == RenderArgs::RenderMode::MIRROR_RENDER_MODE);
+            renderContext->args->_hudOperator(batch, renderContext->args->_hudTexture);
         }
     });
 #endif

libraries/render-utils/src/hmd_ui.slf

@@ -12,6 +12,7 @@
 //  See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
 //
 <@include render-utils/ShaderConstants.h@>
+<@include gpu/Color.slh@>
 
 LAYOUT(binding=0) uniform sampler2D hudTexture;
 
@@ -36,5 +37,5 @@ void main() {
         discard;
     }
 
-    fragColor0 = color;
+    fragColor0 = color_sRGBAToLinear(color);
 }

libraries/render-utils/src/toneMapping.slf

@@ -20,6 +20,7 @@ struct ToneMappingParams {
     ivec4 _toneCurve_s0_s1_s2;
 };
 
+const float GAMMA_22 = 2.2;
 const float INV_GAMMA_22 = 1.0 / 2.2;
 const int ToneCurveNone = 0;
 const int ToneCurveGamma22 = 1;
@@ -51,13 +52,20 @@ void main(void) {
     vec3 tonedColor = srcColor;
     if (toneCurve == ToneCurveFilmic) {
         vec3 x = max(vec3(0.0), srcColor-0.004);
-        tonedColor = (x * (6.2 * x + 0.5)) / (x * (6.2 * x + 1.7) + 0.06);
+        tonedColor = pow((x * (6.2 * x + 0.5)) / (x * (6.2 * x + 1.7) + 0.06), vec3(GAMMA_22));
     } else if (toneCurve == ToneCurveReinhard) {
         tonedColor = srcColor/(1.0 + srcColor);
-        tonedColor = pow(tonedColor, vec3(INV_GAMMA_22));
     } else if (toneCurve == ToneCurveGamma22) {
-        tonedColor = pow(srcColor, vec3(INV_GAMMA_22));
+        // We use glEnable(GL_FRAMEBUFFER_SRGB), which automatically converts textures from RGB to SRGB
-    } // else None toned = src
+        // when writing from an RGB framebuffer to an SRGB framebuffer (note that it doesn't do anything
+        // when writing from an SRGB framebuffer to an RGB framebuffer).
+        // Since the conversion happens automatically, we don't need to do anything in this shader
+    } else {
+        // toneCurve == ToneCurveNone
+        // For debugging purposes, we may want to see what the colors look like before the automatic OpenGL 
+        // conversion mentioned above, so we undo it here
+        tonedColor = pow(srcColor, vec3(GAMMA_22));
+    } 
 
     outFragColor = vec4(tonedColor, 1.0);
 }

libraries/render/src/render/Args.h

@@ -136,7 +136,7 @@ namespace render {
         render::ScenePointer _scene;
         int8_t _cameraMode { -1 };
 
-        std::function<void(gpu::Batch&, const gpu::TexturePointer&, bool mirror)> _hudOperator { nullptr };
+        std::function<void(gpu::Batch&, const gpu::TexturePointer&)> _hudOperator { nullptr };
         gpu::TexturePointer _hudTexture { nullptr };
 
         bool _takingSnapshot { false };

libraries/render/src/render/ResampleTask.cpp

@@ -16,6 +16,7 @@
 #include <shaders/Shaders.h>
 
 using namespace render;
+using namespace shader::gpu::program;
 
 gpu::PipelinePointer HalfDownsample::_pipeline;
 
@@ -137,6 +138,7 @@ void Upsample::run(const RenderContextPointer& renderContext, const gpu::Framebu
 }
 
 gpu::PipelinePointer UpsampleToBlitFramebuffer::_pipeline;
+gpu::PipelinePointer UpsampleToBlitFramebuffer::_mirrorPipeline;
 
 void UpsampleToBlitFramebuffer::run(const RenderContextPointer& renderContext, const Input& input, gpu::FramebufferPointer& resampledFrameBuffer) {
     assert(renderContext->args);
@@ -148,10 +150,11 @@ void UpsampleToBlitFramebuffer::run(const RenderContextPointer& renderContext, c
 
     if (resampledFrameBuffer != sourceFramebuffer) {
         if (!_pipeline) {
-            gpu::ShaderPointer program = gpu::Shader::createProgram(shader::gpu::program::drawTransformUnitQuadTextureOpaque);
             gpu::StatePointer state = gpu::StatePointer(new gpu::State());
             state->setDepthTest(gpu::State::DepthTest(false, false));
-            _pipeline = gpu::Pipeline::create(program, state);
+
+            _pipeline = gpu::Pipeline::create(gpu::Shader::createProgram(drawTransformUnitQuadTextureOpaque), state);
+            _mirrorPipeline = gpu::Pipeline::create(gpu::Shader::createProgram(DrawTextureMirroredX), state);
         }
         const auto bufferSize = resampledFrameBuffer->getSize();
         glm::ivec4 viewport{ 0, 0, bufferSize.x, bufferSize.y };
@@ -164,7 +167,7 @@ void UpsampleToBlitFramebuffer::run(const RenderContextPointer& renderContext, c
             batch.setViewportTransform(viewport);
             batch.setProjectionTransform(glm::mat4());
             batch.resetViewTransform();
-            batch.setPipeline(_pipeline);
+            batch.setPipeline(args->_renderMode == RenderArgs::MIRROR_RENDER_MODE ? _mirrorPipeline : _pipeline);
 
             batch.setModelTransform(gpu::Framebuffer::evalSubregionTexcoordTransform(bufferSize, viewport));
             batch.setResourceTexture(0, sourceFramebuffer->getRenderBuffer(0));

libraries/render/src/render/ResampleTask.h

@@ -80,6 +80,7 @@ namespace render {
     protected:
 
         static gpu::PipelinePointer _pipeline;
+        static gpu::PipelinePointer _mirrorPipeline;
     };
 }
 

plugins/oculus/src/OculusDisplayPlugin.cpp

@@ -163,7 +163,7 @@ void OculusDisplayPlugin::hmdPresent() {
             batch.setStateScissorRect(ivec4(uvec2(), _outputFramebuffer->getSize()));
             batch.resetViewTransform();
             batch.setProjectionTransform(mat4());
-            batch.setPipeline(_SRGBToLinearPipeline);
+            batch.setPipeline(_drawTexturePipeline);
             batch.setResourceTexture(0, _compositeFramebuffer->getRenderBuffer(0));
             batch.draw(gpu::TRIANGLE_STRIP, 4);
         });