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

Bad performance for boundaries #2836

Closed
alex-w opened this issue Nov 9, 2022 · 25 comments · Fixed by #3187
Closed

Bad performance for boundaries #2836

alex-w opened this issue Nov 9, 2022 · 25 comments · Fixed by #3187
Assignees
Labels
bug Something likely wrong in the code importance: medium A bit annoying, minor miscalculation, but no crash qt Issues, related to Qt framework state: confirmed A developer can reproduce the issue
Milestone

Comments

@alex-w
Copy link
Member

alex-w commented Nov 9, 2022

Expected Behaviour

Good performance to rendering boundaries of constellations

Actual Behaviour

Bad performance to rendering boundaries of constellations - after enabling displaying boundaries FPS dramatic decreases in 4 times

Steps to reproduce

  1. Run Stellarium
  2. Enable boundaries (B)

System

  • Stellarium version: master
  • Operating system: macOS 13.0 M1

Logfile

log.txt

@alex-w alex-w added this to the 1.2 milestone Nov 9, 2022
@alex-w alex-w added bug Something likely wrong in the code importance: medium A bit annoying, minor miscalculation, but no crash labels Nov 9, 2022
@10110111
Copy link
Contributor

10110111 commented Nov 9, 2022

Does this also happen with --opengl-compat option?

@Atque
Copy link
Contributor

Atque commented Nov 9, 2022

Also to some degree present on Windows. Especially noticeable at wide FOV.

@alex-w
Copy link
Member Author

alex-w commented Nov 10, 2022

Does this also happen with --opengl-compat option?

No

@alex-w alex-w added the state: confirmed A developer can reproduce the issue label Dec 4, 2022
@github-actions
Copy link

github-actions bot commented Dec 4, 2022

Hello @alex-w!

OK, developers can reproduce the issue. Thanks for the report!

@alex-w
Copy link
Member Author

alex-w commented Dec 21, 2022

@10110111 any ideas where is the source of issue?

@10110111
Copy link
Contributor

Apparently, the geometric shader works slowly. But it's so basic, that I don't know how it could be optimized.
A solution of this issue would be to redo StelPainter::drawFromArray() for all line-based primitives, so that it created polygons on the CPU instead of the geometric shader. Not nice technically, and quite a bit of work.

@alex-w
Copy link
Member Author

alex-w commented Dec 22, 2022

Today we tested 1.2RC2 on Intel Mac and the issue is not reproducible, so, apparently this is problem specific for arm-macs (at least for M1).

@alex-w alex-w modified the milestones: 1.2, 23.1 Dec 22, 2022
@gzotti
Copy link
Member

gzotti commented Dec 22, 2022

I can confirm the boundaries are "expensive" also on Win11, dropping from 60 to 52 fps in wide-angle view. But also gridlines, GUI dialogs and InfoText displays are (and always have been) costly. I have not tested previous versions on this notebook, though. A drop to 1/4 would be bad, indeed. Is that only boundaries, or also the gridlines?

@unpacklo
Copy link

unpacklo commented Jan 13, 2023

The core issue is the CPU cost here in drawing lines and text:

image

We should not be creating a new QOpenGLPaintDevice in StelPainter::drawText. The destruction of QOpenGLPaintDevice is 11% of the total CPU time. I've spent some time before in this code in an attempt to optimize a few months ago but I simply don't have enough knowledge about how Qt's OpenGL API and how Stellarium works to be able to meaningfully contribute at this time.

Aside from that, the text drawing appears to be the most costly and should be addressed, but I haven't figured out what the core cause is of the slow performance to have any suggestions about making the text draw faster. I suspect it is due to constant rerasterization or recomputation of the layout of the text but I'm not sure. More profiling will be needed.

@10110111
Copy link
Contributor

We should not be creating a new QOpenGLPaintDevice in StelPainter::drawText.

This might be one of the reasons, but not the only one. Lines used to work faster. But, do you get any improvement if you make this variable static?

@unpacklo
Copy link

I finally managed to build stellarium on my macos and I can confirm that simply making the variable static makes my frame rate go from about 7 FPS to about 20 FPS (from about 143 ms per frame to about 50 ms per frame) for about a 2.8x speedup. Totally worth doing, but given that the CPU time associated with the destructor only takes up 10%, there must be some sort of GPU pipeline flush or sync that occurs with the construction/destruction of the QOpenGLPaintDevice that causes a much larger performance problem on the GPU side. I'll try to dig more but simply making that var static has a really big boost.

@10110111
Copy link
Contributor

I finally managed to build stellarium on my macos and I can confirm that simply making the variable static makes my frame rate go from about 7 FPS to about 20 FPS

This is not related to boundaries, right? Because I don't see any text associated with them.

@alex-w
Copy link
Member Author

alex-w commented Jan 14, 2023

@unpacklo thank you for investigating the problem. Could you send us pull request with patch?

@unpacklo
Copy link

I finally managed to build stellarium on my macos and I can confirm that simply making the variable static makes my frame rate go from about 7 FPS to about 20 FPS

This is not related to boundaries, right? Because I don't see any text associated with them.

@10110111 ah sorry, I was confused with another issue. I don't see the problem with boundaries at the moment, but I am seeing big performance problems on my Intel MacBook Pro (2019).

@unpacklo thank you for investigating the problem. Could you send us pull request with patch?

@alex-w Yes, I intend to open a PR once I've figured out why making it static improves performance so much.

@krstnrogers
Copy link

Very slow and lags on version 1.2 when enabling the "show boundaries" option. Macbook air running MacOS Ventura 13.2.

@alex-w
Copy link
Member Author

alex-w commented Feb 18, 2023

This is weird. macOS 13.2.1 (M1), current master + Qt5 - no penalty of performance when boundaries are enabled. Current master + Qt6 - the issue is here.

@alex-w alex-w added the qt Issues, related to Qt framework label Feb 18, 2023
@gzotti
Copy link
Member

gzotti commented Feb 18, 2023

We can so much need a profiling expert to point out bottlenecks!

@10110111
Copy link
Contributor

One major difference between Qt5 and Qt6 is that in Qt5 only integral scaling factors are supported (any fraction is rounded), while Qt6 supports fractional factors. You might be observing no slowdown with e.g. 100% effective (i.e. from Qt's PoV) pixel ratio, while a slowdown appears with higher pixel ratio.

@alex-w
Copy link
Member Author

alex-w commented Feb 18, 2023

One major difference between Qt5 and Qt6 is that in Qt5 only integral scaling factors are supported (any fraction is rounded), while Qt6 supports fractional factors. You might be observing no slowdown with e.g. 100% effective (i.e. from Qt's PoV) pixel ratio, while a slowdown appears with higher pixel ratio.

But why Qt6-based packages doesn't have the issue on Mac's with Intel CPU?

@10110111
Copy link
Contributor

If the issue only appears on non-Intel, this may just mean that Intel have been capable of making GPUs that run geometry shaders with sensible performance.

Just to check, what pixel ratio do you have on the machines where this slowdown happens? I'm interested in both Qt5 and Qt6 perspectives. Also, was the line width setting changed from the default on these machines?

@alex-w
Copy link
Member Author

alex-w commented Feb 18, 2023

Device pixel ratio is 2

@10110111
Copy link
Contributor

In both cases?

@alex-w
Copy link
Member Author

alex-w commented Feb 18, 2023

In both cases?

Yep

@alex-w alex-w modified the milestones: 23.1, 23.2 Mar 14, 2023
@alex-w alex-w added the state: published The fix has been published for testing in weekly binary package label May 2, 2023
@github-actions
Copy link

github-actions bot commented May 2, 2023

Hello @alex-w!

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 Jul 2, 2023
@github-actions
Copy link

github-actions bot commented Jul 2, 2023

Hello @alex-w!

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
bug Something likely wrong in the code importance: medium A bit annoying, minor miscalculation, but no crash qt Issues, related to Qt framework state: confirmed A developer can reproduce the issue
Development

Successfully merging a pull request may close this issue.

6 participants