triplea-game / triplea

TripleA is a turn based strategy game and board game engine, similar to Axis & Allies or Risk.
https://triplea-game.org/
GNU General Public License v3.0
1.34k stars 393 forks source link

OpenGL buggy UI rendering #4941

Closed DanVanAtta closed 5 years ago

DanVanAtta commented 5 years ago

Seeing rendering problems with latest TripleA version. After hovering over buttons, the display becomes corrupted:

Screenshot from 2019-08-07 19-05-20

I bisected this problem to this commit: eb017eb5d1c01e126630e6a789f0db312d687b2e

Removing this line fixes the problem:

<option name="VM_PARAMETERS" value="-Dsun.java2d.opengl=true" />

https://github.com/triplea-game/triplea/pull/4921/commits/e1c8be6e04c64875b1fff8fe4ee5a7b02ba337c0#diff-be47e7f6444da73a9a1ce891248933dbR5

I notice we have this configuration in install4j, presumably we could see issues on the built artifacts, I have not checked. @RoiEXLab , would you have a look and advise? I wonder how universally the flag is supported.

RoiEXLab commented 5 years ago

Ok, looks like this is a lot buggier on Ubuntu. On Windows there are just a couple of smaller glitches (flickering when some components are loaded but nothing dramatic). I originally added this flag to make scrolling fast enough on Windows, rendering otherwise takes a little bit too long on my machine to feel 60-FPS-smooth

RoiEXLab commented 5 years ago

Try playing a basic game and zoom and drag around the map a little bit. If that works without any performance problems for you then we might want to drop it on Ubuntu.

DanVanAtta commented 5 years ago

@RoiEXLab with the flag, the UX on the main menus is very bad. The disappearing text impacts text that is entered and makes it difficult to even log in to lobby. Even without hovering over buttons, text disappears, eg: Screenshot from 2019-08-08 00-16-44

I played a map with the flag, the scrolling is reasonable, but it seems to be better without.

@RoiEXLab are you sure this flag works well on Mac and the various versions of Windows with various graphics cards?

DanVanAtta commented 5 years ago

@RoiEXLab how important is the openGL flag for performance on windows?

On the ubuntu case it needs to be removed, all swing components are glitchy, the UI is very difficult to use. I'm concerned we could see this problem in unexpected places, for specific java versions, other OS or graphics cards.

I do not know the background to why the flag is beneficial, or needed, is there a good reference on why it's useful? Is there other background I can read to learn more about the performance improvement/impact it has for scrolling/rendering?

RoiEXLab commented 5 years ago

Well the basic difference is that without this flag all rendering operations are calculated on the CPU, which makes this obviously pretty slow. With this flag drawing the tiles took less than 3ms on my machine (the time drawImage took aggregated for all tiles), whereas without this flag it took <40ms, which made movement really clunky. It definitely way usable, but it sure wasn't the best experience. We could disable this flag via code if we do this before swing is being invoked (i.e. the look and feel is loaded) on specific oses if that helps. See https://stackoverflow.com/questions/35641229/java2d-opengl-graphics-acceleration-not-working for more information

DanVanAtta commented 5 years ago

OS specific code is something to avoid, it multiplies test burden as you have to test the same thing on multiple systems. To boot, none of us AFAIK have all three OS available, which further makes testing more difficult and adds more liability to silently break something.

I think it might be safer to instead of blacklisting some OS it would be better to conditionally enable if we can detect proper support (which means we'd have to know which conditions are necessary for the flag to work properly vs not). For example, perhaps other flavors of linux this flag would work, maybe I'm just missing an openGL library.. Hence, enabling the flag when support is known to be available I think would be far safer.

DanVanAtta commented 5 years ago

Another thought, if we can't make great progress to knowingly determine when the flag would be supported, we could move it to a setting and allow users to turn it on. In this case it could be turned off again readily if there are rendering issues.

RoiEXLab commented 5 years ago

Hmm, the OpenGL java2d pipeline seems to be a feature that was introduced after the "normal" implementation just for the performance. So I don't think any OS is really 100% supported, but a the same time that could be said about the swing framework that isn't really updated either anymore.

Having a user setting for this might be an option. Another option I just thought of, but haven't tried yet. JavaFX has those swing adapter nodes. So we could in theory use a JFXPanel to do all the rendering in, which should support hardware acceleration by default, there might be some problems with this approach though: I'm not sure how JFXPanel relays rendering. If it just delegates the rendering to swing, then the problem wouldn't be solved at all. In this case we'd have to make the outer JFrame its own javafx application embed all swing components in SwingNodes except for MapPanel, which would have to be adjusted accordingly and use this approach.

RoiEXLab commented 5 years ago

Ok, did some testing and I have good news: JFXPanel does indeed skip swing rendering so that it uses the 3D capabilities of the GPU to do some rendering (fortunately windows task manager finally allows users to see how much load the GPU has). There are some limitations to this (it looks like JavaFX really doesn't like the swing event model) so that resizing the surrounding swing window really slows everything down. Once the new size is fixed though, everything works normal again.

There's also a catch to this approach I haven't thought of before: Currently javafx is only a game-headed dependency, so we'd have to create it in game-headed code (or have at least the code there and pass a supplier) and pass the object through the window model.

Also I removed the flag on my machine to get the direct comparison: With OpenGL, the screen update feels near instantly and 60FPS smooth, and without it it seems like the rendering can't really keep up with the fast scrolling, so it skips some frames which makes the movement feel choppy.

But at the same time the non GL version is definitely playable. I mean most of the time is spent in menus buying troops and other stuff, moving troops around, watching battles happen and observing other players movement, but I'm that especially on systems with a slow CPU, people will notice a serious drop in performance.

TL;DR having a switch that might or might not default to a value based on the OS is probably the best short term solution.

I wish I had the time to rewrite the rendering in JavaFX for better performance and quality at the same time, but it looks like this will stay a dream until I have a little bit more free time.

ron-murhammer commented 5 years ago

@RoiEXLab I think we need to remove the opengl flag across the board. Did you want to revisit the original PR that introduces changes related to zooming that implemented it? Or do we just want to remove the flag and see if performance is good enough?

RoiEXLab commented 5 years ago

Well after getting used to it, it works well enough in my opinion, so simply removing the flag from the install4j config should be enough IMO.

Just haven't had the time to create a pr for this

ron-murhammer commented 5 years ago

PR: https://github.com/triplea-game/triplea/pull/5026

DanVanAtta commented 5 years ago

One comment, I noted that we saw this in Ubuntu and I think that biased things a bit to think the problem was OS specific. On one hand I did not want to infer the blast radius of this problem was larger than what I concretely knew, on the other hand assuming the problem is related to the one OS of the person reporting the problem was a mistake. This does not seem to be necessarily OS related, perhaps video card, graphics driver, or even java version related. Without deeply knowing the root cause, removing the flag is right. We do not know which conditions are necessary yet for a game to be able to offer this flag. We can still perhaps present this as an option for users to be able to turn on if the benefit is really large, we'd need to be careful to clearly note that there could be rendering problems. The rendering problems could be severe enough so that the feature could not be turned off again, in which case we'd need to offer a 'safe-mode' to start the application, overall it's pretty painful to support such an optional and potentially unstable (but beneficial) flag.