glTF 2.0 #67

lilleyse · 2017-04-19T22:09:24Z

This has initial support for gltf 2.0.

I would like some input on my approach on converting traditional diffuse/specular materials to pbr metallic-roughness materials. These are the basic steps:

Specular intensity is extracted from the specular color and treated as the metallic factor.
Specular shininess is typically an exponent from 0 to 1000, and is converted to a 0-1 range as the roughness factor.
Diffuse stays as is

Also I have one comment left in the code that I'm wondering about. Is there a convention for lighting models when they don't contain normals? Should pbrMetallicRoughness be left undefined and let the implementation figure it out? Right now I do keep the pbr material but set all the values to either black or 0.0, and set the emissive color instead. I'm wondering what different glTF 2.0 viewers might do with a material like this.

To-do:

More manual testing. Fix tests.
Figure out how to get pngjs to write out a 24-bit png with their sync packer. May require a PR there.

pjcozzi · 2017-04-20T14:36:27Z

@sbtron @bghgary @lexaknyazev @mlimper @cedricpinson @erich666 do you have any input on this approach to convert diffuse/specular materials to PBR metal roughness? See #67 (comment)

emackey · 2017-04-20T15:09:38Z

My $0.02, I don't agree with the traditional specular intensity being used as PBR metalness. Traditional diffuse/specular (non-PBR) models never look metallic, so I think it's fine to hard-code metalness to zero for such conversions.

The roughness value comes from both specular intensity and shininess. When specular intensity is very low or zero, roughness should be very high, regardless of the shininess setting. When specular intensity is above near-zero, then you have to look at the shininess factor to determine the roughness. I think generally the higher shininess exponents correspond to smoother surfaces, and lower ones to rough surfaces. I would have to play around with some sample models to see what looks visually similar, though.

xelatihy · 2017-04-20T15:24:21Z

The approach you mention is quite lossy. Things will look very poor. We has this discussion some time ago when I was pushing the diffuse-specular instead, but I got voted out.

Simple thing to do:

convert to PBR diffuse-specular extension
from that loossily convert to PBR metal-roughness
for the specular exponent, use the conversion suggest in the EGSR paper on GGX

Example code (not finished) in yocto-gl.

bghgary · 2017-04-20T17:04:59Z

@xelatihy Can you point to a link for the EGSR paper for the conversion you mentioned?

I did some experiments with this when we were talking about PBR parameter sets. For context, here is the thread. I would recommend doing what @xelatihy said in 1 and 2, not sure about 3. The conversion from spec-gloss to metal-rough is lossy and none of them are perfect.

The live demo of my conversion is here. It is based on the math described here. It's documented in the specs here and here. There is also a note about conversion in the spec-gloss extension appendix. The conversion is not perfect and materials sometimes end up more metallic than they should be. I've been meaning to document how I ended up with the conversion equations, but haven't gotten around to it. Let me know if documenting it will be useful.

We have been using this conversion to convert spec-gloss to metal-rough for a few of our models in our testing repo. The Avocado and BarramundiFish are examples of the conversion.

Spec-Gloss:

Converted to Metal-Rough:

You can toggle back and forth between the two workflows with the drop-down.

bghgary · 2017-04-20T17:15:42Z

Is there a convention for lighting models when they don't contain normals?

We had a small discussion about calculating normals KhronosGroup/glTF@fb8b472#commitcomment-21728860. We ended up saying that flat normals will be used if they are not specified in the asset.

This is specified in an implementation note in https://github.com/KhronosGroup/glTF/tree/2.0/specification/2.0#meshes

pjcozzi · 2017-04-21T11:41:06Z

There's some great discussion on twitter, https://twitter.com/pjcozzi/status/855067965309558784

(also above, #67 (comment))

xelatihy · 2017-04-21T14:16:25Z

BTW, for the PBR material I should also mention that you should look at the OBJ illum property.

emackey · 2017-04-21T14:27:04Z

This is great discussion of the ideal math by @xelatihy and @bghgary. But allow me to offer a completely different point of view, from a typical artist-type user attempting to use this obj2gltf tool. I feel like I have a half-decent understanding of the current workflow for at least some artists who are using OBJ and PBR, after talking to a few in person, watching many online tutorials, and using some of the tools myself to create some sample models (via Collada since this OBJ tool is not yet up to the task). The current workflow goes something like this:

Start creating the model in a package such as Modo, Blender, etc. Much of the focus at this stage is on geometry, normals, and a reasonable UV map. Some tools at this stage have some awareness of PBR (I think Modo can show PBR in its viewport, and Blender has plans to do likewise sometime next year), but most modeling tools do not have the final say on PBR. Generally the model only has "placeholder" textures at this point, or has an "ID" map (one solid color per material, just to indicate material locations on a model).
The model is exported to an intermediate format, such as Collada, FBX, or OBJ. Since this PR is for the OBJ converter, we'll assume OBJ export here. Since we're assuming OBJ format, we know that no actual PBR information was written in step 1, since OBJ does not support that. This is common practice.
A different set of tools come into play to create a full set of PBR textures for the OBJ file. This could be Substance Painter, Marmoset Toolbag, or even just Photoshop if the artist has some way to preview what the OBJ looks like when combined with PBR textures. This could even mean loading the model into an engine like Unity 3D, and interactively assigning PBR textures there.

The important thing to understand here is the situation the artist is placed in by the end of step 3: The OBJ still exists, un-modified from step 2. Likewise, some PBR textures now exist, created in step 3 to pair up with the UV map that's baked into the OBJ. But the OBJ doesn't call out the PBR textures by filename, as there are several of those per-material, and they didn't exist yet when the OBJ was created.

In fact, in the particular case of a truck model I got from one of our 3D model artists here (Scott), the texturemap name baked into the OBJ is unreadable ("F:\Scott\GroundVehicle\groundvehicle.png doesn't exist"). He sent me the OBJ and a small stack of PBR textures exported from Substance Painter, for two different materials in this particular OBJ. The PBR maps are named like this, which is quite typical:

GroundVehicle_body_BaseColor.png
GroundVehicle_body_MetalRough.png
GroundVehicle_body_Occlusion.png
GroundVehicle_body_Normal.png
GroundVehicle_wheels_BaseColor.png
GroundVehicle_wheels_MetalRough.png
GroundVehicle_wheels_Occlusion.png
GroundVehicle_wheels_Normal.png

So for him to use this obj2gltf tool himself, he would need a way to specify the OBJ file, and then specify all eight of these PBR textures, and have the tool understand which texture was assigned to what PBR channel of which material in the OBJ. Plus, the tool would need to disregard the actual texture filename that was specified in the OBJ file itself, as that was just a work-in-progress, non-PBR texture name.

The command-line may not be the right approach here. Of course artists have never liked command lines, but in this case it's particularly egregious as the number of files involved increases by 4 per material.

I think what artists like Scott need would be more along the lines of a GUI that can load an OBJ, and then allow interactive assignment of textures to PBR channels per material, ideally with a live preview window, and then save the result to glTF 2.0. Actually I have some hope that my vscode extension will someday fill this role, but currently vscode extensions don't have a way to open file dialogs (there's an issue filed) and that puts a crimp in those plans for now.

It may be fair to say that interactive assignment of PBR maps is out-of-scope for obj2gltf for now. If that's the case, then some replacement priorities bubble up:

obj2gltf should be easy to embed and use within larger tools, such as the interactive one I describe.
obj2gltf should be quite tolerant of OBJ textures that don't exist, as the textures there may be completely meaningless. Maybe even put a flag to disregard any textures called for by the OBJ (I don't have an F drive on my machine for example, and I don't want the tool reading from any filenames it found in the OBJ file). Keep in mind that the contents of the OBJ pre-date the artists's own efforts to create PBR textures for the model.
obj2gltf should correctly preserve geometry, normals, and especially preserve UV maps even if the texture is not found or intentionally disregarded.
obj2gltf should create stubs for the materials with the correct material names assigned. These could have a simple base-color map assigned from the OBJ's texture filename, unless the user opted to disregard OBJ textures, in which case these would just be named empty material stubs.

With these qualities, obj2gltf could be used as the initial model load stage of a future interactive tool for assigning PBR textures to glTF files. For users who aren't using PBR, they should just stick to KHR_material_common, or the tool could attempt to do some sort of up-conversion via the spec/gloss PBR workflow as described by other participants here. But I see this as lower priority, mostly because the result can't compete with actual PBR assets like Scott's truck. OBJ simply doesn't hold the full suite of PBR texturemaps, and there's no authoring tool that tries to squeeze PBR into OBJ itself, so there's limited value in attempting to extract it back out.

I would much rather see obj2gltf focus itself on enabling professional PBR, and not attempting to up-convert from non-PBR. All of this is just my personal opinion of course :)

donmccurdy · 2017-04-26T19:49:18Z

I think what artists like Scott need would be more along the lines of a GUI that can load an OBJ, and then allow interactive assignment of textures to PBR channels per material, ideally with a live preview window, and then save the result to glTF 2.0.

I think this could be supported in the gltf-viewer I've been working on (demo, source). Potential workflow:

Import .gltf asset without PBR maps.
Select a mesh from scene graph.
Drag-and-drop PBR maps and assign.
Preview.
Export .gltf result.

I would have a few more questions before trying to implement something like that, probably more appropriate for a separate issue on the glTF repo.

In any case, +1 from me on trying to avoid up-conversion to PBR in obj2gltf.

pjcozzi · 2017-04-27T11:49:42Z

It sounds great to have the flexibility that @emackey suggests (contributions welcome, of course!); however, obj2gltf will still need some reasonable default way to create glTF models without the user having to think about the materials. like today's workflow. This can be built on top of the flexibility, but we are making the barrier-to-entry for glTF too high if we go without it.

donmccurdy · 2017-04-27T13:21:29Z

obj2gltf will still need some reasonable default way to create glTF models without the user having to think about the materials. like today's workflow.

I agree that we need good defaults, and an OBJ going through obj2gltf should yield a similar-as-possible glTF asset. But why should obj2gltf up-convert from phong to PBR by default, when phong can be defined simply with the KHR_materials_common extension? That conversion seems more appropriate for gltf-pipeline than obj2gltf, in my opinion.

pjcozzi · 2017-04-27T14:48:24Z

If KHR_materials_common becomes widely supported, that could be the right approach, but as of now, the extension is not finished, ratified, or widely implemented, so we still need a mapping to PBR.

emackey · 2017-04-28T20:32:07Z

so we still need a mapping to PBR.

Perhaps we can make this tool align with what COLLADA2GLTF is doing? Rob (@lasalvavida) describes that process in a comment here.

lasalvavida · 2017-04-28T20:48:18Z

You can find the C++ implementation for this here.

In general:
diffuse(Color) -> baseColorFactor
diffuse(Texture) -> baseColorTexture
emissive(Color) -> emissiveFactor
emissive(Texture) -> emissiveTexture
ambient(Texture) -> occlusionTexture
bump -> normalTexture

If we have pbrSpecularGlossiness:
diffuse(Color) -> diffuseFactor
diffuse(Texture) -> diffuseTexture
specular(Color -> specularFactor
specular(Texture) -> specularGlossinessTexture
shininess -> glossinessFactor

And then metallicFactor, roughnessFactor, and metallicRoughnessTexture all have to be specified via command-line as there is no direct mapping.

lasalvavida · 2017-04-28T20:49:49Z

Also if anyone has any suggested changes or improvements on this approach I'm happy to hear them; I'm sure many of the people reading this know a ton more about rendering than I do, and may be able to help make this mapping more robust

emackey · 2017-05-02T20:02:22Z

And then metallicFactor, roughnessFactor, and metallicRoughnessTexture all have to be specified via command-line as there is no direct mapping.

Actually, for metal/rough models, the ambient texture might actually be a combination of occlusion (R), roughness (G), and metalness (B) in a single map. I could imagine a command-line switch to enable usage of such a combined texture from the ambient channel in both the occlusionTexture and metallicRoughnessTexture channels (using the same texture index for both).

pjcozzi · 2017-05-10T17:05:41Z

Adding @bghgary who has been thinking about this.

lilleyse · 2017-05-10T17:52:54Z

The code is still not 100% ready, but I've pretty much converged on the workflow for this tool.

The command line now has three options:

metallicRoughness - The values in the mtl file are already metallic-roughness PBR values and no conversion step should be applied. Metallic is stored in the Ks and map_Ks slots and roughness is stored in the Ns and map_Ns slots.
specularGlossiness - The values in the mtl file are already specular-glossiness PBR values and no conversion step should be applied. Specular is stored in the Ks and map_Ks slots and glossiness is stored in the Ns and map_Ns slots. The glTF will be saved with the KHR_materials_pbrSpecularGlossiness extension.
materialsCommon - The glTF will be saved with the KHR_materials_common extension. Good for assets with traditional materials and engines that support this extension.
TODO : add command that outputs in KHR_technique_webgl

If none of these are supplied, the material is converted from traditional to metallic-roughness PBR.

The alternative of passing in metallic/roughness/specular/glossiness values/textures via command line seemed like too much work and maintenance, particularly when multiple materials are involved. This method keeps it simple, but requires that the obj is ready-to-go. As @emackey and @donmccurdy discussed a gui may be more suitable for the task of connecting pbr maps to fix the possibly irrelevant .mtl file.

I've decided to keep the traditional->metallic-roughness conversion pretty simple. I started to watch a video about how an artist would convert traditional textures to pbr, and there's too much magic involved to code a proper conversion in here. Along those lines I couldn't find a good way to convert traditional to specular-glossiness, they sound the same but seem to represent completely different concepts. Looking at some sample PBR values here the traditional diffuse would not map well to the specular-glossiness diffuse.

So I mainly went with @emackey's suggestion and decided that this tool should just produce good enough values for simple models that are exported at default settings from blender and other programs, but should not be a full blown converter. For example I don't bother converting the specular/shininess textures to PBR because it just feels beyond the scope.

The new conversion looks like:

diffuse -> base color
specular intensity and specular shininess -> roughness
metallic -> 0

emackey · 2017-05-10T18:08:59Z

@lilleyse This all sounds great. One question though (I haven't looked at the code changes yet), why do you use both map_Ks and map_Ns? Are you reading those maps, combining channels, and outputting a new map made of combined channels? What if the user already has the maps combined with the channels?

And perhaps most importantly, what if the user has a map with combined occlusion (R), roughness (G), and metalness (B) as a single texture already, do you support that? That should be the most common case at least for coming out of Substance Painter (and Unity I think) and going to glTF.

lilleyse · 2017-05-10T18:41:39Z

Are you reading those maps, combining channels, and outputting a new map made of combined channels? What if the user already has the maps combined with the channels?

Right now the assumption is that no textures are packed together. The converter will pack metallic, roughness, and occlusion maps together.

emackey · 2017-05-10T18:54:40Z

The converter will pack metallic, roughness, and occlusion maps together.

I'm sure there are situations where that could be useful. But of course "classic" OBJ models don't have these new-fangled PBR maps to start with, so only PBR-aware models would ever need this channel re-packing. Most tools that produce PBR maps will produce them pre-packed, either with configurable mapping or with their own hard-coded mapping. But generally these tools do not produce a pile of loose channels like this in my experience. So having the OBJ converter do this automatically, by default, strikes me as problematic.

There should at the very least be an option to enable or disable texture repacking (vs just embedding texture names in the glTF and not loading or altering the textures themselves). Probably this should be off-by-default, as neither classic OBJs nor PBR OBJs typically need this kind of remapping.

lilleyse · 2017-05-10T19:07:01Z

I'm not super familiar with all the exporters, do they generally follow the same packing conventions?

likangning93

I think that's it for part 2.

likangning93 · 2017-07-26T17:48:28Z

lib/gltfToGlb.js

+        binaryBuffer = dataUriToBuffer(buffer.uri);
+        delete buffer.uri;
+    } else {
+        binaryBuffer = Buffer.alloc(0);


Does this mean that the glb must have external files if the resources are too large to base64 encode? That seems like it should be noted in the README since the glb spec doesn't seem to have a limit for buffer size.

Yeah this is a weird limitation right now, I'll try fixing it.

likangning93 · 2017-07-26T17:50:48Z

lib/gltfToGlb.js

+module.exports = gltfToGlb;
+
+/**
+ * Convert a glTF to binary glTF.


For future devs, make note of any assumptions this function makes about the glTF.

likangning93 · 2017-07-26T18:55:15Z

lib/loadObj.js

            })
            .catch(function() {
                logger('Could not read image file at ' + imagePath + '. Material will ignore this image.');
            });
    }, {concurrency : 10})
-        .thenReturn(images);
+        .then(function() {


why not thenReturn?

likangning93 · 2017-07-26T19:06:12Z

lib/obj2gltf.js

+ * @param {Boolean} [options.metallicRoughness=false] The values in the mtl file are already metallic-roughness PBR values and no conversion step should be applied. Metallic is stored in the Ks and map_Ks slots and roughness is stored in the Ns and map_Ns slots.
+ * @param {Boolean} [options.specularGlossiness=false] The values in the mtl file are already specular-glossiness PBR values and no conversion step should be applied. Specular is stored in the Ks and map_Ks slots and glossiness is stored in the Ns and map_Ns slots. The glTF will be saved with the KHR_materials_pbrSpecularGlossiness extension.
+ * @param {Boolean} [options.materialsCommon=false] The glTF will be saved with the KHR_materials_common extension.
+ * @param {String} [options.metallicRoughnessOcclusionTexture] Path to the metallic-roughness-occlusion texture used by the model, where occlusion is stored in the red channel, roughness is stored in the green channel, and metallic is stored in the blue channel. This may be used instead of setting texture paths in the .mtl file. The model will be saved with a pbrMetallicRoughness material.


Do the path to XXX texture parameters disallow materials referencing multiple images in the obj? If so, we should document that users of these "advanced features" need to make sure the input texture covers those channels across the entire input obj.

likangning93 · 2017-07-26T19:11:07Z

lib/obj2gltf.js

-        kmcOptions : kmcOptions,
-        textureCompressionOptions : textureCompressionOptions
+
+    var jsonOptions = {


For a very slight performance bump, we could set this to undefined when outputting .glb.

likangning93 · 2017-07-26T19:16:04Z

lib/obj2gltf.js

+    return Promise.map(imagePaths, function(imagePath) {
+        var imageOptions = (imagePath === options.baseColorTexture) ? checkTransparencyOptions : undefined;
+        return loadImage(imagePath, imageOptions);
+    }).then(function(images) {


Include a concurrency limit in case someone throws an obj with a very large number of textures at this.

This place will be ok since it is only loading images passed in above, which would be at most 6.

Images that are loaded from the .mtl file has a concurrency limit.

likangning93 · 2017-07-26T19:28:29Z

lib/writeUris.js

    }
+
+    var source = getBufferPadded(Buffer.concat(sources));


Do the buffers within sources also need to be 4-byte aligned? It seems possible for image sources to not be aligned.

I don't think they need to be. I forget the exact rules but I think byte alignment is only required for buffer views holding accessor data.

likangning93 · 2017-07-26T19:32:51Z

specs/lib/createGltfSpec.js

-        expect(Object.keys(gltf.meshes).length).toBe(3);
+        expect(gltf.materials.length).toBe(3);
+        expect(gltf.nodes.length).toBe(4);
+        expect(gltf.nodes[0].mesh).toBeUndefined();


I guess for "correctness" we would want this to check the node(s) in gltf.scenes[gltf.scene].

likangning93 · 2017-07-26T19:37:27Z

specs/lib/createGltfSpec.js

+        });
+    });
+
+    describe('specularGlosiness', function() {


specularGlosiness -> specularGlossiness?

lilleyse · 2017-07-27T15:26:29Z

@likangning93 I think I address everything except #67 (comment). I'm going to work on that a bit later.

donmccurdy · 2017-07-27T15:46:31Z

I just tested this on a bunch of simple OBJs I had lying around. Looks good and nice work!

likangning93

A few more comments, and I think I replied to some older comments after the most recent commit. Otherwise, new changes look pretty good.

likangning93 · 2017-07-27T19:23:34Z

lib/createGltf.js


    if (packMetallic) {
        // Write into the B channel
-        var metallicChannel = getImageChannel(metallicImage, 0, width, height);
-        writeChannel(pixels, metallicChannel, 2, width, height);
+        var metallicChannel = getImageChannel(metallicImage, 0, width, height, scratchChannel);


Since it's possible that the returned channel is a newly allocated buffer, should this (and below) update scratchChannel as well? Otherwise this might still cause repeated reallocation in some cases. Seems like it's "fine" if scratchChannel is bigger than it needs to be.

scratchChannel will stay the same size, but sometimes scratchResizeChannel may need to be re-allocated. I think this is okay given that best practice is to pass textures that are the same dimensions.

likangning93 · 2017-07-27T19:26:03Z

lib/createGltf.js

+        var blueChannel = getImageChannel(specularImage, 2, width, height, scratchChannel);
+        writeChannel(pixels, redChannel, 0);
+        writeChannel(pixels, greenChannel, 1);
+        writeChannel(pixels, blueChannel, 2);


Should these be interleaved? Otherwise it seems like this will writeChannel the blue channel repeatedly.

Whoops... yes!

likangning93 · 2017-07-27T19:27:38Z

lib/loadImage.js

+                    info.transparent = hasTransparency(info);
+                }
+                return info;
+            });
    }
    return info;


Should this return Promise.resolve(info)?

I refactored this area a bit so this is no longer a problem.

However, this was okay because nothing was calling .then on the result, and a promise chain had already started before.

pjcozzi · 2017-08-02T16:53:59Z

@lilleyse what is the plan here? 😄

lilleyse · 2017-08-02T17:43:03Z

Still finishing up the tests after reorganizing how materials are loaded. It won't be read by the glTF bof, but is close.

pjcozzi · 2017-08-02T17:52:10Z

Rock on and merge when ready!

…riting file

lilleyse · 2017-08-08T19:18:52Z

@likangning93 this is now ready for final review. I got a bit carried away and went on a cleaning/reorganization spree. Sorry about the large diff yet again. I believe it is final

The major changes since before are:

No longer saves the converted glTF from within the API, instead returns the JSON or glb and the caller is responsible for saving it. Saving separate resources is still supported - but requires either passing an outputDirectry or a custom writer callback.
Writing glb's no longer has the buffer size limitation
Materials, PBR texture creation, and handling of overriding textures is now all in loadMtl. This keeps createGltf a lot leaner.
Consistent about naming for image/texture. Texture is now used throughout, except when referring to gltf.images.

mramato · 2017-08-08T19:30:51Z

No longer saves the converted glTF from within the API, instead returns the JSON or glb and the caller is responsible for saving it. Saving separate resources is still supported - but requires either passing an outputDirectry or a custom writer callback.

Doesn't have to be this PR, but eventually we should support a stream-based implementation so that you simply write objects to a user-supplied stream, that way you could handle multi-file output without having to write to a specific directory (think DB import)

lilleyse · 2017-08-10T13:52:54Z

@likangning93 merged in master, all ready now.

likangning93

Just a few more comments, mostly about future stuff or documentation.

likangning93 · 2017-08-10T17:22:15Z

lib/createGltf.js

+    return gltf;
+}
+
+function addBufferView(gltf, buffers, accessors, byteStride, target) {


Doesn't have to be in this PR, but just since there's this many parameters and it seems like there's some assumptions being made, it might be nice for future devs if there's a private JSDoc.

likangning93 · 2017-08-10T18:06:41Z

lib/createGltf.js

-
-        // Add meshes to node
-        var meshes = node.meshes;
+        var meshes = nodes[i].meshes;


Probably should be done in a separate PR, but should this check be per-mesh instead of over the whole asset? I guess the added complexity of supporting different types of indices within one gltf might mess with buffer/bufferView code in here too.

That's what I used to do before I realized Cesium does not like different index types being in the same buffer view. I think the situation has changed a bit since different sized indices wouldn't be allowed in the same bufferView anyways in gltf 2.0.

I do like the simplicity of this approach though, but I'll open an issue since I can think of some models that would require more memory because of this.

likangning93 · 2017-08-10T18:37:19Z

bin/obj2gltf.js

+if (argv.metallicRoughness + argv.specularGlossiness + argv.materialsCommon > 1) {
+    console.error('Only one material type may be set from [--metallicRoughness, --specularGlossiness, --materialsCommon].');
+    process.exit(1);
+}


It looks like these checks duplicate what's in lib/obj2gltf, I guess is this because we don't want CLI users to see DeveloperErrors?

Yeah that was my only reason.

likangning93 · 2017-08-10T18:53:29Z

lib/writeGltf.js

+            var separateTextures = options.separateTextures;
+
+            var promises = [];
+            if (separateTextures) {


Should this also check separate? Looks like otherwise images end up in the external buffer. If that's intended behavior we should probably document it since it's new to 2.0.

So it's a bit hard to notice, but if separate is true separateTextures will also be true.

In obj2gltf.js

options.separateTextures = defaultValue(options.separateTextures, defaults.separateTextures) || options.separate;

likangning93 · 2017-08-10T18:55:35Z

lib/writeGltf.js

+    }
+}
+
+function encodeTextures(gltf) {


Something for a later PR, can we determine cases where the alpha channel isn't needed and encode to JPEG instead? Seems like something that users will want for smaller assets (spec permitting).

Opened #100

likangning93 · 2017-08-10T19:08:01Z

specs/lib/loadMtlSpec.js

+            options.packOcclusion = true;
+
+            var material = loadMtl._createMaterial({
+                ambientTexture : ambientTexture, // Is a .gif which can't be decoded


It doesn't look like there's any special handling in loadTexture if a gif source is given, should there be a warning to the dev/user? Similar to places where textures can't be found or a default is substituted?

The warning is provided later, but only if the PBR texture is created. Otherwise it will just ignore that it couldn't be decoded.

https://github.com/AnalyticalGraphicsInc/obj2gltf/pull/67/files#diff-959e13a8da36b7c10b96349c54350791R323

https://github.com/AnalyticalGraphicsInc/obj2gltf/pull/67/files#diff-959e13a8da36b7c10b96349c54350791R199

Okay. Since these are warnings instead of early-fails, this seems fine but might warrant a little documentation.

Added doc to loadMtl.

likangning93 · 2017-08-11T16:06:32Z

Thanks @lilleyse!

lilleyse added 3 commits April 19, 2017 17:53

2.0

ce5221c

Merge branch 'master' into gltf-2.0

d0e8b90

Remove occurences of khr_materials_common for code (not tests yet)

d6d0d39

xeolabs mentioned this pull request Apr 20, 2017

Automatic Phong -> PBR Metal/Rough conversion xeolabs/xeogl#179

Closed

lilleyse added 3 commits May 4, 2017 14:25

Add metallicRoughness and specularGlosiness output

c9ad66f

Merge branch 'master' into gltf-2.0

dc4ba7f

Add back KHR_materials_common

44e9d5f

Tests

c2af8f5

likangning93 reviewed Jul 26, 2017

View reviewed changes

lilleyse force-pushed the gltf-2.0 branch from 51dde96 to 03e8822 Compare July 27, 2017 15:25

lilleyse force-pushed the gltf-2.0 branch from 03e8822 to 9606137 Compare July 27, 2017 15:27

Async image reading/writing and other cleanup

e54f3af

lilleyse force-pushed the gltf-2.0 branch from 9606137 to e54f3af Compare July 27, 2017 15:28

likangning93 reviewed Jul 27, 2017

View reviewed changes

lilleyse added 3 commits July 28, 2017 17:01

Cleanup overriding images

3da691d

Move incompatible argument checking to bin file

487eca9

Fixed interleaving and added comment

cda657e

pjcozzi mentioned this pull request Aug 3, 2017

glTF roadmap - what would you like to see next in glTF? KhronosGroup/glTF#1051

Open

lilleyse force-pushed the gltf-2.0 branch from 1d118f4 to 1e74209 Compare August 8, 2017 19:01

Reorganization of material loading and returning buffer rather than w…

60a080b

…riting file

lilleyse force-pushed the gltf-2.0 branch from 1e74209 to 60a080b Compare August 8, 2017 19:09

Merge branch 'master' into gltf-2.0

72baced

likangning93 reviewed Aug 10, 2017

View reviewed changes

lilleyse mentioned this pull request Aug 11, 2017

Store either uint16 or uint32 indices on a per mesh basis #99

Open

Add doc

b2e4fb6

likangning93 merged commit 7190fe7 into master Aug 11, 2017

likangning93 deleted the gltf-2.0 branch August 11, 2017 16:06

lilleyse mentioned this pull request Aug 23, 2017

bypassPipeline does not allow converting to binary glTF #73

Closed

lilleyse mentioned this pull request Oct 19, 2018

Upgrade KHR_materials_common to PBR CesiumGS/gltf-pipeline#384

Open

glTF 2.0 #67

glTF 2.0 #67

Conversation

lilleyse commented Apr 19, 2017 • edited Loading

pjcozzi commented Apr 20, 2017

emackey commented Apr 20, 2017

xelatihy commented Apr 20, 2017

bghgary commented Apr 20, 2017 • edited Loading

bghgary commented Apr 20, 2017 • edited Loading

pjcozzi commented Apr 21, 2017

xelatihy commented Apr 21, 2017

emackey commented Apr 21, 2017

donmccurdy commented Apr 26, 2017

pjcozzi commented Apr 27, 2017

donmccurdy commented Apr 27, 2017

pjcozzi commented Apr 27, 2017

emackey commented Apr 28, 2017

lasalvavida commented Apr 28, 2017

lasalvavida commented Apr 28, 2017

emackey commented May 2, 2017

pjcozzi commented May 10, 2017

lilleyse commented May 10, 2017 • edited Loading

emackey commented May 10, 2017

lilleyse commented May 10, 2017

emackey commented May 10, 2017

lilleyse commented May 10, 2017

likangning93 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lilleyse commented Jul 27, 2017

donmccurdy commented Jul 27, 2017

likangning93 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pjcozzi commented Aug 2, 2017

lilleyse commented Aug 2, 2017

pjcozzi commented Aug 2, 2017

lilleyse commented Aug 8, 2017

mramato commented Aug 8, 2017

lilleyse commented Aug 10, 2017

likangning93 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

likangning93 commented Aug 11, 2017

lilleyse commented Apr 19, 2017 •

edited

Loading

bghgary commented Apr 20, 2017 •

edited

Loading

bghgary commented Apr 20, 2017 •

edited

Loading

lilleyse commented May 10, 2017 •

edited

Loading