Graphics cleanup/improvments

paulwedeck commented 6 years ago

Changes:

adds vaos to reduce opengl calls/overhead
replaces all cpu bound buffers (and corresponding functions) with gpu bound ones (vbos)
replaced dynamically allocated vbos in Background with two big static one (one for the position data and one for the color/surface). 3.1. this makes zooming much smother
replaces the exception based error reporting with a more universal debug mode (--debug-opengl) 4.1. IllegalBufferException is not yet removed because this would be a dramatic performance decrease (dont ask why) 4.2. If the view is partially outside of the map Background floats the log with draw errors
sets proper performance hints in glBufferData
replaces x,y,z coordinates with x,y coordinates in geometries where z is always 0

Still to do:

[x] debug context creation with egl (swing requires #738; ~android would require docs EDIT: and api level 21~)
[x] testing of wgl and
[x] jogl debug context creation (EDIT: tested with linux as jogl is platform independent)
[x] stop doing illegal render calls in Background/ stop spamming error messages
[x] filtering of opengls debug output
[x] assigning names to buffers and textures for easier debugging
[x] fix android text color
[ ] fix gles2 on some devices
[ ] ~solve and remove player border object todo~ (no longer urgent)

andreas-eberle commented 6 years ago

@tom-pratt: Can we switch to API level 21 for Android? Since that is Android 5.0, I guess it should not be an issue for any reasonable device running the game. What do you think?

michaelzangl commented 6 years ago

I would be fine with API 21, it would still run on my Galaxy S2 then ;-). And I think you won't find much older hardware out there...

tom-pratt commented 6 years ago

Yes lets do it. Is it required by something in this PR?

andreas-eberle commented 6 years ago

The first checkbox point mentioned by @paulwedeck in the description of the PR mentioned it:

[ ] debug context creation with egl (swing requires #738; android would require docs EDIT: and api level 21)

@paulwedeck: Feel free to update it to API level 21 if you need it.

paulwedeck commented 6 years ago

gles-debugging.patch This patch should do all necessary work for debugging OpenGL ES but Android is filtering the calls or only pretends to have this extension.

E/AndroidRuntime: FATAL EXCEPTION: GLThread 5727 Process: jsettlers.main.android, PID: 25570 java.lang.UnsupportedOperationException: not yet implemented at android.opengl.GLES31Ext.glDebugMessageCallbackKHR(Native Method) at go.graphics.android.AndroidDrawContext.(AndroidDrawContext.java:56) at go.graphics.android.GOSurfaceView$Renderer.onSurfaceCreated(GOSurfaceView.java:259) at android.opengl.GLSurfaceView$GLThread.guardedRun(GLSurfaceView.java:1516) at android.opengl.GLSurfaceView$GLThread.run(GLSurfaceView.java:1259)

andreas-eberle commented 6 years ago

@paulwedeck: I see all your checkboxes are marked. Is this ready for review?

paulwedeck commented 6 years ago

Yes

andreas-eberle commented 6 years ago

Text rendering in the upper right corner is a bit broken and it should not be totally on the border: grafik

tom-pratt commented 6 years ago

Just tried this on my phone and the zooming is much much smoother. Thanks! Makes it way more playable

tom-pratt commented 6 years ago

Just tried to play on a Pixel 2 XL and get a black screen when the game starts. I saw the game graphics flash on screen quickly and then disappear to black.

It worked fine on a Samsung s7

andreas-eberle commented 6 years ago

@paulwedeck: Did you see the comments of @tom-pratt? If you don't have access to a Pixel2XL, Tom might be able to help you debug it.

paulwedeck commented 6 years ago

@tom-pratt Does it run on the Pixel 2 XL now ?

tom-pratt commented 6 years ago

@paulwedeck Still the same. This time I took a logcat. This line is red

08-16 20:56:40.179 3111-3111/jsettlers.main.android E/libEGL: call to OpenGL ES API with no current context (logged once per thread)

Here's the rest

08-16 20:56:39.405 3111-3343/jsettlers.main.android I/System.out: grid filled
08-16 20:56:39.422 3111-3359/jsettlers.main.android D/AudioTrack: Client defaulted notificationFrames to 296 for frameCount 890
08-16 20:56:39.422 3111-3354/jsettlers.main.android D/AudioTrack: Client defaulted notificationFrames to 296 for frameCount 890
08-16 20:56:39.422 3111-3358/jsettlers.main.android D/AudioTrack: Client defaulted notificationFrames to 296 for frameCount 890
08-16 20:56:39.422 3111-3357/jsettlers.main.android D/AudioTrack: Client defaulted notificationFrames to 296 for frameCount 890
08-16 20:56:39.422 3111-3356/jsettlers.main.android D/AudioTrack: Client defaulted notificationFrames to 296 for frameCount 890
08-16 20:56:39.423 3111-3355/jsettlers.main.android D/AudioTrack: Client defaulted notificationFrames to 296 for frameCount 890
08-16 20:56:39.427 3111-3358/jsettlers.main.android W/AudioTrack: Use of stream types is deprecated for operations other than volume control
    See the documentation of AudioTrack() for what to use instead with android.media.AudioAttributes to qualify your playback use case
08-16 20:56:39.427 3111-3354/jsettlers.main.android W/AudioTrack: Use of stream types is deprecated for operations other than volume control
    See the documentation of AudioTrack() for what to use instead with android.media.AudioAttributes to qualify your playback use case
08-16 20:56:39.427 3111-3359/jsettlers.main.android W/AudioTrack: Use of stream types is deprecated for operations other than volume control
    See the documentation of AudioTrack() for what to use instead with android.media.AudioAttributes to qualify your playback use case
08-16 20:56:39.427 3111-3357/jsettlers.main.android W/AudioTrack: Use of stream types is deprecated for operations other than volume control
    See the documentation of AudioTrack() for what to use instead with android.media.AudioAttributes to qualify your playback use case
08-16 20:56:39.428 3111-3355/jsettlers.main.android W/AudioTrack: Use of stream types is deprecated for operations other than volume control
    See the documentation of AudioTrack() for what to use instead with android.media.AudioAttributes to qualify your playback use case
08-16 20:56:39.429 3111-3356/jsettlers.main.android W/AudioTrack: Use of stream types is deprecated for operations other than volume control
    See the documentation of AudioTrack() for what to use instead with android.media.AudioAttributes to qualify your playback use case
08-16 20:56:40.179 3111-3111/jsettlers.main.android E/libEGL: call to OpenGL ES API with no current context (logged once per thread)
08-16 20:56:41.223 3111-3381/jsettlers.main.android I/System.out: Background texture generated in 6ms
08-16 20:56:41.419 3111-3354/jsettlers.main.android W/AudioTrack: releaseBuffer() track 0x75216cc400 disabled due to previous underrun, restarting
08-16 20:56:42.670 3111-3359/jsettlers.main.android W/AudioTrack: releaseBuffer() track 0x752e6a2000 disabled due to previous underrun, restarting
08-16 20:56:42.774 3111-3358/jsettlers.main.android W/AudioTrack: releaseBuffer() track 0x7531481400 disabled due to previous underrun, restarting
08-16 20:56:43.502 3111-3117/jsettlers.main.android I/zygote64: Do full code cache collection, code=1009KB, data=561KB
08-16 20:56:43.504 3111-3117/jsettlers.main.android I/zygote64: After code cache collection, code=878KB, data=459KB
08-16 20:56:43.880 3111-3357/jsettlers.main.android W/AudioTrack: releaseBuffer() track 0x752409ec00 disabled due to previous underrun, restarting
08-16 20:56:44.077 3111-3355/jsettlers.main.android W/AudioTrack: releaseBuffer() track 0x7523fadc00 disabled due to previous underrun, restarting
08-16 20:56:44.381 3111-3356/jsettlers.main.android W/AudioTrack: releaseBuffer() track 0x75217ccc00 disabled due to previous underrun, restarting
08-16 20:56:46.990 3111-3357/jsettlers.main.android W/AudioTrack: releaseBuffer() track 0x752409ec00 disabled due to previous underrun, restarting
08-16 20:56:49.304 3111-3354/jsettlers.main.android W/AudioTrack: releaseBuffer() track 0x75216cc400 disabled due to previous underrun, restarting
08-16 20:56:49.401 3111-3355/jsettlers.main.android W/AudioTrack: releaseBuffer() track 0x7523fadc00 disabled due to previous underrun, restarting
08-16 20:56:50.401 3111-3342/jsettlers.main.android I/System.out: Scheduled SyncTasksPacket(lockstep: 102 tasks: [SimpleGuiTask: BUILD playerId: 1] for lockstep: 102 (game time: 10200ms / 00:00:10:200)

paulwedeck commented 6 years ago

~~go to GOSurfaceView.Factory#createContext does it return a context or null ?~~

EDIT: this error message is another bug

paulwedeck commented 6 years ago

@tom-pratt is it actually black or dark gray ?

paulwedeck commented 6 years ago

screenshot_jsettlers_20180817-195119

tom-pratt commented 6 years ago

Completely black like the second image. Even stranger, a couple of times, after it's happened my home screen wallpaper has disappeared and turned black too until I rebooted!

paulwedeck commented 6 years ago

JSettlers should not have the permission to change your wallpaper, so this should be an unrelated bug.

andreas-eberle commented 6 years ago

Maybe this is less related to the phone and more to the newest Android? Did you guys test on another pixel/nexus device with newest android?

tom-pratt commented 6 years ago

Just updated to android p on this phone and the problem is the same. Maybe we should just merge anyway. It seems much improved on all other phones.

paulwedeck commented 6 years ago

does this change anything ?

tom-pratt commented 6 years ago

Very noticeable difference but still not quite working.

No terrain graphics at all
Very laggy and low FPS
Crashes after 20 seconds or so after the map loads
Wouldn't take a screenshot when I tried, gave me a notification saying that screenshot failed
Definitely made my wallpaper go black! It hadn't changed the wallpaper, just something about the graphics and device load causing it to go weird

img-20180820-wa0009

paulwedeck commented 6 years ago

Could you send me a Screenshot from Android Profiler>jsettlers.main.android>GLThread xxx>FlameChart somewhere between the game is fully loaded and it crashes ?

What are the last lines before the process crashes ?

paulwedeck commented 6 years ago

Is the background fixed ?

tom-pratt commented 6 years ago

Good progress!

The background looks normal. All of the graphics appear correctly.

The lagginess is still there, it didn't seem to crash so easily although it did crash a couple of times after leaving it a while.

screenshot 2018-08-21 21 51 11

paulwedeck commented 6 years ago

Please add three counters in Background.java. They should increment when:

Background#updateGeometry
byte fow = ... (line 1280)
context.getGl().updateGeometryAt(... (line1288) is called. What are the values after ~5 and ~10 seconds ?

andreas-eberle commented 5 years ago

@paulwedeck: What's the status of this? Will it be ready to merge anytime soon?

paulwedeck commented 5 years ago

Its working on my devices but I do not know the status of @tom-pratt s Pixel 2 XL. I think its fine to merge because he has not replied in a month.

tom-pratt commented 5 years ago

Hi, sorry never got round to trying the debugging steps you suggested, I think it's fine to merge, shouldn;t let device specific issues block!

paulwedeck commented 5 years ago

@andreas-eberle Are you going to merge it ?

andreas-eberle commented 5 years ago

@paulwedeck: From the checklist on top of this PR, I see that one issue is still open. What is the status on that? Do you think we should merge it first and you continoue on the further fixes?

paulwedeck commented 5 years ago

I think we should merge it and open an issue for the pixel 2 problem.

andreas-eberle commented 5 years ago

@paulwedeck: There seems to be an issue with travis. I'll have a look to get it merged.

andreas-eberle commented 5 years ago

@paulwedeck: Thanks for all these improvements. I'm looking forward to futher ones ;)

jsettlers / settlers-remake

Graphics cleanup/improvments #747