renderer: implement native sRGB support #1687

illwieckz · 2025-06-25T00:26:41Z

Implement native sRGB support.

Obsoletes #1543:

WIP: implement colorspace support (sRGB, etc.) #1543

It includes the commit from #1686:

tr_shader: fix “flags & RSF_2D” #1686

This is based on an even older branch from 2019 as I initially tested native sRGB support before attempting to implement it in GLSL.

I rebased on current master and merged improvements from the srgb-naive branch as it implemented more corner cases than this one.

It still suffers from the problems I faced in 2019: 2D colors are wrong. I need help on that part.

Status:

color source	conversion implemented	when is converted	how is converted
color map	✅️	sampling	metadata
diffuse map	✅️	sampling	metadata
glow map	✅️	sampling	metadata
specular map	✅️	sampling	metadata
skybox	✅️	sampling	metadata
light map	✅️	sampling	metadata
light grid	✅️	loading	conversion
map tris vertex color	✅️	loading	conversion
map patch vertex color	✅️	loading	conversion
expression_t	✅️	uploading	conversion
CGEN_CONST	✅️	uploading	conversion
CGEN_WAVEFORM	✅️	uploading	conversion
CGEN_ENTITY	✅️	uploading	conversion
CGEN_ONE_MINUS_ENTITY	✅️	uploading	conversion
polyVert_t::modulate	✅️	uploading	conversion

illwieckz · 2025-06-25T07:56:04Z

Hmmm… I don't know where to write the line glEnable( GL_FRAMEBUFFER_SRGB );, should it be called at the beginning of every frame?

VReaperV · 2025-06-25T08:23:38Z

Hmmm… I don't know where to write the line glEnable( GL_FRAMEBUFFER_SRGB );, should it be called at the beginning of every frame?.

You only really need to call it once, it's global state and will affect all sRGB framebuffers.

illwieckz · 2025-06-25T14:36:50Z

Which framebuffer should be sRGB there?

src/engine/renderer/tr_bsp.cpp

src/engine/renderer/tr_backend.cpp

src/engine/renderer/tr_shade.cpp

illwieckz · 2025-06-30T17:52:15Z

As a matter of fact, I have zero confidence this implementation is correct, unlike the GLSL alternative implementation.
I expect this to be wrong because I lack some understanding of how sRGB stuff are done the OpenGL way.

Said otherway, I know how to implement the whole computation myself in GLSL, but I don't know how to configure OpenGL to let it do the computation by itself without me writing the explicit computation.

The GLSL implementation is a math algorithm, the OpenGL implementation is a puzzle.

illwieckz · 2025-07-06T00:59:43Z

@VReaperV what is expected if I, for example, set upload an image with GL_SRGB format instead of GL_RGB but do glDisable( GL_FRAMEBUFFER_SRGB )?

illwieckz · 2025-07-07T06:20:06Z

what is expected if I, for example, set upload an image with GL_SRGB format instead of GL_RGB but do glDisable( GL_FRAMEBUFFER_SRGB )?

The question is now likely irrelevant.

illwieckz · 2025-07-07T06:22:16Z

It now works with both maps built with and without the -sRGB q3map2 compiler option.

But you have to disable the tone mapper when running maps compiled with -sRGB, otherwise that looks very surreal.

I don't know anything about the tone mapper so I don't know if something is wrong.

Right now the camera post-process GLSL shader doesn't run in linear space because colorgrades were done for the non-linear space.

illwieckz · 2025-07-07T08:20:48Z

The 2D rendering is done like before, as @slipher suggested:

We should probably stick with naive blending for the 2D rendering since that's what HTML/CSS do.
-- Unvanquished/Unvanquished#3390 (comment)

Actually it makes many things far easier, especially it doesn't require to know how the map should be rendered to render the UI, and it means the UI isn't affected by what's done there.

slipher · 2025-07-08T02:56:25Z

Wait, why would we even want GL_FRAMEBUFFER_SRGB? I don't think it can do anything useful for us. Apparently it is only useful for converting from colors linear to sRGB at a final stage of rendering, as described here. In principle, this is something we need to do at the point of the cameraEffects shader. But our current color grade and tonemap operations are designed for sRGB inputs. So it would work a lot more smoothly if we convert the colors from from linear space ourselves in cameraEffects before feeding them into color grades and tonemapping.

I wonder why GL_FRAMEBUFFER_SRGB would have any effect in the current version of the code though. Supposedly you have to create the framebuffer with an SRGB format for it to have an effect but the code does not appear to do that.

illwieckz · 2025-07-08T03:52:39Z

What I understood (but maybe I'm wrong), is that once GL_FRAMEBUFFER_SRGB is enabled, any subsequent framebuffer operation will assume any input and output is in sRGB and will convert them before and after processing (and also converting framebuffers back and forth when blending). I'm not saying this is truth, but I read in various places something that seemed to mean that, and it looks like doing things that way makes it work (I also read things that meant totally different things, but that seemed wrong).

Maybe we also need to set sRGB on framebuffers, maybe that's required for blending. I don't know, everyone is saying something different on the Internet.

illwieckz · 2025-07-08T03:56:48Z

Also, something I've read (but I may be wrong) is that sRGB variants only exist for 8-bit formats, and some of our framebuffers are 16-bit… I don't know what to do with that information.

slipher · 2025-07-08T04:33:21Z

I found various people complaining of buggy handling of the framebuffer SRGB stuff. Like Nvidia applies it even when the framebuffer was not created as sRGB, or that Intel doesn't do it at all... another reason not to use it.

Some more random considerations:

Vertex colors of models are another thing that theoretically probably needs to be converted. Not sure if we actually have any models that use those.
Vertex colors of (3D) polygons too? OTOH we could just say that is the responsibility of the caller to use a linear space.
I guess the 2D vs. 3D shader registration flag stuff will become important now (since you only want sRGB-to-linear conversion when reading the latter) and probably uncover some bugs.

src/engine/renderer/tr_shader.cpp

illwieckz · 2025-07-08T05:27:36Z

Hmm, comparing a Xonotic map like afterslime on Dæmon and Darkplaces reveals there is still something wrong.

illwieckz · 2025-07-13T11:17:19Z

So unless I'm wrong it looks like CGEN_ENTITY and CGEN_ONE_MINUS_ENTITY may not be implemented to begin with, I only see the worldspawn having a shaderRGBA color, and it is set to white.

illwieckz · 2025-07-13T12:22:56Z

I implemented the conversion of polyVert_t::modulate.

illwieckz · 2025-07-13T12:26:04Z

So unless I'm wrong it looks like CGEN_ENTITY and CGEN_ONE_MINUS_ENTITY may not be implemented to begin with, I only see the worldspawn having a shaderRGBA color, and it is set to white.

Ah I missed that answer:

For example CG_DrawSphere and CG_RenderParticle.

It is implemented in the cgame…

illwieckz · 2025-07-13T12:39:13Z

OK so e.shaderRGBA is now properly taken care of, so CGEN_ENTITY and CGEN_ONE_MINUS_ENTITY are now doing the conversions.

I added a table in the first post to check the status.

slipher · 2025-07-14T00:46:30Z

src/common/Color.h

@@ -256,6 +281,22 @@ class BasicColor
 		data_[ 3 ] = v;
 	}

+	CONSTEXPR_FUNCTION_RELAXED component_type ConvertFromSRGB( component_type v, bool accurate = true ) NOEXCEPT


should be static

Do I just need to a static keyword before the return type?

Do I need to do the same on the next ConvertFromSRGB() returning BasicColor?

Is the addition of static there to help the compiler to optimize out the accurate selector and drop the alternate code?

Also what to do with CONSTEXPR_FUNCTION_RELAXED or CONSTEXPR_FUNCTION, which one to use? I have no idea what is the difference, I just copy-pasted that from previous lines.

Do I need to do the same on the next ConvertFromSRGB() returning BasicColor?

I tested this one, this one I cannot.

static because you don't need a Color object to use the function. static means there is no this argument.

Also what to do with CONSTEXPR_FUNCTION_RELAXED or CONSTEXPR_FUNCTION, which one to use? I have no idea what is the difference, I just copy-pasted that from previous lines.

Just use constexpr, those are workarounds for the compilers of 10 years ago. Also you can add const since for functions constexpr does not imply const.

slipher · 2025-07-14T00:59:24Z

src/engine/renderer/tr_surface.cpp

@@ -728,7 +734,12 @@ static void Tess_SurfacePolychain( srfPoly_t *p )
 				normals[i], qtangents);

 			VectorCopy(p->verts[i].xyz, tess.verts[tess.numVertexes + i].xyz);
-			tess.verts[tess.numVertexes + i].color = Color::Adapt(p->verts[i].modulate);
+
+			Color::Color color =  Color::Adapt( p->verts[ i ].modulate );


Color::Color is a float color while the source and destination are bytes, so you could avoid a round trip by using Color32Bit. (The API ought to be changed so it is not so easy to accidentally do that.) Also it is pointless to clamp with byte colors.

Also done in two similar places in tr_backend.cpp.

slipher · 2025-07-14T01:13:24Z

src/engine/renderer/tr_surface.cpp

-	Tess_AddQuadStamp( backEnd.currentEntity->e.origin, left, up,
-		backEnd.currentEntity->e.shaderRGBA );
+	Color::Color color = backEnd.currentEntity->e.shaderRGBA;
+	color.Clamp();


Useless clamping of byte colors

Revamp `R_LoadEntities()`: - Use `std::string` as much as possible. - Parse `_q3map2_cmdline` using Cmd::Args, so options are read in expected order. - Let `deluxeMapping` override `_q3map2_cmdline`. - Check for deluxe mapping being explicitely disabled. - Check for the deluxe map being in model space. - Make `vertexremapshader` do something when vertex lighting is enabled.

illwieckz · 2025-07-15T08:26:38Z

What the heck is that (macOS CI)?

In file included from /Users/runner/work/1/s/src/common/Common.h:48:
/Users/runner/work/1/s/src/common/Color.h:291:29: error: 'constexpr' non-static member function
 will not be implicitly 'const' in C++14; add 'const' to avoid a change in behavior [-Werror,-Wconstexpr-not-const]
        const constexpr BasicColor ConvertFromSRGB( bool accurate = true ) NOEXCEPT
                                   ^
                                                                           const
/Users/runner/work/1/s/src/common/Color.h:286:9: error: variable declaration in a constexpr function
 is a C++14 extension [-Werror,-Wc++14-extensions]
                float f = float( v ) / float( component_max );
                      ^
/Users/runner/work/1/s/src/common/Color.h:287:3: error: use of this statement in a constexpr function
 is a C++14 extension [-Werror,-Wc++14-extensions]
                f = convertFromSRGB( f, accurate );
                ^

illwieckz · 2025-07-15T08:31:02Z

I added on the testing server the maps citadel gloom2 utcs veddak wrecktify rebuilt for this branch.

illwieckz · 2025-07-15T10:19:04Z

Some gameplay textures are loaded as linear when they should be sRGB, like the flames.

illwieckz added A-Renderer T-Feature-Request Proposed new feature labels Jun 25, 2025

illwieckz force-pushed the illwieckz/srgb-native branch 2 times, most recently from c914db4 to ce0a6e3 Compare June 25, 2025 07:46

VReaperV reviewed Jun 30, 2025

View reviewed changes

illwieckz force-pushed the illwieckz/srgb-native branch from ce0a6e3 to 387cac8 Compare June 30, 2025 17:41

illwieckz force-pushed the illwieckz/srgb-native branch from 387cac8 to feee7ff Compare July 6, 2025 00:45

illwieckz mentioned this pull request Jul 6, 2025

engine: load worldspawn separately from other map data #1692

Open

illwieckz force-pushed the illwieckz/srgb-native branch 2 times, most recently from 86e853b to b2f3100 Compare July 7, 2025 06:19

illwieckz force-pushed the illwieckz/srgb-native branch 2 times, most recently from 8702bec to b2a9fba Compare July 7, 2025 06:43

illwieckz mentioned this pull request Jul 7, 2025

2024-10: Project direction; Urgent, Important and Fancy things to do Unvanquished/Unvanquished#3167

Open

9 tasks

illwieckz force-pushed the illwieckz/srgb-native branch 2 times, most recently from 5383610 to 4e28980 Compare July 8, 2025 04:24

slipher reviewed Jul 8, 2025

View reviewed changes

src/engine/renderer/tr_shader.cpp Outdated Show resolved Hide resolved

illwieckz force-pushed the illwieckz/srgb-native branch from 4e28980 to acceb04 Compare July 8, 2025 05:09

illwieckz force-pushed the illwieckz/srgb-native branch 2 times, most recently from 8c6565a to 772a065 Compare July 13, 2025 06:03

illwieckz force-pushed the illwieckz/srgb-native branch 4 times, most recently from bf930dd to 22ffeb5 Compare July 13, 2025 18:34

slipher reviewed Jul 14, 2025

View reviewed changes

illwieckz added 2 commits July 14, 2025 08:16

fixup: fixup

dcd6e17

renderer: add the EXP_NORM bit to normalize the expression value

22aa0a1

illwieckz force-pushed the illwieckz/srgb-native branch from 32f7d0b to 1c181ef Compare July 14, 2025 06:28

illwieckz force-pushed the illwieckz/srgb-native branch 8 times, most recently from 47bb6dd to 27a80b0 Compare July 14, 2025 08:43

illwieckz added 2 commits July 15, 2025 08:40

fixup: const constexpr

d21fcfc

fixup: warning C4018: '<': signed/unsigned mismatch

bdd8d6d

let's try something

45834ab

illwieckz force-pushed the illwieckz/srgb-native branch from da16c59 to 45834ab Compare July 15, 2025 08:30

renderer: implement native sRGB support #1687

Are you sure you want to change the base?

renderer: implement native sRGB support #1687

Conversation

illwieckz commented Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

illwieckz commented Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

VReaperV commented Jun 25, 2025

Uh oh!

illwieckz commented Jun 25, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

illwieckz commented Jun 30, 2025

Uh oh!

illwieckz commented Jul 6, 2025

Uh oh!

illwieckz commented Jul 7, 2025

Uh oh!

illwieckz commented Jul 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

illwieckz commented Jul 7, 2025

Uh oh!

slipher commented Jul 8, 2025

Uh oh!

illwieckz commented Jul 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

illwieckz commented Jul 8, 2025

Uh oh!

slipher commented Jul 8, 2025

Uh oh!

Uh oh!

illwieckz commented Jul 8, 2025

Uh oh!

illwieckz commented Jul 13, 2025

Uh oh!

illwieckz commented Jul 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

illwieckz commented Jul 13, 2025

Uh oh!

illwieckz commented Jul 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

illwieckz Jul 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

illwieckz commented Jul 15, 2025

Uh oh!

illwieckz commented Jul 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

illwieckz commented Jul 15, 2025

illwieckz commented Jun 25, 2025 •

edited

Loading

illwieckz commented Jun 25, 2025 •

edited

Loading

illwieckz commented Jul 7, 2025 •

edited

Loading

illwieckz commented Jul 8, 2025 •

edited

Loading

illwieckz commented Jul 13, 2025 •

edited

Loading

illwieckz commented Jul 13, 2025 •

edited

Loading

illwieckz Jul 14, 2025 •

edited

Loading

illwieckz commented Jul 15, 2025 •

edited

Loading