mrdoob / three.js

JavaScript 3D Library.
https://threejs.org/
MIT License
101.98k stars 35.33k forks source link

Suggestion: Refactor MeshToonMaterial #16257

Closed WestLangley closed 4 years ago

WestLangley commented 5 years ago

As suggested in https://github.com/mrdoob/three.js/pull/15105#issuecomment-453540970.

I have never seen a toon shader that supports hotspots, environment maps, or even maps for that matter... Or multiple lights.

A toon shader can be written in a few lines of code, and should not have extended Phong, which is way too flexible. I would have implemented it like MeshMatcapMaterial.

Alternative opinions, welcome.

Mugen87 commented 5 years ago

Ups, we've created an issue at the same time^^

16258

Mugen87 commented 5 years ago

I've closed my issue...

gkjohnson commented 5 years ago

I'm all for rethinking the toon shader but can you elaborate on this a bit more?

I have never seen a toon shader that supports hotspots, environment maps, or even maps for that matter... Or multiple lights.

My understanding is that a toon shader is basically just mapping a color ramp to light intensity so it would seem that that should enable support for specular highlights through the stepped shading (though maybe not in the way they're represented right now). The Octane and Arnold toon material docs both reference rendering specular highlights with the toon material, for example. Regarding maps the Octane docs give examples of using a roughness map to augment the shape of the color bands and something like a normal map should have the same effect.

I can't find anything explicitly showing a toon material rendering with multiple lights (other than the final image in the Octane docs that shows two highlights) but this seems like a stylistic choice. I'm not sure if THREE allows a way to limit or choose which lights a material renders with but that would be a nice option. This write up on Zelda Wind Waker discusses how lighting changes dynamically based on closest light source, but maybe that's considered to use case specific.

WestLangley commented 5 years ago

The additional features we support are really up to us. I agree supporting scene lights is reasonable. And I think OutlineEffect should remain separate from Toon.

If others are happy the way it is, I'll close this. Suggestions welcome.

gkjohnson commented 5 years ago

I didn't necessarily mean the suggest that the shader shouldn't change. I'm not familiar enough with the shader to have an opinion on whether or not it should be refactored away from extending Phong but I do agree that the way the specular is handled right now is not what I would expect from a toon shader built to emulate a cel-shading look. I would think they should have a firm edge instead of the soft one they have now (from the toon shader example):

image

Compared to the Octane docs:

image

WestLangley commented 5 years ago

the specular is handled right now is not what I would expect from a toon shader built to emulate a cel-shading look. I would think they should have a firm edge instead of the soft one they have now

Agreed - if the shader is going to continue to support hot-spots.

@takahirox made a nice contribution with MeshToonMaterial. Let's see if there is consensus on what features it should support. Then I would suggest using MeshMatcapMaterial as a guideline, and refactor the shader.

mrdoob commented 5 years ago

I think a toon material should support multiple lights, use a gradientMap to map the resulting color and that's about it?

May be good to see what VRM is doing: https://vrm.dev/en/univrm/shaders/mtoon/

WestLangley commented 5 years ago

If MeshToonMaterial is to be full-featured, this would be the list I would begin with. If you want to remove some of these features (or add some), that would be fine with me.

everything Material Supports, including lights, plus
color
gradientMap (specific to Toon)
lightMap
lightMapIntensity
aoMap
aoMapIntensity
emissive
emissiveIntensity
emissiveMap
bumpMap
bumpScale
normalMap
normalMapType
normalScale
displacementMap
displacementScale
displacementBias
alphaMap
skinning
morphTargets
morphNormals
Hot-spots as you see fit. If not, then just Lambertian-based.
takahirox commented 5 years ago

Thanks for trying to refactor MeshToonMaterial.

About specular, the reason why I implemented so was from MMD default shader. Like her sleeve in the first picture in the link https://ascii.jp/elem/000/000/714/714544/index-2.html MMD specular is soft edge.

I personally want to remain the soft edge for MMD but I can understand it isn't standard. So I don't block changing. Instead if we change I'll think of customizing shader for MMD with Node-based material (when it's available in core), .onBeforeCompile(), or ShaderMaterial.

WestLangley commented 5 years ago

I am flexible on this -- although, I still would prefer a separate shader.

  1. After studying this more closely, I realized that the only properties I would remove are those related to envMap (because Toon is an NPR shader; realistic reflections do not seem appropriate to me) and map.

  2. If you want to support multiple lights, then I think you should calculate the irradiance from all the lights first, and then apply the gradient. When you apply the gradient with each light, you end up with too much banding. (Multiple lights with different colors may also be a problem.)

  3. In the Octane example above, is the specular spot really a specular reflection with a hard edge, or is it just the gradient applied to the diffuse reflection only? /ping @gkjohnson

gkjohnson commented 5 years ago

@WestLangley The Octane docs refer to it as a specular reflection so I expect that's what it is but I haven't played with it myself. Some of the other images in those docs make it look like a specular highlight, as well.

WestLangley commented 5 years ago

@gkjohnson If you are able to experiment and determine for sure, that would be great. Assuming the direct light is fixed, if it moves with the view direction it is specular; if not, it is a diffuse gradient effect.

gkjohnson commented 5 years ago

I haven't used octane myself and I haven't had a chance to download the demo but after searching around briefly I came across these videos.

This one shows the editor for the Octane toon material. The editor shows a roughness parameter on the material which wouldn't be needed if there were no hot spot being rendered. And at around 7:27 you can see the highlights change when the camera moves in the render view: https://youtu.be/G_ciCcSrHr0?t=447

image

This video also shows roughness and specular components to the octane toon material in blender: https://youtu.be/Ni5Oz2QZ8xA?t=124

image

At the moment, at least, I'm pretty convinced that the toon material here includes rendering highlights.

Prinzhorn commented 4 years ago

From the perspective of a three.js user I found getting started with the MeshToonMaterial a little bumpy, because it looks weird/unusable without a gradientMap (at least in my personal setup). It took me a while to realize that the docs use a fiveTone map by default. So from a user perspective, would it make sense if the API simply had a numColors (or whatever) parameter in addition to the gradientMap? gradientMap (if set) would take precedence over numColors, which defaults to sth. like 5.

Right now I'm using a constant like this, since I didn't want to have load an image for five whole pixels.

const fiveTone = new THREE.DataTexture(
  Uint8Array.from([0, 0, 0, 64, 64, 64, 128, 128, 128, 192, 192, 192, 255, 255, 255]),
  5,
  1,
  THREE.RGBFormat
);

fiveTone.needsUpdate = true;

I guess this could be done internally and hidden from the user? So that toon "just works" out of the box and you can quickly play with different gradients? Apart from that I don't have anything to add to this discussion, as I'm a newb when it comes to the detail aspects of the rendering stuff or phong.

WestLangley commented 4 years ago

Closing since MeshToonMaterial no longer extends MeshPhongMaterial.

Discussions regarding improvements to the toon illumination model can continue in a new thread.

takahirox commented 4 years ago

About specular, the reason why I implemented so was from MMD default shader. Like her sleeve in the first picture in the link https://ascii.jp/elem/000/000/714/714544/index-2.html MMD specular is soft edge.

I personally want to remain the soft edge for MMD but I can understand it isn't standard. So I don't block changing. Instead if we change I'll think of customizing shader for MMD with Node-based material (when it's available in core), .onBeforeCompile(), or ShaderMaterial.

As #19609 I am going to implement MMD specific material in MMDLoader, so we no longer need to take care MMD for refactoring Toon material.