Bug 232071 – Hardware PCF filtering does not work on iOS 15 when using WebGL2

• Status: RESOLVED
• Resolution: FIXED
• Component: WebGL

[Martin]

See https://github.com/playcanvas/engine/issues/3590#issuecomment-945994262

Hardware PCF filtering (compare sampling of depth shadow) does not seem to work on iOS 15.
It works on all other devices and platforms, including Safari 15 on MacOS.

Steps to repro:
http://playcanvas.github.io/#/graphics/shadow-cascades
in Controls, set Count to 1, resolution to around 512, filtering PCF3 (it's easier to click on sliders than slide them)
and notice how pixelated the shadows are on iOS 15 platform.

[Radar WebKit Bug Importer]

<rdar://problem/84524233>

[Kenneth Russell]

Only Apple engineers will be able to triage this effectively but is there any way you can provide a smaller test case?

Although - I think depth comparisons in the direct-to-Metal translator were a known missing feature and might have been implemented upstream. Kyle, do you remember? Searching anglebug.com I couldn't find recent CLs.

[Kyle Piddington]

(In reply to Kenneth Russell from comment #2)
> Only Apple engineers will be able to triage this effectively but is there
> any way you can provide a smaller test case?
> 
> Although - I think depth comparisons in the direct-to-Metal translator were
> a known missing feature and might have been implemented upstream. Kyle, do
> you remember? Searching anglebug.com I couldn't find recent CLs.

I believe the CL you were thinking of was here: https://chromium-review.googlesource.com/c/angle/angle/+/3232818, which just added some workarounds for platforms that didn't correctly implement sample_compare for various options.

Martin, which iOS device are you seeing this regression on? Could you attach a screenshot so I could compare on my own devices?

[Kyle Piddington]

Answered my own question by looking at the forums :)

Perhaps you can answer me a second question then: What pixel format are you using to back the depth buffer? 

on iOS, we have several formats that cannot be filtered on iOS that can on macOS. Are you perhaps using Depth32Float or D32S8 to back your depth buffer? Does changing that to Depth16 improve the issue?


https://developer.apple.com/metal/Metal-Feature-Set-Tables.pdf

[Martin]

Yes, it uses Depth32Float format. I'll test out the 16bit format next week when back from holidays. Is there some way to test for the filtering support of a format?

Thanks!

[Martin]

I've tested this on iPhone XR, OS 15.0.

Originally we used 32bit float depth texture:
    glFormat = gl.DEPTH_COMPONENT;
    glInternalFormat = gl.DEPTH_COMPONENT32F;
    glPixelType = gl.FLOAT;

I now tried 16 bit:
    glFormat = gl.DEPTH_COMPONENT;
    glInternalFormat = gl.DEPTH_COMPONENT16;
    glPixelType = gl.UNSIGNED_INT;

And also 24bit:
    glFormat = gl.DEPTH_COMPONENT;
    glInternalFormat = gl.DEPTH_COMPONENT24;
    glPixelType = gl.UNSIGNED_INT;

They all work as expected on MacOS, but I still get no filtering on iPhone XR.
I also tried Google Pixel 3A and all 3 versions work fine there.

[Kyle Piddington]

Thanks for having a look.

I've found a related issue from the SPIRV-Cross team:
https://github.com/KhronosGroup/SPIRV-Cross/issues/533

It seems by inserting a constexpr sampler into our shadow sampler function, we're able to return PCF functionality to this shader. Right now, I'm double checking if this is a case of state initialization gone wrong, or if we've missed some restriction.

[Kyle Piddington]

Created attachment 443049 [details]
Patch

[EWS Watchlist]

Note that there are important steps to take when updating ANGLE. See https://trac.webkit.org/wiki/UpdatingANGLE

[Kimmo Kinnunen]

Comment on attachment 443049 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=443049&action=review

Some comments that might come up later in the upstream.
I'm not saying if it's 100% required to fix the comments here to land this here now.

> Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/DisplayMtl.mm:-941
> -

Would it be more in line with the project code structure to rename this to supports32BitFloatFiltering and then have the .json query this property?
then move maybe also the conditions `display->supportsMacGPUFamily(1) ||` somehow into this FeatureMtl trigger?

> Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/DisplayMtl.mm:1086
> +    return [mMetalDevice supports32BitFloatFiltering];

I think your compile failure is the case where this one is a "property" name, where as below is a "getter method name".

it looks a bit odd to have one "supports" and the other "is...Supported".

> Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/DisplayMtl.mm:1090
> +bool DisplayMtl::supportsDepth24Stencil8PixelFormat() const

Would it be more in line with the project structure to add this as a FeaturesMtl, too?

Note: allowSeparatedDepthStencilBuffers is already a related feature query, so this one should be maybe added in such a way that it's clear that both are internally consistent.

FWIW:
If I understand correctly, the current DEPTH24_STENCIL8 emulation implementation is triggered based on `allowSeparatedDepthStencilBuffers`. 
It kind of makes sense today, but it appears funny after supportsDepth24Stencil8PixelFormat is introduced as this would be the more appropriate flag to check in many places.

> Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/TextureMtl.mm:799
> +        !mFormat.getCaps().filterable)
>      {

now you can use `webkit-patch format -g HEAD` to get this formatted according to ANGLE rules.

[Kyle Piddington]

Created attachment 443747 [details]
Patch

[Kyle Piddington]

Created attachment 443754 [details]
Patch

[Kenneth Russell]

Surfacing a conversation we've had on Slack on this topic:

Depth textures in OpenGL ES 3.0 are actually specified as not being filterable; i.e. using GL_LINEAR filtering for the min/mag filter is supposed to make the texture non-renderable. See:

https://github.com/KhronosGroup/WebGL/issues/3353

Martin, is it specifically linear filtering of depth textures that your algorithm relies on? Or is it something else about how depth comparisons are being performed that might be different?

[Kenneth Russell]

More feedback based on chats. It seems likely that linear filtering is being allowed because the TEXTURE_COMPARE_MODE is set to COMPARE_REF_TO_TEXTURE. The depth texture would be considered incomplete specifically if TEXTURE_COMPARE_MODE were set to NONE.

https://github.com/KhronosGroup/WebGL/issues/3359

If the proposed patch is applied, what happens to Gregg's test from:
https://github.com/KhronosGroup/WebGL/issues/3353

specifically,
https://jsgist.org/?src=493218139726a53b7b3affbb636148e9

and the d16 linear case?

If that continues to be rejected as non-renderable - then I think this patch is OK. ANGLE must then be enforcing the renderability / filterability requirements at higher levels than the Metal backend.

[Martin]

Yes, the PCF depends on LINEAR filtering of the depth map. But as you said, this is only supported when COMPARE_REF_TO_TEXTURE is set - in which case the hardware should do 4 samples, compare them to passed in depth value, and return the interpolated result.

At the moment on iOS15 it seems it only does single sample, or does not interpolate 4 samples.

Other WebGL2 platforms give us this result (source is a very low res shadow map to see the points clearly)
https://user-images.githubusercontent.com/59932779/137777777-6ffa5fdb-5b43-4d27-9b45-6361be4871e3.png

but iOS15 gives us this: 
https://user-images.githubusercontent.com/59932779/137777731-62d53fe4-c274-43ea-b0b6-6d6c8cb5a462.png

These are the settings we use
https://user-images.githubusercontent.com/59932779/137777856-2ffae554-815e-40d0-baaf-21c252984b92.png

[Martin]

See the page 28 here: https://www.slideserve.com/myrrh/shadow-mapping-powerpoint-ppt-presentation

it seems like iOS15 is doing GL_NEAREST instead of GL_LINEAR.

[Kyle Piddington]

Using Gregg's test here: https://jsgist.org/?src=493218139726a53b7b3affbb636148e9
D16 and D32 Linear sampled filtering still correctly does not render, though sample_compare now correctly filters. I think this patch brings us into spec alignment.

[Kenneth Russell]

Comment on attachment 443754 [details]
Patch

Thanks for confirming that Gregg's sample renders correctly on iOS with this change. r+

[EWS]

Committed r286323 (244682@main): <https://commits.webkit.org/244682@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 443754 [details].

[Brent Fulgham]

This change should be present in STP 139, iOS 15.4 Beta, and macOS 12.3 Beta.

[Martin]

I'd like to report, that the original repro case I made (see here: https://github.com/playcanvas/engine/issues/3590#issuecomment-945994262 ) still does not render correctly on iPhone XR 16.1.1, tested in Safari.