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

(WIP) More flexible image export handling for extensions. #1410

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

netpro2k
Copy link
Contributor

@netpro2k netpro2k commented Jun 8, 2021

There are now several GLTF extensions for alternate image formats like KHR_texture_basisu and EXT_texture_webp. It would be nice to add support for these to to other addons built against this extension without them having to duplicate the node traversal and channel combination/remapping logic already implemented in ExportImage.

This is a quick attempt at abstracting things just enough to allow that. I am not particularly happy with this solution but I think its a useful starting point for discussion.

Using this in the Hubs Blender Exporter to support .hdr (which blender already natively understands) files on lightmaps like this:
https://github.com/MozillaReality/hubs-blender-exporter/blob/76579e38ad2b3ad6768eef7b1acf575b2fe4302d/gather_properties.py#L181-L193
https://github.com/MozillaReality/hubs-blender-exporter/blob/76579e38ad2b3ad6768eef7b1acf575b2fe4302d/gather_properties.py#L215

For formats Blender does not already natively support you could instead completely override encode(mime):

ImageData.extension_for_mime["image/ktx2"] = ".ktx"
class BasisExportImage(ExportImage):
    def preferred_mime_type(self) -> str:
        return "image/ktx2"

    def encode(self, mime_type: Optional[str]) -> bytes:
        # use self.fills to generate a ktx2 image
        return bytes()

Again this feels a bit messy and its stretching the ExportImage abstraction quite a bit, but hopefully serves to show what an addon might want to be able to do. It would likely be a better solution to pull out the bits of gather_image and ExportImage that are generically useful and have the addon call those instead, skipping gather_image entirely, but I wanted to start with a more direct approach just to see the minimal set of changes needed.


Some related issues I ran into while looking into this: #1234 #1308

@@ -27,14 +27,16 @@
from io_scene_gltf2.io.exp.gltf2_io_user_extensions import export_user_extensions


@cached
# @cached
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

export_image_class appears to not be hashable which causes this to fail. Not ideal.

@@ -93,20 +95,14 @@ def __gather_extras(sockets, export_settings):


def __gather_mime_type(sockets, export_image, export_settings):
# force png if Alpha contained so we can export alpha
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the user specifically sets their export mode to jpeg, I think things should always export as a jpeg. When set to auto the new preffered_mime_type() check would naturally output as a png unless the input image was already a jpg

if extension.lower() in ['.png', '.jpg', '.jpeg']:
if name:
return name
return name
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Attempting to remove as much as possible that is assuming a particular filetype. This check seems superfluous anyway.

@@ -19,6 +19,13 @@ class ImageData:
# FUTURE_WORK: as a method to allow the node graph to be better supported, we could model some of
# the node graph elements with numpy functions

extension_for_mime = {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We end up with multiple spots with very similar mappings... should be centralized somehow

def gather_image(
blender_shader_sockets: typing.Tuple[bpy.types.NodeSocket],
export_settings):
export_settings,
export_image_class = ExportImage
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic here is that all of the extensions related to image formats right now actually start at the texture level, so its likely they will be calling gather_image and can provide an override. Its possible this could instead be handled in reverse via something like a pre_gather_image_hook that could return some data, but it would likely require duplicating some functionality.

@netpro2k
Copy link
Contributor Author

netpro2k commented Jun 9, 2021

For comparison, this is the same thing but built against the existing exporter without modification. In many ways its cleaner, but there is some code duplicated (namely __encode_from_image) and I am intentionally skipping many edge cases (unsaved image, compositing multiple image channels, input images in a different format than export format) that ExportImage handles.

https://github.com/MozillaReality/hubs-blender-exporter/blob/hdr-lightmaps/gather_properties.py#L177

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant