state parameters should be an array instead of a list of named parameters

[RemiArnaud]

for example:

                    "states": {
                        "blendEnable": false,
                        "depthMask": true,
                        "depthTestEnable": true
                    },

the spec needs the entire list to be defined so the application know what to expect

[fabrobinet]

Yes, but in the meantime you can have a look at the schemas, the states are listed there.

[RemiArnaud]

https://github.com/KhronosGroup/glTF/blob/master/specification/states.schema.json

[RemiArnaud]

cullFaceEnabled default value is set to 'false' in the schema.
I think it should be 'true'

[RemiArnaud]

depthTestEnable default values is set to 'false' and should be 'true'

[RemiArnaud]

ditherEnable is listed ... I thought glFT would not support dither?

[RemiArnaud]

I would like to change the spec. Instead of using objects when there are multiple parameters, I would like to use an array for the parameters to array.
Since those parameters need to be provided to the gl function in the same order, this would make the code simpler and faster

so instead of

states": {  
   "stencilFunc" : {
           "func" : "LESS",
           "ref" : 0,
            "mask" : 0 }
        },

this would be (I'm not sure of the schema

states": {  
   "stencilFunc" : ["LESS" ,0 ,0] ,
        },

[fabrobinet]

We still need to address the parameters by index, so using keys instead doesn't seem a bit deal.
As a side effect, I wonder if having an array instead of an object here won't make the validation harder. I am not sure if rules can be set per index... I would be surprised if it can. Otherwise the check would be moved to the consistency check but it would be unfortunate if that's just to test value type...

[RemiArnaud]

It's way more complicated in the code, since object properties are not
ordered. Way simpler to get an array and call function.apply(this, array);

The difference is about 50 lines of code, and lost of possible typos.

On Fri, May 3, 2013 at 9:27 AM, Fabrice Robinet notifications@github.comwrote:

We still need to address the parameters by index, so using keys instead
doesn't seem a bit deal.
As a side effect, I wonder if having an array instead of an object here
won't make the validation harder. I am not sure if rules can be set per
index... I would be surprised if it can. Otherwise the check would be moved
to the consistency check but it would be unfortunate if that's just to test
value type...

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/85#issuecomment-17403984
.

[tparisi]

In general I think Fabrice got carried away using properties instead of
arrays. I had this same grip about lists of children in the scene
hierarchy; I thought they should just be in an array and let the loader or
viewer build its own hashed properties. Fab talked me out of this by
convincing me that would require more processing in the loader, so I
backed off. But in this case if properties make this harder to code, let's
please use arrays.

On Fri, May 3, 2013 at 10:11 AM, Rémi Arnaud notifications@github.comwrote:

It's way more complicated in the code, since object properties are not
ordered. Way simpler to get an array and call function.apply(this, array);

The difference is about 50 lines of code, and lost of possible typos.

On Fri, May 3, 2013 at 9:27 AM, Fabrice Robinet notifications@github.comwrote:

We still need to address the parameters by index, so using keys instead
doesn't seem a bit deal.
As a side effect, I wonder if having an array instead of an object here
won't make the validation harder. I am not sure if rules can be set per
index... I would be surprised if it can. Otherwise the check would be
moved
to the consistency check but it would be unfortunate if that's just to
test
value type...

—
Reply to this email directly or view it on GitHub<
https://github.com/KhronosGroup/glTF/issues/85#issuecomment-17403984>
.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/85#issuecomment-17406477
.

Tony Parisi tparisi@gmail.com
CTO at Large 415.902.8002
Skype auradeluxe
Follow me on Twitter! http://twitter.com/auradeluxe
Read my blog at http://www.tonyparisi.com/
Learn WebGL http://learningwebgl.com/

Read my book! WebGL, Up and Running
http://shop.oreilly.com/product/0636920024729.do
http://www.amazon.com/dp/144932357X

[fabrobinet]

@tparisi it's not the same situation here. These properties are scoped within the described function, these are not uniqueIDs meant to be referred by other objects. So arrays can work, but I brought issues related to validation in this case.

[fabrobinet]

@RemiArnaud I see, apply is indeed convenient in this case for WebGL. I just have 2 concerns:

1. How will look the schema ? I'd prefer to not give away validation if possible... (I understand JSON validation is not too official anyway, but it would be nice to check how this will look with what we have today...).
2. It makes glTF tied not only the GL parameters, but to the function signatures also. I don't know if this can bite us someday.

[fabrobinet]

@pjcozzi What do you think ?

[tparisi]

Well how important is validation in this case? I have seen many instances of specs in the past that were designed in less than ideal ways in order to be able to validate input (X3D comes to mind). We can't let validation be the enemy of ease of implementation; the two need to work together in synergy.

Patrick and I were discussing validation the other day and I can't remember what the feature was, but he was saying there were already a few features that couldn't be validated by schema and would require additional tools. @pjcozzi , do you recall?

[fabrobinet]

@RemiArnaud looks like this issue is tracking at least 2 different things.

• missing states
• the fact you would like an array to store states (matching the signature of the GL functions), this would have to be per profile.

To ease @pjcozzi catching up with all this,
Please clean-up the issue by renaming the title, or open another more focused if needed. Thanks.

[pjcozzi]

cullFaceEnabled default value is set to 'false' in the schema. I think it should be 'true'

This does not match the WebGL spec.

depthTestEnable default values is set to 'false' and should be 'true'

This does not match the WebGL spec.

ditherEnable is listed ... I thought glFT would not support dither?

We just haven't removed it yet.

the fact you would like an array to store states (matching the signature of the GL functions), this would have to be per profile.

Things I like about this:

• More concise. Smaller file.
• Sort-of looks like a WebGL call.

Things I don't like:

• What is the performance of function.apply(this, array); vs a direct call? Perhaps insignificant?
• Shouldn't the client convert strings, e.g., "LESS" in the example above, to WebGL constants anyway?
• Harder to understand unless you know the WebGL parameter order. Things are implicit, based on index, instead of explicit.
• Many engines will not call WebGL directly; instead, they will use the glTF render state to assign values to their engine render state. Using arrays to store states will make this code much worse.

I see the trade-off like this: if most clients will never need to access individual properties, then @RemiArnaud's proposal is OK; otherwise, the existing design is.

As a side effect, I wonder if having an array instead of an object here won't make the validation harder

In general, I agree with @tparisi's point that we can't let JSON validation drive the design. Our plan - I believe - has always been to use JSON schema for coarse-grained validation as best as it can, and then to write a fine-grained validator (perhaps in JavaScript using node.js), which can go way beyond what JSON schema can check for.

[tparisi]

r/e validation, we have to get our priorities straight. My favorite war
story is that XML validation drove the design of X3D and, well, you see how
that ended up. (I can throw stones in this case, I was on the core team)

Our priorities should be

1. completeness
2. efficiency
3. understandability (don't get to fancy that implementers can't build a
   reader)
4. validatibility

On Tue, Jun 4, 2013 at 5:25 AM, Patrick Cozzi notifications@github.comwrote:

cullFaceEnabled default value is set to 'false' in the schema. I think it
should be 'true'

This does not match the WebGL spec.

depthTestEnable default values is set to 'false' and should be 'true'

This does not match the WebGL spec.

ditherEnable is listed ... I thought glFT would not support dither?

We just haven't removed it yet.

the fact you would like an array to store states (matching the signature
of the GL functions), this would have to be per profile.

Things I like about this:

• More concise. Smaller file.
• Sort-of looks like a WebGL call.

Things I don't like:

• What is the performance of function.apply(this, array); vs a direct
  call? Perhaps insignificant?
• Shouldn't the client convert strings, e.g., "LESS" in the example
  above, to WebGL constants anyway?
• Harder to understand unless you know the WebGL parameter order.
  Things are implicit, based on index, instead of explicit.
• Many engines will not call WebGL directly; instead, they will use
  the glTF render state to assign values to their engine render state. Using
  arrays to store states will make this code much worse.

I see the trade-off like this: if most clients will never need to access
individual properties, then @RemiArnaud https://github.com/RemiArnaud's
proposal is OK; otherwise, the existing design is.

As a side effect, I wonder if having an array instead of an object here
won't make the validation harder

In general, I agree with @tparisi https://github.com/tparisi's point
that we can't let JSON validation drive the design. Our plan - I believe -
has always been to use JSON schema for coarse-grained validation as best as
it can, and then to write a fine-grained validator (perhaps in JavaScript
using node.js), which can go way beyond what JSON schema can check for.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/85#issuecomment-18905500
.

Tony Parisi tparisi@gmail.com
CTO at Large 415.902.8002
Skype auradeluxe
Follow me on Twitter! http://twitter.com/auradeluxe
Read my blog at http://www.tonyparisi.com/
Learn WebGL http://learningwebgl.com/

Read my book! WebGL, Up and Running
http://shop.oreilly.com/product/0636920024729.do
http://www.amazon.com/dp/144932357X

[fabrobinet]

@RemiArnaud @pjcozzi
interesting page: http://jsperf.com/function-versus-function-call-versus-function-apply
-> On my computer, apply is 77% slower than direct call !!.

@pjcozzi anyhow, as a related fact we need to separate states per profile.

Overall, as pointed out by Patrick, the current design addresses more use cases since one can still access properties by name, for individual access it would be cumbersome to do it by index.
My take on this is that the current solution is less "risky"... and having implemented it was not such a big deal.

[fabrobinet]

I tried a bit more, it looks than on Chrome / Firefox apply is way slower, but on safari direct,call and apply are more of less equivalent.

[RemiArnaud]

I don't understand this discussion. How did we go from using array instead of list of objects in the glTF format end up at looking at performance of apply() vs. native call ? This is a mystery to me... I don't even see the connection.

Can we delete all this discussion about apply and get back to the core of the issue please?

[fabrobinet]

@RemiArnaud this was your comment to propose this:

"states": {
"stencilFunc" : ["LESS" ,0 ,0] ,
},
"
And "It's way more complicated in the code, since object properties are not ordered. Way simpler to get an array and call function.apply(this, array); The difference is about 50 lines of code, and lost of possible typos.
"

[fabrobinet]

@RemiArnaud @pjcozzi if we re-focus on the original issue - matching the title of this issue, it's fine with me too.
Please see some comments above, Patrick answered, looks like the only action that should be taken for sure on our side is to remove dither.

[RemiArnaud]

Ok, my bad, I mentioned implementation specific detail as an example, which de-routed the entire conversation to discuss the example implementation rather than the design itself.

So to get back to the core issue: parameters in javascript are provided/defined by their order in the call. The way to represent this in JSON is to provide an array, which keeps the parameters in the same order of the parameters of the WebGL function call.

In the current specification, the application has to know for every state function which function parameter matches with witch parameter name. - instead of just applying the parameters in the order they come in the glTF.

So, it what strikes me is that we are adding named parameters in glTF, and in turn need to also parse and manage those names for each state in the loader. This is more work, more data, more parsing for nothing. In fact, this is not friendly to adding new states in the future, as applications will have to be edited and maintain this list of named parameters for each new state added.

I have a hard time to see any reason for the current specification.

[RemiArnaud]

I 'fixed' the title

[pjcozzi]

If I understand @RemiArnaud correctly, it sounds like if we design states carefully (especially with respect to enable/disable), then an implementation could do a very small implementation like:

for each property in states
    gl[state].apply(gl, states[state]);

Although I love the conciseness, I don't know how many folks will do this in practice. Most engines will want to access specific states as cleanly as possible (without index). Examples:

• Translate glTF states to the engine's state data structure
• Sort by state
• Only apply state that isn't already set

In addition, using arrays here is inconsistent with the rest of glTF, which maps WebGL parameters to names, not array elements.

[RemiArnaud]

I don't understand

The application can and should manage states, but all the parameters are needed for each state change.

So what is the point of managing each parameter individually?

[RemiArnaud]

Maybe to provide more explanation, I not understand why having access to parameters by name of a state is necessary,

If you want and choke at the idea of using indexes, you can wrap this easily like that:

   paramX: { //paramX is at position 2 in the array of parameters for this state
            get: function() {
                return this._a[1];
            },
            set: function(value) {
                if (this._a[1] != value) {
                    this._a[1] = value;
                }
            }
        },

Another way, less modern, is to create your state object at load time

   if (state.type === 'stateX')   {  
      this.type = state.type;
      this.x = state.values[0];
      this.y = state.values[1];
      this.z = state.values[2];
  }

This is one optional file to write once for everyone to use.

On the other hand, having objects instead of a simple array in the glTF file MANDATE that everybody uses the following type of code:

switch (stateID) {
    case 'blendEquationSeparate':
      arg.push(arg_json.rgb); 
/// Have to list all the possible named parameters for all states and get them in the right order
      arg.push(arg_json.alpha);
      break;

And it make the glFT file bigger for no reason other than code styling.

Like I said there is no need to separate each value of a state with its own name, because a state is atomic, you can't change a part of it, you have to have all the parameters, in the exact order defined for that state.

glTF should be as close to the target platform and as straight forward to load as possible. The current specification mandates the glTF importer to have a parser, when it is definitely not necessary.

So overall, I am saying that in the code you can use array/index or properties (.xxx notation), regardless of how it is stored in glTF, but that I am advocating is that the loader code does not have to do any parsing when not necessary, and that the specification statically defined information should not be part of glTF. There is already a spec that says what parameter is where in each state. So why do I have to duplicate this in each glTF document? Why do I have to have a parser code that duplicate that in the code ?

[fabrobinet]

I agree that this can be more straight forward if we go #96 (comment)

@RemiArnaud @pjcozzi Please try to reach agreement ASAP, I need to move on the code, quite late already.

[pjcozzi]

I still don't agree with this design based on everything I've already said, but I don't want to hold us up. If @RemiArnaud wants to update the spec (and related examples), then it is OK with me if it helps us move forward.

[RemiArnaud]

Will be able to update the spec when back end of July.
I would not get offended if someone does this before I am back online, or if you wait for me to be back.

Also, would love to use values rather than strings in the state parameters.

Regards

[pjcozzi]

Talked to @fabrobinet offline, we agreed to leave the stencil functions for a minor revision and to change the enable property to map from ENUM value to boolean. The full example is:

{
  // This object will be empty/not-present by default since all states are disabled by default
  "enable" : {
    BLEND : false,
    CULL_FACE : false,
    DEPTH_TEST : false,
    POLYGON_OFFSET_FILL : false,
    SAMPLE_ALPHA_TO_COVERAGE : false,
    SAMPLE_COVERAGE : false,
    SCISSOR_TEST : false
  },
  "functions" : {
    "blendColor": [0.0, 0.0, 0.0, 0.0], // (red, green, blue, alpha)
    "blendEquationSeparate" : [FUNC_ADD, FUNC_ADD], // (rgb, alpha)
    "blendFuncSeparate" : [ONE, ONE, ZERO, ZERO], // (srcRGB, srcAlpha, dstRGB, dstAlpha)
    "colorMask" : [true, true, true, true], // (red, green, blue, alpha)
    "cullFace" : [BACK],
    "depthFunc" : [LESS],
    "depthMask" : [true],
    "depthRange" : [0.0, 1.0], // (zNear, zFar)
    "frontFace" : [CCW],
    "lineWidth" : [1.0],
    "polygonOffset" : [0.0, 0.0], // (factor, units)
    "sampleCoverage" : [1.0, false], // (value, invert)
    "scissor" : [0, 0, 0, 0], // (x, y, width, height)
  }
}

Any feedback?

If we are all in agreement, @fabrobinet can you do the converter updates?

Outstanding questions from above (I am OK pushing both of them):

• At some point, we'll need a way to specify gl_PointSize since we allow POINT primitives. Should we add it now?
• Do we want to include color/depth/stencil clears for 1.0 or save them for multipass? Perhaps we could even ask the same question for the stencil functions since they are not useful outside of multipass.

[fabrobinet]

Thanks @patrick , sure I'll wait a bit a and make the converter updates.

• I am for specifying point size
• I prefer to defer color/depth/stencil clears.

[pjcozzi]

I am for specifying point size

This will require implementations to assign to gl_PointSize in the vertex shader. Should we do this in the content pipeline and provide point size in details? We generally don't require shader stitching client-side. We could also expose a uniform semantic for setting gl_PointSize, again, with the shader stitched in the content pipeline.

[fabrobinet]

I would see it as a semantic in parameters.

[pjcozzi]

OK, let's discuss in #83. Anyone have more feedback here?

[pjcozzi]

FYI - I just made a minor update to the above to remove STENCIL_TEST from enable, which I missed.

[fabrobinet]

moving forward and implementing this now, will merge dev-8 into master once it is done.

[fabrobinet]

actually we finally agreed to keep enable property an array.

[fabrobinet]

@pjcozzi I think we should better not touch SAMPLE_COVERAGE . if someone instantiate a context with anti-aliasing, this will be set to true and we don't want this to be impacted by the state management in glTF at this point. And same applies for the sampleCoverage function.

[fabrobinet]

but still, we want to manage SAMPLE_ALPHA_TO_COVERAGE .

[pjcozzi]

I think that will be fine. Let me confirm on Monday when I am available.

[fabrobinet]

damn, what's exported by the converter is not following the proposal.
The functions aren't under functions but are directly under states.
@pjcozzi what's best for you ? get the fix right away for 0.8 (too late?) or dev-9 ?

[fabrobinet]

And it is ready here: #318

[pjcozzi]

Removing SAMPLE_COVERAGE and sampleCoverage, we now have:

{
  "enable" : [BLEND, CULL_FACE, DEPTH_TEST, POLYGON_OFFSET_FILL, SAMPLE_ALPHA_TO_COVERAGE, SCISSOR_TEST], // empty by default
  "functions" : {
    "blendColor": [0.0, 0.0, 0.0, 0.0], // (red, green, blue, alpha)
    "blendEquationSeparate" : [FUNC_ADD, FUNC_ADD], // (rgb, alpha)
    "blendFuncSeparate" : [ONE, ONE, ZERO, ZERO], // (srcRGB, srcAlpha, dstRGB, dstAlpha)
    "colorMask" : [true, true, true, true], // (red, green, blue, alpha)
    "cullFace" : [BACK],
    "depthFunc" : [LESS],
    "depthMask" : [true],
    "depthRange" : [0.0, 1.0], // (zNear, zFar)
    "frontFace" : [CCW],
    "lineWidth" : [1.0],
    "polygonOffset" : [0.0, 0.0], // (factor, units)
    "scissor" : [0, 0, 0, 0], // (x, y, width, height)
  }
}

I am updating the schema in schema-8 now.

[pjcozzi]

Updated schema-8 branch in 8e56d9c.

[pjcozzi]

Useful comment from #249 (duplicate): #249 (comment)

[pjcozzi]

@tparisi here is content on render states for the draft 1.0 spec:

---

Render states define the fixed-function GL state when a primitive is rendered. Each primitive refers to a material, which refers to a technique, which has a pass, which has a state dictionary object defining the fixed-function GL state that should be applied. states contains two properties:

• enable: an array of integers corresponding to boolean GL states that should be enabled using GL's enable function.
• functions: a dictionary object containing properties corresponding to the names of GL state functions to call. Each property is an array, where the elements correspond to the arguments to the GL function.

Valid values for elements in the enable array are: 3042 (BLEND), 2884 (CULL_FACE), 2929 (DEPTH_TEST), 32823 (POLYGON_OFFSET_FILL), 32926 (SAMPLE_ALPHA_TO_COVERAGE), and 3089 (SCISSOR_TEST). If any of these values are not in the array, the GL state should be disabled (which is the GL default state). If the enable array is not defined in the pass, all of these boolean GL states are disabled.

Each property in functions indicates a GL function to call and the arguments to provide. Valid property names are: "blendColor", "blendEquationSeparate", "blendFuncSeparate", "colorMask", "cullFace", "depthFunc", "depthMask", "depthRange", "frontFace", "lineWidth", "polygonOffset", and "scissor". If a property is not defined, the GL state for that function should be set to the default value(s) shown in the example below.

The following example states object indicates to enable all boolean states (see the enable array) and use the default values for all the GL state functions (which could be omitted). Enum strings, instead of integers, are shown for clarity (@tparisi if you don't like this, perhaps do, for example, 3042 /* BLEND */, which isn't valid JSON, but is fine for an example).

"states" : {
    "enable" : [BLEND, CULL_FACE, DEPTH_TEST, POLYGON_OFFSET_FILL, SAMPLE_ALPHA_TO_COVERAGE, SCISSOR_TEST], // empty by default
    "functions" : {
      "blendColor": [0.0, 0.0, 0.0, 0.0], // (red, green, blue, alpha)
      "blendEquationSeparate" : [FUNC_ADD, FUNC_ADD], // (rgb, alpha)
      "blendFuncSeparate" : [ONE, ONE, ZERO, ZERO], // (srcRGB, srcAlpha, dstRGB, dstAlpha)
      "colorMask" : [true, true, true, true], // (red, green, blue, alpha)
      "cullFace" : [BACK],
      "depthFunc" : [LESS],
      "depthMask" : [true],
      "depthRange" : [0.0, 1.0], // (zNear, zFar)
      "frontFace" : [CCW],
      "lineWidth" : [1.0],
      "polygonOffset" : [0.0, 0.0], // (factor, units)
      "scissor" : [0, 0, 0, 0], // (x, y, width, height)
    }
}

The following example shows a typical "states" object for closed opaque geometry. Culling and the depth test are enabled, and all other GL states are their default value.

"states": {
    "enable": [
        2884,
        2929
    ]
}

Implementation Note: It is recommended that a runtime use the minimal number of GL state function calls. This generally means ordering draw calls by technique, and then only making GL state function calls for the states that vary between techniques.

---

Of course, this will also need a table that containing the schema details: https://github.com/KhronosGroup/glTF/blob/spec-1.0/specification/schema/examples/techniques.json