Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix banding and discoloration issue in 3D scene #629

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 5 additions & 7 deletions src/plugins/minimal_scene/MinimalScene.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1073,7 +1073,10 @@ void TextureNode::NewTexture(void* _texturePtr, const QSize &_size)
/////////////////////////////////////////////////
void TextureNode::PrepareNode()
{
this->rhi->PrepareNode();
{
std::unique_lock<std::mutex> lock(this->renderSync.mutex);
this->rhi->PrepareNode();
}

if (this->rhi->HasNewTexture())
{
Expand All @@ -1085,6 +1088,7 @@ void TextureNode::PrepareNode()
// rendered and it can start rendering to the other one.
// emit TextureInUse(&this->renderSync); See comment below
}

// NOTE: The original code from Qt samples only emitted when
// newId is not null.
//
Expand Down Expand Up @@ -1545,12 +1549,6 @@ void MinimalScene::LoadConfig(const tinyxml2::XMLElement *_pluginElem)
}

renderWindow->SetEngineName(cmdRenderEngine);
// there is a problem with displaying ogre2 render textures that are in
// sRGB format. Workaround for now is to apply gamma correction
// manually.
// There maybe a better way to solve the problem by making OpenGL calls.
if (cmdRenderEngine == std::string("ogre2"))
this->PluginItem()->setProperty("gammaCorrect", true);
}

/////////////////////////////////////////////////
Expand Down
11 changes: 0 additions & 11 deletions src/plugins/minimal_scene/MinimalScene.qml
Original file line number Diff line number Diff line change
Expand Up @@ -54,17 +54,6 @@ Rectangle {
visible: MinimalScene.loadingError.length == 0
}

/*
* Gamma correction for sRGB output. Enabled when engine is set to ogre2
*/
GammaAdjust {
anchors.fill: renderWindow
source: renderWindow
gamma: 2.4
enabled: gammaCorrect
visible: gammaCorrect
}

onParentChanged: {
if (undefined === parent)
return;
Expand Down
72 changes: 70 additions & 2 deletions src/plugins/minimal_scene/MinimalSceneRhiOpenGL.cc
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,19 @@ namespace gz::gui::plugins

class TextureNodeRhiOpenGLPrivate
{
// Current texture id from the render engine
public: GLuint textureId = 0;

// New texture id from the render engine
public: GLuint newTextureId = 0;

// Intermediate FBO for copying texture data
public: GLuint rhiFbo = 0;

// Texture id for internal texture that holds texture data from
// the render engine texture. The texture data is in GL_RGB format
public: GLuint rhiTextureId = 0;

public: QSize size {0, 0};
public: QSize newSize {0, 0};
public: QMutex mutex;
Expand Down Expand Up @@ -195,6 +206,13 @@ void RenderThreadRhiOpenGL::ShutDown()
/////////////////////////////////////////////////
TextureNodeRhiOpenGL::~TextureNodeRhiOpenGL()
{
// clean up opengl resources owned by this rhi
QOpenGLFunctions *f = QOpenGLContext::currentContext()->functions();
if (this->dataPtr->rhiFbo)
f->glDeleteFramebuffers(1, &this->dataPtr->rhiFbo);
if (this->dataPtr->rhiTextureId)
f->glDeleteTextures(1, &this->dataPtr->rhiTextureId);

delete this->dataPtr->texture;
this->dataPtr->texture = nullptr;
}
Expand Down Expand Up @@ -265,13 +283,63 @@ void TextureNodeRhiOpenGL::PrepareNode()
delete this->dataPtr->texture;
this->dataPtr->texture = nullptr;

GLuint texForQt = this->dataPtr->newTextureId;

// Get the internal texture format of the GL texture from the
// rendering engine
QOpenGLFunctions *f = QOpenGLContext::currentContext()->functions();
f->glBindTexture(GL_TEXTURE_2D, this->dataPtr->newTextureId);
GLint texFormat = 0;
glGetTexLevelParameteriv(
GL_TEXTURE_2D,
0,
GL_TEXTURE_INTERNAL_FORMAT,
&texFormat
);
f->glBindTexture(GL_TEXTURE_2D, 0);

// if render engine's texture is in GL_SRGB format, convert it to GL_RGB
// as that is the format of QSG textures, otherwise the scene will appear
// dark.
if (texFormat == GL_SRGB || texFormat == GL_SRGB8 ||
texFormat == GL_SRGB_ALPHA || texFormat == GL_SRGB8_ALPHA8)
{
// Get the currently bound fbo so we can restore it later.
GLint currentFbo;
f->glGetIntegerv(GL_FRAMEBUFFER_BINDING, &currentFbo);

// Bind the texture from the rendering engine to our own RHI fbo
if (!this->dataPtr->rhiFbo)
f->glGenFramebuffers(1, &this->dataPtr->rhiFbo);
f->glBindFramebuffer(GL_FRAMEBUFFER, this->dataPtr->rhiFbo);
f->glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
GL_TEXTURE_2D, this->dataPtr->newTextureId, 0);

// Copy the source render engine texture data to our own internal texture
// The copied texture is the one we will pass to Qt
if (!this->dataPtr->rhiTextureId)
f->glGenTextures(1, &this->dataPtr->rhiTextureId);
f->glBindTexture(GL_TEXTURE_2D, this->dataPtr->rhiTextureId);
f->glCopyTexImage2D(GL_TEXTURE_2D, 0, GL_RGB, 0, 0,
this->dataPtr->newSize.width(), this->dataPtr->newSize.height(), 0);

// Unbind the render engine texture from RHI fbo and rebind the
// previously bound FBO.
f->glFramebufferTexture2D(GL_FRAMEBUFFER, GL_COLOR_ATTACHMENT0,
GL_TEXTURE_2D, 0, 0);
f->glBindTexture(GL_TEXTURE_2D, 0);
f->glBindFramebuffer(GL_FRAMEBUFFER, (GLuint) currentFbo);

texForQt = this->dataPtr->rhiTextureId;
}

#if QT_VERSION < QT_VERSION_CHECK(5, 14, 0)
# ifndef _WIN32
# pragma GCC diagnostic push
# pragma GCC diagnostic ignored "-Wdeprecated-declarations"
# endif
this->dataPtr->texture = this->dataPtr->window->createTextureFromId(
this->dataPtr->newTextureId,
texForQt,
this->dataPtr->newSize,
QQuickWindow::TextureIsOpaque);
# ifndef _WIN32
Expand All @@ -281,7 +349,7 @@ void TextureNodeRhiOpenGL::PrepareNode()
this->dataPtr->texture =
this->dataPtr->window->createTextureFromNativeObject(
QQuickWindow::NativeObjectTexture,
static_cast<void*>(&this->dataPtr->newTextureId),
static_cast<void*>(&texForQt),
0,
this->dataPtr->newSize);
#endif
Expand Down