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

add support for pre-multiplied alpha. #19

Closed
fabrobinet opened this issue Mar 20, 2013 · 16 comments
Closed

add support for pre-multiplied alpha. #19

fabrobinet opened this issue Mar 20, 2013 · 16 comments
Labels

Comments

@fabrobinet
Copy link
Contributor

No description provided.

@pjcozzi
Copy link
Member

pjcozzi commented Apr 18, 2013

CC #15 - related issue for spec.

@fabrobinet
Copy link
Contributor Author

@pjcozzi @RemiArnaud @tparisi we need to specify how shaders are generated. With premultiplied alpha or not ? that's a context info on WebGL. Where should we put this in glTF ? in profile ? a root property to carry all context infos ? any suggestion ?

@pjcozzi
Copy link
Member

pjcozzi commented Oct 22, 2013

Why does glTF need to support the WebGL context-level premultiplied alpha?

Why not just support UNPACK_PREMULTIPLY_ALPHA_WEBGL for textures?

@fabrobinet
Copy link
Contributor Author

Shader are normally written/thought with or without premultiplied , this goes not only for texture but for computed colors in shaders too. We must support both and tell developers how the context should be set to be able to render correctly provided shaders.

@pjcozzi
Copy link
Member

pjcozzi commented Oct 22, 2013

This really feels outside the scope of glTF. This is an asset format, not a scene format. I don't know how we expect assets to dictate how the context should be created. That implies that I'm able to create/recreate the context when I encounter the asset. ha. Not likely for real apps.

@fabrobinet
Copy link
Contributor Author

The following comments only applies when developers want to use shaders generated by the converter. (or any other tool...).

Here is how I see things:

  • It is not an option for glTF to enforce support for only pre-multiplied or not by SPEC. (you didn't suggest that, but I want this to be explicit).
  • The converter should provide options to generate shaders for pre-multiplied alpha or not.
  • 2 assets shouldn't be merged in a scene if one has shaders generated using pre-multiplied and one not. It will look wrong. How can a developer could check this ?
  • Say I have a glTF asset with custom shader, I want to update shaders in this glTF asset as post processing step, how do I know if the shaders have been generated with pre-multiplied on ? And this would not fit in the details property because it would be mandatory and global.

Overall, I understand what your concern is @pjcozzi but if we provide shaders, pre-multiplied alpha is an implementation details that is one aspect driving how shaders are generated.

And shaders are implementation... so we shouldn't be surprised that kind of details surface eventually.
Wherever it is and how it is specified (could be a hint ?) should be exposed somewhere in the glTF file.

Before going further about the way to implement this, I'd like we agree on the principle.

Also I mentioned, again a converter option, let's have a configuration file to manage options better #166

@pjcozzi
Copy link
Member

pjcozzi commented Oct 24, 2013

Ah, I see. This is really just asset-level metadata on how the shader was generated. OK with me. (Also good info on this topic here).

Do you have JSON examples for your proposals? If this were a root-level property, what potential future properties would it include?

@fabrobinet
Copy link
Contributor Author

Well, I have an urgent need for this, so I'll add a converter option and and extra in asset which looks to be a good candidate until we reach an agreement for this.
something like:

    "asset": {        
        "generator": "collada2gltf@042d7d2a3782aaf6d86961d052fc53bea8b3e424",
        "extras" : {
           "premultipliedAlpha" : true
        }
    },

Toggling premultipliedAlpha will impact the generated shaders and states (blend func).

@fabrobinet
Copy link
Contributor Author

Also, after this change the default in the converter will change.
We were generating states for non-premultiplied alpha, but we will align with WebGL who has premultipliedAlpha switched on...

@fabrobinet
Copy link
Contributor Author

just pushed the change (merged from mini branch premulatiplied-alpha-rework) e7a5277

@pjcozzi
Copy link
Member

pjcozzi commented Dec 30, 2013

@fabrobinet do you have any ideas for what the final JSON should look like?

Perhaps it is as simple as

"asset": {        
    "generator": "collada2gltf@042d7d2a3782aaf6d86961d052fc53bea8b3e424",
    "premultipliedAlpha" : true
},

Or do you see premultipliedAlpha being on a sub-property of asset?

@fabrobinet
Copy link
Contributor Author

@pjcozzi Yes, that's what I had in mind, but I have put an extras before we reached agreement.
@RemiArnaud , @tparisi OK with you ? I plan to move forward quickly on this low-hanging fruit.

@pjcozzi
Copy link
Member

pjcozzi commented Feb 19, 2014

I'm working on the schema now (in the new schema branch). Can we move premultipliedAlpha directly under asset for dev-6?

"asset": {        
    "generator": "collada2gltf@042d7d2a3782aaf6d86961d052fc53bea8b3e424",
    "premultipliedAlpha" : true
},

@fabrobinet
Copy link
Contributor Author

Fixed in dev-6.

@pjcozzi
Copy link
Member

pjcozzi commented May 15, 2014

@fabrobinet OK to close?

@fabrobinet
Copy link
Contributor Author

yes

lexaknyazev pushed a commit to lexaknyazev/glTF that referenced this issue Feb 26, 2024
…-anim-notes-and-extension-samples

Added clarification regarding material animation - thx a lot.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants