WIP: resize textures using GL_PROXY mechanism

[illwieckz]

1. Resize textures using GL_PROXY mechanism
2. Introduce picMin and picMax

The ATI X1950 PRO can display the game with medium preset including full size textures except the skybox that is 1024^6 size:

With r_picMax set to 512, everything looks good:

The r_picMax cvar expects the size of an edge, so r_picMax 512 will scale down every image larger than a square image of 512×512 size.

Using r_picMax 512 will scale down textures from tex-vega that are 1024×1024 large but will not scale down textures from tex-pk01 that are 512x512 large. This way a map using both vega and pk01 textures will look highly textured with 512×512 textures everywhere while using r_picmip 1 would have displayed a mix of nice 512x512 and ugly 256×256 images.

Both r_picmip and r_picMax can be used at the same time. For example, by using r_picmip 1 and r_picMax 512, once minimap is set as nopicmip (see https://github.com/Unvanquished/Unvanquished/pull/1230 ), the game will scale down all textures but the minimap to keep it clear, unless it is larger than 512, but minimaps with size up to 512 will not be scaled down…

I added two shader keywords: picMin and picMax.

The picMin one allows an artist to tell the renderer to never downscale a texture under this size. This is useful for models for example, since seeing details like the eyes and others is better.

The picMax allows an artist to tell the renderer to downscale the texture to this size, even if the player is playing full size.

I added another cvar, r_picMin, which allows to override the picMin value set in shader, and when it is set to -1, never downscale images that has picMin value (it would be like making them nopicmip).

[slipher]

It looks like there is some code right next to the code you added which is supposed to do exactly the same thing. What is glConfig2.maxCubeMapTextureSize on your test machine?

[illwieckz]

I get 4096 for glConfig2.maxCubeMapTextureSize on the ATI X1950 PRO.

[illwieckz]

Interestingly those are the values on the original code to scale down lightmaps:

scaledWidth: 1024, scaledHeight: 1024, glConfig2.maxCubeMapTextureSize: 4096

Is glConfig2.maxCubeMapTextureSize the size of a single face or the size of the whole cube? If it's the size of a single face then the driver is buggy, if it's the size of the whole cube then we have to compare scaledWidth and scaledHeight with glConfig2.maxCubeMapTextureSize / 6 instead.

That said, this hardware is not meant to support 4096 sized standard flat textures, so it's probably the summed size of all the faces of the skybox.

Testing against glConfig2.maxCubeMapTextureSize / 6 fixes the issue, but it may be wrong.

I still think the r_picMax feature is good even if unneeded to fix bugs. For example setting r_picMax to 512 still produces high definition rendering and it's better than doing r_picmip 1 that would reduce 4096 sided large textures to still-heavy 2048 ones and ok-ish 512 sided textures to ugly 256 ones.

Basically r_picmip looks to be a relic from a past where all textures had the same size and were low definition anyway, and then user would somewhat expect the resulting images having the same size. To me this r_picMax feature is more useful than r_picmip, this is what I would naturally look for instead.

[illwieckz]

I added a commit to fix skybox scaling according to the hardware limits.

[illwieckz]

I rewrote entirely the code to compute an image size, or to be more precise, to compute a pixel size in memory (see #378).

I added two shader keywords, picMin and picMax.

The picMin one allows an artist to tell the renderer to never downscale a texture under this size. This is useful for models for example, since seeing details like the eyes and others is better.

The picMax allows an artist to tell the renderer to downscale the texture to this size, even if the player is playing full size. This can be useful for example if a texture is shipped in 4096×4096 size but is only displayed on a tiny surface. This allow to ship full size textures even if they are used downscaled in game. It's better for conservancy.

I added another cvar, r_picMin, which allows to override the picMin value set in shader.

So basically, texture is downscaled using picMax (or r_picMax if positive) unless a shader uses picMin. If a shader uses picMin, the texture is downscaled to the largest value between picMin or picMax unless r_picMin is set. If a shader uses picMin and r_picMin is set to something greater than zero, then texture is downscaled using r_picMin. There is a special value for r_picMin: if a shader uses picMin and r_picMin is set to -1, then texture is not downscaled at all. This makes possible to downscale a lot what is known to be downscalable safely, and to not downscale at all what is known to not downscale properly. This is useful for competitive scenes: it makes possible to make world surfaces using only one color but models would be rendered properly.

Pictures may help to understand, in all those screenshots, models shaders have picMin 128 instruction.

r_picMax 512 and r_picMin 0, every texture is loaded with 512×512 size maximum.

r_picMax 64 and r_picMin 0, every texture is loaded with 64×64 size maximum, except models that are loaded with 128×128 size.

r_picMax 1 and r_picMin 0, every texture is loaded with 1×1 size maximum, except models that are loaded with 128×128 size.

r_picMax 1 and r_picMin -1, every texture is loaded with 1×1 size maximum, except models that are loaded full size.

[slipher]

The picMax allows an artist to tell the renderer to downscale the texture to this size, even if the player is playing full size. This can be useful for example if a texture is shipped in 4096×4096 size but is only displayed on a tiny surface. This allow to ship full size textures even if they are used downscaled in game.

This doesn't sound like a great practice to encourage, since you would still pay full price in file size and load times. Wouldn't something like this be better as part of the map build process?

[illwieckz]

This is better as part of a map build process, but a mapper can still write shaders based on third-party texture set and other situations.

Note that I did not added that because of a urge need. I see a real need for picMin so I did it, and right after than I've seen picMax could be done for free. There would be no issue to not have it, but once picMin is implemented, picMax costs nothing, so why not.

[illwieckz]

I've added a commit temporarily named wat that may fix an issue… While reading the original code, I thought this part looked weird and that it sounded like a typo. I would appreciate opinions on that.

[illwieckz]

This is basically what does the wat commit, before:

Debug: Loading compressed image models/buildables/barricade/body_d with 2048×2048 original size, mip 0, layer 0
Debug: Loading compressed image models/buildables/barricade/body_d with 2048×2048 original size, mip 1, layer 0
Debug: Loading compressed image models/buildables/barricade/body_d with 2048×2048 original size, mip 2, layer 0
Debug: Loading compressed image models/buildables/barricade/body_d with 2048×2048 original size, mip 3, layer 0
Debug: Loading compressed image models/buildables/barricade/body_d with 2048×2048 original size, mip 4, layer 0
Debug: Loading compressed image models/buildables/barricade/body_d with 2048×2048 original size, mip 5, layer 0
Debug: Loading compressed image models/buildables/barricade/body_d with 2048×2048 original size, mip 6, layer 0
Debug: Loading compressed image models/buildables/barricade/body_d with 2048×2048 original size, mip 7, layer 0
Debug: Loading compressed image models/buildables/barricade/body_d with 2048×2048 original size, mip 8, layer 0
Debug: Loading compressed image models/buildables/barricade/body_d with 2048×2048 original size, mip 9, layer 0
Debug: Loading compressed image models/buildables/barricade/body_d with 2048×2048 original size, mip 10, layer 0
Debug: Loading compressed image models/buildables/barricade/body_d with 2048×2048 original size, mip 11, layer 0

after:

Debug: Loading compressed image models/buildables/barricade/body_d with 2048×2048 original size, mip 0, layer 0
Debug: Loading compressed image models/buildables/barricade/body_d with 1024×1024 size, mip 1, layer 0
Debug: Loading compressed image models/buildables/barricade/body_d with 512×512 size, mip 2, layer 0
Debug: Loading compressed image models/buildables/barricade/body_d with 256×256 size, mip 3, layer 0
Debug: Loading compressed image models/buildables/barricade/body_d with 128×128 size, mip 4, layer 0
Debug: Loading compressed image models/buildables/barricade/body_d with 64×64 size, mip 5, layer 0
Debug: Loading compressed image models/buildables/barricade/body_d with 32×32 size, mip 6, layer 0
Debug: Loading compressed image models/buildables/barricade/body_d with 16×16 size, mip 7, layer 0
Debug: Loading compressed image models/buildables/barricade/body_d with 8×8 size, mip 8, layer 0
Debug: Loading compressed image models/buildables/barricade/body_d with 4×4 size, mip 9, layer 0
Debug: Loading compressed image models/buildables/barricade/body_d with 2×2 size, mip 10, layer 0
Debug: Loading compressed image models/buildables/barricade/body_d with 1×1 size, mip 11, layer 0

Original behavior sounded wrong, right?

[illwieckz]

Unfortunately the new code to detect images being too large for the hardware does not work, or not the way we expect it. I never see them being downscaled.

[illwieckz]

It works, but not the way we expect it for cube maps. If I double width and height when passing them to CheckImage2D, the obviously too large size is detected and proper order is given to downscale, and the skybox is rendered properly (because effectively downscaled):

Debug: Uploading image env/shared_space_src/sky07 with 256×256 size, downscaled from 1024×1024 size, layer 0
Debug: Uploading image env/shared_space_src/sky07 with 256×256 size, downscaled from 1024×1024 size, layer 1
Debug: Uploading image env/shared_space_src/sky07 with 256×256 size, downscaled from 1024×1024 size, layer 2
Debug: Uploading image env/shared_space_src/sky07 with 256×256 size, downscaled from 1024×1024 size, layer 3
Debug: Uploading image env/shared_space_src/sky07 with 256×256 size, downscaled from 1024×1024 size, layer 4
Debug: Uploading image env/shared_space_src/sky07 with 256×256 size, downscaled from 1024×1024 size, layer 5

Edit: hu, no, that does not work, I forgot I have previously set r_picMax… There is no downscaling based on testing the upload, at all.

[illwieckz]

Well, if I do this (notice the << 3):

	scalingStep = CheckImage2D( internalFormat, scaledWidth << 3, scaledHeight << 3, 0, format, GL_UNSIGNED_BYTE );

it works:

Warn: Image env/shared_space_src/sky07 too large, downscaling 1 time(s) to fit the hardware limits 
Debug: Uploading image env/shared_space_src/sky07 with 512×512 size, downscaled from 1024×1024 size, layer 0 
Debug: Uploading image env/shared_space_src/sky07 with 512×512 size, downscaled from 1024×1024 size, layer 1 
Debug: Uploading image env/shared_space_src/sky07 with 512×512 size, downscaled from 1024×1024 size, layer 2 
Debug: Uploading image env/shared_space_src/sky07 with 512×512 size, downscaled from 1024×1024 size, layer 3 
Debug: Uploading image env/shared_space_src/sky07 with 512×512 size, downscaled from 1024×1024 size, layer 4 
Debug: Uploading image env/shared_space_src/sky07 with 512×512 size, downscaled from 1024×1024 size, layer 5 

[illwieckz]

I noticed that, on this computer reporting 4096 max cubemap texture size, if I multiply a 1024 sized texture by 4, it does not scale down, then display garbage, meaning 4096 is expected to be OK but 1024 is already not. While multiplying by 5 downscaling happens (and then displays the skybox properly because being downscaled to 512).

[illwieckz]

The current branch works, but I have no explanation for the multiplier. I've set 6 like the amount of faces a cube has, and a comment explaining there is no explanation.

[illwieckz]

So, I renamed the wat commit to upload image mips using mipWidth and mipHeight size, that makes sense to me.

[illwieckz]

At this point I think it's ready.

[slipher]

Have you checked the OpenGL error codes to see if there's anything relevant in the cubemap bug case? I opened https://github.com/DaemonEngine/Daemon/pull/395 to enable this checking all the time.

[illwieckz]

I made an extra function to downscale dimensions that not only applies on scaleHeight and scaleWidth but also on some other array and mip data. I noticed the extra computation was only done with picmip (and the variants I introduced), but not when the image is larger than the hardware can do.

We may technically do the custom downscale (picmip & all) and and the hardware-limited downscale at the same time after internalFormat is determined, and call the function to do the downscaling computation, but we have code to convert formats in the code determining internalFormat, so doing the custom downscale before determining the internal format may make conversion faster.

[illwieckz]

Have you checked the OpenGL error codes to see if there's anything relevant in the cubemap bug case? I opened #395 to enable this checking all the time.

If I run your PR (i.e. without my own patches from my PR), I get Warn: OpenGL error GL_INVALID_ENUM detected in GLShaderManager::buildPermutation, but no error when loading the skybox (that displays garbage).

[illwieckz]

Except those two questions:

https://github.com/DaemonEngine/Daemon/pull/394/files#r519607355 https://github.com/DaemonEngine/Daemon/pull/394/files#r519510857

I consider this PR ready again.

Thanks to GL_CheckErrors I spotted some issues I introduced in my code, now fixed.

Note: The skybox issue is still not fixed but I give up on getting it fixed for 0.52. Anyway, hardware reproducing the issue runs better with low preset and this PR now introduces a nice feature that would make the game look better with low preset while workarounding the issue.

[illwieckz]

Also, r_picMin, r_picMax, picMin and picMax keywords are temporary.

I'm looking for better cvar name than r_picMin and r_picMax, some base name that would fit well when ported to “dot” naming. Something like maximumEdge and minimumEdge, so we can do renderer.textures.maximumEdge or something like that. Material keywords would be renamed accordingly.

Does anyone have an idea about that?

[slipher]

For cvar naming how about maxImageDimension?

Did you ever find any cases where the texture proxy successfully identified a texture that couldn't be loaded?

[illwieckz]

I did imageMinDimension/imageMaxDimension, r_imageMinDimension/r_imageMaxDimension.

[illwieckz]

I got this on debug build, I don't know how to fix it:

#0  0x00005555558d618e in Str::AssertOnTinyFormatError (reason="\"tinyformat: Not enough conversion specifiers in format string\"") at /home/illwieckz/dev/buildme-unvanquished/Daemon/src/common/String.cpp:37
#1  0x0000555555650b73 in tinyformat::detail::streamStateFromFormat (out=..., spacePadPositive=@0x7fffffff4181: false, ntrunc=@0x7fffffff4188: -1, fmtStart=0x5555559bcf38 "", formatters=0x7fffffff4410, argIndex=@0x7fffffff4184: 5, numFormatters=7) at /home/illwieckz/dev/buildme-unvanquished/Daemon/libs/tinyformat/tinyformat.h:607
#2  0x00005555556518e2 in tinyformat::detail::formatImpl (out=..., fmt=0x5555559bcf38 "", formatters=0x7fffffff4410, numFormatters=7) at /home/illwieckz/dev/buildme-unvanquished/Daemon/libs/tinyformat/tinyformat.h:807
#3  0x0000555555651da9 in tinyformat::vformat (out=..., fmt=0x5555559bcef8 "Uploading compressed image %s with %d×%d size, mip %d, layer %d", list=...) at /home/illwieckz/dev/buildme-unvanquished/Daemon/libs/tinyformat/tinyformat.h:963
#4  0x00005555558223c1 in tinyformat::format<char [64], int, int, unsigned short, unsigned short, int, int> (out=..., fmt=0x5555559bcef8 "Uploading compressed image %s with %d×%d size, mip %d, layer %d") at /home/illwieckz/dev/buildme-unvanquished/Daemon/libs/tinyformat/tinyformat.h:973
#5  0x0000555555821a7f in tinyformat::format<char [64], int, int, unsigned short, unsigned short, int, int> (fmt=0x5555559bcef8 "Uploading compressed image %s with %d×%d size, mip %d, layer %d") at /home/illwieckz/dev/buildme-unvanquished/Daemon/libs/tinyformat/tinyformat.h:982
#6  0x0000555555820e8b in Str::Format<char (&) [64], int&, int&, unsigned short&, unsigned short&, int&, int&> (format=...) at /home/illwieckz/dev/buildme-unvanquished/Daemon/src/common/String.h:315
#7  0x000055555581faa9 in Log::Logger::Debug<char (&) [64], int&, int&, unsigned short&, unsigned short&, int&, int&> (this=0x555556236c80 <Log::defaultLogger>, format=...) at /home/illwieckz/dev/buildme-unvanquished/Daemon/src/common/Log.h:220
#8  0x000055555581e932 in Log::Debug<char (&) [64], int&, int&, unsigned short&, unsigned short&, int&, int&> (format=...) at /home/illwieckz/dev/buildme-unvanquished/Daemon/src/common/Log.h:272
#9  0x00005555558189c2 in R_UploadImage (dataArray=0x7fffffff4b18, numLayers=1, numMips=8, image=0x7fffd5f3b200, imageParams=0x7fffffffcf70) at /home/illwieckz/dev/buildme-unvanquished/Daemon/src/engine/renderer/tr_image.cpp:1127