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

Render some wide lines as sets of triangles #3187

Merged
merged 1 commit into from
Apr 23, 2023

Conversation

10110111
Copy link
Contributor

@10110111 10110111 commented Apr 15, 2023

This is an experimental partial conversion of some wide-line LineStrip primitives to rendering via manual tesselation. May fix #2836.

Please check whether

  1. It renders correctly, including when the lines are wide;
  2. Performance improves.

@github-actions
Copy link

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
Copy link
Member

alex-w commented Apr 16, 2023

Tested in macOS/arm64 and yes, this patch improves performance and solves #2836

@10110111
Copy link
Contributor Author

@gzotti you noted in #2836 that you saw performance problems on Windows. Does this PR change anything?

@gzotti
Copy link
Member

gzotti commented Apr 16, 2023

Just developing my own stuff around finding out why the transitions are so damn slow. (#3183). Will do in 1-2 hrs.

@10110111 10110111 force-pushed the lines-as-quads branch 2 times, most recently from 35951af to bc54f80 Compare April 16, 2023 12:38
@gzotti
Copy link
Member

gzotti commented Apr 16, 2023

Screenshot (33)
Right: 23.1. Left: This branch. Synced with RemoteSync but then running without connection. It seems the new version is marginally slower. This is on a Geforce RTX3700 Ti (Laptop).
(Framerate looks very low because I had way too many asteroids. With default ssystem_minor.ini I see 33-35 fps. Still the new version appears marginally slower.)

@10110111
Copy link
Contributor Author

Aren't asteroids created at random times? This might introduce some uncertainties.
Besides, running two apps simultaneously on Windows gives the one with the active window higher priority, which once again can skew the benchmarks.

@gzotti
Copy link
Member

gzotti commented Apr 16, 2023

Aren't asteroids created at random times? This might introduce some uncertainties.

No, maybe you mean meteors/shooting stars? I mean the minor planets.

Besides, running two apps simultaneously on Windows gives the one with the active window higher priority, which once again can skew the benchmarks.

But not by much. Both show a few tenths of fps up and down, but maybe its something like 37.2 vs 36.8. ±0.5. I have plenty of cores free, overall CPU load at 15%. (And yes, we should identify ways to distribute load better!)

The major effect of #2836 was on Apple. If this solves it, it's OK for me.

@10110111 10110111 force-pushed the lines-as-quads branch 2 times, most recently from b667901 to 278d653 Compare April 16, 2023 16:31
@10110111 10110111 marked this pull request as ready for review April 16, 2023 16:33
@github-actions github-actions bot requested a review from gzotti April 16, 2023 16:34
@gzotti
Copy link
Member

gzotti commented Apr 16, 2023

Seems to be the same as before.
Is this now also used in the gridlines? I see a drop from 37 to 32fps when activating both equatorial and horizon grid.
The Borders are far fewer lines, probably making detection of differences more difficult.

@alex-w
Copy link
Member

alex-w commented Apr 16, 2023

macOS 13.3.1 / M1

stellarium-002
stellarium-003

@10110111
Copy link
Contributor Author

@alex-w Is this lines disabled vs lines enabled? What about the case with lines enabled before this commit?

@10110111
Copy link
Contributor Author

10110111 commented Apr 16, 2023

Is this now also used in the gridlines?

Should be I think.

I see a drop from 37 to 32fps when activating both equatorial and horizon grid.

Is it much worse than before this commit? Do you run a Release build?

The Borders are far fewer lines, probably making detection of differences more difficult.

You can enable all of them for greater effect. The borders had a large impact in #2836.

@alex-w
Copy link
Member

alex-w commented Apr 16, 2023

@alex-w Is this lines disabled vs lines enabled?

Yes

What about the case with lines enabled before this commit?

OK, the screenshots from the current master:
stellarium-004
stellarium-005

Same master, but with --opengl-compat option:
stellarium-006
stellarium-007

Copy link
Member

@alex-w alex-w left a comment

Choose a reason for hiding this comment

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

Great!

@10110111
Copy link
Contributor Author

Your screenshots have strangely inconsistent frame rates, maybe because of the time taken to switch to screenshot mode. Could you type the values that you see on the screen in real time instead of the screenshots?

@gzotti
Copy link
Member

gzotti commented Apr 16, 2023

Is this now also used in the gridlines?

Should be I think.

I see a drop from 37 to 32fps when activating both equatorial and horizon grid.

Is it much worse than before this commit? Do you run a Release build?

23.1: 37fps
This: 32fps.
All in Release build. I activated Equatorial and Horizontal grids. Framerate intent was higher, so it's maxed out.

The Borders are far fewer lines, probably making detection of differences more difficult.

You can enable all of them for greater effect. The borders had a large impact in #2836.

Yes, I have read and seen that. However strange that sounds if there apparently were no issues around gridlines on Macs.

@10110111
Copy link
Contributor Author

All in Release build. I activated Equatorial and Horizontal grids.

Strange behavior (and not really acceptable). Is the CPU used 100%? What about the GPU?

@alex-w
Copy link
Member

alex-w commented Apr 16, 2023

Your screenshots have strangely inconsistent frame rates, maybe because of the time taken to switch to screenshot mode. Could you type the values that you see on the screen in real time instead of the screenshots?

master, FOV 180 degrees, OpenGL compatibility mode - same FPS (17.9) for both cases - without and with borders.

master, FOV 180 degrees, OpenGL core profile mode - 17.9 and 2.4 FPS for without and with borders respectively.

current branch, FOV 180 degrees, OpenGL core profile mode - same FPS (17.9) for both cases - without and with borders.

P.S. v23.1, FOV 180 degrees, OpenGL core profile mode - 17.9 and 10.9 FPS for without and with grids (equatorial + horizontal) respectively.

P.P.S. this branch, FOV 180 degrees, OpenGL core profile mode - same FPS (17.9) for both cases - without and with grids (equatorial + horizontal).

P.P.P.S. Max FPS for master and this branch is over 30.

@gzotti
Copy link
Member

gzotti commented Apr 16, 2023

All in Release build. I activated Equatorial and Horizontal grids.

Strange behavior (and not really acceptable). Is the CPU used 100%? What about the GPU?

The resource monitor reports several core are "parked".

Screenshot (34)

@10110111
Copy link
Contributor Author

@gzotti When you have time, please test the following scenario with the new debug commit:

  1. Set frame rate to virtually unlimited
  2. Set large FOV
  3. Disable everything: stars, planets, atmosphere, DSO markers, DSO, cardinal directions, etc. — so that the sceen is black
  4. Enable azimuthal grid
  5. Note the frame rate
  6. Note CPU & GPU load
  7. Disable azimuthal grid, re-enable it (this should toggle the tessellation, you can see it in the log)
  8. Note the frame rate
  9. Note CPU & GPU load

I think this will be the fairest comparison of the two modes.

@gzotti
Copy link
Member

gzotti commented Apr 16, 2023

I had to switch several grids to go below 60fps. Both ways fluctuate 36-40 fps, with 9-10% CPU and 24% GPU load. I may have to reboot to activate >60fps which I will not do right now, I seriously must do sth else now.

@gzotti
Copy link
Member

gzotti commented Apr 16, 2023

With VSync disabled and only horizontal grid
enabled: 85-93fps GPU fluctuating 28-30% CPU 10-11%
disabled: 88...95fps GPU fluctuating 28..34% CPU 9-12%

Still almost inconclusive. Probably a draw on the latest and greatest system. I can test on Intel only later next week.

@alex-w
Copy link
Member

alex-w commented Apr 17, 2023

Intel UHD Graphics 630; Windows 10

A black screen, FOV 235 - average FPS 59; azimuthal grid - avr. FPS 46; equatorial grid - avr. FPS 47; azimuthal + equatorial grids - avr. FPS 32

@alex-w
Copy link
Member

alex-w commented Apr 17, 2023

This is full crap - this branch with Qt 6.5 on M1 (OpenGL core profile mode) is slow as it was before now :(

@10110111
Copy link
Contributor Author

10110111 commented Apr 17, 2023

Have you checked in the log that tessellation is enabled? Or simply checkout the previous commit, 278d653.

@alex-w
Copy link
Member

alex-w commented Apr 17, 2023

Have you checked in the log that tessellation is enabled? Or simply checkout the previous commit, 278d653.

No tessellation in the log 🤪

@10110111
Copy link
Contributor Author

No tessellation — no optimization. You're in the wrong mode :P . Press Z to enable azimuthal grid and tessellation. Each enable toggles. Or, once again, just go to the commit before the debug one.

@alex-w
Copy link
Member

alex-w commented Apr 17, 2023

Aargh... A debug message for tesselation mode for azimuthal grid only. OK, azimuthal grid + boundaries = good rendering. Sorry for false panic 😳

@10110111
Copy link
Contributor Author

Still almost inconclusive. Probably a draw on the latest and greatest system.

I measured a tiny difference on GeForce GTX 750Ti, which is not really latest nor greatest. On my Intel UHD Graphics 620 the difference is also imperceptible. So if you don't mind, I can simply merge this PR.

@gzotti
Copy link
Member

gzotti commented Apr 17, 2023

Maybe I should still test on Intel i5-4xxx and RPi3/4. Sorry, next weekend.

@gzotti
Copy link
Member

gzotti commented Apr 23, 2023

Intel Core i5-4570 (HD4600), Qt6.5: disabled: 15.2±0.3 -- enabled: 15.0±0.3. Again, hard to see the difference.
Intel Core i5-4570 (HD4600), Qt5.15.2: disabled: 17.5±0.3 -- enabled: 17.2±0.4. Wide lines show about 0.5fps penalty in both paths.
Intel Core i5-4570 (HD4600), Qt5.15.2, ANGLE: disabled: 31.4±0.3 -- enabled: 31.5±0.3. Still no wide lines on ANGLE!

(The fps difference between OpenGL and ANGLE may come from a different FoV.)

RPi3 (smaller screen, not comparable): disabled: 9.37±0.1 -- enabled: 9.43±0.2. A tiny improvement!
RPi4/UbuntuMate22.04/OGL2.1/GLSL1.2/Mesa22.2.5 (same screen as RPi3): disabled: 32.4±0.1 -- enabled: 32.4±0.2. No change.

@10110111
Copy link
Contributor Author

disabled: 31.4±0.3 -- enabled: 31.5±0.3. Still no wide lines on ANGLE!

What does this mean? Nothing rendered when thickness is >1?

@gzotti
Copy link
Member

gzotti commented Apr 23, 2023

Yes, lines stay thin. This was always the case on ANGLE (~GLES2.0). I just hoped this may enable wide lines. StelPainter::setLineWidth(float width) does nothing when not on a Core profile. Is that outdated?

@10110111
Copy link
Contributor Author

10110111 commented Apr 23, 2023

StelPainter::setLineWidth(float width) does nothing when not on a Core profile. Is that outdated?

It doesn't do nothing, it sets glState.lineWidth that is used when rendering wide lines. But yes, you pointed me to the reason of this problem. It's because wideLineMode checks whether Core profile is active, so in GLES mode we don't set line width. I'll try to fix this.

Does your testing show that this PR is OK other than this ANGLE problem?

@gzotti
Copy link
Member

gzotti commented Apr 23, 2023

Yes, I think other than that this would be OK. If this enables wide lines in ANGLE, it is also visible progress here!

@10110111
Copy link
Contributor Author

OK, it should work in GLES2 now. Caveat as before: wide lines with color varying per vertex are still rendered with geometry shader on Core profile, and as thin lines in GLES2. This use case is not so frequent though, so I think it should be OK.

@10110111
Copy link
Contributor Author

10110111 commented Apr 23, 2023

@alex-w Please check that the last commit hasn't broken anything on macOS. The logic is a bit convoluted now.

@gzotti
Copy link
Member

gzotti commented Apr 23, 2023

OK, it should work in GLES2 now. Caveat as before: wide lines with color varying per vertex are still rendered with geometry shader on Core profile, and as thin lines in GLES2. This use case is not so frequent though, so I think it should be OK.

Confirm this works! I think we currently (never had) lines with separate vertex colors. Ah, not sure about the technical details of meteors. They should not be affected, though. OK, from my side it's fine now, thank you!

@alex-w
Copy link
Member

alex-w commented Apr 23, 2023

@alex-w Please check that the last commit hasn't broken anything on macOS. The logic is a bit convoluted now.

30FPS with boundaries and 37FPS without boundaries - so, this is acceptable

@alex-w alex-w added this to the 23.2 milestone Apr 23, 2023
Additionally, support for wide lines is enabled on OpenGL ES 2.

This currently only affects lines with primitives being GL_FLOAT-based
vectors and scalars. Also, the case when indices or color buffer are
used is not affected by this commit.

The cases not affected by this commit continue being handled via
geometry shader. As most of the lines currently rendered are
fixed-color without index buffer, this shouldn't have a noticeable
performance impact.
@10110111 10110111 merged commit 38dbf6a into Stellarium:master Apr 23, 2023
@10110111 10110111 deleted the lines-as-quads branch April 23, 2023 19:11
@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 @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 Jul 2, 2023
@github-actions
Copy link

github-actions bot commented Jul 2, 2023

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
Development

Successfully merging this pull request may close these issues.

Bad performance for boundaries
3 participants