Skip to content

Commit b7fdc5f

Browse files
adam-ceadam-ce
authored
Merge pull request #225 from adam-ce/main
fix handling of gamma encoding
2 parents 9090382 + 26a45cd commit b7fdc5f

6 files changed

Lines changed: 61 additions & 48 deletions

File tree

‎gl_engine/Framebuffer.cpp‎

Lines changed: 27 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -43,14 +43,16 @@ QOpenGLTexture::TextureFormat internal_format_qt(Framebuffer::ColourFormat f)
4343
return QOpenGLTexture::TextureFormat::RGB8_UNorm;
4444
case Framebuffer::ColourFormat::RGBA8:
4545
return QOpenGLTexture::TextureFormat::RGBA8_UNorm;
46+
// case Framebuffer::ColourFormat::SRGBA8:
47+
// return QOpenGLTexture::TextureFormat::SRGB8_Alpha8;
4648
case Framebuffer::ColourFormat::RG16UI:
4749
return QOpenGLTexture::TextureFormat::RG16U;
48-
case Framebuffer::ColourFormat::Float32:
49-
return QOpenGLTexture::TextureFormat::R32F;
50-
case Framebuffer::ColourFormat::RGB16F:
51-
return QOpenGLTexture::TextureFormat::RGB16F;
52-
case Framebuffer::ColourFormat::RGBA16F:
53-
return QOpenGLTexture::TextureFormat::RGBA16F;
50+
// case Framebuffer::ColourFormat::Float32:
51+
// return QOpenGLTexture::TextureFormat::R32F;
52+
// case Framebuffer::ColourFormat::RGB16F:
53+
// return QOpenGLTexture::TextureFormat::RGB16F;
54+
// case Framebuffer::ColourFormat::RGBA16F:
55+
// return QOpenGLTexture::TextureFormat::RGBA16F;
5456
case Framebuffer::ColourFormat::R32UI:
5557
return QOpenGLTexture::TextureFormat::R32U;
5658
case Framebuffer::ColourFormat::RGBA32F:
@@ -69,14 +71,16 @@ GLenum format(Framebuffer::ColourFormat f)
6971
return GL_RGB;
7072
case Framebuffer::ColourFormat::RGBA8:
7173
return GL_RGBA;
74+
// case Framebuffer::ColourFormat::SRGBA8:
75+
// return GL_RGBA;
7276
case Framebuffer::ColourFormat::RG16UI:
7377
return QOpenGLTexture::PixelFormat::RG_Integer;
74-
case Framebuffer::ColourFormat::Float32: // reading Float32 is inefficient, see read_colour_attachment() for details.
75-
return GL_RED;
76-
case Framebuffer::ColourFormat::RGB16F:
77-
return GL_RGB;
78-
case Framebuffer::ColourFormat::RGBA16F:
79-
return GL_RGBA;
78+
// case Framebuffer::ColourFormat::Float32: // reading Float32 is inefficient, see read_colour_attachment() for details.
79+
// return GL_RED;
80+
// case Framebuffer::ColourFormat::RGB16F:
81+
// return GL_RGB;
82+
// case Framebuffer::ColourFormat::RGBA16F:
83+
// return GL_RGBA;
8084
case Framebuffer::ColourFormat::R32UI:
8185
return GL_RED_INTEGER;
8286
case Framebuffer::ColourFormat::RGBA32F:
@@ -114,16 +118,17 @@ GLenum type(Framebuffer::ColourFormat f)
114118
switch (f) {
115119
case Framebuffer::ColourFormat::R8:
116120
case Framebuffer::ColourFormat::RGBA8:
121+
// case Framebuffer::ColourFormat::SRGBA8:
117122
case Framebuffer::ColourFormat::RGB8:
118123
return GL_UNSIGNED_BYTE;
119124
case Framebuffer::ColourFormat::RG16UI:
120125
return QOpenGLTexture::PixelType::UInt16;
121-
case Framebuffer::ColourFormat::Float32:
126+
// case Framebuffer::ColourFormat::Float32:
122127
case Framebuffer::ColourFormat::RGBA32F:
123128
return GL_FLOAT;
124-
case Framebuffer::ColourFormat::RGB16F:
125-
case Framebuffer::ColourFormat::RGBA16F:
126-
return GL_HALF_FLOAT;
129+
// case Framebuffer::ColourFormat::RGB16F:
130+
// case Framebuffer::ColourFormat::RGBA16F:
131+
// return GL_HALF_FLOAT;
127132
case Framebuffer::ColourFormat::R32UI:
128133
return GL_UNSIGNED_INT;
129134
}
@@ -159,8 +164,8 @@ QImage::Format qimage_format(Framebuffer::ColourFormat f)
159164
return QImage::Format_RGBA8888;
160165
case Framebuffer::ColourFormat::RGB8:
161166
return QImage::Format_RGB888;
162-
case Framebuffer::ColourFormat::RGB16F:
163-
return QImage::Format_RGB16;
167+
// case Framebuffer::ColourFormat::RGB16F:
168+
// return QImage::Format_RGB16;
164169
default:
165170
throw std::logic_error("unsupported, QImage does not support the color format of the texture");
166171
}
@@ -304,16 +309,17 @@ T Framebuffer::read_colour_attachment_pixel(unsigned int index, const glm::dvec2
304309
case Framebuffer::ColourFormat::R8:
305310
case Framebuffer::ColourFormat::RGB8:
306311
case Framebuffer::ColourFormat::RG16UI: // unsupported on android emulator (and webassembly linux firefox?)
307-
case Framebuffer::ColourFormat::Float32:
308-
case Framebuffer::ColourFormat::RGB16F:
309-
case Framebuffer::ColourFormat::RGBA16F:
312+
// case Framebuffer::ColourFormat::Float32:
313+
// case Framebuffer::ColourFormat::RGB16F:
314+
// case Framebuffer::ColourFormat::RGBA16F:
310315
case Framebuffer::ColourFormat::R32UI: // fails on linux firefox
311316
// unsupported or untested.
312317
// you really should add a unit test if you move something down to the supported section
313318
// as the support accross platforms (webassembly, android, ios?) is patchy
314319
assert(false);
315320
return {};
316321
case Framebuffer::ColourFormat::RGBA8:
322+
// case Framebuffer::ColourFormat::SRGBA8:
317323
assert(sizeof(T) == 4);
318324
if (sizeof(T) != 4)
319325
return {};

‎gl_engine/Framebuffer.h‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,12 +50,13 @@ class Framebuffer
5050
R8,
5151
RGB8,
5252
RGBA8,
53+
// SRGBA8, // not supported as a framebuffer on android
5354
RG16UI,
54-
RGB16F, // NOT COLOR RENDERABLE ON OPENGLES
55-
RGBA16F, // NOT COLOR RENDERABLE ON OPENGLES
55+
// RGB16F, // NOT COLOR RENDERABLE ON OPENGLES
56+
// RGBA16F, // NOT COLOR RENDERABLE ON OPENGLES
5657
R32UI,
57-
Float32, // NOT COLOR RENDERABLE ON OPENGLES
58-
RGBA32F, // NOT COLOR RENDERABLE ON OPENGLES (weirdly it works, maybe because of extension, that qt activates?)
58+
// Float32, // NOT COLOR RENDERABLE ON OPENGLES
59+
RGBA32F, // NOT COLOR RENDERABLE ON OPENGLES (weirdly it works, maybe because of extension, that qt activates?)
5960
};
6061

6162
private:

‎gl_engine/Texture.cpp‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,8 @@ GlParams gl_tex_params(gl_engine::Texture::Format format)
4848
return { GLint(gl_engine::Texture::compressed_texture_format()), 0, 0, 0, 0, true };
4949
case F::RGBA8:
5050
return { GL_RGBA8, GL_RGBA, GL_UNSIGNED_BYTE, 4, 1, true };
51+
case F::SRGBA8:
52+
return { GL_SRGB8_ALPHA8, GL_RGBA, GL_UNSIGNED_BYTE, 4, 1, true };
5153
case F::RGBA8UI:
5254
return { GL_RGBA8UI, GL_RGBA_INTEGER, GL_UNSIGNED_BYTE, 4, 1 };
5355
case F::RGBA32F:
@@ -135,12 +137,13 @@ void gl_engine::Texture::upload(const nucleus::utils::ColourTexture& texture)
135137
f->glPixelStorei(GL_UNPACK_ALIGNMENT, 1);
136138
const auto width = GLsizei(texture.width());
137139
const auto height = GLsizei(texture.height());
140+
const auto p = gl_tex_params(m_format);
138141
if (m_format == Format::CompressedRGBA8) {
139142
assert(m_min_filter != Filter::MipMapLinear);
140143
const auto format = gl_engine::Texture::compressed_texture_format();
141144
f->glCompressedTexImage2D(GLenum(m_target), 0, format, width, height, 0, GLsizei(texture.n_bytes()), texture.data());
142-
} else if (m_format == Format::RGBA8) {
143-
f->glTexImage2D(GLenum(m_target), 0, GL_RGBA8, width, height, 0, GL_RGBA, GL_UNSIGNED_BYTE, texture.data());
145+
} else if (m_format == Format::RGBA8 || m_format == Format::SRGBA8) {
146+
f->glTexImage2D(GLenum(m_target), 0, p.internal_format, width, height, 0, p.format, p.type, texture.data());
144147
if (m_min_filter == Filter::MipMapLinear)
145148
f->glGenerateMipmap(GLenum(m_target));
146149
} else {
@@ -163,7 +166,7 @@ void gl_engine::Texture::upload(const nucleus::utils::ColourTexture& texture, un
163166
if (m_format == Format::CompressedRGBA8) {
164167
const auto format = gl_engine::Texture::compressed_texture_format();
165168
f->glCompressedTexSubImage3D(GLenum(m_target), 0, 0, 0, GLint(array_index), width, height, 1, format, GLsizei(texture.n_bytes()), texture.data());
166-
} else if (m_format == Format::RGBA8) {
169+
} else if (m_format == Format::RGBA8 || m_format == Format::SRGBA8) {
167170
f->glTexSubImage3D(GLenum(m_target), 0, 0, 0, GLint(array_index), width, height, 1, GL_RGBA, GL_UNSIGNED_BYTE, texture.data());
168171
} else {
169172
assert(false);
@@ -188,7 +191,7 @@ void gl_engine::Texture::upload(const nucleus::utils::MipmappedColourTexture& mi
188191
const auto format = gl_engine::Texture::compressed_texture_format();
189192
f->glCompressedTexSubImage3D(
190193
GLenum(m_target), mip_level, 0, 0, GLint(array_index), width, height, 1, format, GLsizei(texture.n_bytes()), texture.data());
191-
} else if (m_format == Format::RGBA8) {
194+
} else if (m_format == Format::RGBA8 || m_format == Format::SRGBA8) {
192195
f->glTexSubImage3D(GLenum(m_target), mip_level, 0, 0, GLint(array_index), width, height, 1, GL_RGBA, GL_UNSIGNED_BYTE, texture.data());
193196
} else {
194197
assert(false);
@@ -274,18 +277,18 @@ GLenum gl_engine::Texture::compressed_texture_format()
274277
const ext = gl.getExtension("WEBGL_compressed_texture_etc");
275278
if (ext === null)
276279
return 0;
277-
return ext.COMPRESSED_RGB8_ETC2;
280+
return ext.COMPRESSED_SRGB8_ETC2;
278281
});
279282
// qDebug() << "gl_engine::Texture::compressed_texture_format: gl_texture_format from js: " << gl_texture_format;
280283
// clang-format on
281284
if (gl_texture_format == 0) {
282-
gl_texture_format = GL_COMPRESSED_RGB_S3TC_DXT1_EXT; // not on mobile
285+
gl_texture_format = GL_COMPRESSED_SRGB_S3TC_DXT1_EXT; // not on mobile
283286
}
284287
return gl_texture_format;
285288
#elif defined(__ANDROID__)
286-
return GL_COMPRESSED_RGB8_ETC2;
289+
return GL_COMPRESSED_SRGB8_ETC2;
287290
#else
288-
return GL_COMPRESSED_RGB_S3TC_DXT1_EXT;
291+
return GL_COMPRESSED_SRGB_S3TC_DXT1_EXT;
289292
#endif
290293
}
291294

‎gl_engine/Texture.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ class Texture {
3333
enum class Target : GLenum { _2d = GL_TEXTURE_2D, _2dArray = GL_TEXTURE_2D_ARRAY }; // no 1D textures in webgl
3434
enum class Format {
3535
RGBA8, // normalised on gpu
36+
SRGBA8, // normalised on gpu
3637
CompressedRGBA8, // normalised on gpu, compression format depends on desktop/mobile
3738
RGBA8UI,
3839
RGBA32F,

‎gl_engine/shaders/compose.frag‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,4 +260,6 @@ void main() {
260260
}
261261
}
262262

263+
// srgb framebuffer support is patchy. encoding manually here.
264+
out_Color = vec4(pow(out_Color.rgb, vec3(1.0/2.2)), out_Color.a);
263265
}

‎unittests/gl_engine/texture.cpp‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -291,9 +291,9 @@ TEST_CASE("gl texture")
291291
const auto g = qGreen(result_pixel);
292292
const auto b = qBlue(result_pixel);
293293

294-
diff += std::abs(r - ref_pixel.x) / 255.0;
295-
diff += std::abs(g - ref_pixel.y) / 255.0;
296-
diff += std::abs(b - ref_pixel.z) / 255.0;
294+
diff += std::abs(r / 255.0 - std::pow(ref_pixel.x / 255.0, 2.2));
295+
diff += std::abs(g / 255.0 - std::pow(ref_pixel.y / 255.0, 2.2));
296+
diff += std::abs(b / 255.0 - std::pow(ref_pixel.z / 255.0, 2.2));
297297
}
298298
}
299299
CAPTURE(resolution);
@@ -381,7 +381,7 @@ TEST_CASE("gl texture")
381381
for (auto texture_type : texture_types) {
382382
CAPTURE(texture_type.first);
383383
CAPTURE(texture_type.second);
384-
const auto format = (texture_type.first == ColourTexture::Format::Uncompressed_RGBA) ? gl_engine::Texture::Format::RGBA8
384+
const auto format = (texture_type.first == ColourTexture::Format::Uncompressed_RGBA) ? gl_engine::Texture::Format::SRGBA8
385385
: gl_engine::Texture::Format::CompressedRGBA8;
386386
const auto use_mipmaps = texture_type.second;
387387
gl_engine::Texture opengl_texture(gl_engine::Texture::Target::_2dArray, format);
@@ -433,9 +433,9 @@ TEST_CASE("gl texture")
433433
double diff = 0;
434434
for (int i = 0; i < render_result.width(); ++i) {
435435
for (int j = 0; j < render_result.height(); ++j) {
436-
diff += std::abs(qRed(render_result.pixel(i, j)) - test_raster.pixel({ i, j }).x) / 255.0;
437-
diff += std::abs(qGreen(render_result.pixel(i, j)) - test_raster.pixel({ i, j }).y) / 255.0;
438-
diff += std::abs(qBlue(render_result.pixel(i, j)) - test_raster.pixel({ i, j }).z) / 255.0;
436+
diff += std::abs(qRed(render_result.pixel(i, j)) / 255.0 - std::pow(test_raster.pixel({ i, j }).x / 255.0, 2.2));
437+
diff += std::abs(qGreen(render_result.pixel(i, j)) / 255.0 - std::pow(test_raster.pixel({ i, j }).y / 255.0, 2.2));
438+
diff += std::abs(qBlue(render_result.pixel(i, j)) / 255.0 - std::pow(test_raster.pixel({ i, j }).z / 255.0, 2.2));
439439
}
440440
}
441441
CHECK(diff / (256 * 256 * 3) < 0.017);
@@ -446,25 +446,25 @@ TEST_CASE("gl texture")
446446
double diff = 0;
447447
for (int i = 0; i < render_result.width(); ++i) {
448448
for (int j = 0; j < render_result.height(); ++j) {
449-
diff += std::abs(qRed(render_result.pixel(i, j)) - 42) / 255.0;
450-
diff += std::abs(qGreen(render_result.pixel(i, j)) - 142) / 255.0;
451-
diff += std::abs(qBlue(render_result.pixel(i, j)) - 242) / 255.0;
449+
diff += std::abs(qRed(render_result.pixel(i, j)) / 255.0 - std::pow(42 / 255.0, 2.2));
450+
diff += std::abs(qGreen(render_result.pixel(i, j)) / 255.0 - std::pow(142 / 255.0, 2.2));
451+
diff += std::abs(qBlue(render_result.pixel(i, j)) / 255.0 - std::pow(242 / 255.0, 2.2));
452452
}
453453
}
454-
CHECK(diff / (256 * 256 * 3) < 0.017);
454+
CHECK(diff / (256 * 256 * 3) < 0.02);
455455
}
456456
{
457457
const QImage render_result = framebuffer.read_colour_attachment(2);
458458
// render_result.save("render_result2.png");
459459
double diff = 0;
460460
for (int i = 0; i < render_result.width(); ++i) {
461461
for (int j = 0; j < render_result.height(); ++j) {
462-
diff += std::abs(qRed(render_result.pixel(i, j)) - 222) / 255.0;
463-
diff += std::abs(qGreen(render_result.pixel(i, j)) - 111) / 255.0;
464-
diff += std::abs(qBlue(render_result.pixel(i, j)) - 0) / 255.0;
462+
diff += std::abs(qRed(render_result.pixel(i, j)) / 255.0 - std::pow(222 / 255.0, 2.2)) / 255.0;
463+
diff += std::abs(qGreen(render_result.pixel(i, j)) / 255.0 - std::pow(111 / 255.0, 2.2)) / 255.0;
464+
diff += std::abs(qBlue(render_result.pixel(i, j)) / 255.0 - std::pow(0 / 255.0, 2.2)) / 255.0;
465465
}
466466
}
467-
CHECK(diff / (256 * 256 * 3) < 0.017);
467+
CHECK(diff / (256 * 256 * 3) < 0.02);
468468
}
469469
}
470470
}

0 commit comments

Comments
 (0)