Skip to content

Make it possible to restrict texture size of GUI panel button pixmaps#2982

Merged
10110111 merged 1 commit into
Stellarium:masterfrom
10110111:panel-button-pixmap-size
Feb 8, 2023
Merged

Make it possible to restrict texture size of GUI panel button pixmaps#2982
10110111 merged 1 commit into
Stellarium:masterfrom
10110111:panel-button-pixmap-size

Conversation

@10110111
Copy link
Copy Markdown
Contributor

@10110111 10110111 commented Jan 7, 2023

A few times @gzotti complained that the new high-resolution pixmaps used for GUI panel buttons make performance worse on low-spec systems like Raspberry or something like that. This patch introduces a config file setting, gui/pixmaps_scale, which can vary from 1 to 5, to reduce the pixmaps loaded from resources to corresponding scale factor.

E.g., on a machine with a low-DPI monitor, where scale factor higher than 100% is not supposed to be useful, we can set this setting to 1, and avoid using excessive VRAM for 500%-scale textures. Or, if the scaling factor of the monitor is higher than 100% but much lower than 500%, we can set that exact scaling factor as the maximum scale of the pixmaps, thus reducing VRAM use.

I'm not sure if this 500% increase of texture size really influences performance, but, since @gzotti complained more than once, I propose this patch, which might help.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jan 7, 2023

Great PR! Please pay attention to the following items before merging:

Files matching src/**/*.cpp:

  • Are possibly unused includes removed?

This is an automatically generated QA checklist based on modified files.

@alex-w alex-w added this to the 23.1 milestone Jan 7, 2023
@alex-w alex-w removed their request for review January 7, 2023 18:51
@gzotti
Copy link
Copy Markdown
Member

gzotti commented Jan 7, 2023

I am trying to at least keep (actually already: restore) some way of running on RPi3 (at ~3-5fps it still may have its use), even if some features will no longer be available in full quality. Currently it fails when zooming in while showing the DSO texture loading bar. It seems now it barely loaded again (I saw M42, but it crashed again on further zoom in). Logfile says nothing, though.

Not loading DSO and just using icons would probably be OK (seems stable). Mars renders, but the Moon has no surface rendering. Equirectangular projection with "spherical" landscapes works. If this PR reduces texture memory used for the actual buttons, even just a few kB, I approve it. Maybe change name to [gui]/pixmap_scale. I may try reducing DSO texture sizes (or even numbers) as well. A way to limit texture size on loading may also be good for such limited systems.

But I have severely not enough time for testing all platforms with the many GLSL updates (although OTOH I welcome them!).

@10110111
Copy link
Copy Markdown
Contributor Author

10110111 commented Jan 8, 2023

The main question is: does this patch change anything usefully? If the change is not noticeable, this would be just a useless complication.

@10110111
Copy link
Copy Markdown
Contributor Author

10110111 commented Jan 8, 2023

Also, it would be useful to have the output of glxinfo -l to know the limits of this platform.

@gzotti
Copy link
Copy Markdown
Member

gzotti commented Jan 8, 2023

I wonder about texture compression. The RPi supports ETC1 compression, but what does it take to activate this?

I just converted all DSO textures to 64x64, and it runs well again. Better blurred than crash, IMO. Maybe we can configure another (config.ini and/or GUI-available?) switch for "limit texture use"?

RPi3_glxinfo-l.txt
log_RPi3-master-a195de0aae8186_out-of-memory.txt
log_pr2982_maybeSlightlyBetter.txt

@10110111
Copy link
Copy Markdown
Contributor Author

10110111 commented Jan 8, 2023

GL_MAX_TEXTURE_SIZE = 2048

So this is why the Moon is not rendered: our texture is 4096×2048. May be worth it to resize each texture being loaded if it doesn't fit OpenGL limits.

Video memory: 743MB

This is a lot. Do DSO really take so much to hit this limit?

@10110111
Copy link
Copy Markdown
Contributor Author

10110111 commented Jan 8, 2023

I just converted all DSO textures to 64x64, and it runs well again.

Where are they stored? What's their initial size?

Also, the log doesn't appear to say that the crash is due to lack of memory. How did you come to this conclusion?

@gzotti
Copy link
Copy Markdown
Member

gzotti commented Jan 8, 2023

GL_MAX_TEXTURE_SIZE = 2048

So this is why the Moon is not rendered: our texture is 4096×2048. May be worth it to resize each texture being loaded if it doesn't fit OpenGL limits.

Yes, such test and auto-resize should be added in any case! 2048 was the max texture size ever used just for that reason. (And as said, this is why old_style landscapes were required and are still useful.)

Video memory: 743MB

This is a lot. Do DSO really take so much to hit this limit?

I am not sure if this number is correctly reported. The total RAM of this device is 1GB, and raspi-config is used to set a part of this memory for GPU use (64/128/256MB). The documentation is unclear, though, about the right split. Some say the new driver does not use it (set to 64MB), but now I tried 256MB. Maybe this is the total of remaining memory, and the 256MB is even lost...

@gzotti
Copy link
Copy Markdown
Member

gzotti commented Jan 8, 2023

I just converted all DSO textures to 64x64, and it runs well again.

Where are they stored? What's their initial size?

nebulae/default/*.png
Sizes vary from 256x256 to 2048x2048 IIRC, depending on size in the sky. They could as well become JPG, but I understand this would not reduce texture mem needed. Most are 24bit sRGB PNG, others are 256color sRGB PNGs, a few 256 shade B/W PNG (reported by ImageMagick's identify).

Also, the log doesn't appear to say that the crash is due to lack of memory. How did you come to this conclusion?

The program hangs for a minute, then exits. dmesg indicates memory issues (The exact wording comes from the VC4 driver. I think there are no gl_error messages available on this platform. )

@gzotti
Copy link
Copy Markdown
Member

gzotti commented Jan 8, 2023

The main question is: does this patch change anything usefully? If the change is not noticeable, this would be just a useless complication.

How does it work currently? You load large button textures and show them (usually) smaller? Why not make the actual textures the same size as later used on the screen? Or do you fight the rare occasion when a Stellarium window is moved between different screens of different scalings?

@10110111
Copy link
Copy Markdown
Contributor Author

10110111 commented Jan 8, 2023

The program hangs for a minute, then exits. dmesg indicates memory issues

This sounds like out of system RAM, rather than VRAM. Namely, heavy swapping (which looks like a hang to the user), and then process termination. Does dmesg say something like "Out of memory: Killed process ..."?

How does it work currently? You load large button textures and show them (usually) smaller?

I load large textures, and resize them to the configured scale. Then I render them as usual, scaling according to current scale factor as needed.

Why not make the actual textures the same size as later used on the screen? Or do you fight the rare occasion when a Stellarium window is moved between different screens of different scalings?

Exactly. Stellarium is not always run in fullscreen mode. And when it's windowed, it's not really rare to move the window around. If you want to use one particular scaling factor you want, and don't care about differences between screens, just put this value in the config. This is the purpose of this PR, after all.

@gzotti gzotti mentioned this pull request Jan 21, 2023
15 tasks
@gzotti
Copy link
Copy Markdown
Member

gzotti commented Jan 28, 2023

The texture decimation (#3009) seemed to have remedied the basic problem around RPi3. Can you estimate how many MB this patch here would save additionally? The RPi3 has no separate texture memory, just 1GB of system memory for everything. Assume no screen scaling, just FullHD screen (at most) for the RPi3. If this is a few MB only, we can probably close this if you think it just complicates matters. If it's more, we should use it.

@10110111
Copy link
Copy Markdown
Contributor Author

Can you estimate how many MB this patch here would save additionally?

About 10 MiB. Maybe less.

@gzotti
Copy link
Copy Markdown
Member

gzotti commented Jan 28, 2023

Hmm, that's just the size of about 3-5 large DSO textures. Still it is "something", so if sufficiently commented, I still would use it.

@10110111 10110111 marked this pull request as ready for review February 2, 2023 20:53
@github-actions github-actions Bot requested a review from alex-w February 2, 2023 20:54
Copy link
Copy Markdown
Member

@gzotti gzotti left a comment

Choose a reason for hiding this comment

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

I think conserving memory has its merits on limited systems, so yes, Approved!

@10110111 10110111 merged commit ad696b2 into Stellarium:master Feb 8, 2023
@10110111 10110111 deleted the panel-button-pixmap-size branch February 8, 2023 19:54
@github-actions
Copy link
Copy Markdown

Hello @10110111!

The fix has been merged into source code and you may test it via building Stellarium from source code or wait the weekly development snapshot...

@alex-w alex-w added state: published The fix has been published for testing in weekly binary package and removed state: fixed labels Mar 13, 2023
@github-actions
Copy link
Copy Markdown

Hello @10110111!

Please check the fresh version (development snapshot) of Stellarium:
https://github.com/Stellarium/stellarium-data/releases/tag/weekly-snapshot

@alex-w alex-w removed the state: published The fix has been published for testing in weekly binary package label Mar 27, 2023
@github-actions
Copy link
Copy Markdown

Hello @10110111!

Please check the latest stable version of Stellarium:
https://github.com/Stellarium/stellarium/releases/latest

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants