Feature: Add vertex color alpha channel support. #20975

chubei · 2020-12-31T08:24:17Z

Related issue: Fixed #16403.

Description

A new shader macro USE_VERTEX_OPACITY is added to control if the attribute color is a vec3 or vec4.

Corresponding flag in Material is .vertexOpacities.

Does this need a new example? @donmccurdy @mrdoob @Mugen87

Mugen87 · 2020-12-31T10:17:42Z

I think I prefer the name vertexAlpha.

Please also update the TS declaration file and the documentation.

Does this need a new example?

It's good if at least one example demonstrates the usage. However, I would enhance an existing buffer geometry example like webgl_buffergeometry.

chubei · 2020-12-31T10:24:04Z

I think I prefer the name vertexAlpha.

No problem.

Please also update the TS declaration file and the documentation.

I wasn't aware of the TS files. Will do.

The contribution guide says please make a new PR for the appropriate doc changes. Is that out of date?

It's good if at least one example demonstrates the usage. However, I would enhance an existing buffer geometry example like webgl_buffergeometry.

I'll modify that example.

Mugen87 · 2020-12-31T10:35:13Z

The contribution guide says please make a new PR for the appropriate doc changes. Is that out of date?

Oh, I was not aware of that line. TBH, I think it's better if the documentation is always updated with the same PR that changes code.

chubei · 2020-12-31T10:38:10Z

TBH, I think it's better if the documentation is always updated with the same PR that changes code.

I actually agree. Let's break that rule in this PR 😈.

This PR clarifies the update of documentation pages, see #20975 (comment).

src/loaders/MaterialLoader.js

examples/webgl_buffergeometry.html

chubei · 2021-01-16T03:33:59Z

Thanks for your review @Mugen87. Are we waiting for other collaborators’ review now?

Mugen87 · 2021-01-16T11:53:34Z

Yep. @mrdoob will have a look at this PR and merge it when everything is fine.

Mugen87 · 2021-02-01T10:15:04Z

@chubei Do you mind updating the PR? There is now a merge conflict since all TS type declaration files were removed.

chubei · 2021-02-04T09:08:42Z

Rebased and force pushed. Sorry for the delay.

noamgat · 2021-03-04T17:01:37Z

Wow, this looks great! We would be really happy it it was merged.

src/materials/Material.js

Code based has changed since last approval.

Mugen87

Using vec4 by default is a breaking change and can't be accepted.

chubei · 2021-03-15T11:15:30Z

I tried to implement automatic inferring vertexAlpha and force pushed the code. Because most of the original modification is not used any longer, I rebased and made a new commit. Please let me know if you want a merge-then-forward style commit.

Mugen87 · 2021-03-15T11:39:09Z

I think we should stick to vertexAlpha and let the user/loaders decide when to use vertex alpha or not. #20975 (comment) is a good example why automatically determine configurations are problematic.

Mugen87

Not approving an inferring approach.

src/renderers/WebGLRenderer.js

chubei · 2021-03-15T11:42:36Z

@mrdoob What’s your opinion on this? I prefer the inferring approach, but am also fine with merging the previous version.

mrdoob · 2021-03-15T18:01:20Z

Hmm...

On the API ergonomics side, inferring is a big win.
Users no longer need to know about skinning, vertexTangents, vertexColor, vertexAlpha, etc

We can also remove 3 checkboxes from the editor (which is usually a good signal API wise too):

I agree with @chubei that instead of grid1.material.vertexColors = false; the user should be doing grid1.geometry.deleteAttribute('color'); (and I should have done that 😅).

I also understand that this would force people have to duplicate geometries and that could result in increased memory, but I'm not sure how much of a concern it should be for most of the cases.

How about we try the inferred approach in this case so we can see what the real consequences are?
We can add the property to the material later if we found this creates issues.

Mugen87 · 2021-03-15T18:27:02Z

How about we try the inferred approach in this case so we can see what the real consequences are?

Okay, that sounds good to me.

Mugen87 · 2021-03-22T15:17:53Z

src/core/BufferGeometry.js

@@ -879,6 +879,14 @@ BufferGeometry.prototype = Object.assign( Object.create( EventDispatcher.prototy

 	},

+	hasColorAlpha: function () {


I don't think we want to expose this method.

Yes, this also looks strange to me. Any suggestions where I should put this method? Don't feel like to repeat the code in WebGLRenderer.js and WebGLPrograms.js.

I would indeed inline it for now.

Agreed! I was going to clean that up after merging it tomorrow.

I would indeed inline it for now.

Sure thing. Updated.

mrdoob · 2021-03-23T23:33:34Z

@Mugen87

I've spent some idle thinking on this issue over the last few days and I came up with another reason to remove skinning.

There are quite a few bug reports of scene.overrideMaterial not working with scenes that have skinned and not skinned meshes. And that's because we can't reuse the same material in both. If a user wanted to render the whole scene in wireframe for example, it's not possible.

Similarly, WebGLShadowMap has this logic that is a workaround for the same problem.

My hunch is that the extra VAOs are worth it so users can finally reuse materials without bumping into these issues and being forced to understand the internals.

mrdoob · 2021-03-26T21:22:42Z

Hmm, I'm going to merge this and do another PR some changes to it for your review.

mrdoob · 2021-03-26T21:22:51Z

Thanks!

Mugen87 mentioned this pull request Dec 31, 2020

Update CONTRIBUTING.md #20980

Merged

Mugen87 added a commit that referenced this pull request Dec 31, 2020

Update CONTRIBUTING.md

b4be803

This PR clarifies the update of documentation pages, see #20975 (comment).

Mugen87 reviewed Jan 5, 2021

View reviewed changes

src/loaders/MaterialLoader.js Outdated Show resolved Hide resolved

Mugen87 reviewed Jan 5, 2021

View reviewed changes

examples/webgl_buffergeometry.html Outdated Show resolved Hide resolved

Mugen87 reviewed Jan 5, 2021

View reviewed changes

examples/webgl_buffergeometry.html Outdated Show resolved Hide resolved

Mugen87 previously approved these changes Jan 6, 2021

View reviewed changes

mrdoob reviewed Mar 5, 2021

View reviewed changes

src/materials/Material.js Outdated Show resolved Hide resolved

mrdoob added this to the r127 milestone Mar 5, 2021

donmccurdy mentioned this pull request Mar 15, 2021

Vertex color alpha is ignored for GLB files #21466

Closed

Mugen87 requested changes Mar 15, 2021

View reviewed changes

Mugen87 approved these changes Mar 15, 2021

View reviewed changes

Feature: Add vertex color alpha channel support.

0faee99

Mugen87 requested changes Mar 15, 2021

View reviewed changes

Mugen87 reviewed Mar 15, 2021

View reviewed changes

src/renderers/WebGLRenderer.js Show resolved Hide resolved

Mugen87 previously approved these changes Mar 22, 2021

View reviewed changes

Mugen87 reviewed Mar 22, 2021

View reviewed changes

Vertex alpha: Remove hasVertexAlpha from BufferGeometry

526a305

Mugen87 mentioned this pull request Mar 25, 2021

OutlinePass does not support InstancedMesh #18533

Closed

11 tasks

mrdoob merged commit 30189da into mrdoob:dev Mar 26, 2021

This was referenced Mar 26, 2021

WebGLRenderer: Refactored vertex color alpha code #21530

Merged

GLTFLoader: Vertex color alpha support #21533

Closed

chubei deleted the color4 branch March 29, 2021 15:08

This was referenced Jun 18, 2021

[@types/three] add vertexAlphas in Material, supported since r127 DefinitelyTyped/DefinitelyTyped#53906

Closed

[Docs] add vertexAlphas in Material, supported since r127 #22015

Closed

jhurliman mentioned this pull request Mar 8, 2022

Add vertex color alpha channel support to LineMaterial #23680

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: Add vertex color alpha channel support. #20975

Feature: Add vertex color alpha channel support. #20975

chubei commented Dec 31, 2020 •

edited by Mugen87

Loading

Mugen87 commented Dec 31, 2020 •

edited

Loading

chubei commented Dec 31, 2020

Mugen87 commented Dec 31, 2020

chubei commented Dec 31, 2020

chubei commented Jan 16, 2021

Mugen87 commented Jan 16, 2021

Mugen87 commented Feb 1, 2021 •

edited

Loading

chubei commented Feb 4, 2021

noamgat commented Mar 4, 2021

Mugen87 left a comment

chubei commented Mar 15, 2021

Mugen87 commented Mar 15, 2021 •

edited

Loading

Mugen87 left a comment

chubei commented Mar 15, 2021

mrdoob commented Mar 15, 2021

Mugen87 commented Mar 15, 2021

Mugen87 Mar 22, 2021

chubei Mar 23, 2021

Mugen87 Mar 23, 2021

mrdoob Mar 23, 2021

chubei Mar 24, 2021

mrdoob commented Mar 23, 2021

mrdoob commented Mar 26, 2021

mrdoob commented Mar 26, 2021

		@@ -879,6 +879,14 @@ BufferGeometry.prototype = Object.assign( Object.create( EventDispatcher.prototy

		},

		hasColorAlpha: function () {

Feature: Add vertex color alpha channel support. #20975

Feature: Add vertex color alpha channel support. #20975

Conversation

chubei commented Dec 31, 2020 • edited by Mugen87 Loading

Mugen87 commented Dec 31, 2020 • edited Loading

chubei commented Dec 31, 2020

Mugen87 commented Dec 31, 2020

chubei commented Dec 31, 2020

chubei commented Jan 16, 2021

Mugen87 commented Jan 16, 2021

Mugen87 commented Feb 1, 2021 • edited Loading

chubei commented Feb 4, 2021

noamgat commented Mar 4, 2021

Mugen87 left a comment

Choose a reason for hiding this comment

chubei commented Mar 15, 2021

Mugen87 commented Mar 15, 2021 • edited Loading

Mugen87 left a comment

Choose a reason for hiding this comment

chubei commented Mar 15, 2021

mrdoob commented Mar 15, 2021

Mugen87 commented Mar 15, 2021

Mugen87 Mar 22, 2021

Choose a reason for hiding this comment

chubei Mar 23, 2021

Choose a reason for hiding this comment

Mugen87 Mar 23, 2021

Choose a reason for hiding this comment

mrdoob Mar 23, 2021

Choose a reason for hiding this comment

chubei Mar 24, 2021

Choose a reason for hiding this comment

mrdoob commented Mar 23, 2021

mrdoob commented Mar 26, 2021

mrdoob commented Mar 26, 2021

chubei commented Dec 31, 2020 •

edited by Mugen87

Loading

Mugen87 commented Dec 31, 2020 •

edited

Loading

Mugen87 commented Feb 1, 2021 •

edited

Loading

Mugen87 commented Mar 15, 2021 •

edited

Loading