WIP: Alpha fixes - #15573
Conversation
|
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
|
Snapshot stored with reference name: Test environment: To test a playground add it to the URL, for example: https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/refs/pull/15573/merge/index.html#WGZLGJ#4600 Links to test babylon tools with this snapshot: https://playground.babylonjs.com/?snapshot=refs/pull/15573/merge To test the snapshot in the playground with a playground ID add it after the snapshot query string: https://playground.babylonjs.com/?snapshot=refs/pull/15573/merge#BCU1XR#0 |
|
Visualization tests for WebGL 1 have failed. If some tests failed because the snapshots do not match, the report can be found at If tests were successful afterwards, this report might not be available anymore. |
|
WebGL2 visualization test reporter: |
|
Visualization tests for WebGPU (Experimental) |
|
This pull request has been marked as stale because it has been inactive for more than 14 days. Please update to "unstale". |
Popov72
left a comment
Popov72
left a comment
There was a problem hiding this comment.
One change is missing: in shadowGenerator.ts, a call to needAlphaBlending should be replaced with a call to needAlphaBlendingForMesh.
| return this._transparencyModeIsBlend; | ||
| } | ||
|
|
||
| if (this.subSurface?.disableAlphaBlending) { |
There was a problem hiding this comment.
It should be checked first to be backward compatible.
| } | ||
|
|
||
| return this.alpha < 1.0 || this._opacityTexture != null || this._shouldUseAlphaFromAlbedoTexture(); | ||
| return super.needAlphaBlending(); |
There was a problem hiding this comment.
The this._opacityTexture != null || this._shouldUseAlphaFromAlbedoTexture() part is missing.
| */ | ||
| protected _hasAlphaChannel(): boolean { | ||
| return (this._albedoTexture != null && this._albedoTexture.hasAlpha) || this._opacityTexture != null; | ||
| return super.needAlphaTesting(); |
There was a problem hiding this comment.
The this._hasAlphaChannel() part is missing.
|
|
||
| /** @override */ | ||
| public override needAlphaBlending(): boolean { | ||
| return super.needAlphaBlending() || this._opacityFresnelParameters?.isEnabled; |
There was a problem hiding this comment.
As for the PBR material, the checks on this._opacityTexture and this._shouldUseAlphaFromDiffuseTexture() are missing, and needAlphaTesting would also need to be overriden to check this._hasAlphaChannel().
|
@bghgary any update ? |
|
Not yet. On my list. |
|
This pull request has been marked as stale because it has been inactive for more than 14 days. Please update to "unstale". |
|
Superceded by #16144. |
4 participants
Just a test for now. We should not merge this until there is adequate testing for the edge cases.
This will supersede the changes in #15504.