Skip to content
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

renderer: introduce IF_FITSCREEN and RSF_2D #1145

Merged
merged 6 commits into from
Jun 6, 2024

Conversation

illwieckz
Copy link
Member

@illwieckz illwieckz commented May 13, 2024

Introduce IF_FITSCREEN to rescale image to the smaller size larger than screen.

The idea behind this feature is that we have images that should not be rescaled smaller than screen to look good:

  • minimaps,
  • UI background,
  • levelshot,
  • etc.

So for them we use IF_PICMIP, either directly in code, either with material keyword nopicmip.

But then this stupid thing happen, let's imagine someone playing on a low end device with not a lot of GPU memory and a 800×600 screen:

]/listImages
------------------------------------------------------------------
num   width heigth layers mm  type format   wrap.t   wrap.s   name
------------------------------------------------------------------
63      100   2230      1 no  2D   DXT5     t.clmp   s.clmp   ui/assets/background/bg
64       20   1200      1 no  2D   DXT5     t.clmp   s.clmp   ui/assets/background/black
65     2082   1371      1 no  2D   DXT5     t.clmp   s.clmp   ui/assets/background/duel
66       20   1200      1 no  2D   DXT5     t.clmp   s.clmp   ui/assets/background/black2
67     2500   1667      1 no  2D   DXT5     t.clmp   s.clmp   ui/assets/background/circles1
68     2500   1667      1 no  2D   DXT5     t.clmp   s.clmp   ui/assets/background/circles2
69     3500    886      1 no  2D   DXT5     t.clmp   s.clmp   ui/assets/background/fog
121    3840   2160      1 no  2D   DXT1     t.clmp   s.clmp   meta/chasm/chasm

The 4K levelshot is fully loaded in GPU memory just to display it on a 800×600 screen!

Here is what happens once this feature is implemented AND (not done yet) the related images are configured to use IF_FITSCREEN:

]/listImages
------------------------------------------------------------------
num   width heigth layers mm  type format   wrap.t   wrap.s   name
------------------------------------------------------------------
63       50   1115      1 no  2D   DXT5     t.clmp   s.clmp   ui/assets/background/bg
64       20   1200      1 no  2D   DXT5     t.clmp   s.clmp   ui/assets/background/black
65     1041    685      1 no  2D   DXT5     t.clmp   s.clmp   ui/assets/background/duel
66       20   1200      1 no  2D   DXT5     t.clmp   s.clmp   ui/assets/background/black2
67     1250    833      1 no  2D   DXT5     t.clmp   s.clmp   ui/assets/background/circles1
68     1250    833      1 no  2D   DXT5     t.clmp   s.clmp   ui/assets/background/circles2
69      875    221      1 no  2D   DXT5     t.clmp   s.clmp   ui/assets/background/fog
121     960    540      1 no  2D   DXT1     t.clmp   s.clmp   meta/chasm/chasm

This is far better!

So this feature scales down images flagged with IF_FITSCREEN to the smaller size larger than (or equal to) the screen, this way the image is never underscaled compared to the screen, while not being too much large for nothing.

Made for:

@illwieckz illwieckz added A-Renderer T-Feature-Request Proposed new feature labels May 13, 2024
@illwieckz illwieckz changed the base branch from master to for-0.55.0/sync May 13, 2024 13:07
@illwieckz illwieckz force-pushed the illwieckz/image-fitscreen/sync branch from d23c90c to a9463ea Compare May 13, 2024 13:40
@illwieckz illwieckz marked this pull request as draft May 13, 2024 13:42
@illwieckz illwieckz changed the title WIP: renderer: introduce IF_FITSCREEN to rescale image to the smaller size larger than screen WIP: renderer: introduce IF_FITSCREEN and RSF_2D May 13, 2024
@illwieckz
Copy link
Member Author

illwieckz commented May 13, 2024

Example with the chasm map being loaded, with lowest preset and 640×480 screen size:

]/listImages
…
------------------------------------------------------------------ 
40828875 total texels (not including mipmaps) 
49.11 MB total image memory (estimated) 
666 total images 
------------------------------------------------------------------ 

@illwieckz illwieckz force-pushed the illwieckz/image-fitscreen/sync branch from dc84053 to f4dd2b3 Compare May 16, 2024 14:03
@illwieckz
Copy link
Member Author

I just tested it with a very old card (ATi R300 family) with limited memory and other resources.

Before:

20240529-004300-000 unvanquished

20240529-003829-000 unvanquished

After:

20240529-004557-000 unvanquished

20240529-004607-000 unvanquished

It works well. So I'll start to clean-up things to make it ready.

@illwieckz illwieckz force-pushed the illwieckz/image-fitscreen/sync branch from f4dd2b3 to 87d8065 Compare May 28, 2024 23:47
@illwieckz illwieckz changed the title WIP: renderer: introduce IF_FITSCREEN and RSF_2D renderer: introduce IF_FITSCREEN and RSF_2D May 29, 2024
@illwieckz illwieckz force-pushed the illwieckz/image-fitscreen/sync branch from 5ca11c1 to c567434 Compare May 29, 2024 17:59
@illwieckz
Copy link
Member Author

I added a cvar named r_imageFitScreen.

Values:

  • 0: do nothing
  • 1: downscale “fitscreen” images until it is not smaller than the screen size.
  • 2: downscale “fitscreen” images to never be larger than screen size.

The 1 value is enabled by default.

The 2 value is meant to be used in lowest preset, where it's OK to take the risk to not look that good while not being that bad. In practice this is good enough: most UI images don't fill the whole screen, and the one that does are usually background images that can suffer a bit of downscaling.

The following examples are done with the game running on a 640×480 screen, on main menu.

Feature disabled with value 0 (same behavior as in current master):

]/listImages ui/assets/background
------------------------------------------------------------------
num   width heigth layers mm  type format   wrap.t   wrap.s   name
------------------------------------------------------------------
66      100   2230      1 no  2D   DXT5     t.clmp   s.clmp   ui/assets/background/bg
67       20   1200      1 no  2D   DXT5     t.clmp   s.clmp   ui/assets/background/black
68     2082   1371      1 no  2D   DXT5     t.clmp   s.clmp   ui/assets/background/duel
69       20   1200      1 no  2D   DXT5     t.clmp   s.clmp   ui/assets/background/black2
70     2500   1667      1 no  2D   DXT5     t.clmp   s.clmp   ui/assets/background/circles1
71     2500   1667      1 no  2D   DXT5     t.clmp   s.clmp   ui/assets/background/circles2
72     3500    886      1 no  2D   DXT5     t.clmp   s.clmp   ui/assets/background/fog
------------------------------------------------------------------
14561422 total texels (not including mipmaps)
13.88 MB total image memory (estimated)
129 total images
------------------------------------------------------------------

Feature enabled with default 1 value:

]/listImages ui/assets/background
------------------------------------------------------------------
num   width heigth layers mm  type format   wrap.t   wrap.s   name
------------------------------------------------------------------
63       50   1115      1 no  2D   DXT5     t.clmp   s.clmp   ui/assets/background/bg
64       20   1200      1 no  2D   DXT5     t.clmp   s.clmp   ui/assets/background/black
65     1041    685      1 no  2D   DXT5     t.clmp   s.clmp   ui/assets/background/duel
66       20   1200      1 no  2D   DXT5     t.clmp   s.clmp   ui/assets/background/black2
67     1250    833      1 no  2D   DXT5     t.clmp   s.clmp   ui/assets/background/circles1
68     1250    833      1 no  2D   DXT5     t.clmp   s.clmp   ui/assets/background/circles2
69     1750    443      1 no  2D   DXT5     t.clmp   s.clmp   ui/assets/background/fog
------------------------------------------------------------------
3674585 total texels (not including mipmaps)
3.50 MB total image memory (estimated)
88 total images
------------------------------------------------------------------

Feature enabled with 2 value:

]/listImages ui/assets/background
------------------------------------------------------------------
num   width heigth layers mm  type format   wrap.t   wrap.s   name
------------------------------------------------------------------
63       25    557      1 no  2D   DXT5     t.clmp   s.clmp   ui/assets/background/bg
64       10    600      1 no  2D   DXT5     t.clmp   s.clmp   ui/assets/background/black
65      520    342      1 no  2D   DXT5     t.clmp   s.clmp   ui/assets/background/duel
66       10    600      1 no  2D   DXT5     t.clmp   s.clmp   ui/assets/background/black2
67      625    416      1 no  2D   DXT5     t.clmp   s.clmp   ui/assets/background/circles1
68      625    416      1 no  2D   DXT5     t.clmp   s.clmp   ui/assets/background/circles2
69      437    110      1 no  2D   DXT5     t.clmp   s.clmp   ui/assets/background/fog
------------------------------------------------------------------
771835 total texels (not including mipmaps)
0.73 MB total image memory (estimated)
88 total images
------------------------------------------------------------------

@illwieckz illwieckz marked this pull request as ready for review May 29, 2024 18:00
@illwieckz illwieckz force-pushed the illwieckz/image-fitscreen/sync branch from c567434 to e8077d6 Compare May 29, 2024 18:37
@illwieckz
Copy link
Member Author

illwieckz commented May 29, 2024

The distorted images I got with the ATi R300 are the same I got with the Raspberry Pi 3, so this should also fix that bug on RPI3 too. At the time I got a crash later, I don't know if reducing memory usage helps on that other topic. But after that the main menu should be reachable without bug on RPI3, even if other bugs may happen later.

@illwieckz illwieckz force-pushed the illwieckz/image-fitscreen/sync branch from 5164507 to f812bd6 Compare June 1, 2024 19:43
Comment on lines +767 to +772
// If r_imageFitScreen is disabled, use nopicmip instead.
if ( ( image->bits & IF_FITSCREEN ) && !r_imageFitScreen.Get() )
{
image->bits &= ~IF_FITSCREEN;
image->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.

Is there a motivation for having this be something we can disable?

Copy link
Member Author

@illwieckz illwieckz Jun 6, 2024

Choose a reason for hiding this comment

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

This is only for asset debugging. We can for example imagine a game that would have a very large 2D minimap larger than screen and that the player could scroll. If someone finds weird the image is blurry, he can disable the feature and deduce that's because the fitscreen flag is set somewhere for that image.

Copy link
Member Author

Choose a reason for hiding this comment

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

The same would be usable for debugging any image that would have received the fitscreen flag but should not use fitscreen like some 3D textures (or worst, lightmaps…).

@@ -1147,6 +1148,7 @@ enum class dynamicLightRenderer_t { LEGACY, TILED };

int deformIndex;
bool overrideNoPicMip; // for images that must always be full resolution
bool overrideFitScreen;
Copy link
Contributor

Choose a reason for hiding this comment

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

document

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@@ -1282,6 +1284,7 @@ enum class dynamicLightRenderer_t { LEGACY, TILED };
float polygonOffsetValue;

bool noPicMip; // for images that must always be full resolution
bool fitScreen;
Copy link
Contributor

Choose a reason for hiding this comment

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

document

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@@ -150,6 +150,7 @@ Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
cvar_t *r_ignoreMaterialMinDimension;
cvar_t *r_ignoreMaterialMaxDimension;
cvar_t *r_replaceMaterialMinDimensionIfPresentWithMaxDimension;
Cvar::Range<Cvar::Cvar<int>> r_imageFitScreen("r_imageFitScreen", "downscale “fitscreen” images according to screen size: 0: do not downscale, 1: downscale as much as possible without being smaller than screen size, 2: downscale to never be larger then screen size", Cvar::NONE, 1, 0, 2);
Copy link
Contributor

Choose a reason for hiding this comment

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

explicitly note that 0 is "disabled"

Copy link
Member Author

Choose a reason for hiding this comment

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

Also added (default) after the default one.

@@ -1147,6 +1148,7 @@ enum class dynamicLightRenderer_t { LEGACY, TILED };

int deformIndex;
bool overrideNoPicMip; // for images that must always be full resolution
bool overrideFitScreen;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need both overrideFitScreen and fitScreen? it sounds like we could just make do with fitScreen

Copy link
Member Author

Choose a reason for hiding this comment

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

This was bad cargo cult reproducing something stupid already done withshader.noPicMip and stage.overrideNoPicMip, the override name is only meaningful for enums, not boolean, to set something an enum value is used. I'm just renaming that as stage.fitScreen and in the future one will have to rename stage.overrideNoPicMip as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

The reason why there are two variables is because one is for shaders (applying to all stages), one is for stages.

I'm not sure it makes sense to have it as a stage keyword though, but the same image could be used in UI (and then use fitScreen) while also being part of a 3D shader blended with other images that do not need this feature.

@illwieckz illwieckz force-pushed the illwieckz/image-fitscreen/sync branch from f812bd6 to b1a8f01 Compare June 6, 2024 00:38
@illwieckz
Copy link
Member Author

illwieckz commented Jun 6, 2024

I may also add some option to prevent the generation of useless mipmaps on such image.

@illwieckz
Copy link
Member Author

Actually the game use RSF_NOMIP and the engine sets IF_NOPICMIP, in fact we would better have:

  • RSF_NOPICMIP and IF_NOPICMIP (do not automatically downscale the image with r_picmip) .
  • RSF_NOMIPMAP and IF_NOMIPMAP (do not generate mipmaps for the image).

@illwieckz
Copy link
Member Author

I may also add some option to prevent the generation of useless mipmaps on such image.

In fact RSF_2D already does:

imageParams.filterType = filterType_t::FT_LINEAR;

Which already prevents the generation of mipmaps, as mipmaps are only generated with filterType_t::FT_DEFAULT.

@illwieckz illwieckz merged commit 2300888 into for-0.55.0/sync Jun 6, 2024
9 checks passed
@illwieckz illwieckz deleted the illwieckz/image-fitscreen/sync branch June 6, 2024 21:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants