Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

alternative for shaders and states #97

Closed
fabrobinet opened this issue Jun 7, 2013 · 9 comments
Closed

alternative for shaders and states #97

fabrobinet opened this issue Jun 7, 2013 · 9 comments

Comments

@fabrobinet
Copy link
Contributor

For engines ables to generate shaders based on light / material properties for common lighting model we should provide ways to ensure they are not forced to use glTF shaders and even states.

A first approach has been to "specify" parameters. So that just looking at shaders parameters will allow to regenerate shaders. This is documented here

This works for simple cases, but it misses at least 2 things:

First, the ability to associate a material property to texture.
Concretely, if one needs to regenarate a texture fetch for a specular map, he needs to know that say :TEXCOORD_2 is associated with the specular parameter that refer to a texture.
This will allow to write code such as `specularColor = texture2D(u_specularTexture, v_texcoord2).
Thus, This association "material property"(like specular) -> mesh attribute (TEXCOORD_2 in the example), is missing and this is tracked here:
#47

Then we also need to be able to regenerate state. So we should write higher level info, for instance double_sided information and so on... (possibly coming from COLLADA extras) here.

My proposal (a bit rough, but to get is idea..) is to add something like this in technique,
And we'll find a better name than "generator"..

"generator" : {   
               "type" : "COLLADA-1.4.1/commonProfile",
               "commonProfile" : {
                   "technique" : "Phong",
                   "diffuse" : "aColor"               <--- point to paramerer
               "texcoordsBindings" : {
                      "diffuse" : "TEXCOORD_0,
                      "specular": "TEXCOORD_2
                },
               "extras" : {
                   "double_sided" : true;                    
                }
               }
           },
@fabrobinet
Copy link
Contributor Author

@tparisi @pjcozzi Remi and I reached agreement about that with the following changes:
We should export a technique without program for the ones who don't want the use provided shaders and state. I will propose a JSON that show such techniques.

@pjcozzi
Copy link
Member

pjcozzi commented Jun 10, 2013

The rough idea is OK with.

We should export a technique without program for the ones who don't want the use provided shaders and state. I will propose a JSON that show such techniques.

I'm not so sure about this - it would be possible for glTF content to not have the shaders, right? A conformant glTF renderer will need to be able to generate the shaders, which goes against our ideals of keeping the client simple and fast.

I guess we could give the converter an option to drop the shaders to create a non-conformant glTF asset as an optimization, but that defeats the whole purpose of a standard.

@RemiArnaud
Copy link
Contributor

The client is king.

It should be able to ask the converter for what it needs, and get it in a straight forward standardized way.

So we have a few options:

  • declare threejs and other technologies non webGL compliant, and then either fix those technologies to be webGL compliant or wait for the respective communities to make their tools webGL compliant.
  • consider those technologies as clients for webGL, and provide the proper converter options and specification so they can use webGL. This can be done either by providing a common profile in webGL, or specific profiles for each target framework
  • provide the exact list of all the shaders that may be generated by the converter this is my prefered solution. This way it is possible to write a threejs loader that simply look at the name of the shader and map the parameters to corresponding materials.

Note that this conversation is limited to a subset of shaders anyways, as those that can't be described as COLLADA common profile (+ extras) will only work with a client that can read shaders.

  • Remi

@fabrobinet
Copy link
Contributor Author

@RemiArnaud I believe we all agree with the idea here, but precisely, we will not provide a list of all shaders that may be generated - this is totally up to the converter implementation. But maybe you meant "lighting model" (e.g technique name) instead of shader and this way I understand more your conclusion (which I agree with) : "This way it is possible to write a threejs loader that simply look at the name of the shader and map the parameters to corresponding materials" - and this is already the case.

@pjcozzi I hear you. Initially I was against this, I considered a "canonical" GLTF asset to be made of shaders - e.g to contain everything needed for rendering, but now, I am not so sure if allowing to specify an output without shaders would be a bad thing, but we can try a compromise...

One way could be to minimize the generated shaders impact when the "generator" information is requested.
by:

  • sharing parameters for both shaders and "generator" info.
  • force external shaders (not inlined).

This way, the impact of shaders should be pretty small, only the instanceProgram object would have to be ignored by clients who do not want shaders.

@ghost ghost assigned fabrobinet Jun 14, 2013
@fabrobinet
Copy link
Contributor Author

Following our call, we decided to keep shaders in the generated output. Having them external does not impact client, so can just ignore them.

@fabrobinet
Copy link
Contributor Author

Implemented as -d option on the converter 0445397
@tparisi I will follow-up on this by adding examples in /model (at the root of this repo). Just ping me if you need them sooner...

@fabrobinet
Copy link
Contributor Author

Just updated duck.json , so that it gives an idea (in this repo).

@fabrobinet
Copy link
Contributor Author

I'll try to make to time to update the SPEC for this one.

@pjcozzi pjcozzi mentioned this issue Apr 30, 2014
8 tasks
@fabrobinet fabrobinet removed their assignment Mar 19, 2015
@pjcozzi pjcozzi removed this from the Spec 1.0 milestone Aug 27, 2015
@pjcozzi
Copy link
Member

pjcozzi commented Sep 1, 2015

Closing as duplicate with #361.

@pjcozzi pjcozzi closed this as completed Sep 1, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants