StainedGlassLamp model #292

echadwick-artist · 2021-01-29T17:23:24Z

An example model to test and demonstrate KHR_materials_clearcoat, KHR_materials_ior, KHR_materials_translucency, KHR_materials_transmission, KHR_materials_variants, KHR_materials_volume, and KHR_texture_basisu.

Re-posting #288 into its own branch, with slight updates, and including the source model.

An example model to test and demonstrate KHR_materials_clearcoat, KHR_materials_ior, KHR_materials_translucency, KHR_materials_transmission, KHR_materials_variants, KHR_materials_volume, and KHR_texture_basisu.

cx20 · 2021-01-29T20:54:38Z

I think this model is very nice. However, I am worried about the bloated repository size.
The average size of the folders in the other sample models is 12-13MB, so this model is much larger than that.

Folder Name	Folder Size
2.0/StainedGlassLamp/glTF	29.3 MB
2.0/StainedGlassLamp/glTF-Binary	29.3 MB
2.0/StainedGlassLamp/glTF-KTX	13.2 MB
2.0/StainedGlassLamp/glTF-Translucency	27.5 MB
2.0/StainedGlassLamp/glTF-Variants	29.4 MB
2.0/StainedGlassLamp/screenshot	3.4 MB
sourceModels/StainedGlassLamp	60.5 MB
Total	192.6 MB

For reference, here is the folder size information for other samples.

echadwick-artist · 2021-02-01T15:09:44Z

Thanks for this. A few ideas:

Should I compress each folder with KTX, then remove the redundant \glTF-KTX ?
Should we keep the textures in sourceModels as PNG files?
Should I replace the (larger) .Max model in sourceModels with FBX or OBJ (smaller file)?
Should down-sample the PNGs in sourceModels to match the image sizes used in the glTFs? These are mostly 2048 PNGs.
Should I remove any of the model versions?

lexaknyazev · 2021-02-01T15:35:24Z

Should I compress each folder with KTX, then remove the redundant \glTF-KTX ?

No, since people implementing new PBR extensions may lack KTX support.

Should we keep the textures in sourceModels as PNG files?

Yes.

Should I replace the (larger) .Max model in sourceModels with FBX or OBJ (smaller file)?

The sourceModels asset should contain proper materials, so OBJ is probably not an option.

Should down-sample the PNGs in sourceModels to match the image sizes used in the glTFs? These are mostly 2048 PNGs.

This repo has lots of 2048 PNGs, so it should be fine. In fact, it may be even better to use the same PNGs (if possible) for source models and glTFs so that git could deduplicate them internally.

Should I remove any of the model versions?

In my opinion, the GLB version doesn't add much value to this sample model.

Also please remove the validation info from the readme since that'll change eventually.

cx20 · 2021-02-02T12:32:15Z

Should I remove any of the model versions?

I compared each model with Babylon.js. It looks the same as glTF / glTF-Binary / glTF-KTX / glTF-Variants (Lamp on).
What about the idea of removing the same looking glTF / glTF-Binary and promoting glTF-Variants to glTF?
If we remove the two variations, we save 60 MB.

I will link to the following issue as a related topic.
#272

* Redundant GLB removed. * Variant promoted to \glTF. * 2048x2048 textures duplicated from sourceModel, to enable git to deduplicate them internally. * Renamed assets to use "StainedGlassModel" prefix instead of "FDLV3095" SKU prefix. * glTF-KTX recompressed using ETC1S, to get the asset closer to the file size recommended by 3D Commerce guidelines. * DS-PBR render updated. * Removed validation info from README as it will be outdated soon. * Various small README improvements.

* Removed the model with KHR_materials_translucency, until it's been ratified. * Changed the KTX model to use no extensions except KHR_textures_basisu. * Added a model using JPG and PNG textures, with no extensions, to compare file sizes between it and the KTX version.

cx20 · 2021-02-26T18:09:17Z

@echadwick-wayfair Thank you for organizing the folder of models.

Is it possible to change the folder name to glTF-BasisU instead of glTF-KTX?
The reason for this is that the folder name for the KHR_texture_basisu version of FlightHelmet is glTF-BasisU.
See: #285

And if possible, To be consistent with other samples, it is better to name the sample model file the same as the folder name is a good idea.

Folder Name	File Name
`Duck`/glTF	`Duck`.gltf
`Duck`/glTF-Binary	`Duck`.glb

Folder Name	File Name
`StainedGlassLamp`/glTF	`StainedGlassLamp`.gltf
`StainedGlassLamp`/glTF-JPG-PNG	`StainedGlassLamp`.gltf
`StainedGlassLamp`/glTF-BasisU	`StainedGlassLamp`.gltf

rsahlin · 2021-03-04T08:27:16Z

Is it possible to change the folder name to glTF-BasisU instead of glTF-KTX?
The reason for this is that the folder name for the KHR_texture_basisu version of FlightHelmet is glTF-BasisU.
See: #285

Hi @cx20 and thanks for your comments.
I think we should think about this for a moment.
As I understand we should start using KTX2 (or simply KTX) when referring to 'supercompressed' (BasisU) assets.
This is so that we have a consistent way of communicating.

I know that the extension that adds the support for supercompressed textures is called KHR_texture_basisu but the carrier is KTX (version 2)

cx20 · 2021-03-04T11:40:39Z

@rsahlin Thanks for letting me know about your concerns.
I'm not too particular about the folder name glTF-BasisU. I simply suggested it as a way to make folders with similar meanings more consistent in the repository.

@bghgary Do you have any advice on folder names for this model?

bghgary · 2021-03-05T00:57:06Z

As I understand we should start using KTX2 (or simply KTX) when referring to 'supercompressed' (BasisU) assets.

Hmm, I haven't heard this. KTX files are not necessarily supercompressed BasisU files.

I would recommend calling it glTF-BasisU.

cx20 · 2021-03-05T03:25:25Z

For reference, I have listed the names currently used for glTF folders. So far, glTF-JPG-PNG and glTF-KTX have not been used.

glTF
glTF-BasisU
glTF-Binary
glTF-Draco
glTF-Embedded
glTF-Embedded-buffer
glTF-IBL
glTF-pbrSpecularGlossiness
glTF-Quantized

(I don't think there will be any significant impact by increasing the folder name. The only impact would be that I would have to add a few more if statements when I use gltf-test to display it.)

rsahlin · 2021-03-05T07:35:10Z

I would recommend calling it glTF-BasisU.

What would the drawback be with calling it glTF-KTX2?

From a broader ecosystem perspective I think it makes sense to try to unify how we name our features.
I know that historically we usually use the technical name of the feature.
In this case, to make it easier for the rest of the ecosystem to understand I think we should consider using KTX2 instead of BasisU.

lexaknyazev · 2021-03-05T09:12:00Z

The glTF extension is called KHR_texture_basisu.
KTX v2 can handle much more payload formats than just Basis Universal supercompression.
There will be other KTX use cases within glTF.

That said, we could use glTF-KTX-BasisU to give the most precise context possible.

emackey · 2021-03-07T14:23:41Z

Once the folder name is settled (and it looks like it is, from @lexaknyazev's comment above), we should update the files here and in other PRs that offer KTX, such as #285.

@rsahlin

* Copyright section added, Generator section shortened. * glTF files renamed to match root folder. * JPG-PNG version recompressed to use mostly JPG files. * KTX replaced with high-quality version created by @rsahlin . * KTX folder renamed to match convention. * Readme updated with new screenshots and information.

For details, see readme.md in push 8bee860 in pull request KhronosGroup/glTF-Sample-Models#292

Correcting the readme: only the baseColor textures with alpha were kept as PNG. Normal maps were not kept as PNG, they were converted to JPGs along with all the rest. Also added the reasoning behind keeping those PNGs.

Model proportions adjusted to match reference photo better. Stained-glass textures recreated to reduce stretching & aliasing. Stained-glass transmission roughness reduced to improve translucency. Normal map "scale" reset to 1 to improve bump quality. KTX vs JPG chart added to readme.

echadwick-artist · 2021-03-30T15:44:32Z

I noticed KTX2 model has some problems:

The glTF is missing extensionsUsed and extensionsRequired
The model doesn't finish loading in https://modelviewer.dev/examples/tester.html
glTF Sample Viewer can't load the model either:

I'm re-compressing the textures to see if this fixes the problems.

echadwick-artist · 2021-03-30T16:43:19Z

Ah it was missing the extension definitions in the Textures section!

I'll submit a fix soon.

The KTX glTF was refusing the load in glTF Sample Viewer and <model-viewer>, so I added missing data to the glTF: extensionsUsed, extensionsRequired, extensions within Textures. Recompressed the textures too, just in case.

bghgary · 2021-04-07T17:01:17Z

Somewhat minor, but there is a seam on the lamp. It appears in at least in the bump normal, roughness, metallic maps. Not sure what's causing it.

* No emissive on gems, beads. * Roughness adjusted for glass w/ transmission. * Volume & IOR added to gems/beads. * Various geometry fixes: Cord exit added under base, fixed interpenetrations between steel hangers and amber beads. * Texture tiling fixes on stained glass. (thanks for the nudge Gary!) * Brighter emissive for top part of base. * Re-compress all KTX with latest RDO & Linear parameters. * A/B/C versions created: no extensions, clearcoat/transmission/variants, clearcoat/transmission/variants/volume/IOR. * File size/memory size data collected for all versions. * Pathtracer version created for dsPBR use, with emissive textures disabled and bulb emissiveFactor increased to 100. * Screenshots created with Babylon.js, using manual override to avoid transmission roughness bug (Volume IOR = 1.15). * GLBs uploaded to 3D-Formats-Guidelines for demo use.

echadwick-artist

The pathtracer version created for dsPBR use is actually in a separate commit, see https://github.com/KhronosGroup/3D-Formats-Guidelines/tree/main/samples/StainedGlassLamp

cx20 · 2021-04-10T01:06:44Z

@echadwick-wayfair

I tried the latest version of the model locally.
However, Lamp on is not displayed in Variants. Is this the intended change? I think it was displayed before.

"extensions": {
"KHR_materials_variants": {
    "variants": [
        {
            "name": "Lamp off"
        }
    ]
}

echadwick-artist · 2021-04-10T12:22:12Z

Thanks for the review. Yes that is intended. It seems to me the "lamp on" variant is redundant since it's a duplicate of the original state.

cx20 · 2021-04-10T16:30:55Z

Thanks for the review. Yes that is intended. It seems to me the "lamp on" variant is redundant since it's a duplicate of the original state.

I see, it is the same idea as the default camera.
I will have to modify the example program because I have not taken into account the default state of variants in the gltf-test.
Currently, gltf-test just shows the list in the extension in variants.

[updated 2021.04.11] I added the default state to variants in gltf-test.
I have modified the gltf-test example to show the default state to variants in Babylon.js and Three.js. I don't know how to change the default material in RedCube.js, so I need to check with the library author.

* Model proportions adjusted to match reference photo better. * Stained-glass textures recreated to reduce stretching & aliasing. * Stained-glass transmission roughness reduced to improve translucency. * No emissive on gems, beads. * Volume & IOR added to gems/beads. * Various geometry fixes: Cord exit added under base, fixed interpenetrations between steel hangers and amber beads. * Texture tiling fixes on stained glass. (thanks for the nudge Gary!) * Brighter emissive for top part of base.

echadwick-artist · 2021-06-07T13:39:20Z

Is there anything else that needs to be done with this asset to resolve the merge? Thanks.

emackey · 2021-06-07T14:36:03Z

The choice between "Lamp off" vs "default" seems far less intuitive than the old "Lamp off" vs "Lamp on". I think the KHR_material_variants spec itself needs a bit of clarification. Perhaps we can give the default state a name somehow, or ask clients to detect whether the default state is redundant with the named states and simply not show it when it's redundant. Or we could ask implementors of variants to always include the default state among the variants, and say that clients are not required to offer the default state, and should instead detect its name from matching it to the list of named states or something.

It seems like something that needs discussion at the spec level before we commit to a model like this one that forces clients to show a nameless state.

1. Variant added for the "on" state, so the model has a specific variant for the default state. See pull request 292 for discussion on default variant states. 2. Links to specific pages on Wayfair.com have been removed from the readme; this is no longer an active product so it no longer has a "live" webpage.

emackey · 2021-11-03T20:44:34Z

Thanks again @echadwick-wayfair for this amazing model!

StainedGlassLamp model

de58574

An example model to test and demonstrate KHR_materials_clearcoat, KHR_materials_ior, KHR_materials_translucency, KHR_materials_transmission, KHR_materials_variants, KHR_materials_volume, and KHR_texture_basisu.

echadwick-artist mentioned this pull request Jan 29, 2021

StainedGlassLamp #288

Closed

echadwick-artist added 2 commits February 8, 2021 17:01

echadwick-artist added a commit to KhronosGroup/3D-Formats-Guidelines that referenced this pull request Mar 8, 2021

high quality KTX by @rsahlin

4fa1ecf

For details, see readme.md in push 8bee860 in pull request KhronosGroup/glTF-Sample-Models#292

echadwick-artist added 2 commits March 8, 2021 19:40

JPG-PNG info fixed

875ed27

Correcting the readme: only the baseColor textures with alpha were kept as PNG. Normal maps were not kept as PNG, they were converted to JPGs along with all the rest. Also added the reasoning behind keeping those PNGs.

glTF formatting, textures recompressed

f4d6a0f

The KTX glTF was refusing the load in glTF Sample Viewer and <model-viewer>, so I added missing data to the glTF: extensionsUsed, extensionsRequired, extensions within Textures. Recompressed the textures too, just in case.

echadwick-artist mentioned this pull request Apr 3, 2021

KHR_materials_volume KhronosGroup/glTF#1726

Merged

echadwick-artist commented Apr 9, 2021

View reviewed changes

echadwick-artist mentioned this pull request Apr 12, 2021

Refraction? KhronosGroup/glTF-Sample-Environments#7

Closed

cx20 mentioned this pull request Oct 10, 2021

glTF2 models all using png instead of ktx2 #334

Closed

emackey merged commit 7100cd5 into master Nov 3, 2021

emackey deleted the stained-glass-lamp branch November 3, 2021 20:44

emackey mentioned this pull request Nov 3, 2021

Add KHR_texture_basisu version of the FlightHelmet #285

Merged

2 tasks

donmccurdy mentioned this pull request Sep 26, 2022

KHR_texture_basisu samples KhronosGroup/glTF#1750

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

StainedGlassLamp model #292

StainedGlassLamp model #292

echadwick-artist commented Jan 29, 2021 •

edited

Loading

cx20 commented Jan 29, 2021 •

edited

Loading

echadwick-artist commented Feb 1, 2021

lexaknyazev commented Feb 1, 2021

cx20 commented Feb 2, 2021

cx20 commented Feb 26, 2021

rsahlin commented Mar 4, 2021

cx20 commented Mar 4, 2021

bghgary commented Mar 5, 2021

cx20 commented Mar 5, 2021

rsahlin commented Mar 5, 2021

lexaknyazev commented Mar 5, 2021

emackey commented Mar 7, 2021

echadwick-artist commented Mar 30, 2021

echadwick-artist commented Mar 30, 2021

bghgary commented Apr 7, 2021 •

edited

Loading

echadwick-artist left a comment

cx20 commented Apr 10, 2021

echadwick-artist commented Apr 10, 2021

cx20 commented Apr 10, 2021 •

edited

Loading

echadwick-artist commented Jun 7, 2021

emackey commented Jun 7, 2021

emackey commented Nov 3, 2021

StainedGlassLamp model #292

StainedGlassLamp model #292

Conversation

echadwick-artist commented Jan 29, 2021 • edited Loading

cx20 commented Jan 29, 2021 • edited Loading

echadwick-artist commented Feb 1, 2021

lexaknyazev commented Feb 1, 2021

cx20 commented Feb 2, 2021

cx20 commented Feb 26, 2021

rsahlin commented Mar 4, 2021

cx20 commented Mar 4, 2021

bghgary commented Mar 5, 2021

cx20 commented Mar 5, 2021

rsahlin commented Mar 5, 2021

lexaknyazev commented Mar 5, 2021

emackey commented Mar 7, 2021

echadwick-artist commented Mar 30, 2021

echadwick-artist commented Mar 30, 2021

bghgary commented Apr 7, 2021 • edited Loading

echadwick-artist left a comment

Choose a reason for hiding this comment

cx20 commented Apr 10, 2021

echadwick-artist commented Apr 10, 2021

cx20 commented Apr 10, 2021 • edited Loading

echadwick-artist commented Jun 7, 2021

emackey commented Jun 7, 2021

emackey commented Nov 3, 2021

echadwick-artist commented Jan 29, 2021 •

edited

Loading

cx20 commented Jan 29, 2021 •

edited

Loading

bghgary commented Apr 7, 2021 •

edited

Loading

cx20 commented Apr 10, 2021 •

edited

Loading