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

Rework infoLOS shader. Use a maximum intel based approach instead of additive. #1931

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

DevJac
Copy link
Contributor

@DevJac DevJac commented Jul 5, 2023

Overview

This PR simplifies the infoLOS shader and renders the fog of war based upon the maximum level of intel instead of the purely addition based approach of the old shader (I'll explain why this additive approach was a problem below.)

Also, the old shader was doing some really weird things. I believe the first evidence of this is how much simpler this shader is than the old one while still achieving very similar results. Another small example is the line mix(diagLines2(texCoord), diagLines1(texCoord), heightStatus) from the old shader, in which heightStatus could be -3 (negative three); that's quite unusual. The old shader also had unreachable code which I removed.

My goal in doing this was to make the areas that are not covered by radar more visible. To achieve this, I increased the width of the so-called "terra-incognita" lines. I also (ab)used the radarColor1 to allow the darkness of the terra-incognita lines to be adjusted outside of the shader. I will personally be adjusting this in a widget, but tried to leave the default colors and shader subtle like the existing one.

Why a maximum intel approach is better than an additive approach.

A problem with the old shader is that every form of intel adds to the brightness. You want radar to add some brightness to the map to indicate coverage, and, of course, you also want direct line-of-sight to add brightness. The problem then is that radar coverage makes the areas with direct LOS even brighter. This is goofy.

To illustrate the problem, imaging you have several grunts spread loosely around the screen. They have direct LOS over their area and so that area is fairly bright; there is no fog of war around your grunts. Now imagine someone builds a T3 radar half-way across the map, and now the area around your grunts becomes even brighter. That's weird. A radar half-way across the map adds no intel to the area with your grunts, why should it become brighter?

This is how the additive approach must be. If the radar contributes significant brightness, and direct LOS contributes significant brightness, then together they are too bright.

This shader uses maximum instead of addition. So someone building a T3 radar across the map will brighten the appropriate dark areas, but it will not brighten the areas where you already have direct LOS. This way both radar and direct LOS can contribute a significant amount of brightness without adding to become too bright.

Screenshots

Here's a screenshot comparing the old and new shader. The first screenshot shows the old shader.

It's worth noting that the first screenshot demonstrates a bug with the old shader. If you look closely at the water covered by radar, you can see that there are still subtle terra-incognita lines that were not fully removed. I think this is because the old shader was returning values greater than 1.0 for gl_FragColor. This bug does not occur with the new proposed shader.

screen00003
screen00007

@DevJac
Copy link
Contributor Author

DevJac commented Jul 5, 2023

This probably fixes #1817

@Beherith
Copy link
Collaborator

Beherith commented Jul 6, 2023

Have you perhaps seen my reimagings in df06d63 ?

@Beherith
Copy link
Collaborator

Beherith commented Jul 6, 2023

@icexuick have a looksie here

@DevJac
Copy link
Contributor Author

DevJac commented Jul 6, 2023

I see that the old shader code is duplicated in df06d63 ; I don't understand why.

@DevJac
Copy link
Contributor Author

DevJac commented Feb 14, 2024

It's been many months since I made this PR. I play in a lot of noob lobbies and I get the impression that people don't understand areas that are and aren't covered by radar and I think this would help. What is holding this up?

@sprunk
Copy link
Collaborator

sprunk commented Feb 15, 2024

One of your major premises is false, radar is not obsoleted by sight because it can see cloak where sight cannot.

@DevJac
Copy link
Contributor Author

DevJac commented Feb 15, 2024

@sprunk technically true. As you correctly pointed out, LOS without radar coverage and LOS with radar coverage are different in some cases, but notably, the game currently doesn't distinguish the two visually; it's never been a problem before, so I hope it wont be considered a problem now. Also, there is a limited amount of contrast available, and using some of that contrast on the edge case of cloaked units isn't worth it IMO.

@DevJac
Copy link
Contributor Author

DevJac commented Feb 15, 2024

Here's some additional screenshots of the difference. These are heavily compressed, so don't judge too closely, just look at the overall effect. Also, the difference pictured here is more extreme than what I've done in this PR.

Screenshot from 2024-02-15 13-33-22
Screenshot from 2024-02-15 13-34-09

@icexuick
Copy link
Collaborator

My only 2 'things' to talk about are:

  1. Cloaked units: With the old shader you could typically see this, though the effect is very subtle (radar+LOS vs LOS only) and i'd be surprised how many could tell the difference at a glance.
  2. Subtle immersion - where venturing out into new areas, without proper radar coverage, will have a (subtle) effect because it's less bright compared to fully radar-covered.

This PR does do what it says, it simplifies the LOS layers and also cleans up some strange/goofy effects.
I think in general this PR is better.

  • Better/more consistent contrast between in-view or no-view
  • Removes noise/overlap between radar | radar&LOS | LOS only
  • Should improve consistency behavior/looks on both bright & dark maps

Things I'd like to see improved:

  • Diagonal lines maybe too big on zoom in - perhaps look/try a system with 'fixed' diagonal lines (regardless of zoom)
  • Diagonal lines seem to show inside of radar range (should be a clear 'line' that aligns perfectly with the radar-range circle.

image

  • Ideally I'd like to add sliders to settings so people can tweak the presence/darkness of both radar and no-radar (diag lines) visibility
  • Current setup in this PR is too dark for the map Forge

@WatchTheFort
Copy link
Member

* Ideally I'd like to add sliders to settings so people can tweak the presence/darkness of both radar and no-radar (diag lines) visibility

This is something we need to take care of ourselves, it is not the player's responsibility to adjust the shaders for each map. If we need to create a per-map config, then that's what we'll need to do.

I have never been a fan of the diagonal lines, but that is outside the scope of this PR.

@DevJac
Copy link
Contributor Author

DevJac commented Feb 25, 2024

I'll reply to @icexuick's comment in the same order he made his points:

Cloaked units: With the old shader you could typically see this, though the effect is very subtle (radar+LOS vs LOS only) and i'd be surprised how many could tell the difference at a glance.

Regarding cloaked units. I've maximized the fog of war opacity, but I cannot see any indication of radar coverage inside LOS. For example, here's some ticks on Forge, without radar (there isn't a single radar anywhere on the map). I think this might be a case where you have edited your widgets to achieve an effect that is not normally possible in the game? See the screens below, these are from the default game.

Screenshot from 2024-02-25 15-08-10

Screencast.from.2024-02-25.15-09-32.webm

As I mentioned above, the game doesn't normally show radar coverage within LOS currently, and as far as I'm aware nobody has complained about it. We need to decide whether that's a concern that will be addressed in this PR, and if so, I don't know if I'll be able to fix it.

Diagonal lines maybe too big on zoom in - perhaps look/try a system with 'fixed' diagonal lines (regardless of zoom)

This also is beyond my current knowledge to solve. The LOS shader adjusts colors based only on map coordinates. It does not know the position of the players camera, it does not know the players zoom level, etc. Also, the LOS shader doesn't know the color of the map.

We can easily adjust the width of the diagonal lines, but I don't know how to to make them dependent on player zoom level or map color.

Diagonal lines seem to show inside of radar range (should be a clear 'line' that aligns perfectly with the radar-range circle.

This is true but was was not caused by my change. I remember talking with @Beherith about this on Discord, but haven't been able to find the conversation. What I remember is that the texture containing the LOS information is lower resolution that the actual map (for performance reasons), and a lot of blurring happens when applying that lower resolution texture to the map. This is why the edges are not sharp, and I don't know how to make them sharp. This has always been a problem but the fog of war effects have been so subtle nobody noticed. Is this something we need to fix in this PR? See this screencast from the default game (without my changes):

Screencast.from.2024-02-25.15-37-11.webm

Ideally I'd like to add sliders to settings so people can tweak the presence/darkness of both radar and no-radar (diag lines) visibility

Agreed, currently there are no adjustable settings, and this change breaks the opacity slider because I couldn't get the opacity calculation in lua to work well with the shader. See relevant code comment: https://github.com/beyond-all-reason/Beyond-All-Reason/pull/1931/files#diff-12d6cb52e62d8431817023ddaa609f3de5f8b416917474f4d4ea5f7803f71ca5L61

Current setup in this PR is too dark for the map Forge

As mentioned, the shader does not have access to the color of the map. If it did, we could adjust the shading to handle brighter / darker maps, but we cannot. I don't know how to fix this. Is this something that needs to be fixed in this PR?

Notice my screenshot of Forge from above, it comes from the default game. The brightness seems about the same to me.

@DevJac
Copy link
Contributor Author

DevJac commented Feb 25, 2024

Here's some screenshots from Forge. The brightness is not that different IMO.

With this PR:

Screenshot from 2024-02-25 15-56-07
Screenshot from 2024-02-25 15-56-27

Without this PR (this is the default game, with 100% fog of war opacity):

Screenshot from 2024-02-25 15-59-07
Screenshot from 2024-02-25 16-00-17

This last screenshot shows bug #1817

Note, the custom shader I put on Discord is not the same as this PR. The custom shader on Discord makes more extreme changes, and this PR makes more subtle changes. So be careful if you are judging this PR based on the Discord download.

@icexuick
Copy link
Collaborator

Not sure why my Forge game shots are much darker.
Are you on Nvidia or AMD?

Regarding your other feedback @DevJac:

  • Regarding the sharp edges: I don't need exact sharp edges, I also know of the lower-res maps. But what triggers me is that it's such far off. Maybe you can dig in and find something to improve this a bit. Ivand might be able to help here.
  • Other remarks are good/fair - I don't see any major deal breakers - though I want to tweak it a bit and test/check bright/dark maps more.

@DevJac
Copy link
Contributor Author

DevJac commented Feb 26, 2024

I'm using AMD. Were you using this PR or were you using the custom shader I posted on Discord? They are different.

Something is weird with the green radar ring from the commander. The fog of war is a more accurate indicator of radar range than the green ring.

Take a look at these screenshots. The first has /globallos, and I've marked the exact position of the Grunts (:strong:). The second has fog of war enabled; the exact positions are still marked and only those standing within the LOS radar range are visible.

Screenshot from 2024-02-26 11-40-54
Screenshot from 2024-02-26 11-41-06

The green ring is wrong. It might be an air LOS indicator or something?

@DevJac
Copy link
Contributor Author

DevJac commented Feb 27, 2024

Scratch that, I don't know what I was thinking. I'm using an Nvidia GPU on Linux.

@Ruwetuin Ruwetuin requested a review from Beherith May 6, 2024 11:39
@icexuick
Copy link
Collaborator

Was jamming-range also like this in original?

image

@icexuick
Copy link
Collaborator

First test on this map I think that difference between LOS and Radar is (too) small.

image

@icexuick
Copy link
Collaborator

I can confirm that the radar on the dotted line is indeed too large, and the LOS shader seems to be on point.

@WatchTheFort
Copy link
Member

I think the big problem we have with differentiating areas from one another is that we use a gradient where they meet, instead of a hard border. That problem isn't specific to this pull request, but I think solving that will go a long way to solving this issue, no matter how we style the different areas.

@DevJac
Copy link
Contributor Author

DevJac commented Jun 23, 2024

@WatchTheFort: I believe the radar coverage (and jammer, etc) is stored in a texture that is lower resolution than the map. That is why the boarder is fuzzy, because we don't have the information needed to draw a perfectly sharp boarder. There would be a performance cost if we store that data in a higher resolution texture.

@DevJac
Copy link
Contributor Author

DevJac commented Jun 23, 2024

@icexuick What map is that? I want to compare the radar / los contrast with the base game. As before, I suspect we will find that the base game already has a difficult to see transition between radar / los, but it will be good to check.

Actually, did we ever resolve this? I showed screenshots (from my computer) that showed little difference between this shader and the default game, but it sounds like you were seeing something else? Are we seeing the same things?

I'm open to talking on Discord and sharing screens or whatever.

@icexuick
Copy link
Collaborator

@WatchTheFort: I believe the radar coverage (and jammer, etc) is stored in a texture that is lower resolution than the map. That is why the boarder is fuzzy, because we don't have the information needed to draw a perfectly sharp boarder. There would be a performance cost if we store that data in a higher resolution texture.

Well i think hardlines will not look that 'pretty'.
Example:
image

@DevJac
Copy link
Contributor Author

DevJac commented Jun 23, 2024

I don't think the hard lines look that bad, but I don't know how to change them. Maybe half way between the current smoothing and what we currently have?

@icexuick
Copy link
Collaborator

Yes, I might be wise to jump in a VC / screenshare to discuss some of the topics.
Indeed it seemed pretty similar, though I was checking the (older) screenshots in this thread, and seeing similar results in my game.

Is there an option to 'sharpen' the edge with your new shader?

@icexuick
Copy link
Collaborator

PS. please make a thread on Discord so we can channel all feedback there.

@DevJac
Copy link
Contributor Author

DevJac commented Jun 23, 2024

Pinged you on Discord.

But I want to reply to your feedback about that map and the low los / radar contrast.

I'm calling the 3 intel levels "los" / "radar" / "fow". "fow" meaning no radar coverage. (There are also things like los with radar and los without radar, but we've talked about that previously in this PR. There's only so many information channels available.)

That's an unavoidable trade-off here. We want los to remain roughly the same brightness, and fow to remain roughly the same brightness, and radar to be somewhere in between.

Currently radar has the same brightness as fow. That's my complaint and the reason I made this shader, there is no contrast between radar and fow areas.

To fix this I moved the radar brightness to be more in the middle between the two extremes. The brightness follows a roughly linear progression from los -> radar -> fow.

But, moving the radar brightness adds to one contrast while taking away from the other. In this case, the contrast between los and radar is reduced.

Let me illustrate it with some ascii art:

From most to least bright:

Current situation:  | los ------------------------------------------------ radar - fow |
In this shader:        | los ----------------------- radar -------------------------- fow |

Yes, the contrast (color distance) between los and radar has decreased, but the contrast between radar and fow increased.

I think I've beat this point to death now.

Of the two, I prefer having the radar contrast closer to los, because los is usually quite predictable, and there are also los lines that can be turned on in the settings (I have them on).

(I've said similar on Discord if you want to discuss there: https://discord.com/channels/549281623154229250/1132003042007449681/1254549192961953903 I wanted to keep major points of discussion here in GitHub as well.)

@WatchTheFort
Copy link
Member

The fact that there are LOS rings at all is completely bonkers. There is zero reason they should ever exist.

@sprunk
Copy link
Collaborator

sprunk commented Jun 24, 2024

Units can be in a position where their LoS range is not evident regardless of how you improve LoS coverage visibility on the map:

  • a unit can be in your base where everything is covered by LoS anyway so you don't see what the unit is contributing.
  • a unit can be in a ditch where terrain blocks its LoS from all sides so you can't see how far it could reach if you get out.
  • a unit could be under a status effect (your modding support is currently fake but you claim eventually it won't be) that reduces LoS and you might want to know its real LoS.
  • you can learn still learn the value from a number in a stats spreadsheet in each case but that's lame.

@WatchTheFort
Copy link
Member

@icexuick What is the status of this? Is more work on it needed? Are we planning to merge it to the game?

@icexuick
Copy link
Collaborator

@icexuick What is the status of this? Is more work on it needed? Are we planning to merge it to the game?

Thats a good question - this has slipped through my fingers.

@icexuick
Copy link
Collaborator

icexuick commented Sep 20, 2024

I tried running this, but I think it's not working anymore.
I would have to ask @DevJac to check and/or maybe update with latest game-code I presume?
I'd like to test it in the latest game-version and tweak the shader stuff a bit + test on bright + dark maps.

When I hit ; to disable/change the with/without radar i looks roughly like in the screenshots.
What I still dislike is the strong diagonals - especially when fighting in no-radar zones, it's heavy on the eyes.
But this is tweakable.

I do like no radar + no vision is more clear.

image

@WatchTheFort
Copy link
Member

Can we please get rid of L and ; functionality? It's our job to ensure the game is visible no matter which map and infoLOS state, not the players.

@DevJac
Copy link
Contributor Author

DevJac commented Sep 20, 2024

It's been a long time since I played BAR, and has been over a year since I looked at the code in this PR. I'm not interesting in working on this or contributing to BAR at the moment. If you want to close this PR, I understand.

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.

5 participants