Custom three.js shader material #4

Aadjou · 2017-06-21T14:43:23Z

See issue #3

I took the shader & shader includes directly from three.js and created our own base-shaderMaterial based on their MeshPhong Material:
MeshPhong Fragment
MeshPhong Vertex
I stripped it from includes that we currently are not using, eg. skinning, shadowmaps etc.
we can add any of these easily if it turns out that we need them.
finally I added the lightmap controls (based on the editor) in the fragment shader.

looking forward to your review on this pull request

AVGP

-- removed --

AVGP · 2017-06-23T10:19:43Z

lib/core/three/data3d-view.js

@@ -88,7 +89,8 @@ View.prototype = {
        // three.js materials
        if (!self._materials3d[ meshId ]) {
          // (one material pro mesh, because some of our mesh properties are material properties and it does not matter performance wise)
-          material3d = new THREE.MeshPhongMaterial({ opacity: 0.5, transparent: true })
+          //material3d = new THREE.MeshPhongMaterial({ opacity: 0.5, transparent: true})
+          material3d = BaseMaterial()


Not a problem, but I think we should restructure it later to be a proper constructor. Depending on how it'll evolve we might even be able to inherit a bunch of stuff from MeshPhongMaterial

I would like to get your help with this. @AVGP

4cc39cb \o/

AVGP · 2017-06-26T13:11:29Z

.gitignore

+.apdisk
+node_modules
+build


@tomas-polach the build folder can be added to .gitignore right?

AVGP · 2017-06-26T13:13:59Z

examples/browser/aframe/index.html


+  <a-entity light="type: point; intensity: 0.75; distance: 50; decay: 2" position="-3.2 -2.2 -4.5"></a-entity>


So the light in the example here is there to replace the standard light, right?

How are we going to deal with people who have only the default light in the scene?

@tomas-polach wants to build a a-frame component for an archilogic light scene.

the problem with the lights is that if there is no light present, any object with no lightmap (eg. all products) are not illuminated at all and therefore appear black.
if you add a three.js light, you can illuminate the scene (which is a pretty cool feature IMHO that comes for free) but the lightmapped objects will react much stronger to it.
It would be an option to use the lightMapIntensity parameter for a base illumination factor. (i tested that before and the results were good)

AVGP · 2017-06-26T13:26:08Z

lib/core/three/data3d-view/set-material.js


  // specular coefficient

-  material3d.shininess = _attributes.specularCoef !== undefined ? _attributes.specularCoef : 0.1
+  material3d.shininess = (_attributes.specularCoef !== undefined) ? (_attributes.specularCoef / 100) : 0.1


Not sure if it's "shininess" or "specular" that's the problem but the sink isn't particularly shiny :(

okay so it turns out that three.js is super inconsistent and "shininess" is one of the rare parameters that goes from 0 to 100 and instead of using a unit float. My bad.

and thanks for the catch.

AVGP · 2017-06-26T13:28:38Z

lib/core/three/data3d-view/set-material.js


-  if (_attributes.colorEmissive) {
+  if (_attributes.colorEmissive && material3d.emissive) {


Can we reformat this to match diffuse and specular?
Also: Where is the corresponding uniform being set?

f71d1b0

the emission color is not yet implemented in the shader, because we currently do not support that in the editor. I could however implement it, if that is something we want to support.

AVGP · 2017-06-26T13:29:36Z

lib/core/three/data3d-view/set-material.js

@@ -117,9 +121,12 @@ export default function setMaterial (args) {
  // lightmap settings
  if (_attributes.mapLight || _attributes.mapLightPreview) {
    material3d.enhancedLightMap = material3d.enhancedLightMap || {}


Do we still need the enhancedLightMap property? It was used in the editor as a switch to enable our lightmap shader...

i removed enhancedLightMap because we don't need it anymore

commit:
487a76a

AVGP · 2017-06-26T13:37:01Z

lib/core/three/data3d-view/load-textures.js

@@ -267,6 +267,7 @@ export default function loadTextures ( queue, TEXTURE_TYPES, vm, _attributes, ma
          }
          // add new texture
          material3d[ texture3dKeys[ i ] ] = textures[ i ]
+          material3d.uniforms[ texture3dKeys[ i ] ].value = textures[ i ]


I assume this sets .lightMap?

also specularMap, normalMap, alphaMap und map

AVGP · 2017-06-26T13:38:15Z

lib/core/three/material/base-fragment.glsl

+
+    vec3 totalEmissiveRadiance = emissive;
+
+    #include <logdepthbuf_fragment>


Is logdepthbuf_pars_fragment missing in the includes previously?

Considering the comment of Stavros, we say nay to logdepthbuffer for now
a54d300

IMHO there's no need for a logarithmic depth buffer in our context (interior scenes). It usually helps when you're doing very large-scale stuff, such as terrain rendering.

AVGP · 2017-06-26T13:40:52Z

lib/core/three/material/base-fragment.glsl

+        RE_IndirectDiffuse( irradiance, geometry, material, reflectedLight );
+
+    #endif
+


Mark end-of-replaced-block here, please

AVGP · 2017-06-26T14:03:15Z

lib/core/three/material/base-material.js

+    lightMapCenter: { value: 0.5 },
+    normalMap: { value: null },
+    shininess: { value: 1.0 },
+    specular: { value: new THREE.Color(0.0, 0.0, 0.0) },


Is this meant to be black by default?

good point. I will consider this together with the general specularity issue.

AVGP · 2017-06-26T14:11:20Z

lib/core/three/material/base-vertex.glsl

+
+#include <uv_pars_vertex>
+#include <uv2_pars_vertex>
+


logdepthbuffer yea/nay?

AVGP · 2017-06-26T14:12:52Z

lib/core/three/material/base-vertex.glsl

+
+  #include <begin_vertex>
+  #include <project_vertex>
+


logdepthbuffer yea/nay?

IMHO there's no need for a logarithmic depth buffer in our context (interior scenes). It usually helps when you're doing very large-scale stuff, such as terrain rendering.

thanks @stavros-papadopoulos. i will remove the one include then that accidentally is there

AVGP · 2017-06-27T13:00:11Z

That's weird, but cool that you found it :-) 2017-06-27 14:58 GMT+02:00 Madlaina Kalunder <notifications@github.com>:

…

***@***.**** commented on this pull request. ------------------------------ In lib/core/three/data3d-view/set-material.js <#4 (comment)> : > // specular coefficient - material3d.shininess = _attributes.specularCoef !== undefined ? _attributes.specularCoef : 0.1 + material3d.shininess = (_attributes.specularCoef !== undefined) ? (_attributes.specularCoef / 100) : 0.1 okay so it turns out that three.js is super inconsistent and "shininess" is one of the rare parameters that goes from 0 to 100 and instead of using a unit float. My bad. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#4 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAWmRhtpnoFYKpGj7rABbyRaNSV2SNeuks5sIPx0gaJpZM4OBEQ-> .

AVGP

Looks good to me

AVGP · 2017-06-28T11:54:55Z

lib/core/three/data3d-view/set-material.js

+    emissive.g = _attributes.colorEmissive[ 1 ]
+    emissive.b = _attributes.colorEmissive[ 2 ]
+  } else if (_attributes.lightEmissionCoef) {
+    var emissiveIntensity = _attributes.lightEmissionCoef / 10


I guess this is reasonable but I'd welcome a small comment explaining why we're dividing by 10

madlaina-kalunder added 21 commits June 12, 2017 16:05

Created draft for custom three.js shaderMaterial

be76720

Added support for diffuse and lightmaps.

dea25a7

Added glsl rollup plugin.

c441357

Switched glsl rollup plugin.

549667f

Added product to example

b952c14

Added specular color to uniforms.

eeff9cd

Split shaderMaterial definition to new file.

50d7493

Replaced uv&uv2 with includes.

731b86b

Added map includes

d9d4eae

Rename

4ad84ad

Fixed rebase-conflict

e51a8dc

Added relevant includes from THREE phong material.

afdda9a

Fixed empty line in vertex.glsl.

ddafad1

Added scene lights (three.js light sources) to uniforms.

b2d18f4

Fixed vUv, added offsetRepeat uniform

633911c

Added lightMapIntensity to uniforms

00f3953

Added lightMap controls to uniforms

8d712d3

Fixed fragment uniforms

d5515c9

Fixed typo

266670e

- Implemented lightmap controls (resolved #3)

ee37194

Clean up

fdf48ef

Aadjou requested review from tomas-polach and AVGP June 21, 2017 14:44

Added alpha map uniform

41a05ac

AVGP reviewed Jun 26, 2017

View reviewed changes

.gitignore

.apdisk

node_modules

build

Copy link

Contributor

AVGP Jun 26, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tomas-polach the build folder can be added to .gitignore right?

AVGP reviewed Jun 26, 2017

View reviewed changes

madlaina-kalunder added 3 commits June 27, 2017 13:42

Reformated emissive color and set the uniform for emissive color.

f71d1b0

Improved comments that triggered glsl preprocessor

168be3f

Replaced enhancedLightMap properties with straightforward names

487a76a

madlaina-kalunder added 2 commits June 27, 2017 14:30

Removed unused include

a54d300

Marked replaced block clearly

f2c8d60

madlaina-kalunder added 3 commits June 27, 2017 15:10

Fixed specularity

4df98de

Set emissiveColor if the material has lightEmissionCoef

fe337bd

- Restructured BaseMaterial with proper constructor

4cc39cb

AVGP approved these changes Jun 28, 2017

View reviewed changes

AVGP merged commit 02a306f into master Jun 28, 2017

Aadjou deleted the custom-three-material branch June 30, 2017 15:48

frederic-schwarz mentioned this pull request Jul 18, 2017

Design API for custom materials in a-frame component "io3d-data3d" #15

Open

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Custom three.js shader material #4

Custom three.js shader material #4

Aadjou commented Jun 21, 2017

AVGP left a comment •

edited

Loading

AVGP Jun 23, 2017

Aadjou Jun 27, 2017

Aadjou Jun 28, 2017

AVGP Jun 26, 2017

AVGP Jun 26, 2017

Aadjou Jun 27, 2017 •

edited

Loading

AVGP Jun 26, 2017

AVGP Jun 26, 2017

Aadjou Jun 27, 2017

Aadjou Jun 27, 2017

Aadjou Jun 27, 2017

AVGP Jun 26, 2017

Aadjou Jun 27, 2017

AVGP Jun 26, 2017

Aadjou Jun 27, 2017 •

edited

Loading

Aadjou Jun 27, 2017

AVGP Jun 26, 2017

Aadjou Jun 27, 2017

Aadjou Jun 27, 2017

AVGP Jun 26, 2017

Aadjou Jun 27, 2017

AVGP Jun 26, 2017

Aadjou Jun 27, 2017

AVGP Jun 26, 2017

Aadjou Jun 27, 2017

AVGP Jun 26, 2017

Aadjou Jun 27, 2017

AVGP Jun 26, 2017

stavros-papadopoulos Jun 27, 2017

Aadjou Jun 27, 2017

AVGP commented Jun 27, 2017 via email

AVGP left a comment

AVGP Jun 28, 2017


		<a-entity light="type: point; intensity: 0.75; distance: 50; decay: 2" position="-3.2 -2.2 -4.5"></a-entity>


		if (_attributes.colorEmissive) {
		if (_attributes.colorEmissive && material3d.emissive) {


		vec3 totalEmissiveRadiance = emissive;

		#include <logdepthbuf_fragment>

		RE_IndirectDiffuse( irradiance, geometry, material, reflectedLight );

		#endif

Custom three.js shader material #4

Custom three.js shader material #4

Conversation

Aadjou commented Jun 21, 2017

AVGP left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Aadjou Jun 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Aadjou Jun 27, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AVGP commented Jun 27, 2017 via email

AVGP left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AVGP left a comment •

edited

Loading

Aadjou Jun 27, 2017 •

edited

Loading

Aadjou Jun 27, 2017 •

edited

Loading