-
Notifications
You must be signed in to change notification settings - Fork 64
Add support for the GL_RED format, use it, also rework format detection #1793
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.
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
base: master
Are you sure you want to change the base?
Conversation
|
We may also upload standalone height maps as |
I'm doing the same mistake again… DXT images can't be uploaded as |
src/engine/renderer/tr_image.cpp
Outdated
| out[ 1 ] = out[ 3 ] = 255; | ||
| } | ||
|
|
||
| imageParams.bits = IF_NOPICMIP; |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
src/engine/renderer/tr_image.cpp
Outdated
| bool hasAlpha = false; | ||
|
|
||
| if ( !( image->bits & IF_LIGHTMAP ) ) | ||
| for ( i = 0; i < c * 4; i += 4 ) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
|
The fog image is pretty pointless, all it really does it calculate |
Oh, that's interesting! Would you also know what computation to expect when blending the linear way? Or maybe it should have been
If we can that would be nice. |
a596ee5 to
217dcd0
Compare
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 |
217dcd0 to
3efb6e7
Compare
|
That looks ready to me. |
4746e50 to
76300e5
Compare
|
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: Unrelated to that branch, but I notice we have both |
|
Very good: |
76300e5 to
26914ef
Compare
26914ef to
d29eb7b
Compare
|
This now also support and detect RG textures, and use explicit |
0e30853 to
7042d99
Compare
|
So yes there was a pre-existing confusion on |
9ef2aa9 to
2b5d7aa
Compare
|
Detection works well: |
46b9e4a to
1d4ab4d
Compare
|
I found a bug in the code in |
|
I noticed that would modify an undocumented feature of Because all its channels were set to But when stored as RED the alpha channel is opaque (so, But I have seen no map in the whole betaserv corpus that used any Actually it bugged me that the black image was not opaque black. |
|
Fog image stuff can be dropped. Do we really need to have the |
|
Yes we may purge those color variants. |
|
This PR is expected to be rebased after #1804 is merged: So this PR is currently not in mergeable state. |
1d4ab4d to
3b88a3a
Compare
|
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 Something not implemented in that PR is to flag images with 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. |
There was a problem hiding this 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 ) ); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
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 |
|
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. |
Previous message:
That's just beautifying, following patches are written above this
Imported from WIP: tr_shader: linearize fog colors and fog image #1730
but without the colorspace-related HACKs
Imported from WIP: tr_shader: linearize fog colors and fog image #1730
Imported from WIP: tr_shader: linearize fog colors and fog image #1730
Useful for when we know in advance an image has no alpha channel
Actually
GL_COMPRESSED_*formats can be used (they let the driver decide), but we don't use them.