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

ShaderChunks: Ignore alpha in output_fragment when transparent = false #22414

Closed
wants to merge 2 commits into from

Conversation

mrdoob
Copy link
Owner

@mrdoob mrdoob commented Aug 25, 2021

Related issue: #15483 #18631 #21744 #22196

Description

When rendering with alpha: true opaque materials with textures with alpha channel were "carving out" pixels from the output render.

Before After
Screen Shot 2021-08-25 at 1 57 31 AM Screen Shot 2021-08-25 at 1 57 01 AM

/cc @elalish

@mrdoob mrdoob added this to the r132 milestone Aug 25, 2021
@mrdoob
Copy link
Owner Author

mrdoob commented Aug 25, 2021

Crazy... Totally forgot I tried doing this last year... #18631 (What a year...)

The implementation is very similar and I suspect this comment by @WestLangley is relevant here too:

Confirmed. This breaks AdditiveBlending when texture.a < 1 and material.transparent is false.

And this other comment:

I don't think this can be retained. I think you will have to implement something different -- something specific to the glTF alphaMode setting.

So maybe the solution is to introduce OpaqueBlending?

/cc @donmccurdy

@elalish
Copy link
Contributor

elalish commented Aug 25, 2021

It'll be nice to remove my hack for fixing this with glTF.

@donmccurdy
Copy link
Collaborator

donmccurdy commented Aug 25, 2021

So maybe the solution is to introduce OpaqueBlending?

I'm not sure I understand the suggestion, when would this be used?

I have trouble wrapping my head around this because .transparent=true flag has compound effects... I wonder if we could make those effects separately configurable, and decouple this? Something like this, where .blend is a new boolean property:

class Material {
  set transparent ( transparent ) {
    this.blend = transparent;
    this.depthWrite = ! transparent;
    this.renderPass = transparent ? TransparentPass : OpaquePass;
  }
  get transparent () {
    return this.blend && ! this.depthWrite && this.renderPass === TransparentPass;
  }
  ...
}

Then to enable .blending=AdditiveBlending + .transparent=false the user would set .blend = true manually.

I've also suggested a more direct copy of glTF's .alphaBlend = OPAQUE | BLEND | MASK | ... property in some other thread, which has certain advantages over a boolean .blend = true | false (e.g. forward-compatible with alpha hash?) but it isn't necessary here.

@mrdoob
Copy link
Owner Author

mrdoob commented Aug 25, 2021

Hmm, let me try the OpaqueBlending approach... 🧐

Edit: Maybe we can just use NoBlending for this...

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

Successfully merging this pull request may close these issues.

None yet

3 participants