Merge passes in Vulkan mobile renderer

[BastiaanOlij]

After various discussions with @darksylinc, did the work to merge passes that didn't need to be separate subpasses in the mobile renderer. This greatly simplifies things.

There are now 3 scenarios:

1. The most optimal scenario is where Opaque, sky and transparent all run in a single subpass, and tonemapping runs in a second subpass. In this approach our 3D buffer is never written out to texture memory and the only output is our render target
2. The slightly less optimal scenario is where Opaque, sky and transparent all run in a single pass. Tonemapping runs in a separate pass. This will happen if: MSAA, glow or DOF are enabled.
3. The least optimal scenario is where Opaque and sky run in a single pass, transparent runs in a separate pass and finally tonemapping runs in a separate pass. This will happen if reading from screen texture or depth texture is enabled.

This PR also has an optimalisation that when the background is copied from canvas, this no longer happens in a separate pass, but becomes the first step in the opaque pass.

Fixes #82175
Fixes #81910
Fixes #83481

[BastiaanOlij]

I'll be adding some before/after timing info as soon as I have recharged some phones :)

I suspect this will have no impact on desktop GPUs, but in theory there should be a measurable difference on phones and standalone VR hardware.

[BastiaanOlij]

Testing scene on Samsung S21 MALI

Master
MSAA 3D	off	off	on	on
Glow	off	on	off	on
GPU time	9 - 14ms	19-23ms	11-13 ms	19-23 ms

This PR
MSAA 3D	off	off	on	on
Glow	off	on	off	on
GPU time	9 - 12ms	18-22ms	10-12 ms	19-24 ms

I'm not surprised by these results, according to ARM MALI implements subpasses well.

Test project used (could use something better):
TestMSAA.zip

[BastiaanOlij]

Testing scene on Quest Pro

Master
MSAA 3D	off	off	on	on
Glow	off	on	off	on
GPU time	9 - 10ms	15-16ms	19-20 ms	27-29 ms

This PR
MSAA 3D	off	off	on	on
Glow	off	on	off	on
GPU time	9 - 10ms	15-16ms	16-17 ms	20-22 ms

Totally not what I was expecting...

Test project used:
https://www.dropbox.com/scl/fi/pcm5xo42zi4xt0ksf62cx/TestXR.zip?rlkey=cpogyt2w70frgkadvg0ofottp&dl=0
(requires installation of appropriate android templates but otherwise configured for Quest)

[BastiaanOlij]

I'm wondering if #82175 was really a bug, so might try and "unfix" that and see if that actually works. In theory we should be able to render MSAA in a pass, resolve it, and then do tonemapping in a subpass, which should result in no data being written our to MSAA or 3D buffers, only to our render target.

[BastiaanOlij]

After doing some more testing with this, found out the conclusion in 82175 was indeed wrong, the problems were due to the transparent subpass getting skipped on Adreno hardware and MSAA thus not getting resolved, not because MSAA doesn't work with the subpass approach.

Merging the passes already solves the Adreno issue so I've undone the exception and we now use the 2-subpass approach with MSAA enabled.

@darksylinc this could use a review from you. I know that there is more to MSAA especially on Forward+, but for Mobile all scenarios are now working and I think this would be a good thing to merge ASAP once our merge window for 4.3 is open (really wish this had ended up in 4.2 as it solves a number of issues people are currently running into in VR).
@clayjohn could use your stamp of approval too :)

[clayjohn]

Testing a modified version of the TPS demo with this PR. I am seeing a slight GPU time regression (~16mspd -> ~18mspf) using intel integrated graphics.

I'll investigate a bit more as it is not obvious where that regression would come from, especially on desktop hardware

[BastiaanOlij]

Testing a modified version of the TPS demo with this PR. I am seeing a slight GPU time regression (~16mspd -> ~18mspf) using intel integrated graphics.

I'll investigate a bit more as it is not obvious where that regression would come from, especially on desktop hardware

Indeed, weird, seeing it should do less, not more ;)

[clayjohn]

Tested again from clean builds and I can't reproduce the regression anymore.

The code looks good to me and feels a lot cleaner, so this is a nice win

[Alex2782]

Fixes #83481

tested only my 'minimal reproduction project' (comment)

[YuriSizov]

Needs a rebase, and then can be merged!

[warent]

Any chance of merging this one in soon before conflicts are introduced?

[akien-mga]

@warent There are conflicts already, as pointed out 5 days ago :) So it can't be merged.

But be assured that we keep track of approved and ready to merge rendering PRs, there are just many of them with interdependencies which we need to merge in a specific order. Clay is managing that.

[BastiaanOlij]

Sorry, missed the last bit of feedback, its been a bit of a weird week. Should all be ok now.

[clayjohn]

Great! Should be good to merge now

[YuriSizov]

Thanks!

[RomanMatl]

Are there some updates on this issue? I am still able to reproduce it with v4.3.stable.official [77dcf97] :(

[clayjohn]

Are there some updates on this issue? I am still able to reproduce it with v4.3.stable.official [77dcf97] :(

Did you mean to post on this PR? This isn't a bug report, this is a PR that was merged a year ago. There won't be any updates since it was already merged

[RomanMatl]

Ah, sorry wrong tab 😅. Wanted to post here #86098