Skip to content

Conversation

@illwieckz
Copy link
Member

@illwieckz illwieckz commented Sep 8, 2025

  • tr_image: rework the alpha detection
  • renderer: implement and detect RED and RG images
  • renderer: introduce and use IF_NOALPHA
  • tr_backend: fix a comment
  • tr_image: flag glyph image with IF_ALPHA
  • sdl_glimp: fix the EXT/ARB texture_rg confusion
  • tr_image: document the GL_RGB format
  • tr_image: add some GL format checks and asserts
  • tr_image: reshape the cinematic image generation a bit
  • tr_image: reshape the default image painting a bit
  • tr_image: make some iterators more local

Previous message:

  • tr_image: reword the fog generation code
    That's just beautifying, following patches are written above this
  • tr_image: store the fog alpha channel in red one
    Imported from WIP: tr_shader: linearize fog colors and fog image #1730
    but without the colorspace-related HACKs
  • renderer: add support for the GL_RED image format
    Imported from WIP: tr_shader: linearize fog colors and fog image #1730
  • tr_image: upload _fog image as GL_RED
    Imported from WIP: tr_shader: linearize fog colors and fog image #1730
  • tr_image: upload _black, _red and _blackCube images as GL_RED
  • tr_image: rework the alpha detection
  • tr_image: detect RED images
  • renderer: introduce and use IF_NOALPHA
    Useful for when we know in advance an image has no alpha channel
  • tr_image: do not rely on previously set data for _cinematic, make sure it cannot be detected as RED
  • tr_backend: fix a comment
    Actually GL_COMPRESSED_* formats can be used (they let the driver decide), but we don't use them.
@illwieckz illwieckz added T-Improvement Improvement for an existing feature A-Renderer labels Sep 8, 2025
@illwieckz
Copy link
Member Author

We may also upload standalone height maps as GL_RED too, unfortunately doing that will loose precision since as far as I know the DXT storage is 5:6:5 and then the red component features less bits than the green one.

@illwieckz
Copy link
Member Author

We may also upload standalone height maps as GL_RED too, unfortunately doing that will loose precision since as far as I know the DXT storage is 5:6:5 and then the red component features less bits than the green one.

I'm doing the same mistake again… DXT images can't be uploaded as GL_RED. 🤣

out[ 1 ] = out[ 3 ] = 255;
}

imageParams.bits = IF_NOPICMIP;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem to do what the commit message says?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That removes the IF_RED flag for the next textures that cannot be uploaded as GL_RED.

bool hasAlpha = false;

if ( !( image->bits & IF_LIGHTMAP ) )
for ( i = 0; i < c * 4; i += 4 )
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this actually work? I assume the check for IF_LIGHTMAP could've been there because lightmaps might contain some garbage in the alpha channel.

Copy link
Member Author

@illwieckz illwieckz Sep 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Long time ago the code was doing:

if ( image->bits & IF_NORMALMAP )	{ if ( image->bits & ( IF_DISPLACEMAP | IF_ALPHATEST ) )	{ samples = 4;	} else	{ samples = 3;	}	} else if ( image->bits & IF_LIGHTMAP )	{ samples = 3;	}

It was just a code setting known formats for known input.

The current code in master (that is modified by this PR) was a modification I made: 071c789

The IF_LIGHTMAP test was just there because we already knew there is no alpha channel in a lightmap. Q3map2 doesn't produce alpha channel (and should not).

I added a more generic IF_NOALPHA flag that can be used for other things than lightmaps, like the deluxemaps, and other things, instead of only checking for lightmaps as a known input with known format.

Copy link
Member Author

@illwieckz illwieckz Sep 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This alpha channel detection code is never expected to be run on lightmaps to begin with, and now also not on other images I flagged with IF_NOALPHA.

@slipher
Copy link
Member

slipher commented Sep 8, 2025

The fog image is pretty pointless, all it really does it calculate sqrt(x). We should just do that in GLSL. I think it only exists because someone ported the code from GL1 in an overly mechanical way.

@illwieckz
Copy link
Member Author

illwieckz commented Sep 8, 2025

The fog image is pretty pointless, all it really does it calculate sqrt(x).

Oh, that's interesting! Would you also know what computation to expect when blending the linear way? Or maybe it should have been sqrt() in linear space but the naive blending did garbage and we're accustomed to it?

We should just do that in GLSL. I think it only exists because someone ported the code from GL1 in an overly mechanical way.

If we can that would be nice.

@illwieckz illwieckz changed the title Add suppot for GL_RED format, use it, also rework format detection Add support for GL_RED format, use it, also rework format detection Sep 8, 2025
@illwieckz illwieckz force-pushed the illwieckz/red branch 2 times, most recently from a596ee5 to 217dcd0 Compare September 8, 2025 22:50
@illwieckz illwieckz changed the title Add support for GL_RED format, use it, also rework format detection Add support for the GL_RED format, use it, also rework format detection Sep 9, 2025
@slipher
Copy link
Member

slipher commented Sep 9, 2025

Oh, that's interesting! Would you also know what computation to expect when blending the linear way? Or maybe it should have been sqrt() in linear space but the naive blending did garbage and we're accustomed to it?

Square root doesn't make much sense as a model for the fog; probably they just tried random functions and picked something that looked OK. Instead of sqrt(a*x), 1 - exp(b*x) would be an obvious model to use (as if the fog is formed from layered alpha blending). a/b is a constant based on the fog density. You can make these functions match up somewhat using appropriate constants: https://www.desmos.com/calculator/abvkogdqzy

@illwieckz
Copy link
Member Author

That looks ready to me.

@illwieckz illwieckz force-pushed the illwieckz/red branch 2 times, most recently from 4746e50 to 76300e5 Compare September 13, 2025 18:29
@illwieckz
Copy link
Member Author

illwieckz commented Sep 13, 2025

I fixed an non-optimal code path, it was testing for RED before testing for ALPHA, meaning a red texture with an opaque alpha channel was only detected as RGB, not as RED. Now fixed:

]/listImages *red* ------------------------------------------------------------------------- num width height layers mm type format space wrap.t wrap.s name ------------------------------------------------------------------------- 3 8 8 1 no 2D R8 linear t.rept s.rept _red 99 64 64 1 no 2D DXT5 linear t.clmp s.clmp emoticons/featured 119 256 256 1 yes 2D RGB8 linear t.rept s.rept textures/pulse/metal-red.tga 120 256 256 1 yes 2D RGB8 linear t.rept s.rept textures/pulse/metal-red 158 256 256 1 yes 2D RGB8 linear t.rept s.rept textures/pulse/e8base_red 159 256 256 1 yes 2D RGB8 linear t.rept s.rept textures/pulse/e8base_red.tga 233 16 16 1 no 2D R8 linear t.rept s.rept textures/pulse/red16x16.tga 262 16 16 1 yes 2D DXT1 linear t.rept s.rept gfx/buildables/common/redbuild 281 16 256 1 yes 2D RGB8 linear t.rept s.rept gfx/cgrading/red-only ------------------------------------------------------------------------- 0 total texels (not including mipmaps) 1.01 MB total image memory (estimated) 689 total images ------------------------------------------------------------------------- 

Unrelated to that branch, but I notice we have both textures/pulse/metal-red.tga and textures/pulse/metal-red loaded… that doesn't look right. That should be investigated separately.

@illwieckz
Copy link
Member Author

Very good:

2 8 8 1 no 2D R8 linear t.rept s.rept _black 3 8 8 1 no 2D R8 linear t.rept s.rept _red 39 256 32 1 yes 2D R8 linear t.clmp s.clmp _fog 47 32 32 6 no CUBE R8 linear t.eclmp s.eclmp _blackCube 519 32 32 1 yes 2D R8 linear t.rept s.rept textures/atcshd/bulb_red.tga 522 64 64 1 yes 2D R8 linear t.rept s.rept textures/atcshd/black 
@illwieckz
Copy link
Member Author

This now also support and detect RG textures, and use explicit GL_R8 instead of GL_RED when supported.

]/listImages *16x16* ------------------------------------------------------------------------- num width height layers mm type format space wrap.t wrap.s name ------------------------------------------------------------------------- 205 16 16 1 no 2D R8 linear t.rept s.rept textures/pulse/black16x16.tga 280 16 16 1 yes 2D RGB8 linear t.rept s.rept textures/pulse/white16x16.tga 283 16 16 1 no 2D RGB8 linear t.rept s.rept textures/pulse/white16x16.tga 291 16 16 1 yes 2D R8 linear t.rept s.rept textures/pulse/black16x16.tga 296 16 16 1 no 2D R8 linear t.rept s.rept textures/pulse/red16x16.tga 302 16 16 1 no 2D RG8 linear t.rept s.rept textures/pulse/green16x16.tga ------------------------------------------------------------------------- 0 total texels (not including mipmaps) 0.00 MB total image memory (estimated) 752 total images ------------------------------------------------------------------------- 
@illwieckz illwieckz force-pushed the illwieckz/red branch 3 times, most recently from 0e30853 to 7042d99 Compare September 14, 2025 00:19
@illwieckz
Copy link
Member Author

So yes there was a pre-existing confusion on master related to some RG extension but it was less problematic than what I thought at first, it was just a naming mess.

@illwieckz illwieckz force-pushed the illwieckz/red branch 4 times, most recently from 9ef2aa9 to 2b5d7aa Compare September 14, 2025 01:07
@illwieckz
Copy link
Member Author

Detection works well:

]/listImages *shared_colors* ------------------------------------------------------------------------- num width height layers mm type format space wrap.t wrap.s name ------------------------------------------------------------------------- 614 1 1 1 yes 2D R8 sRGB t.rept s.rept textures/shared_colors_src/black_d 615 1 1 1 yes 2D RGB8 sRGB t.rept s.rept textures/shared_colors_src/gray_d 616 1 1 1 yes 2D RGB8 sRGB t.rept s.rept textures/shared_colors_src/white_d 617 1 1 1 yes 2D R8 sRGB t.rept s.rept textures/shared_colors_src/red_d 618 1 1 1 yes 2D RG8 sRGB t.rept s.rept textures/shared_colors_src/orange_d 619 1 1 1 yes 2D RG8 sRGB t.rept s.rept textures/shared_colors_src/yellow_d 620 1 1 1 yes 2D RG8 sRGB t.rept s.rept textures/shared_colors_src/chartreuse_d 621 1 1 1 yes 2D RG8 sRGB t.rept s.rept textures/shared_colors_src/green_d 622 1 1 1 yes 2D RGB8 sRGB t.rept s.rept textures/shared_colors_src/spring_d 623 1 1 1 yes 2D RGB8 sRGB t.rept s.rept textures/shared_colors_src/cyan_d 624 1 1 1 yes 2D RGB8 sRGB t.rept s.rept textures/shared_colors_src/azure_d 625 1 1 1 yes 2D RGB8 sRGB t.rept s.rept textures/shared_colors_src/blue_d 626 1 1 1 yes 2D RGB8 sRGB t.rept s.rept textures/shared_colors_src/violet_d 627 1 1 1 yes 2D RGB8 sRGB t.rept s.rept textures/shared_colors_src/magenta_d 628 1 1 1 yes 2D RGB8 sRGB t.rept s.rept textures/shared_colors_src/rose_d 629 1 1 1 yes 2D RGBA8 sRGB t.rept s.rept textures/shared_colors_src/transparent_d ------------------------------------------------------------------------- 0 total texels (not including mipmaps) 0.00 MB total image memory (estimated) 630 total images ------------------------------------------------------------------------- 
@illwieckz illwieckz force-pushed the illwieckz/red branch 2 times, most recently from 46b9e4a to 1d4ab4d Compare September 14, 2025 01:59
@illwieckz
Copy link
Member Author

I found a bug in the code in master where we only iterate the first layer of an image for detecting the format. Now fixed.

@illwieckz
Copy link
Member Author

illwieckz commented Sep 14, 2025

I noticed that would modify an undocumented feature of _black, previously it was usable to make a transparent texture:

transparent {	map _black	blend blend } 

Because all its channels were set to 0, including the alpha one.

But when stored as RED the alpha channel is opaque (so, 1).

But I have seen no map in the whole betaserv corpus that used any _black, *black, $black, $blackimage keywords at all, so even not used as trick with blend to get a fully transparent texture.

Actually it bugged me that the black image was not opaque black.

@slipher
Copy link
Member

slipher commented Sep 17, 2025

Fog image stuff can be dropped.

Do we really need to have the _red image at all?

@illwieckz
Copy link
Member Author

Yes we may purge those color variants.

@illwieckz
Copy link
Member Author

This PR is expected to be rebased after #1804 is merged:

So this PR is currently not in mergeable state.

@illwieckz illwieckz marked this pull request as draft September 17, 2025 23:05
@illwieckz illwieckz changed the title Add support for the GL_RED format, use it, also rework format detection WIP: Add support for the GL_RED format, use it, also rework format detection Sep 17, 2025
@illwieckz illwieckz changed the title WIP: Add support for the GL_RED format, use it, also rework format detection Add support for the GL_RED format, use it, also rework format detection Sep 29, 2025
@illwieckz illwieckz marked this pull request as ready for review September 29, 2025 18:45
@illwieckz
Copy link
Member Author

illwieckz commented Sep 29, 2025

Detecting the RG images is less needed after we drop all the color image variants that are not white and black, as seen in:

Though, this can still be useful for optimizing the upload of legacy images from legacy paks, that are not DXT-compressed, and then for which we can detect if they use their blue channel or not.

This PR actually fixes a bug in detecting the alpha channel presence (it only looked for the first layer of an image).

This also adds IF_NOALPHA to skip the detection of the alpha channel when it is known the image is fully opaque, it already uses it for a bunch of builtin images.

Something not implemented in that PR is to flag images with IF_NOALPHA when they are part of a shader without blend functions, something we can do, but I'm postponing this. Especially, we may need some revamp to be done first, as some images are loaded before the stage is entirely parsed (and then the blending is unknown). But that's likely something I want to do later.

Also something I want to do later is to reimplement the GL compression of non-DXT-compressed images, for use with the legacy assets full of non-crunched JPG or TGA files. I actually have a working branch for that, based on an older version of this branch, but then again that's for later.

Some commits do some beautifying or improve consistency with other parts of the code around to ease the reading.

Copy link
Member

@slipher slipher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How much memory do we expect to save from this red image detection?

/* Generate cinematic frames.
It is empty data to be filled by the cinematic code, but
we fill it with non-zero values so the format detector keeps all color channels. */
memset( data, 255, sizeof( data ) );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we even passing in data there? Can't we put in nullptr for the data like when making an FBO?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no idea, but that would have to be investigated for another PR, not this one.

@illwieckz
Copy link
Member Author

illwieckz commented Oct 1, 2025

How much memory do we expect to save from this red image detection?

That's one of my concern. Now that we ditched the color images, and after we will use q3shader code for common colors instead of pixmaps, not only RG detection but also RED detection becomes less useful.

This can still benefit some legacy maps, for example the pulse maps has some plain red and green images for displaying the door closed/open status in the control room, legacy maps may have things like that, pure black, red or green images, the win isn't very big.

@slipher
Copy link
Member

slipher commented Oct 1, 2025

Yeah I don't think the complexity is worth it if we are going to only save like 10 KB or something. The color detection code is rather convoluted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Renderer T-Improvement Improvement for an existing feature

4 participants