Skip to content

Commit a3ec365

Browse files
shanzzamshanzzam
authored
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
2 parents a3f0b2c + f44bc52 commit a3ec365

22 files changed

+51
-105
lines changed

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

-6
Original file line numberDiff line numberDiff line change
@@ -113,14 +113,8 @@ gpu::PipelinePointer Basic2DWindowOpenGLDisplayPlugin::getRenderTexturePipeline(
113113
#if defined(Q_OS_ANDROID)
114114
return _linearToSRGBPipeline;
115115
#else
116-
117-
#ifndef USE_GLES
118-
return _SRGBToLinearPipeline;
119-
#else
120116
return _drawTexturePipeline;
121117
#endif
122-
123-
#endif
124118
}
125119

126120
void Basic2DWindowOpenGLDisplayPlugin::compositeExtra() {

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

+9-18
Original file line numberDiff line numberDiff line change
@@ -392,13 +392,11 @@ void OpenGLDisplayPlugin::customizeContext() {
392392

393393
_drawTexturePipeline = gpu::Pipeline::create(gpu::Shader::createProgram(DrawTexture), scissorState);
394394

395-
_linearToSRGBPipeline = gpu::Pipeline::create(gpu::Shader::createProgram(DrawTextureGammaLinearToSRGB), scissorState);
395+
_linearToSRGBPipeline = gpu::Pipeline::create(gpu::Shader::createProgram(DrawTextureLinearToSRGB), scissorState);
396396

397-
_SRGBToLinearPipeline = gpu::Pipeline::create(gpu::Shader::createProgram(DrawTextureGammaSRGBToLinear), scissorState);
397+
_SRGBToLinearPipeline = gpu::Pipeline::create(gpu::Shader::createProgram(DrawTextureSRGBToLinear), scissorState);
398398

399-
_hudPipeline = gpu::Pipeline::create(gpu::Shader::createProgram(DrawTexture), blendState);
400-
401-
_mirrorHUDPipeline = gpu::Pipeline::create(gpu::Shader::createProgram(DrawTextureMirroredX), blendState);
399+
_hudPipeline = gpu::Pipeline::create(gpu::Shader::createProgram(DrawTextureSRGBToLinear), blendState);
402400

403401
_cursorPipeline = gpu::Pipeline::create(gpu::Shader::createProgram(DrawTransformedTexture), blendState);
404402
}
@@ -413,7 +411,6 @@ void OpenGLDisplayPlugin::uncustomizeContext() {
413411
_SRGBToLinearPipeline.reset();
414412
_cursorPipeline.reset();
415413
_hudPipeline.reset();
416-
_mirrorHUDPipeline.reset();
417414
_compositeFramebuffer.reset();
418415

419416
withPresentThreadLock([&] {
@@ -582,20 +579,18 @@ void OpenGLDisplayPlugin::updateFrameData() {
582579
});
583580
}
584581

585-
std::function<void(gpu::Batch&, const gpu::TexturePointer&, bool mirror)> OpenGLDisplayPlugin::getHUDOperator() {
582+
std::function<void(gpu::Batch&, const gpu::TexturePointer&)> OpenGLDisplayPlugin::getHUDOperator() {
586583
auto hudPipeline = _hudPipeline;
587-
auto hudMirrorPipeline = _mirrorHUDPipeline;
588584
auto hudStereo = isStereo();
589585
auto hudCompositeFramebufferSize = _compositeFramebuffer->getSize();
590586
std::array<glm::ivec4, 2> hudEyeViewports;
591587
for_each_eye([&](Eye eye) {
592588
hudEyeViewports[eye] = eyeViewport(eye);
593589
});
594-
return [=](gpu::Batch& batch, const gpu::TexturePointer& hudTexture, bool mirror) {
595-
auto pipeline = mirror ? hudMirrorPipeline : hudPipeline;
596-
if (pipeline && hudTexture) {
590+
return [=](gpu::Batch& batch, const gpu::TexturePointer& hudTexture) {
591+
if (hudPipeline && hudTexture) {
597592
batch.enableStereo(false);
598-
batch.setPipeline(pipeline);
593+
batch.setPipeline(hudPipeline);
599594
batch.setResourceTexture(0, hudTexture);
600595
if (hudStereo) {
601596
for_each_eye([&](Eye eye) {
@@ -642,7 +637,7 @@ void OpenGLDisplayPlugin::compositeScene() {
642637
batch.setStateScissorRect(ivec4(uvec2(), _compositeFramebuffer->getSize()));
643638
batch.resetViewTransform();
644639
batch.setProjectionTransform(mat4());
645-
batch.setPipeline(getCompositeScenePipeline());
640+
batch.setPipeline(_drawTexturePipeline);
646641
batch.setResourceTexture(0, _currentFrame->framebuffer->getRenderBuffer(0));
647642
batch.draw(gpu::TRIANGLE_STRIP, 4);
648643
});
@@ -905,7 +900,7 @@ OpenGLDisplayPlugin::~OpenGLDisplayPlugin() {
905900
void OpenGLDisplayPlugin::updateCompositeFramebuffer() {
906901
auto renderSize = glm::uvec2(getRecommendedRenderSize());
907902
if (!_compositeFramebuffer || _compositeFramebuffer->getSize() != renderSize) {
908-
_compositeFramebuffer = gpu::FramebufferPointer(gpu::Framebuffer::create("OpenGLDisplayPlugin::composite", gpu::Element::COLOR_RGBA_32, renderSize.x, renderSize.y));
903+
_compositeFramebuffer = gpu::FramebufferPointer(gpu::Framebuffer::create("OpenGLDisplayPlugin::composite", gpu::Element::COLOR_SRGBA_32, renderSize.x, renderSize.y));
909904
}
910905
}
911906

@@ -964,7 +959,3 @@ gpu::PipelinePointer OpenGLDisplayPlugin::getRenderTexturePipeline() {
964959
return _drawTexturePipeline;
965960
}
966961

967-
gpu::PipelinePointer OpenGLDisplayPlugin::getCompositeScenePipeline() {
968-
return _drawTexturePipeline;
969-
}
970-

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

+1-2
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ class OpenGLDisplayPlugin : public DisplayPlugin {
8080
// Three threads, one for rendering, one for texture transfers, one reserved for the GL driver
8181
int getRequiredThreadCount() const override { return 3; }
8282

83-
virtual std::function<void(gpu::Batch&, const gpu::TexturePointer&, bool mirror)> getHUDOperator() override;
83+
virtual std::function<void(gpu::Batch&, const gpu::TexturePointer&)> getHUDOperator() override;
8484
void copyTextureToQuickFramebuffer(NetworkTexturePointer source,
8585
QOpenGLFramebufferObject* target,
8686
GLsync* fenceSync) override;
@@ -162,7 +162,6 @@ class OpenGLDisplayPlugin : public DisplayPlugin {
162162
float _compositeHUDAlpha{ 1.0f };
163163

164164
virtual gpu::PipelinePointer getRenderTexturePipeline();
165-
virtual gpu::PipelinePointer getCompositeScenePipeline();
166165

167166
struct CursorData {
168167
QImage image;

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

+3-10
Original file line numberDiff line numberDiff line change
@@ -174,10 +174,6 @@ float HmdDisplayPlugin::getLeftCenterPixel() const {
174174
return leftCenterPixel;
175175
}
176176

177-
gpu::PipelinePointer HmdDisplayPlugin::getRenderTexturePipeline() {
178-
return _SRGBToLinearPipeline;
179-
}
180-
181177
void HmdDisplayPlugin::internalPresent() {
182178
PROFILE_RANGE_EX(render, __FUNCTION__, 0xff00ff00, (uint64_t)presentCount())
183179

@@ -417,15 +413,15 @@ void HmdDisplayPlugin::HUDRenderer::build() {
417413
pipeline = gpu::Pipeline::create(program, state);
418414
}
419415

420-
std::function<void(gpu::Batch&, const gpu::TexturePointer&, bool mirror)> HmdDisplayPlugin::HUDRenderer::render() {
416+
std::function<void(gpu::Batch&, const gpu::TexturePointer&)> HmdDisplayPlugin::HUDRenderer::render() {
421417
auto hudPipeline = pipeline;
422418
auto hudFormat = format;
423419
auto hudVertices = vertices;
424420
auto hudIndices = indices;
425421
auto hudUniformBuffer = uniformsBuffer;
426422
auto hudUniforms = uniforms;
427423
auto hudIndexCount = indexCount;
428-
return [=](gpu::Batch& batch, const gpu::TexturePointer& hudTexture, bool mirror) {
424+
return [=](gpu::Batch& batch, const gpu::TexturePointer& hudTexture) {
429425
if (hudPipeline && hudTexture) {
430426
batch.setPipeline(hudPipeline);
431427

@@ -440,9 +436,6 @@ std::function<void(gpu::Batch&, const gpu::TexturePointer&, bool mirror)> HmdDis
440436

441437
auto compositorHelper = DependencyManager::get<CompositorHelper>();
442438
glm::mat4 modelTransform = compositorHelper->getUiTransform();
443-
if (mirror) {
444-
modelTransform = glm::scale(modelTransform, glm::vec3(-1, 1, 1));
445-
}
446439
batch.setModelTransform(modelTransform);
447440
batch.setResourceTexture(0, hudTexture);
448441

@@ -475,7 +468,7 @@ void HmdDisplayPlugin::compositePointer() {
475468
});
476469
}
477470

478-
std::function<void(gpu::Batch&, const gpu::TexturePointer&, bool mirror)> HmdDisplayPlugin::getHUDOperator() {
471+
std::function<void(gpu::Batch&, const gpu::TexturePointer&)> HmdDisplayPlugin::getHUDOperator() {
479472
return _hudRenderer.render();
480473
}
481474

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

+2-4
Original file line numberDiff line numberDiff line change
@@ -48,11 +48,9 @@ class HmdDisplayPlugin : public OpenGLDisplayPlugin {
4848

4949
void pluginUpdate() override {};
5050

51-
std::function<void(gpu::Batch&, const gpu::TexturePointer&, bool mirror)> getHUDOperator() override;
51+
std::function<void(gpu::Batch&, const gpu::TexturePointer&)> getHUDOperator() override;
5252
virtual StencilMaskMode getStencilMaskMode() const override { return StencilMaskMode::PAINT; }
5353

54-
virtual gpu::PipelinePointer getRenderTexturePipeline() override;
55-
5654
signals:
5755
void hmdMountedChanged();
5856
void hmdVisibleChanged(bool visible);
@@ -125,6 +123,6 @@ class HmdDisplayPlugin : public OpenGLDisplayPlugin {
125123
static const int VERTEX_STRIDE { sizeof(Vertex) };
126124

127125
void build();
128-
std::function<void(gpu::Batch&, const gpu::TexturePointer&, bool mirror)> render();
126+
std::function<void(gpu::Batch&, const gpu::TexturePointer&)> render();
129127
} _hudRenderer;
130128
};

‎libraries/gpu/src/gpu/DrawTextureGammaLinearToSRGB.slf renamed to ‎libraries/gpu/src/gpu/DrawTextureLinearToSRGB.slf

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
<$VERSION_HEADER$>
33
// Generated on <$_SCRIBE_DATE$>
44
//
5-
// DrawTextureGammaLinearToSRGB.frag
5+
// DrawTextureLinearToSRGB.frag
66
//
77
// Draw texture 0 fetched at texcoord.xy, and apply linear to sRGB color space conversion
88
//

‎libraries/gpu/src/gpu/DrawTextureGammaLinearToSRGB.slp renamed to ‎libraries/gpu/src/gpu/DrawTextureLinearToSRGB.slp

File renamed without changes.

‎libraries/gpu/src/gpu/DrawTextureMirroredX.slf

+2-1
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,13 @@
1313
// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
1414
//
1515

16+
<@include gpu/Color.slh@>
1617

1718
LAYOUT(binding=0) uniform sampler2D colorMap;
1819

1920
layout(location=0) in vec2 varTexCoord0;
2021
layout(location=0) out vec4 outFragColor;
2122

2223
void main(void) {
23-
outFragColor = texture(colorMap, vec2(1.0 - varTexCoord0.x, varTexCoord0.y));
24+
outFragColor = vec4(texture(colorMap, vec2(1.0 - varTexCoord0.x, varTexCoord0.y)).xyz, 1.0);
2425
}

‎libraries/gpu/src/gpu/DrawTextureMirroredX.slp

+1-1
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
VERTEX DrawUnitQuadTexcoord
1+
VERTEX DrawTransformUnitQuad

‎libraries/gpu/src/gpu/DrawTextureOpaque.slf

-2
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,6 @@
1414
// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
1515
//
1616

17-
<@include gpu/ShaderConstants.h@>
18-
1917
LAYOUT(binding=0) uniform sampler2D colorMap;
2018

2119
layout(location=0) in vec2 varTexCoord0;

‎libraries/gpu/src/gpu/DrawTextureGammaSRGBToLinear.slf renamed to ‎libraries/gpu/src/gpu/DrawTextureSRGBToLinear.slf

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
<$VERSION_HEADER$>
33
// Generated on <$_SCRIBE_DATE$>
44
//
5-
// DrawTextureGammaSRGBToLinear.frag
5+
// DrawTextureSRGBToLinear.frag
66
//
77
// Draw texture 0 fetched at texcoord.xy, and apply sRGB to Linear color space conversion
88
//

‎libraries/gpu/src/gpu/DrawTextureGammaSRGBToLinear.slp renamed to ‎libraries/gpu/src/gpu/DrawTextureSRGBToLinear.slp

File renamed without changes.

‎libraries/plugins/src/plugins/DisplayPlugin.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ class DisplayPlugin : public Plugin, public HmdDisplay {
210210
// for updating plugin-related commands. Mimics the input plugin.
211211
virtual void pluginUpdate() = 0;
212212

213-
virtual std::function<void(gpu::Batch&, const gpu::TexturePointer&, bool mirror)> getHUDOperator() { return nullptr; }
213+
virtual std::function<void(gpu::Batch&, const gpu::TexturePointer&)> getHUDOperator() { return nullptr; }
214214
virtual StencilMaskMode getStencilMaskMode() const { return StencilMaskMode::NONE; }
215215
using StencilMaskMeshOperator = std::function<void(gpu::Batch&)>;
216216
virtual StencilMaskMeshOperator getStencilMaskMeshOperator() { return nullptr; }

‎libraries/render-utils/src/RenderCommonTask.cpp

+4-45
Original file line numberDiff line numberDiff line change
@@ -128,52 +128,11 @@ void Blit::run(const RenderContextPointer& renderContext, const gpu::Framebuffer
128128
gpu::doInBatch("Blit", renderArgs->_context, [&](gpu::Batch& batch) {
129129
batch.setFramebuffer(blitFbo);
130130

131-
if (renderArgs->_renderMode == RenderArgs::MIRROR_RENDER_MODE) {
132-
if (renderArgs->isStereo()) {
133-
gpu::Vec4i srcRectLeft;
134-
srcRectLeft.z = width / 2;
135-
srcRectLeft.w = height;
136-
137-
gpu::Vec4i srcRectRight;
138-
srcRectRight.x = width / 2;
139-
srcRectRight.z = width;
140-
srcRectRight.w = height;
141-
142-
gpu::Vec4i destRectLeft;
143-
destRectLeft.x = srcRectLeft.z;
144-
destRectLeft.z = srcRectLeft.x;
145-
destRectLeft.y = srcRectLeft.y;
146-
destRectLeft.w = srcRectLeft.w;
147-
148-
gpu::Vec4i destRectRight;
149-
destRectRight.x = srcRectRight.z;
150-
destRectRight.z = srcRectRight.x;
151-
destRectRight.y = srcRectRight.y;
152-
destRectRight.w = srcRectRight.w;
153-
154-
// Blit left to right and right to left in stereo
155-
batch.blit(primaryFbo, srcRectRight, blitFbo, destRectLeft);
156-
batch.blit(primaryFbo, srcRectLeft, blitFbo, destRectRight);
157-
} else {
158-
gpu::Vec4i srcRect;
159-
srcRect.z = width;
160-
srcRect.w = height;
161-
162-
gpu::Vec4i destRect;
163-
destRect.x = width;
164-
destRect.y = 0;
165-
destRect.z = 0;
166-
destRect.w = height;
167-
168-
batch.blit(primaryFbo, srcRect, blitFbo, destRect);
169-
}
170-
} else {
171-
gpu::Vec4i rect;
172-
rect.z = width;
173-
rect.w = height;
131+
gpu::Vec4i rect;
132+
rect.z = width;
133+
rect.w = height;
174134

175-
batch.blit(primaryFbo, rect, blitFbo, rect);
176-
}
135+
batch.blit(primaryFbo, rect, blitFbo, rect);
177136
});
178137
}
179138

‎libraries/render-utils/src/RenderForwardTask.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -144,9 +144,9 @@ void RenderForwardTask::build(JobModel& task, const render::Varying& input, rend
144144

145145
// Just resolve the msaa
146146
const auto resolveInputs = ResolveFramebuffer::Inputs(scaledPrimaryFramebuffer, static_cast<gpu::FramebufferPointer>(nullptr)).asVarying();
147-
const auto resolvedFramebuffer = task.addJob<ResolveFramebuffer>("Resolve", resolveInputs);
147+
const auto resolvedFramebuffer = task.addJob<ResolveFramebuffer>("Resolve", resolveInputs);
148148

149-
const auto toneMappedBuffer = resolvedFramebuffer;
149+
const auto toneMappedBuffer = resolvedFramebuffer;
150150
#else
151151
const auto newResolvedFramebuffer = task.addJob<NewOrDefaultFramebuffer>("MakeResolvingFramebuffer");
152152

‎libraries/render-utils/src/RenderHUDLayerTask.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ void CompositeHUD::run(const RenderContextPointer& renderContext, const gpu::Fra
3434
batch.setFramebuffer(inputs);
3535
}
3636
if (renderContext->args->_hudOperator) {
37-
renderContext->args->_hudOperator(batch, renderContext->args->_hudTexture, renderContext->args->_renderMode == RenderArgs::RenderMode::MIRROR_RENDER_MODE);
37+
renderContext->args->_hudOperator(batch, renderContext->args->_hudTexture);
3838
}
3939
});
4040
#endif

‎libraries/render-utils/src/hmd_ui.slf

+2-1
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
1313
//
1414
<@include render-utils/ShaderConstants.h@>
15+
<@include gpu/Color.slh@>
1516

1617
LAYOUT(binding=0) uniform sampler2D hudTexture;
1718

@@ -36,5 +37,5 @@ void main() {
3637
discard;
3738
}
3839

39-
fragColor0 = color;
40+
fragColor0 = color_sRGBAToLinear(color);
4041
}

‎libraries/render-utils/src/toneMapping.slf

+12-4
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ struct ToneMappingParams {
2020
ivec4 _toneCurve_s0_s1_s2;
2121
};
2222

23+
const float GAMMA_22 = 2.2;
2324
const float INV_GAMMA_22 = 1.0 / 2.2;
2425
const int ToneCurveNone = 0;
2526
const int ToneCurveGamma22 = 1;
@@ -51,13 +52,20 @@ void main(void) {
5152
vec3 tonedColor = srcColor;
5253
if (toneCurve == ToneCurveFilmic) {
5354
vec3 x = max(vec3(0.0), srcColor-0.004);
54-
tonedColor = (x * (6.2 * x + 0.5)) / (x * (6.2 * x + 1.7) + 0.06);
55+
tonedColor = pow((x * (6.2 * x + 0.5)) / (x * (6.2 * x + 1.7) + 0.06), vec3(GAMMA_22));
5556
} else if (toneCurve == ToneCurveReinhard) {
5657
tonedColor = srcColor/(1.0 + srcColor);
57-
tonedColor = pow(tonedColor, vec3(INV_GAMMA_22));
5858
} else if (toneCurve == ToneCurveGamma22) {
59-
tonedColor = pow(srcColor, vec3(INV_GAMMA_22));
60-
} // else None toned = src
59+
// We use glEnable(GL_FRAMEBUFFER_SRGB), which automatically converts textures from RGB to SRGB
60+
// when writing from an RGB framebuffer to an SRGB framebuffer (note that it doesn't do anything
61+
// when writing from an SRGB framebuffer to an RGB framebuffer).
62+
// Since the conversion happens automatically, we don't need to do anything in this shader
63+
} else {
64+
// toneCurve == ToneCurveNone
65+
// For debugging purposes, we may want to see what the colors look like before the automatic OpenGL
66+
// conversion mentioned above, so we undo it here
67+
tonedColor = pow(srcColor, vec3(GAMMA_22));
68+
}
6169

6270
outFragColor = vec4(tonedColor, 1.0);
6371
}

‎libraries/render/src/render/Args.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ namespace render {
136136
render::ScenePointer _scene;
137137
int8_t _cameraMode { -1 };
138138

139-
std::function<void(gpu::Batch&, const gpu::TexturePointer&, bool mirror)> _hudOperator { nullptr };
139+
std::function<void(gpu::Batch&, const gpu::TexturePointer&)> _hudOperator { nullptr };
140140
gpu::TexturePointer _hudTexture { nullptr };
141141

142142
bool _takingSnapshot { false };

0 commit comments

Comments
 (0)