MeshPhysicalMaterial: Added sheen #16971

DanielSturk · 2019-07-03T19:52:33Z

Added sheen for cloth materials such as velvet. The BDRF used is the same as Google's Filament (NDF is Estevez's and Kulla's "Charlie" sheen + Neubelt's visibility term https://google.github.io/filament/Filament.md.html).

I also added support for sheen to the StandardNodeMaterial (@sunag)

The demo for testing is webgl_materials_sheen.html

# Conflicts: # examples/jsm/nodes/materials/nodes/StandardNode.js # examples/webgl_materials_nodes.html

…js repos

DanielSturk · 2019-07-03T19:54:22Z

By the way, this implementation only covers direct lighting for the time being. There's a bit of confusion/complication in regards to IBL and ambient lighting because sheen seems to be dependent on the direction of the light source

bhouston · 2019-07-03T21:18:07Z

We should never support ambient lighting on this.

sunag · 2019-07-04T01:27:47Z

Thank you for your contribution once again...
The parameter and example sheen does not seem to be working.
I believe that changes in the core and in the NodeMaterial should be sent separately but I think if possible unify this feature using nodes only without modify the core...

DanielSturk · 2019-07-04T14:21:35Z

@sunag

The parameter and example sheen does not seem to be working.

What do you mean by this? I asked my colleague tested it and it runs fine on his machine

sciecode · 2019-07-04T14:41:20Z

@sunag

If you are using something like "githack" to test, it won't work.
Three.module.js needs to be updated, because of new sheen parameter.
Running it locally works as intended.

bhouston · 2019-07-04T15:56:23Z

unify this feature using nodes only without modify the core...

The reason we are adding this is that it is a BRDF improvement and it is to align with the Enterprise PBR model: https://github.com/DassaultSystemes-Technology/EnterprisePBRShadingModel

bhouston · 2019-07-04T16:11:46Z

This is being done in this later context of adopting PBR Next / Enterprise PBR #16977 and ensuring Node-graph materials are awesome #16440

sunag · 2019-07-05T18:39:11Z

@DanielSturk @sciecode Yeah, I run locally and work thanks.

The reason we are adding this is that it is a BRDF improvement and it is to align with the Enterprise PBR model: https://github.com/DassaultSystemes-Technology/EnterprisePBRShadingModel

The roadmap is very good. My think is advance in BRDF using nodes only to create better support for it and facilitate the merger of PR. Once implemented, create other BRDF features will be easier and optimized since the code can be calculated modularly. Sheen could be one of these new BRDF nodes.

bhouston · 2019-07-05T19:15:18Z

Usually the brdf parameters are part of the base brdf node. Having brdf parameters as separate nodes generally is not how brdfs are implemented.

…

On Fri, Jul 5, 2019, 2:41 PM sunag ***@***.***> wrote: @DanielSturk <https://github.com/DanielSturk> @sciecode <https://github.com/sciecode> Yeah, I run locally and work thanks. The reason we are adding this is that it is a BRDF improvement and it is to align with the Enterprise PBR model: https://github.com/DassaultSystemes-Technology/EnterprisePBRShadingModel The roadmap is very good. My think is advance in BRDF using nodes only to create better support for it and facilitate the merger of PR. Once implemented, create other BRDF features will be easier and optimized since the code can be calculated modularly. Sheen could be one of these new BRDF nodes. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#16971?email_source=notifications&email_token=AAEPV7L44GWGK4U4LYSFEGLP56IU3A5CNFSM4H5JCQY2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZKDUZY#issuecomment-508836455>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAEPV7NFFSDBDCNEG4OS5L3P56IU3ANCNFSM4H5JCQYQ> .

sunag · 2019-07-05T19:40:07Z

Usually the brdf parameters are part of the base brdf node. Having brdf
parameters as separate nodes generally is not how brdfs are implemented.

I was not thinking about separating the code from BRDF class, just implementing it as a process tree a bit more organized.
brdf/SheenNode for example...

So we can add relatives sheen functions if the parameter .sheen is really used.
Relative the performance, it is good too, even unused function adds an increased time to program creation.

sunag · 2019-07-17T01:25:30Z

@mrdoob @WestLangley I do not want to block this PR, maybe I have caused this impression, it looks good to me. I can improve these details relative to the NodeMaterial later with other PR.

examples/webgl_materials_sheen.html

Mugen87 · 2019-07-25T09:42:20Z

https://raw.githack.com/DanielSturk/three.js/sheen/examples/webgl_materials_sheen.html

It seems it's not possible to control the arm with the keys like in the original physics example. Besides, the arm jitters right now in a strange way. At least on my system.

If no interaction is intended, consider to remove the arm or make it static.

yomboprime · 2019-07-25T11:12:08Z

https://raw.githack.com/DanielSturk/three.js/sheen/examples/webgl_materials_sheen.html

It seems it's not possible to control the arm with the keys like in the original physics example. Besides, the arm jitters right now in a strange way. At least on my system.

If no interaction is intended, consider to remove the arm or make it static.

He has put a Math.random() in the arm control.

# Conflicts: # examples/jsm/nodes/materials/nodes/StandardNode.js

…HEAD

Mugen87 · 2019-08-07T13:51:51Z

@DanielSturk Can you please resolve the merge conflicts?

# Conflicts: # src/materials/MeshPhysicalMaterial.d.ts # src/materials/MeshPhysicalMaterial.js # src/renderers/shaders/ShaderChunk/lights_physical_pars_fragment.glsl.js # src/renderers/shaders/ShaderLib.js

Mugen87 · 2019-08-08T15:08:14Z

@mrdoob Looks ready to merge 👍

bhouston · 2019-08-20T12:57:10Z

Is there a reason this isn't being merged?

Mugen87 · 2019-08-20T13:53:35Z

Sorry, I'm sure this PR was just overlooked.

@DanielSturk Can you please resolve the merge conflicts one more time?

romainguy · 2019-08-20T16:06:05Z

@DanielSturk What problems did you run into with IBL support for sheen? Folks on Babylon.js and I both got it to work, we can help if you'd like.

DanielSturk · 2019-08-20T21:05:14Z

@Mugen87 Np. Merge conflicts resolved :)

DanielSturk · 2019-08-20T21:07:17Z

@DanielSturk What problems did you run into with IBL support for sheen? Folks on Babylon.js and I both got it to work, we can help if you'd like.

Thank you, I appreciate the support! I've got it working now (was a little confused by the DFG (DG) LUT but it's working now). As soon as we can get this merged, I'll create the PR for the IBL Sheen!

mrdoob · 2019-08-20T23:07:55Z

Thanks!

mrdoob · 2019-08-20T23:54:34Z

@DanielSturk @bhouston I have renamed the property to sheen to match emissive, specular, ... 3e9af1f

mrdoob · 2019-08-21T00:13:50Z

@sunag on one of my computers (MacBook Air, Early 2015, Intel HD Graphics 6000) the sheen example runs at 20fps when using the node material, and at 60fps when not using it.

In this screenshot, the fps graph shows when nodematerial was enabled and disabled:

sunag · 2019-08-21T00:28:43Z

@sunag on one of my computers (MacBook Air, Early 2015, Intel HD Graphics 6000) the sheen example runs at 20fps when using the node material, and at 60fps when not using it.

This because the NodeMaterial is being compiled every frame: nodeMaterial.needsCompile = true; is added in render loop. I already send a PR to fix this.

mrdoob · 2019-08-21T00:35:36Z

This because the NodeMaterial is being compiled every frame: nodeMaterial.needsCompile = true; is added in render loop. I already send a PR to fix this.

Oh, what's the PR that fixes it?

sunag · 2019-08-21T00:46:32Z

Oh, what's the PR that fixes it?

Ops, I'm already sending, sry my english 😳

DanielSturk · 2019-08-21T14:03:25Z

Thanks for the merge @mrdoob and everyone involved
Make sure to check out my follow up PR for IBL support @romainguy #17303

donmccurdy · 2020-09-21T03:05:57Z

The glTF sheen extension (KHR_materials_sheen) is nearly complete, although still welcoming feedback for a bit longer. To implement in MeshPhysicalMaterial, it would require:

sheen
sheenMap
sheenRoughness
sheenRoughnessMap

The two maps use .rgb and .a channels respectively; they can be combined.

DanielSturk added 9 commits July 3, 2019 15:18

Added sheen to physical material

d268b80

Moved sheen to PhysicalMaterial struct

9428b70

Fixed renaming mistake

fcd8fda

Added sheen to StandardNodeMaterial

1ae0a65

# Conflicts: # examples/jsm/nodes/materials/nodes/StandardNode.js # examples/webgl_materials_nodes.html

Fixed node material crashing, added more options to sheen demo

b728371

Fixed node material crashing, added more options to sheen demo

ef38e81

Switched to Google's Filament sheen BDRF

8597102

Fixed errors due to discrepancies between mainline and private three.…

79cd845

…js repos

Removed (unused) inverted Guassian distribution from bsdfs

350553d

bhouston mentioned this pull request Jul 4, 2019

Intent to implement/contribute Enterprise PBR improvements #16977

Closed

9 tasks

Added sheen to copy and toJSON functions

59f5500

DanielSturk added 2 commits July 9, 2019 10:10

Fixed StandardNode.copy

a9518cc

Merge branch 'dev' into sheen

4466b48

DanielSturk closed this Jul 24, 2019

DanielSturk reopened this Jul 24, 2019

Mugen87 reviewed Jul 25, 2019

View reviewed changes

examples/webgl_materials_sheen.html Outdated Show resolved Hide resolved

Merge branch 'dev' of https://github.com/mrdoob/three.js into sheen

b81c8c4

# Conflicts: # examples/jsm/nodes/materials/nodes/StandardNode.js

Merge branch 'sheen' of https://github.com/DanielSturk/three.js into …

3cf6f6e

…HEAD

DanielSturk added 2 commits August 8, 2019 10:00

Merge remote-tracking branch 'mrdoob/dev' into sheen

9099439

# Conflicts: # src/materials/MeshPhysicalMaterial.d.ts # src/materials/MeshPhysicalMaterial.js # src/renderers/shaders/ShaderChunk/lights_physical_pars_fragment.glsl.js # src/renderers/shaders/ShaderLib.js

fixed formatting

c42ccfb

wip: proof of concept sheen IBL, needs cleanup (at least)

52ef062

bhouston mentioned this pull request Aug 20, 2019

What would you like to see in next gen materials? KhronosGroup/glTF#1442

Closed

DanielSturk added 6 commits August 20, 2019 14:08

replaced sheen with sheenColor, and removed IBL sheen (for now)

74e5482

fixed StandardNodeMaterial's sheen

22fce3b

removed DFG LUT for IBL sheen

796c5b3

Removed Sheen IBL DFG LUT from ShaderLib

bbfee95

Merge remote-tracking branch 'mrdoob/dev' into sheen

b2ab31d

fixed formatting

7b14d5c

mrdoob changed the title ~~Added sheen~~ MeshPhysicalMaterial: Added sheen Aug 20, 2019

mrdoob merged commit 1855c15 into mrdoob:dev Aug 20, 2019

sunag mentioned this pull request Aug 21, 2019

Fix sheen example needsCompile #17298

Merged

DanielSturk mentioned this pull request Aug 21, 2019

IBL Sheen Support #17303

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MeshPhysicalMaterial: Added sheen #16971

MeshPhysicalMaterial: Added sheen #16971

DanielSturk commented Jul 3, 2019 •

edited

Loading

DanielSturk commented Jul 3, 2019

bhouston commented Jul 3, 2019

sunag commented Jul 4, 2019

DanielSturk commented Jul 4, 2019

sciecode commented Jul 4, 2019 •

edited

Loading

bhouston commented Jul 4, 2019

bhouston commented Jul 4, 2019

sunag commented Jul 5, 2019

bhouston commented Jul 5, 2019 via email

sunag commented Jul 5, 2019

sunag commented Jul 17, 2019

Mugen87 commented Jul 25, 2019 •

edited

Loading

yomboprime commented Jul 25, 2019

Mugen87 commented Aug 7, 2019

Mugen87 commented Aug 8, 2019

bhouston commented Aug 20, 2019

Mugen87 commented Aug 20, 2019

romainguy commented Aug 20, 2019

DanielSturk commented Aug 20, 2019

DanielSturk commented Aug 20, 2019

mrdoob commented Aug 20, 2019

mrdoob commented Aug 20, 2019

mrdoob commented Aug 21, 2019

sunag commented Aug 21, 2019

mrdoob commented Aug 21, 2019

sunag commented Aug 21, 2019

DanielSturk commented Aug 21, 2019

donmccurdy commented Sep 21, 2020

MeshPhysicalMaterial: Added sheen #16971

MeshPhysicalMaterial: Added sheen #16971

Conversation

DanielSturk commented Jul 3, 2019 • edited Loading

DanielSturk commented Jul 3, 2019

bhouston commented Jul 3, 2019

sunag commented Jul 4, 2019

DanielSturk commented Jul 4, 2019

sciecode commented Jul 4, 2019 • edited Loading

bhouston commented Jul 4, 2019

bhouston commented Jul 4, 2019

sunag commented Jul 5, 2019

bhouston commented Jul 5, 2019 via email

sunag commented Jul 5, 2019

sunag commented Jul 17, 2019

Mugen87 commented Jul 25, 2019 • edited Loading

yomboprime commented Jul 25, 2019

Mugen87 commented Aug 7, 2019

Mugen87 commented Aug 8, 2019

bhouston commented Aug 20, 2019

Mugen87 commented Aug 20, 2019

romainguy commented Aug 20, 2019

DanielSturk commented Aug 20, 2019

DanielSturk commented Aug 20, 2019

mrdoob commented Aug 20, 2019

mrdoob commented Aug 20, 2019

mrdoob commented Aug 21, 2019

sunag commented Aug 21, 2019

mrdoob commented Aug 21, 2019

sunag commented Aug 21, 2019

DanielSturk commented Aug 21, 2019

donmccurdy commented Sep 21, 2020

DanielSturk commented Jul 3, 2019 •

edited

Loading

sciecode commented Jul 4, 2019 •

edited

Loading

Mugen87 commented Jul 25, 2019 •

edited

Loading