QOpenGLShaderProgram: how to update fragment shader on-the-fly?

SGaist

Hi,

WARNING not tested
Quick idea: shouldn't you remove the shader, build it again (or create a new one) and then add it again to the program ?

Violet Giraffe

@SGaist said in QOpenGLShaderProgram: how to update fragment shader on-the-fly?:

Hi,

WARNING not tested
Quick idea: shouldn't you remove the shader, build it again (or create a new one) and then add it again to the program ?

Hi,
You mean without recreating the program object? Something like this?

void MainWidget::setFragmentShader(const QString& shaderSource)
{
	QMutexLocker locker(&m);

	if (_program)
	{
		_program->removeShader(_fragmentShader.get());
	}
	else
	{
		_program = std::make_unique<QOpenGLShaderProgram>();

		// Compile vertex shader
		if (!_program->addShaderFromSourceCode(QOpenGLShader::Vertex, vshader))
			qDebug() << "Failed to add vertex shader:\n" << _program->log();
	}

	_fragmentShader = std::make_unique<QOpenGLShader>(QOpenGLShader::Fragment);


	if (!_fragmentShader->compileSourceCode(shaderSource))
	{
		qDebug() << "Error compiling fragment shader:";
		qDebug() << _fragmentShader->log();
		return;
	}

	if (!_program->addShader(_fragmentShader.get()))
		qDebug() << "Failed to add fragment shader:\n" << _program->log();

	if (!_program->link())
		qDebug() << "Failed to link the program\n:" << _program->log();

	if (!_program->bind())
		qDebug() << "Failed to bind the program\n:" << _program->log();
}

This code crashes. The callstack is always the same:

alt text

SGaist

Are you using the mutex to avoid this call being made while the widget is being repainted ?

Violet Giraffe

@SGaist
Yes, but it doesn't matter on Windows because there's no separate OpenGL thread on this OS.

Violet Giraffe

Still can't get it working, any advice?
The code that crashes on my home PC with AMD graphics adapter simply does nothing on a built-in Intel GPU, so the crash is probably just an AMD driver bug, but clearly that code is not the proper way to replace the shader on the fly.

SGaist

Can you share a minimal sample so I can give it a spin on my machine ?

Violet Giraffe

This post is deleted!

johngod

I admit I havent check your code sample, but why dont you just creat 2 shader programs at the initialization and then change between the shader programs on the fly, i.e change the binding between them ? AFAIK this is the best pratice, changing bindings, and not compiling shader on the fly, performance wise this seems bad.

Violet Giraffe

@johngod said in QOpenGLShaderProgram: how to update fragment shader on-the-fly?:

change between the shader programs on the fly

That is exactly what I have not been able to achieve. But the shader code is dynamic, I can't pre-compile it because it's determined at runtime.

johngod

Hi

Just tested your sample program, it works for me if we force the program shader binding in paintGL:

QMatrix4x4 pmvMatrix;
pmvMatrix.ortho(rect());

_program->bind(); //add this line

_program->enableAttributeArray("vertexPosition");
LogGlError;

Violet Giraffe

@johngod said in QOpenGLShaderProgram: how to update fragment shader on-the-fly?:

Hi

Just tested your sample program, it works for me if we force the program shader binding in paintGL:

Interesting, thanks a bunch!
What do you think is wrong with bind() in setFragmentShader() ? Why is it not enough / has no effect?
As a reminder, I'm currently testing on Windows where there's not even a separate OpenGL thread to which such peculiarities could otherwise be attributed.

johngod

Hi Violet Giraffe

OpenGL being such a tricky beast I usually play safe, if I need to use a program shader, bind it first,
same goes for buffers, need to use it, bind it first.

That being said, afaik when deleting or detaching a shader, that doenst happen right away, that only marks the shader
for deletion, that can happen later when its not being used by the program, so I think its safe for you to forget about
QMutexLocker, and not worrying about locking.
This would perhaps also explain why binding the program in setFragmentShader() doesnt work, because you're binding
the program that is not yet in the desired state.

Also for the sake of safety, I find a bit risky that you're not compiling the shaders and linking them to
the program explicitly in the initGL() function, I would definitly do that.

Also in setFragmentShader() I dont think is necessary to remove all shaders, since you're only updating the fragment
shader I think ist safe to only remove it, compile it with the new source code, and link the program.

Hope it helps, let me know how it goes :)

Violet Giraffe

@johngod said in QOpenGLShaderProgram: how to update fragment shader on-the-fly?:

I think its safe for you to forget about
QMutexLocker, and not worrying about locking.

Makes sense. I'm sure OpenGL itself handles it correctly and is thread-safe. The mutex is only for protecting the QOpenGLShaderProgram object itself.

Also for the sake of safety, I find a bit risky that you're not compiling the shaders and linking them to
the program explicitly in the initGL() function, I would definitly do that.

Also in setFragmentShader() I dont think is necessary to remove all shaders, since you're only updating the fragment
shader I think ist safe to only remove it, compile it with the new source code, and link the program.

Yep, those are the first things I did after applying your answer, and everything works as expected (thanks again!). I just needed a working program before I could start optimizing it.