Skip to content

fix handling of gamma encoding #225

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.

Sign up for GitHub

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

Merged
adam-ce merged 6 commits into from
Oct 27, 2025
Merged

fix handling of gamma encoding #225

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
37e3e09
experiment with srgb: atmosphere looks nicer, but problem with labels…
adam-ce Oct 25, 2025
c258e19
i do not understand, why we need to disable gamma encoding for text. …
adam-ce Oct 25, 2025
9f98315
srgba framebuffer support is patchy (not working on android, for webg…
adam-ce Oct 26, 2025
c60dd88
comment disfunctional formats so nobody is tempted to use them.
adam-ce Oct 26, 2025
8422492
fix unit tests
adam-ce Oct 26, 2025
26a45cd
fix unit test #2
adam-ce Oct 27, 2025
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
48 changes: 27 additions & 21 deletions gl_engine/Framebuffer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,16 @@ QOpenGLTexture::TextureFormat internal_format_qt(Framebuffer::ColourFormat f)
return QOpenGLTexture::TextureFormat::RGB8_UNorm;
case Framebuffer::ColourFormat::RGBA8:
return QOpenGLTexture::TextureFormat::RGBA8_UNorm;
// case Framebuffer::ColourFormat::SRGBA8:
// return QOpenGLTexture::TextureFormat::SRGB8_Alpha8;
case Framebuffer::ColourFormat::RG16UI:
return QOpenGLTexture::TextureFormat::RG16U;
case Framebuffer::ColourFormat::Float32:
return QOpenGLTexture::TextureFormat::R32F;
case Framebuffer::ColourFormat::RGB16F:
return QOpenGLTexture::TextureFormat::RGB16F;
case Framebuffer::ColourFormat::RGBA16F:
return QOpenGLTexture::TextureFormat::RGBA16F;
// case Framebuffer::ColourFormat::Float32:
// return QOpenGLTexture::TextureFormat::R32F;
// case Framebuffer::ColourFormat::RGB16F:
// return QOpenGLTexture::TextureFormat::RGB16F;
// case Framebuffer::ColourFormat::RGBA16F:
// return QOpenGLTexture::TextureFormat::RGBA16F;
case Framebuffer::ColourFormat::R32UI:
return QOpenGLTexture::TextureFormat::R32U;
case Framebuffer::ColourFormat::RGBA32F:
Expand All @@ -69,14 +71,16 @@ GLenum format(Framebuffer::ColourFormat f)
return GL_RGB;
case Framebuffer::ColourFormat::RGBA8:
return GL_RGBA;
// case Framebuffer::ColourFormat::SRGBA8:
// return GL_RGBA;
case Framebuffer::ColourFormat::RG16UI:
return QOpenGLTexture::PixelFormat::RG_Integer;
case Framebuffer::ColourFormat::Float32: // reading Float32 is inefficient, see read_colour_attachment() for details.
return GL_RED;
case Framebuffer::ColourFormat::RGB16F:
return GL_RGB;
case Framebuffer::ColourFormat::RGBA16F:
return GL_RGBA;
// case Framebuffer::ColourFormat::Float32: // reading Float32 is inefficient, see read_colour_attachment() for details.
// return GL_RED;
// case Framebuffer::ColourFormat::RGB16F:
// return GL_RGB;
// case Framebuffer::ColourFormat::RGBA16F:
// return GL_RGBA;
case Framebuffer::ColourFormat::R32UI:
return GL_RED_INTEGER;
case Framebuffer::ColourFormat::RGBA32F:
Expand Down Expand Up @@ -114,16 +118,17 @@ GLenum type(Framebuffer::ColourFormat f)
switch (f) {
case Framebuffer::ColourFormat::R8:
case Framebuffer::ColourFormat::RGBA8:
// case Framebuffer::ColourFormat::SRGBA8:
case Framebuffer::ColourFormat::RGB8:
return GL_UNSIGNED_BYTE;
case Framebuffer::ColourFormat::RG16UI:
return QOpenGLTexture::PixelType::UInt16;
case Framebuffer::ColourFormat::Float32:
// case Framebuffer::ColourFormat::Float32:
case Framebuffer::ColourFormat::RGBA32F:
return GL_FLOAT;
case Framebuffer::ColourFormat::RGB16F:
case Framebuffer::ColourFormat::RGBA16F:
return GL_HALF_FLOAT;
// case Framebuffer::ColourFormat::RGB16F:
// case Framebuffer::ColourFormat::RGBA16F:
// return GL_HALF_FLOAT;
case Framebuffer::ColourFormat::R32UI:
return GL_UNSIGNED_INT;
}
Expand Down Expand Up @@ -159,8 +164,8 @@ QImage::Format qimage_format(Framebuffer::ColourFormat f)
return QImage::Format_RGBA8888;
case Framebuffer::ColourFormat::RGB8:
return QImage::Format_RGB888;
case Framebuffer::ColourFormat::RGB16F:
return QImage::Format_RGB16;
// case Framebuffer::ColourFormat::RGB16F:
// return QImage::Format_RGB16;
default:
throw std::logic_error("unsupported, QImage does not support the color format of the texture");
}
Expand Down Expand Up @@ -304,16 +309,17 @@ T Framebuffer::read_colour_attachment_pixel(unsigned int index, const glm::dvec2
case Framebuffer::ColourFormat::R8:
case Framebuffer::ColourFormat::RGB8:
case Framebuffer::ColourFormat::RG16UI: // unsupported on android emulator (and webassembly linux firefox?)
case Framebuffer::ColourFormat::Float32:
case Framebuffer::ColourFormat::RGB16F:
case Framebuffer::ColourFormat::RGBA16F:
// case Framebuffer::ColourFormat::Float32:
// case Framebuffer::ColourFormat::RGB16F:
// case Framebuffer::ColourFormat::RGBA16F:
case Framebuffer::ColourFormat::R32UI: // fails on linux firefox
// unsupported or untested.
// you really should add a unit test if you move something down to the supported section
// as the support accross platforms (webassembly, android, ios?) is patchy
assert(false);
return {};
case Framebuffer::ColourFormat::RGBA8:
// case Framebuffer::ColourFormat::SRGBA8:
assert(sizeof(T) == 4);
if (sizeof(T) != 4)
return {};
Expand Down
9 changes: 5 additions & 4 deletions gl_engine/Framebuffer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,13 @@ class Framebuffer
R8,
RGB8,
RGBA8,
// SRGBA8, // not supported as a framebuffer on android
RG16UI,
RGB16F, // NOT COLOR RENDERABLE ON OPENGLES
RGBA16F, // NOT COLOR RENDERABLE ON OPENGLES
// RGB16F, // NOT COLOR RENDERABLE ON OPENGLES
// RGBA16F, // NOT COLOR RENDERABLE ON OPENGLES
R32UI,
Float32, // NOT COLOR RENDERABLE ON OPENGLES
RGBA32F, // NOT COLOR RENDERABLE ON OPENGLES (weirdly it works, maybe because of extension, that qt activates?)
// Float32, // NOT COLOR RENDERABLE ON OPENGLES
RGBA32F, // NOT COLOR RENDERABLE ON OPENGLES (weirdly it works, maybe because of extension, that qt activates?)
};

private:
Expand Down
19 changes: 11 additions & 8 deletions gl_engine/Texture.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ GlParams gl_tex_params(gl_engine::Texture::Format format)
return { GLint(gl_engine::Texture::compressed_texture_format()), 0, 0, 0, 0, true };
case F::RGBA8:
return { GL_RGBA8, GL_RGBA, GL_UNSIGNED_BYTE, 4, 1, true };
case F::SRGBA8:
return { GL_SRGB8_ALPHA8, GL_RGBA, GL_UNSIGNED_BYTE, 4, 1, true };
case F::RGBA8UI:
return { GL_RGBA8UI, GL_RGBA_INTEGER, GL_UNSIGNED_BYTE, 4, 1 };
case F::RGBA32F:
Expand Down Expand Up @@ -135,12 +137,13 @@ void gl_engine::Texture::upload(const nucleus::utils::ColourTexture& texture)
f->glPixelStorei(GL_UNPACK_ALIGNMENT, 1);
const auto width = GLsizei(texture.width());
const auto height = GLsizei(texture.height());
const auto p = gl_tex_params(m_format);
if (m_format == Format::CompressedRGBA8) {
assert(m_min_filter != Filter::MipMapLinear);
const auto format = gl_engine::Texture::compressed_texture_format();
f->glCompressedTexImage2D(GLenum(m_target), 0, format, width, height, 0, GLsizei(texture.n_bytes()), texture.data());
} else if (m_format == Format::RGBA8) {
f->glTexImage2D(GLenum(m_target), 0, GL_RGBA8, width, height, 0, GL_RGBA, GL_UNSIGNED_BYTE, texture.data());
} else if (m_format == Format::RGBA8 || m_format == Format::SRGBA8) {
f->glTexImage2D(GLenum(m_target), 0, p.internal_format, width, height, 0, p.format, p.type, texture.data());
if (m_min_filter == Filter::MipMapLinear)
f->glGenerateMipmap(GLenum(m_target));
} else {
Expand All @@ -163,7 +166,7 @@ void gl_engine::Texture::upload(const nucleus::utils::ColourTexture& texture, un
if (m_format == Format::CompressedRGBA8) {
const auto format = gl_engine::Texture::compressed_texture_format();
f->glCompressedTexSubImage3D(GLenum(m_target), 0, 0, 0, GLint(array_index), width, height, 1, format, GLsizei(texture.n_bytes()), texture.data());
} else if (m_format == Format::RGBA8) {
} else if (m_format == Format::RGBA8 || m_format == Format::SRGBA8) {
f->glTexSubImage3D(GLenum(m_target), 0, 0, 0, GLint(array_index), width, height, 1, GL_RGBA, GL_UNSIGNED_BYTE, texture.data());
} else {
assert(false);
Expand All @@ -188,7 +191,7 @@ void gl_engine::Texture::upload(const nucleus::utils::MipmappedColourTexture& mi
const auto format = gl_engine::Texture::compressed_texture_format();
f->glCompressedTexSubImage3D(
GLenum(m_target), mip_level, 0, 0, GLint(array_index), width, height, 1, format, GLsizei(texture.n_bytes()), texture.data());
} else if (m_format == Format::RGBA8) {
} else if (m_format == Format::RGBA8 || m_format == Format::SRGBA8) {
f->glTexSubImage3D(GLenum(m_target), mip_level, 0, 0, GLint(array_index), width, height, 1, GL_RGBA, GL_UNSIGNED_BYTE, texture.data());
} else {
assert(false);
Expand Down Expand Up @@ -274,18 +277,18 @@ GLenum gl_engine::Texture::compressed_texture_format()
const ext = gl.getExtension("WEBGL_compressed_texture_etc");
if (ext === null)
return 0;
return ext.COMPRESSED_RGB8_ETC2;
return ext.COMPRESSED_SRGB8_ETC2;
});
// qDebug() << "gl_engine::Texture::compressed_texture_format: gl_texture_format from js: " << gl_texture_format;
// clang-format on
if (gl_texture_format == 0) {
gl_texture_format = GL_COMPRESSED_RGB_S3TC_DXT1_EXT; // not on mobile
gl_texture_format = GL_COMPRESSED_SRGB_S3TC_DXT1_EXT; // not on mobile
}
return gl_texture_format;
#elif defined(__ANDROID__)
return GL_COMPRESSED_RGB8_ETC2;
return GL_COMPRESSED_SRGB8_ETC2;
#else
return GL_COMPRESSED_RGB_S3TC_DXT1_EXT;
return GL_COMPRESSED_SRGB_S3TC_DXT1_EXT;
#endif
}

Expand Down
1 change: 1 addition & 0 deletions gl_engine/Texture.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ class Texture {
enum class Target : GLenum { _2d = GL_TEXTURE_2D, _2dArray = GL_TEXTURE_2D_ARRAY }; // no 1D textures in webgl
enum class Format {
RGBA8, // normalised on gpu
SRGBA8, // normalised on gpu
CompressedRGBA8, // normalised on gpu, compression format depends on desktop/mobile
RGBA8UI,
RGBA32F,
Expand Down
2 changes: 2 additions & 0 deletions gl_engine/shaders/compose.frag
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,4 +260,6 @@ void main() {
}
}

// srgb framebuffer support is patchy. encoding manually here.
out_Color = vec4(pow(out_Color.rgb, vec3(1.0/2.2)), out_Color.a);
}
30 changes: 15 additions & 15 deletions unittests/gl_engine/texture.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,9 +291,9 @@ TEST_CASE("gl texture")
const auto g = qGreen(result_pixel);
const auto b = qBlue(result_pixel);

diff += std::abs(r - ref_pixel.x) / 255.0;
diff += std::abs(g - ref_pixel.y) / 255.0;
diff += std::abs(b - ref_pixel.z) / 255.0;
diff += std::abs(r / 255.0 - std::pow(ref_pixel.x / 255.0, 2.2));
diff += std::abs(g / 255.0 - std::pow(ref_pixel.y / 255.0, 2.2));
diff += std::abs(b / 255.0 - std::pow(ref_pixel.z / 255.0, 2.2));
}
}
CAPTURE(resolution);
Expand Down Expand Up @@ -381,7 +381,7 @@ TEST_CASE("gl texture")
for (auto texture_type : texture_types) {
CAPTURE(texture_type.first);
CAPTURE(texture_type.second);
const auto format = (texture_type.first == ColourTexture::Format::Uncompressed_RGBA) ? gl_engine::Texture::Format::RGBA8
const auto format = (texture_type.first == ColourTexture::Format::Uncompressed_RGBA) ? gl_engine::Texture::Format::SRGBA8
: gl_engine::Texture::Format::CompressedRGBA8;
const auto use_mipmaps = texture_type.second;
gl_engine::Texture opengl_texture(gl_engine::Texture::Target::_2dArray, format);
Expand Down Expand Up @@ -433,9 +433,9 @@ TEST_CASE("gl texture")
double diff = 0;
for (int i = 0; i < render_result.width(); ++i) {
for (int j = 0; j < render_result.height(); ++j) {
diff += std::abs(qRed(render_result.pixel(i, j)) - test_raster.pixel({ i, j }).x) / 255.0;
diff += std::abs(qGreen(render_result.pixel(i, j)) - test_raster.pixel({ i, j }).y) / 255.0;
diff += std::abs(qBlue(render_result.pixel(i, j)) - test_raster.pixel({ i, j }).z) / 255.0;
diff += std::abs(qRed(render_result.pixel(i, j)) / 255.0 - std::pow(test_raster.pixel({ i, j }).x / 255.0, 2.2));
diff += std::abs(qGreen(render_result.pixel(i, j)) / 255.0 - std::pow(test_raster.pixel({ i, j }).y / 255.0, 2.2));
diff += std::abs(qBlue(render_result.pixel(i, j)) / 255.0 - std::pow(test_raster.pixel({ i, j }).z / 255.0, 2.2));
}
}
CHECK(diff / (256 * 256 * 3) < 0.017);
Expand All @@ -446,25 +446,25 @@ TEST_CASE("gl texture")
double diff = 0;
for (int i = 0; i < render_result.width(); ++i) {
for (int j = 0; j < render_result.height(); ++j) {
diff += std::abs(qRed(render_result.pixel(i, j)) - 42) / 255.0;
diff += std::abs(qGreen(render_result.pixel(i, j)) - 142) / 255.0;
diff += std::abs(qBlue(render_result.pixel(i, j)) - 242) / 255.0;
diff += std::abs(qRed(render_result.pixel(i, j)) / 255.0 - std::pow(42 / 255.0, 2.2));
diff += std::abs(qGreen(render_result.pixel(i, j)) / 255.0 - std::pow(142 / 255.0, 2.2));
diff += std::abs(qBlue(render_result.pixel(i, j)) / 255.0 - std::pow(242 / 255.0, 2.2));
}
}
CHECK(diff / (256 * 256 * 3) < 0.017);
CHECK(diff / (256 * 256 * 3) < 0.02);
}
{
const QImage render_result = framebuffer.read_colour_attachment(2);
// render_result.save("render_result2.png");
double diff = 0;
for (int i = 0; i < render_result.width(); ++i) {
for (int j = 0; j < render_result.height(); ++j) {
diff += std::abs(qRed(render_result.pixel(i, j)) - 222) / 255.0;
diff += std::abs(qGreen(render_result.pixel(i, j)) - 111) / 255.0;
diff += std::abs(qBlue(render_result.pixel(i, j)) - 0) / 255.0;
diff += std::abs(qRed(render_result.pixel(i, j)) / 255.0 - std::pow(222 / 255.0, 2.2)) / 255.0;
diff += std::abs(qGreen(render_result.pixel(i, j)) / 255.0 - std::pow(111 / 255.0, 2.2)) / 255.0;
diff += std::abs(qBlue(render_result.pixel(i, j)) / 255.0 - std::pow(0 / 255.0, 2.2)) / 255.0;
}
}
CHECK(diff / (256 * 256 * 3) < 0.017);
CHECK(diff / (256 * 256 * 3) < 0.02);
}
}
}
Expand Down