ZDoom

applySSAO = mCurrentPortal->AllowSSAO();

 bool HWSkyboxPortal::AllowSSAO() { return !!(Level->flags3 & LEVEL3_SKYBOXAO); } // [MK] sector skyboxes don't allow SSAO by default

Ah heck. Way too late to comment, I know.

But I think it would be better from encapsulation perspective if the line in hw_drawinfo.cpp would be
[code]applySSAO = mCurrentPortal->AllowSSAO();[/code]
while the line in hw_portal.cpp be
[code] bool HWSkyboxPortal::AllowSSAO() { return !!(Level->flags3 & LEVEL3_SKYBOXAO); } // [MK] sector skyboxes don't allow SSAO by default[/code]

Right now you made the global "all portals" code check for "AllowSSAO()" from any portal subclass, but if it says no, you decide to ignore that if the skybox flag is on.

It would "feel more right" if the skybox subclass specifically returned true/false in response to the flag, and the main render code let each subclass decide for itself.
Would be future-proof for hypothetical other portal subclass which might want to control SSAO in response to a different flag or whatever other reason.

That said I'm not sure if Level is accessible in the latter file (or can be made accessible without problems)?

Oh, so in the end it was inverted. Welp.

If you need this feature ASAP, grab [b]Visual Studio 64-bit Release[/b] build artifact from [url=https://github.com/coelckers/gzdoom/actions/runs/452979667]this page[/url]. You should copy all .dll files from existing build in order to run it.

[imgur]https://imgur.com/9LULgdq[/imgur]

[quote="Tormentor667"]Anything wrong with it or was I just too quick and it hasn't been added to the svn devbuild yet?[/quote]

From Github: "madame-rachelle - Modified commit from @OrdinaryMagician #1261 df4f41f [b]3 hours ago[/b] Git stats"

[img]https://i.imgur.com/X7XRlZR.jpg[/img]

;)

map C1M1 lookup "C1M1"	//Tormentor667
{
	levelnum = 1
	music = "NOMUSIC"
	cluster = 1

	// Force SSAO being disabled in skyboxes
	disableskyboxao
}

[b]Update[/b]
Added it and tried it with today's dev-build of GZDoom, didn't work unfortunately:
[code]map C1M1 lookup "C1M1" //Tormentor667
{
levelnum = 1
music = "NOMUSIC"
cluster = 1

// Force SSAO being disabled in skyboxes
disableskyboxao
}[/code]
Anything wrong with it or was I just too quick and it hasn't been added to the svn devbuild yet?

[quote="Blue Shadow"][quote="Tormentor667"]So I wonder, what do I have to add to the MAPINFO entries to activate that feature?[/quote]
Add [c]disableskyboxao[/c] to the maps for which you want it disabled.[/quote]
Thanks kindly, I will just do that - and thanks to anyone involved in this new feature!

Honestly, I don't know. I haven't messed seriously with PBR materials. They're a bit of a dark art to me so far. If the AO thing is in, it would be worth me investigating though.

[quote="Enjay"]Now, if there was also a way to disable SSAO on a texture by texture/surface by surface basis, I really would be happy. That would mean that little light alcoves etc don't become dimmed to almost looking like they are not bright at all despite having LITE5 on each surface and being set to 255 brightness. But I realise that's a whole different problem. :P[/quote]
Doesn't PBR include an AO map that allows doing just this, specifying which parts of the texture can receive AO and which parts shouldn't? Or does GZDoom not implement that part of the pipeline?

[quote="Tormentor667"]So I wonder, what do I have to add to the MAPINFO entries to activate that feature?[/quote]
Add [c]disableskyboxao[/c] to the maps for which you want it disabled.

[quote="Nash"]Why not make it the other way around? Opt in. Surely Torm can spare the 2 minutes to edit BoA's MAPINFO to disable SSAO in BoA's skyboxes... ;)[/quote]
Exactly. I think we can keep the default behaviour as it was (SSAO in skyboxes activated), and I can simply add appropriate lines to the MAPINFO that disables SSAO in the BoA skyboxes, that's just fine and a really good reason compared to the crazy stuff that I must have done without that.

So I wonder, what do I have to add to the MAPINFO entries to activate that feature?

[quote="Nash"]I thought generally the way it goes with GZDoom feature enhancements is always "make sure old stuff always look the same way it did, newer stuff has to be opt in by modders themselves".[/quote]
This, however, is a weird one. If the map was made pre-SSAO then it might have a skybox that doesn't look good with SSAO. So, that would normally dictate that the default should be SSAO = off in skyboxes. However, SSAO has been around long enough that people have made skyboxes expecting it.

So either option is going to "break" something that already exists. ;)

Honestly, even though I personally would prefer SSAO to be on in skyboxes by default, I actually think that it would be better to default to off. Why? Two reasons:

1) The older maps that were not designed with SSAO in mind are probably more numerous.

2) Ultimately, SSAO being off is less "harmful" than being on. A player [i]might[/i] notice something and think "hang on, those buildings/rocks/cliffs in the background look a bit different to the ones that I can walk up to" if the player has SSAO enabled but it isn't present in the skybox. However, everyone is going to notice the huge black lines across the sky that happen when a skybox is set up like the ones in BoA. (Although, it's worth pointing out that the first issue in this reason will potentially occur in BoA if the skyboxes do not have SSAO because the rest of the architecture in the box will also not have SSAO.)


Now, if there was also a way to disable SSAO on a texture by texture/surface by surface basis, I really would be happy. That would mean that little light alcoves etc don't become dimmed to almost looking like they are not bright at all despite having LITE5 on each surface and being set to 255 brightness. But I realise that's a whole different problem. :P

[quote="Graf Zahl"]I don't care about AO so you'll have to make up your minds without me. :P[/quote]
Then to spare any arguments I'll do an arbitration.

I'm going to take on the PR myself, and finish it, it's going to be default allowing SSAO[s], but, it needs an option both to enable it and disable it per-map[/s] (oops, this was there already, my mistake). The defaultmap or gamedefaults block can be used to set it for multiple maps at once.

I don't care about AO so you'll have to make up your minds without me. :P

I thought generally the way it goes with GZDoom feature enhancements is always "make sure old stuff always look the same way it did, newer stuff has to be opt in by modders themselves".