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

Format should support references and inline data #69

Closed
donny-dont opened this issue Apr 16, 2013 · 12 comments
Closed

Format should support references and inline data #69

donny-dont opened this issue Apr 16, 2013 · 12 comments

Comments

@donny-dont
Copy link

As an example the program schema looks like

{
    "attributes": [
        {
            "semantic": "POSITION",
            "symbol": "position",
            "type": "vec3"
        }
    ],
    "fragmentShader": "fragmentShader_id",
    "vertexShader": "vertexShader_id",
    "extra": {
        "Application specific": "The extra object can contain any properties."
    }
}

It would be useful to be able to declare the shaders inline.

{
    "attributes": [
        {
            "semantic": "POSITION",
            "symbol": "position",
            "type": "vec3"
        }
    ],
    "fragmentShader": {
        "name": "application-visibleglobally-uniquename",
        "path": "vertexShader.glsl"
    },
    "uniforms": [],
    "vertexShader": {
        "name": "application-visibleglobally-uniquename",
        "path": "vertexShader.glsl"
    },
    "extra": {
        "Application specific": "The extra object can contain any properties."
    }
}

This should also be true for other portions of the format. As an example the pass format does not support this.

{
    "program": "program_name",
    "states": {
        "depthTestEnable": true
    },
    "extra": {
        "Application specific": "The extra object can contain any properties."
    }
}
@fabrobinet
Copy link
Contributor

I believe having properties which can be either a string for an id or another type like an object to declare the property content directly, would make the implementation more complex on the client side.
This design would also lead to one dead end situation where it is not possible to distinguish between an id and a property value that is a string too.

@donny-dont
Copy link
Author

I would argue that having references, like what currently exists in program, makes the implementation way more complex. Rather than just reading the file now I have to look up the reference to a shader that is parsed in a different location.

I get that there are two forces at play here. On one hand you want a really simple format because it'll be easier for people to parse. On the other hand the killer with rendering is switching states and references make this redundant data go away.

If you had a shader program you wanted to share with me the simplest way to do it would be.

{
    "attributes": [
        {
            "semantic": "POSITION",
            "symbol": "position",
            "type": "vec3"
        }
    ],
    "fragmentShader": {
        "name": "application-visibleglobally-uniquename",
        "path": "vertexShader.glsl"
    },
    "uniforms": [],
    "vertexShader": {
        "name": "application-visibleglobally-uniquename",
        "path": "vertexShader.glsl"
    },
    "extra": {
        "Application specific": "The extra object can contain any properties."
    }
}

Parsing is really straightforward.

Now if I were giving you a large number of shaders and I wanted to be efficient in file size and rendering something like this would be the way to go.

{
  "programs": [
    {
      "name": "alphaedTextureShader",
      "fragmentShader": "texWithAlphaFrag",
      "vertexShader": "simpleTexVert"
    },
    {
      "name": "simpleTextureShader",
      "fragmentShader": "simpleTexFrag",
      "vertexShader": "simpleTexVert"
    }
  ],
  "shaders": [
    {
      "name": "simpleTexVert",
      ....
    },
    {
      "name": "simpleTexFrag",
      ....
    },
    {
      "name": "texWithAlphaFrag",
      ....
    }
  ]
}

I don't see how you reach a dead end with this. If it's a string then its a reference. If its a Map then its a schema that needs to be parsed. In pseudo-code it would look like this.

if (value is String) {
  // I'm a reference you need to find me
} else if (value is Map) {
  // I'm a map so my data is inline just parse me
} else {
  // Somethings wrong here!
}

I really see this format as lego blocks. Each schema is its own thing and I can cherry pick the ones I want for my rendering. When parsing it either the data is handed directly to me or I have to go look it up. By having everything have a global ID I can handle the cases where a reference is needed or if its inline I can just parse it straight away.

Having the simple case makes it easy enough to start supporting the format. The more complex case allows me to create a more efficient rendering.

@RemiArnaud
Copy link
Contributor

I agree that the shaders should be packed in one file, overall size will be small, and reducing the number of http queries is important in that case. On the other hand, it is important that loading several glTFs will not lead into creating the same shader multiple times. I would recommend using unique id for shaders. In that case please rename 'name' into 'id' in the proposal.

note: In COLLADA one can specify both a name and id for an element. They have 2 different purposes. id are unique. names are not. names are used to classify objects, just like 'class' in html. 'id' are exactly like 'id' in XML.

@pjcozzi
Copy link
Member

pjcozzi commented Apr 16, 2013

@RemiArnaud the spec already allows including all (or some shaders) as part of the JSON:

GLSL source can be in external plain-text .glsl files. The URL may also be a plain-text data URI to facilitate storing all model assets in a single .json for easy deployment, drag and drop, etc. Using a data URI reduces the number of requests for a single model. However, if several models are loaded that use the same shader, using separate .glsl files may be better due to HTTP caching. This can be negotiated via a REST API.

@donny-dont if I understand your proposal, you are suggesting that places that take an id should also be able to take the object that id would reference. Correct?

I'm on the fence. I agree, it is not that hard to support on the client, but it does introduce risk of a slow client implementation (e.g., multiple copies of the same shader or other resource) and makes the format more complicated because there are more options. We add options when a strong enough use case supports it. See Reasonable Flexibility in the spec.

Can you tell us more about your lego blocks example? Personally, I've been looking at glTF for a whole asset, but have been thinking about using it's FX system - once we finish it - throughout my engine. So if we have a strong enough use case here, we can consider it.

@donny-dont
Copy link
Author

@donny-dont if I understand your proposal, you are suggesting that places that take an id should also be able to take the object that id would reference. Correct?

You are correct here.

I'm on the fence. I agree, it is not that hard to support on the client, but it does introduce risk of a slow client implementation (e.g., multiple copies of the same shader or other resource) and makes the format more complicated because there are more options. We add options when a strong enough use case supports it. See Reasonable Flexibility in the spec.

Right that's why I said there's the concern of making it easy but at the same time making it fast. To me the fast way is to go references as that way the data is defined once, but it comes at a cost of making the loader more complicated. Having it inline makes it simpler for the client, but if the resource is used in multiple locations there's a chance for duplication.

By having either a reference or inline data you get both and you can have a smart loader or a dumb loader dependent on what you want to do.

Can you tell us more about your lego blocks example? Personally, I've been looking at glTF for a whole asset, but have been thinking about using it's FX system - once we finish it - throughout my engine. So if we have a strong enough use case here, we can consider it.

I have not been looking at glTF as a whole asset solution as there are things I want to support but others I have absolutely no plans to support, such as node and camera. By having glTF be essentially lego blocks I can pick and choose what I actually support. JSON is really amenable to this sort of setup.

So lets say I'm writing a game. I want to load up the shaders that are specific for a level. I go ahead and make a request and the file comes back with all the shader programs and shaders. This would be similar to

{
  "programs": [
    {
      "name": "alphaedTextureShader",
      "fragmentShader": "texWithAlphaFrag",
      "vertexShader": "simpleTexVert"
    },
    {
      "name": "simpleTextureShader",
      "fragmentShader": "simpleTexFrag",
      "vertexShader": "simpleTexVert"
    }
  ],
  "shaders": [
    {
      "name": "simpleTexVert",
      ....
    },
    {
      "name": "simpleTexFrag",
      ....
    },
    {
      "name": "texWithAlphaFrag",
      ....
    }
  ]
}

Now I'm good to go. I want to get a mesh I go ahead and make a request. Maybe its for one mesh maybe its for many. Maybe animations are embedded in the file, maybe each animation is a separate request ala MD5. I get a lot more flexibility if I don't consider glTF a monolithic format but instead a bunch of building blocks I can use or not use.

Additionally with this sort of lego block setup you don't have contention among the community. There's not much one could argue about with regards to loading a shader program. There's a program it has an id. It has a vertex shader which has source code and an id. There's a fragment shader which has source code and an id. Jam them together and you have a program.

Once you go higher is where you get contention. Should a scene graph be in a format? To me no it shouldn't which is why I'm not going to support it. Same goes with a camera. Don't want. Material format? Maybe.

I do plan on hitting a bunch of the lower level functionality and I don't think that's as hard of a sell as the full glTF specification.

@fabrobinet
Copy link
Contributor

Thanks for all input.
Well, defining shaders this way (in a array) instead of using unique ids as the key would be inconsistent with the rest of the spec and I don't see the benefit. it looks also less efficient, because these shaders will be referred from potentially different programs and you just want to use a map and not check names one by one within an array.

Then, maybe the right word was not complex but ambiguous... we should not have a property being one type or another without any other type information to avoid ambiguity.Yes, it could be a specified that we can get just a map or string for values here, but no.. I prefer not going into this kind of corner if not absolutely necessary. Then the code you shown to test the type is not kind of code I'd like client to write, instead if feels more robust to check wether you have a code or path property...

I mentioned it's a dead end because this design couldn't be extended/generalized as it would not be possible to differentiate between a string as a value and an id.

Also I agreed to have the shader inline, so that's the main point of the discussion. not right in the program but instead within a shader object.

something like
"shaders" : { "blahVS" : { "code" : "void main()..." } }

@fabrobinet
Copy link
Contributor

Related to #68 which tracks specifically shaders.

@fabrobinet
Copy link
Contributor

Also, this discuss different issues, having programs is indeed something worth considering. I thought about it too.

@pjcozzi
Copy link
Member

pjcozzi commented Apr 17, 2013

@donny-dont thanks for the detailed writeup. I see your vision and think there is value in adopting glTF piecemeal, especially for effects. However, I'm not sure that allowing objects in place of an id significantly increases the piecemeal usability enough to justify complicating the spec and making a fast and compliant renderer harder to implement. We're trying hard to keep the format fast by default. Given this and that I tend to err on the side of being conservative, I would table this for post glTF 1.0.

@donny-dont this is still well thought-out feedback, and I'm looking forward to more from you.

@fabrobinet
Copy link
Contributor

I believe this is resolved by using the "instance" concept as explained in #92 and also programs will allow to share a program as suggested by don above.

@pjcozzi
Copy link
Member

pjcozzi commented Oct 22, 2013

I believe this is resolved by using the "instance" concept as explained in #92 and also programs will allow to share a program as suggested by don above.

Is this resolved?

@fabrobinet
Copy link
Contributor

Closing this one in favor of tracking inline data in #68 where we got more in details.

@pjcozzi pjcozzi mentioned this issue Mar 7, 2014
10 tasks
javagl added a commit to javagl/glTF that referenced this issue Aug 8, 2022
…-browser/com.fasterxml.jackson.core-jackson-databind-2.13.2.1

Bump jackson-databind from 2.13.1 to 2.13.2.1 in /jgltf-browser
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants