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

Reason to ignore alphaCutoff in BLEND mode? #1158

Closed
donmccurdy opened this issue Nov 20, 2017 · 7 comments
Closed

Reason to ignore alphaCutoff in BLEND mode? #1158

donmccurdy opened this issue Nov 20, 2017 · 7 comments

Comments

@donmccurdy
Copy link
Contributor

From the spec:

When alphaMode is set to MASK the alphaCutoff property specifies the cutoff threshold. If the alpha value is greater than or equal to the alphaCutoff value then it is rendered as fully opaque, otherwise, it is rendered as fully transparent. alphaCutoff value is ignored for other modes.

I came across this Sketchfab model (mildly NSFW) in a three.js issue recently, which provides alphaCutoff: 0.5 and alphaMode: "BLEND":

{
  "alphaCutoff": 0.5,
  "alphaMode": "BLEND",
  "doubleSided": true,
  "emissiveFactor": [
    0,
    0,
    0
  ],
  "extensions": {
    "KHR_materials_pbrSpecularGlossiness": {
      "diffuseFactor": [
        1,
        1,
        1,
        0.82145579270000002
      ],
      "diffuseTexture": {
        "index": 0,
        "texCoord": 0
      },
      "glossinessFactor": 0.17605827430000001,
      "specularFactor": [
        0,
        0,
        0
      ]
    }
  },
  "name": "Model001_Material009",
  "normalTexture": {
    "index": 1,
    "scale": 1,
    "texCoord": 0
  }
}

result in three.js:

ignore alphaCutoff (spec) use alphaCutoff
no_alphacutoff alphacutoff

The spec is clear enough on this, so I'm just wondering:

(1) what is the reasoning for specifying that alphaCutoff should be ignored?
(2) should we check in with the Sketchfab folks about this?
(3) should this be changed in a future version of the spec?

@AurL
Copy link

AurL commented Nov 20, 2017

@donmccurdy I think the spec has been misread, it makes no sense so it's an issue and I'll take a look at it.

The other point is that the model is actually using dithering transparency, and we currently fallback on alpha blending during export, which is probably wrong (alpha masking is more appropriate for this part).
There will also be issues for models using additive transparency (also fallbacking on alpha blending).

If there is an update or an extension that I missed to handle this, it could be updated, but if not I'm not sure yet how to handle this correctly.

@lexaknyazev
Copy link
Member

Upcoming 2.0.0-dev.1.4 validator release will warn on alphaCutoff used with non-MASK modes.

@emackey
Copy link
Member

emackey commented Nov 20, 2017

Much of the original discussion of this happened in #822.

@donmccurdy
Copy link
Contributor Author

The other point is that the model is actually using dithering transparency, and we currently fallback on alpha blending during export, which is probably wrong (alpha masking is more appropriate for this part).

@AurL yes, in this case mask gives a more reasonable result (below). Not sure whether it would be possible to detect which is appropriate in cases like this.

screen shot 2017-11-20 at 9 26 48 am

@emackey Thanks, that's helpful. What I don't see in that discussion is a reply to @erich666's suggestion of an CutoutBlend (or MASK + BLEND) mode. So with the current spec, we cannot have both transparency and cutout, as this model attempts to do.

Is that something we should consider patching in the future, with a 2.X release, by adding a BLEND_AND_MASK mode? Or better to leave alpha coverage as it is, and focus on physically-based transparency later down the road?

/cc @bghgary

@bghgary
Copy link
Contributor

bghgary commented Nov 27, 2017

BLEND_AND_MASK mode

I am not an expert in this area, so I won't dive into details, but this is complicated. There are multiple ways to combine the two modes together and they are not consistent across implementations. It also depends on usage/performance. The implementation note hits on this a bit:

  • BLEND - Support for this mode varies. There is no perfect and fast solution that works for all cases. Implementations should try to achieve the correct blending output for as many situations as possible. Whether depth value is written or whether to sort is up to the implementation. For example, implementations can discard pixels which have zero or close to zero alpha value to avoid sorting issues.

But you are right that there is no way to specify this explicitly in the spec right now.

Or better to leave alpha coverage as it is, and focus on physically-based transparency later down the road?

I would prefer this. Using the current spec for optical transparency is not ideal.

@stevesan
Copy link

Hi all.
Tilt Brush uses additive blending extensively, so +1 on ADDITIVE as an alphaMode.

@donmccurdy
Copy link
Contributor Author

Closing to merge discussion into #1302.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants