glTF Loader issue - duck is dark #702

Closed
cx20 opened this Issue Jan 3, 2017 · 20 comments

Projects

None yet

6 participants

@cx20
cx20 commented Jan 3, 2017

I tried to display the glTF sample using X3DOM 1.7.2.
http://jsdo.it/cx20/40CU

However, the duck seems to be dark.
image

Below is the trace result with WebGL Inspector.
image

I think that it is probably because some uniform variables are not set.

@andreasplesch
Contributor
andreasplesch commented Jan 3, 2017 edited

AFAIK x3dom glTF does not support textures or any kind of material yet, only geometry.

Apologies for any confusion caused by my ignorance.

@mlimper
Member
mlimper commented Jan 4, 2017

This example already loads textures from a glb file, using the ExternalShape node to internally define the right appearance:
https://examples.x3dom.org/gltf/

@skluge Could you have a look?

@mlimper
Member
mlimper commented Jan 4, 2017 edited

I think that it is probably because some uniform variables are not set.

Ah, well, X3DOM does not load ligthing from the glb, only geometry & materials. This is actually intended:

  • ExternalGeometry loads Geometry
  • ExternalShape loads Geometry + Appearance (Material)

The rest should be defined by the usual X3D(OM) ways. Could you try adding a directional light, enabling the headlight, etc.? The headlight should be enabled by default.

@cx20
cx20 commented Jan 4, 2017

Could you try adding a directional light, enabling the headlight, etc.?

I tried adding the following tags, but the result remained unchanged and it remained dark.

<navigationInfo id="head" headlight='true' type='"EXAMINE"'>  </navigationInfo> 

GLSL seems to be using the built-in shader of glTF model.
I think that colors are not displayed unless values are set for u_light0Transform and u_Light0Color.
Below is the GLSL for fragment shader which seems to be used.

// Fragment Shader
precision highp float;
varying vec3 v_normal;
uniform vec4 u_ambient;
varying vec2 v_texcoord0;
uniform sampler2D u_diffuse;
uniform vec4 u_emission;
uniform vec4 u_specular;
uniform float u_shininess;
varying vec3 v_light0Direction; // should be set
varying vec3 v_position;
uniform vec3 u_light0Color; // should be set
void main(void) {
  vec3 normal = normalize(v_normal);
  vec4 color = vec4(0., 0., 0., 0.);
  vec4 diffuse = vec4(0., 0., 0., 1.);
  vec3 diffuseLight = vec3(0., 0., 0.);
  vec4 emission;
  vec4 ambient;
  vec4 specular;
  ambient = u_ambient;
  diffuse = texture2D(u_diffuse, v_texcoord0);
  emission = u_emission;
  specular = u_specular;
  vec3 specularLight = vec3(0., 0., 0.);
  {
    float specularIntensity = 0.;
    float attenuation = 1.0;
    vec3 l = normalize(v_light0Direction);
    vec3 viewDir = -normalize(v_position);
    vec3 h = normalize(l+viewDir);
    specularIntensity = max(0., pow(max(dot(normal,h), 0.) , u_shininess)) * attenuation;
    specularLight += u_light0Color * specularIntensity;
    diffuseLight += u_light0Color * max(dot(normal,l), 0.) * attenuation;
  }
  specular.xyz *= specularLight;
  color.xyz += specular.xyz;
  diffuse.xyz *= diffuseLight;
  color.xyz += diffuse.xyz;
  color.xyz += emission.xyz;
  color = vec4(color.rgb * diffuse.a, diffuse.a);
  gl_FragColor = color;
}

I tried to forcibly set the light in Firefox 's shader editor and confirmed that color will be displayed.
image

@dmorehead

..This likely will not help w/ textures as "ExternalShape" does not allow you to override the the Material/Shader but "ExternalGeometry" does, ..only globally though i.e. not specific materials but *ALL materials in the model

http://codepen.io/mrmernan/pen/VPYJKj?editors=1000#

@mlimper
Member
mlimper commented Jan 6, 2017

Looks like a problem is that the shader from the glTF supposes a specific naming of the uniforms, concretely: that your light has a direction vector called "v_light0Direction". Doing a search for "gltf semantic" let me end up here: KhronosGroup/glTF#93 Ideally, the glTF would tell us which uniform has which semantic, so X3DOM has a chance to name things accordingly (for example, by replacing the string inside the shader code with X3DOMs naming)

@skluge Do you have any thoughts or suggestions on this topic?

@mlimper
Member
mlimper commented Jan 6, 2017
@skluge
Contributor
skluge commented Jan 6, 2017

@mlimper
I think you are correct. The best way would be to use the semantics to map the custom lighting uniforms to the x3dom ones.

I will try to do this, but first i am working on fixing some bugs and i am a bit busy right now. So it will take some days.

@skluge
Contributor
skluge commented Jan 9, 2017 edited

Having looked at the case for some time i have to revert my previous answer.

@mlimper
The semantic mapping won't work, because glTF has no semantics for lights. Only with the KHR_Material_Commons extension you have the ability to define lights with the specific parameters.

As for the duck example:
In my opinion it is the correct result, that the duck is rendered dark.
The duck glTF brings its own custom shader with custom parameters. These parameters should have specified values inside the glTF file. But since there is no value specified for the unifom light0Color, the duck is black.

So I think, with standard glTF we have no way to map glTF technique uniforms to vendor specific uniforms. To maybe solve this I could only think of extending the x3d-ExternalShape node to define custom mappings.

@mlimper
Member
mlimper commented Jan 9, 2017

Thanks!

@pjcozzi
FYI, seems we are stumbling about some issue with undefined semantics for lights used within custom shaders - do you have an idea what the current state of glTF is, with respect to issues like this?

@cx20
cx20 commented Jan 9, 2017

@skluge

I do not understand this very well, but there seems to be light information in the glTF file, but can not this be specified?

But since there is no value specified for the unifom light0Color

https://github.com/javagl/JglTF/releases/tag/v0.0.0-alpha04
image

@skluge
Contributor
skluge commented Jan 9, 2017

@cx20
Thanks for the hint, you are totally right. I did not realize, that the technique parameters can have default values, I only looked at the material values.
This is in fact a bug in the x3dom glTF Loader, because the technique parameter default value is not read and used. I will fix this tomorrow as soon as possible.

@skluge
Contributor
skluge commented Jan 10, 2017

I have added a pull request with the bug fix for the custom glTF Material Shader.
The duck is rendered correct now.

duck

@cx20
Thanks for your help by pointing out the bug.

@cx20
cx20 commented Jan 10, 2017

Thank you for fixing!

I'd like to check the display on other models as well.
Are you sure you want to add X3DOM test results to the following list?
https://github.com/cx20/gltf-test
https://cx20.github.io/gltf-test/

@mlimper
Member
mlimper commented Jan 10, 2017

Thanks a lot! Now we can really render glTF assets with custom shaders! So cool! :-)

@mlimper
Member
mlimper commented Jan 10, 2017

Are you sure you want to add X3DOM test results to the following list?

Hey, that's a pretty cool summary page. If we could have the current state of X3DOMs glTF support there, it will be appreciated.

@mlimper mlimper closed this Jan 10, 2017
@cx20
cx20 commented Jan 10, 2017

OK. I will add it to the list.

By the way, where is the latest version? Is it the URL below?
Https://x3dom.org/download/dev/

@mlimper
Member
mlimper commented Jan 10, 2017

By the way, where is the latest version? Is it the URL below?
Https://x3dom.org/download/dev/

Yes, that's it. We usually have to trigger re-generation to make it up-to-date with very recent commits (such as the merge by @skluge), which I just did. Also, the hash of the respective latest commit can always be seen in the head of the X3DOM js file.

@cx20
cx20 commented Jan 10, 2017

I added the X3DOM samples below :)
https://cx20.github.io/gltf-test/

image

There seems to be some other models that can not be displayed correctly, so we will separately submit Issue.

@pjcozzi
pjcozzi commented Jan 12, 2017

@pjcozzi
FYI, seems we are stumbling about some issue with undefined semantics for lights used within custom shaders - do you have an idea what the current state of glTF is, with respect to issues like this?

The plan is to move GLSL to an extension, part of PBR to core, finish the material_common extension, extract lights from material_common for use with both material_common and PBR (or bring your own engines lights). I'll have more communication on this soon in the main glTF repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment