Fix glTF mask

[elalish]

Fixes #15483

This is a change I'd like to upstream from model-viewer; it's a pretty simple fix that doesn't require any new shader permutations (see the first and last files in the diff). The bulk of the change was updating the example to test it. I added AlphaBlendModeTest and enabled the renderer's alpha channel in order to repro the issue. I also tweaked the camera's near and far plane to reduce Z-fighting. I removed Monster while I was there because it has also been removed from the glTF sample models due to many errors in the file.

[elalish]

Ping @WestLangley

[takahirox]

A bit off topic.

I removed Monster while I was there because it has also been removed from the glTF sample models due to many errors in the file.

Monster is the only model which has lights extension in our examples. Maybe we should have lights extension in existing another model (or new model) for test and to show we support lights extension if we remove Monster.

[elalish]

@takahirox Good idea. Also, there have been a number of updates to the glTF sample models; we might want to pull the latest. Not sure if they have another example in there with lights.

[donmccurdy]

I can help with samples if needed, but let's discuss the code change first.

[donmccurdy]

While glTF materials never use both .alphaTest > 0 and .transparent = true, threejs can — i.e. both a cutoff, and semitransparency. I think this would lose semitransparency in that case. Can't remember if there's a define to check for transparency...

[elalish]

Hmm, I guess I thought that was a bug. Why would you simultaneously use alpha to represent a mask and blending? Seems like it should be either/or.

[donmccurdy]

I haven't personally needed to, and I haven't heard complaints from glTF users, but it's not unheard of:

KhronosGroup/glTF#822 (comment)

You have cutout grass, but really do want to have nice antialiased edges, so you then want both: alpha test for whether the fragment should be discarded (and so not affect the z-buffer ... which can lead to rendering errors) and whether the fragments partially covering a pixel should be blended in...

^this has nothing to do with physically-based rendering, and is (IMO) a better use case for alpha hash or temporal dither, but threejs and glTF don't support either of those now.

[elalish]

yeah, plus it won't help much if your cutoff is at 0.5, since half of the blend is discarded anyway. This can be done in a shader, but not how three is set up.

[donmccurdy]

True, but a careful choice of the cutoff is par for the course here.

[WestLangley]

I wonder if you can just link to
https://github.com/KhronosGroup/glTF-Sample-Models/blob/master/2.0/AlphaBlendModeTest/README.md
instead of including the screenshots in the examples... At least the link is easily viewable...

Regarding changing the behavior of alpha-testing in three.js, I'll defer to whatever @donmccurdy decides.

[donmccurdy]

Unfortunately, I think the size of the problem is not enough to justify preventing simultaneous use of .alphaTest and .transparent=true... so we're probably back to needing another define, per #15483 (comment).

[elalish]

@donmccurdy Fair enough, but if you want alphaTest to work with blending, shouldn't we at least put a remapping of alpha in, so that the test value is mapped to alpha = 0? The fact that wasn't there was what made me assume this combination was never used. Having a feathered edge to a mask is nice, but only if it can become fully transparent at the mask edge.

[donmccurdy]

For the case above, I expect we're talking about alphaTest values of 0.01 or so. It's also not uncommon to use alphaTest for something like a dissolve effect, in which case it would be independent of any semitransparency in the pre-dissolved model (although this involves shader or node work beyond what threejs directly provides). It's best to think of alphaTest and blending as orthogonal properties.

[elalish]

@donmccurdy Ah, thanks for the explanation, that makes sense. When I've done feathered edges it was with compressed textures, so 0.01 wasn't really an option. Well, at least this solution works well for model-viewer, so I'll just keep my hack there until we find a fix for three.

[donmccurdy]

@elalish what issue did model-viewer hit? transparent canvas has opacity where it shouldn't?

[elalish]

@donmccurdy When we use a transparent canvas (which we now use all the time), models using MASK or OPAQUE would end up transparent sometimes, as demonstrated here: google/model-viewer#381

[WestLangley]

@elalish @donmccurdy Can you please explain why you set alpha: true and the div background color to white in the glTF_extensions example?

(red may be better -- also setting the renderer clear color explicitly)

If this is test of some sort, perhaps an inline comment would be beneficial. I would think this would be confusing to users.

[donmccurdy]

I assume it allows us to test this change. I would prefer to revert that change to the example once we're happy with the solution.

[elalish]

@WestLangley @donmccurdy Yes, it was just to demonstrate this existing bug with AlphaModeTest and the solution. It sounds like this PR will be closed anyway, so no harm. I believe the clear color is set via the background property already, but that has no effect on the bug, as the alpha of a non-transparent object punches through everything, right back to the div's color.

[WestLangley]

It sounds like this PR will be closed anyway

Oh.

[mrdoob]

I'll merge this and do these changes so we have time to test.

[mrdoob]

Thanks!

[takahirox]

I opened #18629 for #18579 (comment) as a reminder

[mrdoob]

#18631 completes this PR

[donmccurdy]

Thanks @elalish!