Export textures in original format unless consolidation is requested

[alexey-pelykh]

Current behaviour:

1. Import GLB with separate O/R/M JPEG textures
2. Export GLB
3. Observe growth of file size due to O/R/M textures were packed into single ORM texture and saved as PNG instead of JPEG (while it could've been used)

Proposed behaviour:

1. Import GLB with separate O/R/M JPEG textures
2. Export GLB
3. Observe growth of file size due to O/R/M textures were packed into single ORM texture and saved as JPEG since all source textures were JPEG

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[scurest]

See #818 and #882. The "Automatic" mode for images currently prefers losslessness. If you want to change it, we'll presumably need to bring back the "PNG" option too.

[alexey-pelykh]

@scurest this PR has nothing to do with disabling "losslessness" of Automatic mode. Right now if Blender merges 2 JPEG textures into one, it always saves it as PNG. What's lossless about that? And if does not merge JPEG texture, it saves it as JPEG.

[scurest]

Please look at #818.

If you pack two images into the channels of one image and save as a PNG, you can extract the channels and get the original images back. Thus its "lossless". If you save as JPEG, the channels have artifacts compared to the original images.

[alexey-pelykh]

Indeed, this totally slipped my mind. Yet then it means that ideally it would be "keep images as-is" mode instead of trying to pack them in order to avoid exploding GLB file size, as such packing does more harm than value.

[alexey-pelykh]

@scurest this I guess is a WIP then, as needs more feedback, yet it seems that this part could use a little more attention

[alexey-pelykh]

Would it be possible to request some sort of review on this one? @scurest

[scurest]

You'd need to attract the attention of a maintainer. @julienduroure @emackey @donmccurdy

[donmccurdy]

Is the existing JPEG option not enough here? It seems like the proposed option is very specifically to get O/R/M textures exported as JPEG, but not Normal textures?

[alexey-pelykh]

@donmccurdy not really, JPEG option forces JPEG. Automatic also does a bit more magic that wanted - it repacks separate O/R/M textures (that we have in partially JPEG and PNG) into a single PNG texture, exploding the GLTF size as a result. Intent of the ASIS option is to avoid this behaviour, and basically asking exporter to leave textures as much alone as possible.

[donmccurdy]

Ok, thanks. I think my concern is that we're adding a packing decision to what is (or was) a format dropdown. Could this be done as a separate checkbox, and keep the choice of format independent? For example:

• Pack textures

For the moment that would only affect O/R/M and Clearcoat/ClearcoatRoughness, but there may be others in the future. Then you can choose whether you want textures combined independently of the file format.

[donmccurdy]

^ @scurest I think this would solve the question we discussed earlier, about whether occlusion should still be packed with metal/rough if the images are different sizes?

[alexey-pelykh]

@donmccurdy probably adding a checkbox and keeping format selection just the format selection would be the ultimate solution. What would be the default behaviour? "Automatic" for format, that's obvious, yet what about packing?

[emackey]

Be careful: "Pack textures" has a very specific meaning to Blender users, it means to store the texture data in the Blender project. Users also have a choice of glTF Binary vs glTF Separate, which can pack textures into GLB files or not.

Is there really value in having a format dropdown? I think we've been using it as a texture export strategy dropdown, in practice. If an AS-IS option is really needed, that's probably the place it goes.

[emackey]

Alternately, I could be OK with a "Merge ORM textures" checkbox. But even if you turn that off, metal and rough at least still need to get merged regardless of the user's wishes.

[donmccurdy]

We don't want something specific to ORM textures, do we? The same logic should apply to Clearcoat/ClearcoatRoughness, and maybe other textures on the way?

Another option... maybe we just shouldn't combine textures that don't need to be combined. If the user gives us a single O/R/M texture we export it that way. If they give us O + R/M separately we keep them separate. And if R + M are separate we still have to merge those by necessity.

[alexey-pelykh]

@donmccurdy not that it matters but I completely agree with what you've suggested. Doing extra magic behind the scenes for good ended up a bit for bad in some cases.

[alexey-pelykh]

@donmccurdy @emackey what would be the next steps here?

[alexey-pelykh]

ping?

[donmccurdy]

Another option... maybe we just shouldn't combine textures that don't need to be combined. If the user gives us a single O/R/M texture we export it that way. If they give us O + R/M separately we keep them separate. And if R + M are separate we still have to merge those by necessity.

^This would be my vote... @emackey @julienduroure does that sound okay?

[emackey]

Yeah, I guess I'm OK with that, as long as other people feel this is really needed. The expectations of the test suite would need to change, and regular users would need to learn how to optimize their glTF ORM outside of Blender, but it does give the power user more control. I don't want to stand in the way of this as long as other folks really need it.

[donmccurdy]

@emackey that sounds like a very half-hearted agreement. 😅 If you have concerns, we can skip this. I'm undecided whether the "users would need to learn how to optimize their glTF ORM outside of Blender" thing is an issue, because it's unclear whether merging is an optimization more often than not. Certainly if you're sampler-bound, but for all our discussion of it, I don't run into that issue very often. On the other hand, combining maps that don't need to be combined may reduce compression efficiency (?) or introduce artifacts with JPG or KTX textures.

[scurest]

Well, how about this: we stop combining images like @donmccurdy said, except in the case that the input images and the output image are all .pngs; in that case it's okay to combine. As it happens, the tests for this all use .pngs, so they would keep working, while still solving the issue described in the OP.

In more detail: If we go through all the cases....

1. the textures are already packed in ORM format (no packing needed) => combining ok! (since we're not really "combining")
2. packing is needed, the output is a JPG => don't combine, would be lossy
3. packing is needed, the output is a PNG, all the inputs are PNG => combining ok!
4. packing is needed, the output is a PNG, at least one input is a JPG => don't combine, would bloat filesize

It's a bit hard to detect "already packed in ORM format" from __gather_orm_texture but we can substitute "only one input image" for almost the same effect, which gives this patch

Patch

diff --git a/addons/io_scene_gltf2/blender/exp/gltf2_blender_gather_image.py b/addons/io_scene_gltf2/blender/exp/gltf2_blender_gather_image.py
index 7c1fa86..312e05c 100644
--- a/addons/io_scene_gltf2/blender/exp/gltf2_blender_gather_image.py
+++ b/addons/io_scene_gltf2/blender/exp/gltf2_blender_gather_image.py
@@ -100,7 +100,7 @@ def __gather_mime_type(sockets, export_image, export_settings):
 
     if export_settings["gltf_image_format"] == "AUTO":
         image = export_image.blender_image()
-        if image is not None and __is_blender_image_a_jpeg(image):
+        if image is not None and is_blender_image_a_jpeg(image):
             return "image/jpeg"
         return "image/png"
 
@@ -215,7 +215,7 @@ def __get_tex_from_socket(blender_shader_socket: bpy.types.NodeSocket, export_se
     return result[0]
 
 
-def __is_blender_image_a_jpeg(image: bpy.types.Image) -> bool:
+def is_blender_image_a_jpeg(image: bpy.types.Image) -> bool:
     if image.source != 'FILE':
         return False
     path = image.filepath_raw.lower()
diff --git a/addons/io_scene_gltf2/blender/exp/gltf2_blender_gather_materials.py b/addons/io_scene_gltf2/blender/exp/gltf2_blender_gather_materials.py
index ea597f9..2af3003 100644
--- a/addons/io_scene_gltf2/blender/exp/gltf2_blender_gather_materials.py
+++ b/addons/io_scene_gltf2/blender/exp/gltf2_blender_gather_materials.py
@@ -27,6 +27,7 @@ from ..com.gltf2_blender_extras import generate_extras
 from io_scene_gltf2.blender.exp import gltf2_blender_get
 from io_scene_gltf2.io.exp.gltf2_io_user_extensions import export_user_extensions
 from io_scene_gltf2.io.com.gltf2_io_debug import print_console
+from .gltf2_blender_gather_image import is_blender_image_a_jpeg
 
 
 @cached
@@ -199,6 +200,19 @@ def __gather_orm_texture(blender_material, export_settings):
             "(use same-sized images if you want them combined)")
         return None
 
+    # Avoid combining multiple images if the output will be a .jpg (would be
+    # lossy) or if any input is a .jpg (would bloat filesize).
+    input_images = [__get_image_from_socket(socket) for socket in result]
+    multiple_inputs = any(img is not input_images[0] for img in input_images)
+    if multiple_inputs:
+        output_is_jpeg = (export_settings["gltf_image_format"] == "JPEG")
+        if output_is_jpeg:
+            return None
+
+        input_is_jpeg = any(img and is_blender_image_a_jpeg(img) for img in input_images)
+        if input_is_jpeg:
+            return None
+
     # Double-check this will past the filter in texture_info
     info = gltf2_blender_gather_texture_info.gather_texture_info(result, export_settings)
     if info is None:
@@ -225,6 +239,14 @@ def __gather_pbr_metallic_roughness(blender_material, orm_texture, export_settin
         orm_texture,
         export_settings)
 
+def __get_image_from_socket(socket):
+    result = gltf2_blender_search_node_tree.from_socket(
+        socket,
+        gltf2_blender_search_node_tree.FilterByType(bpy.types.ShaderNodeTexImage))
+    if not result:
+        return None
+    return result[0].shader_node.image
+
 def __has_image_node_from_socket(socket):
     result = gltf2_blender_search_node_tree.from_socket(
         socket,

[emackey]

@scurest That sounds like a good compromise. Do you think it would make the behavior too complex? Or would it just quietly "do the right thing" all the time? Anyway it would certainly work for my own uses, for whatever that's worth, and I hope it would just do the right thing for others as well.

[scurest]

It is complex. Personally I think it's better to try to "do the right thing" instead of adding options though.

We could also continue combining in case (2). That only happens when selecting the JPEG format option, which is already potentially lossy, and since the only reason to pick that is filesize, maybe it's better to combine. If we did that, that would mean only case (4) would change. This would be the minimal change that fixes the issue described in the OP, I think.

[emackey]

Agreed, I don't want UI options asking artists to understand glTF's internals to this depth.

@scurest I'm fine with either of your proposals here. I tend towards the minimal change you mention in (4), and indeed the OP seems to be asking for (2) to combine to JPEG. Let's go with that.

[scurest]

@alexey-pelykh Does amending this to not combine in case (4) sound good to you?

[scurest]

Here's a patch to do the minimal change. It adds a function want_to_combine that checks if we want to combine or not and uses it for ORM. Testing with re-exporting DamagedHelmet results in a 3.7M file, vs 8.2M with master.

Patch

diff --git a/addons/io_scene_gltf2/blender/exp/gltf2_blender_gather_image.py b/addons/io_scene_gltf2/blender/exp/gltf2_blender_gather_image.py
index 7c1fa86..312e05c 100644
--- a/addons/io_scene_gltf2/blender/exp/gltf2_blender_gather_image.py
+++ b/addons/io_scene_gltf2/blender/exp/gltf2_blender_gather_image.py
@@ -100,7 +100,7 @@ def __gather_mime_type(sockets, export_image, export_settings):
 
     if export_settings["gltf_image_format"] == "AUTO":
         image = export_image.blender_image()
-        if image is not None and __is_blender_image_a_jpeg(image):
+        if image is not None and is_blender_image_a_jpeg(image):
             return "image/jpeg"
         return "image/png"
 
@@ -215,7 +215,7 @@ def __get_tex_from_socket(blender_shader_socket: bpy.types.NodeSocket, export_se
     return result[0]
 
 
-def __is_blender_image_a_jpeg(image: bpy.types.Image) -> bool:
+def is_blender_image_a_jpeg(image: bpy.types.Image) -> bool:
     if image.source != 'FILE':
         return False
     path = image.filepath_raw.lower()
diff --git a/addons/io_scene_gltf2/blender/exp/gltf2_blender_gather_materials.py b/addons/io_scene_gltf2/blender/exp/gltf2_blender_gather_materials.py
index 0e76462..7bf06ab 100644
--- a/addons/io_scene_gltf2/blender/exp/gltf2_blender_gather_materials.py
+++ b/addons/io_scene_gltf2/blender/exp/gltf2_blender_gather_materials.py
@@ -196,19 +196,13 @@ def __gather_orm_texture(blender_material, export_settings):
     else:
         result = (occlusion, roughness_socket, metallic_socket)
 
-    if not gltf2_blender_gather_texture_info.check_same_size_images(result):
-        print_console("INFO",
-            "Occlusion and metal-roughness texture will be exported separately "
-            "(use same-sized images if you want them combined)")
-        return None
-
-    # Double-check this will past the filter in texture_info
-    info = gltf2_blender_gather_texture_info.gather_texture_info(result, export_settings)
-    if info is None:
+    # Check if there's any reason not to combine into one image.
+    if not gltf2_blender_gather_texture_info.want_to_combine(result, export_settings):
         return None
 
     return result
 
+
 def __gather_occlusion_texture(blender_material, orm_texture, export_settings):
     if orm_texture is not None:
         return gltf2_blender_gather_texture_info.gather_material_occlusion_texture_info_class(
@@ -266,6 +260,7 @@ def __gather_clearcoat_extension(blender_material, export_settings):
         has_clearcoat_roughness_texture = True
 
     # Pack clearcoat (R) and clearcoatRoughness (G) channels.
+    # TODO: use want_to_combine to check if we should pack or not
     if has_clearcoat_texture and has_clearcoat_roughness_texture:
         clearcoat_roughness_slots = (clearcoat_socket, clearcoat_roughness_socket,)
     elif has_clearcoat_texture:
diff --git a/addons/io_scene_gltf2/blender/exp/gltf2_blender_gather_texture_info.py b/addons/io_scene_gltf2/blender/exp/gltf2_blender_gather_texture_info.py
index e8cc7c5..e627414 100644
--- a/addons/io_scene_gltf2/blender/exp/gltf2_blender_gather_texture_info.py
+++ b/addons/io_scene_gltf2/blender/exp/gltf2_blender_gather_texture_info.py
@@ -21,6 +21,7 @@ from io_scene_gltf2.blender.exp import gltf2_blender_search_node_tree
 from io_scene_gltf2.blender.exp import gltf2_blender_get
 from io_scene_gltf2.io.com.gltf2_io_extensions import Extension
 from io_scene_gltf2.io.exp.gltf2_io_user_extensions import export_user_extensions
+from .gltf2_blender_gather_image import is_blender_image_a_jpeg
 
 
 def gather_texture_info(blender_shader_sockets, export_settings):
@@ -162,19 +163,36 @@ def __get_tex_from_socket(socket):
     return result[0]
 
 
-def check_same_size_images(
-    blender_shader_sockets: typing.Tuple[bpy.types.NodeSocket],
-) -> bool:
-    """Check that all sockets leads to images of the same size."""
-    if not blender_shader_sockets or not all(blender_shader_sockets):
+def want_to_combine(blender_shader_sockets, export_settings):
+    """Check if we want to combine the images from sockets into one texture."""
+    if not blender_shader_sockets:
         return False
 
-    sizes = set()
+    images = []
     for socket in blender_shader_sockets:
         tex = __get_tex_from_socket(socket)
-        if tex is None:
+        if tex and tex.shader_node.image:
+            images.append(tex.shader_node.image)
+
+    if not images:
+        return False
+
+    # Only combine if the same size.
+    all_same_size = all(img.size[:] == images[0].size[:] for img in images)
+    if not all_same_size:
+        return False
+
+    # Don't combine .jpgs into a .png (would bloat filesize).
+    # This should catch most cases....
+    multiple_imgs = any(img is not images[0] for img in images)
+    output_is_png = multiple_imgs and (export_settings["gltf_image_format"] == "AUTO")
+    if output_is_png:
+        if any(is_blender_image_a_jpeg(img) for img in images):
             return False
-        size = tex.shader_node.image.size
-        sizes.add((size[0], size[1]))
 
-    return len(sizes) == 1
+    # Double-check this will pass the filter in texture_info
+    info = gather_texture_info(blender_shader_sockets, export_settings)
+    if info is None:
+        return False
+
+    return True

[alexey-pelykh]

@scurest indeed, not doing extra work (case 2) unless asked to and avoid bloating file size (case 4) is what I ask for :)

[donmccurdy]

@scurest to confirm in your comment #1099 (comment), where you say "packing is needed", I think you mean "packing is possible"? So given separate O + R +M textures, all JPEG, you'll get "O" + "R/M" packed, not "O/R/M"?

Assuming so, I'm OK with your suggestion.¹ @alexey-pelykh would you be willing to update the PR with this change?

---

¹ I'd also be OK with omitting (3) and never combining textures that don't have to be combined, unless some hypothetical future "Always combine textures" option is enabled.

[scurest]

@donmccurdy Yes. I'm looking at it from the perspective of "decide whether to combine these ORM sockets" and the first question is "if we did, would we have to pack them?".

[emackey]

Just so we're thinking ahead: A collection of "PBR Next" extensions are going to start showing up over the next year or so, with potentially lots of new ways to combine things. Previously, "ORM" was the only reasonable combination of separate core textures, but now you could put a clearcoat or transmission layer in the R channel alongside roughnessMetallic or other future parameters in the G, B channels. In the near future, additional channels may be needed for specular, sheen, subsurface, attenuation, anisotropy, translucency, thin film, and other effects. Each will have assigned RGBA channel(s) and the number of possible combinations will blossom.

So in theory, having this exporter be a little more conservative about combining things that aren't already combined might be a reasonably good plan.

[julienduroure]

I'm a bit lost in the discussion. Are any of the in-comment patches are applied?
What is the status of this PR?

[alexey-pelykh]

I believe that next step would be to update the PR, yet I'm a bit lost as well to what we've come up finally.

[globglob3D]

Hi, I just thought I'd add to the discussion since this issue is something I encounter in my daily work, and the reason why I dislike automatic packing (outside of the JPG/PNG problematic). While I agree with pretty much all the discussion I think you missed an important point (correct me if I'm wrong though!)

Let say you make an object and you want to embed multiple different materials variations in the glb. You'll see this happen a lot in product visualization (one of the main area where webGL is used), as it allow customers to create the product they want with the materials and colors they want.
The AO texture will be the same across all materials, but R/M will be different.
The thing is that if you pack the AO in the red channel in all R/M textures you'll end up with a drastic increase in glb size. Doesn't matter if it's JPG or PNG, a red channel that is full black will always be lighter than a red channel with an AO texture in it.

I'd rather have one AO texture of 255ko and multiple R/M textures of 267ko, than multiple O/R/M textures of 461ko.

I honestly don't think automatic packing is that beneficial to the exporter, to me this has always been a feature that ended up causing more problem and debugging than good. I am an advanced user though, but packing doesn't even make that big of a different on texture size, is it really worth imposing it even for all PNG O/R/M? If AO isn't packed in the first place there may be a reason for it, and we should at least have an option for advanced user to keep the way they intended the textures to be.

Currently this can be avoided by forcing JPG export but if you have a Normal map in there that's a terrible idea. You'd have to make sure your AO texture size is different from the R/M to get an efficient export :(

[julienduroure]

Still a bit loss in this discussion, but note that I just merged an option to keep texture images as-is when material node is already a valid PBR node : #1409

[pepperoni505]

Any update on this?

[globglob3D]

There is still a big issue with this #1787