-
Notifications
You must be signed in to change notification settings - Fork 324
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
base: main
Are you sure you want to change the base?
Conversation
@@ -27,14 +27,16 @@ | |||
from io_scene_gltf2.io.exp.gltf2_io_user_extensions import export_user_extensions | |||
|
|||
|
|||
@cached | |||
# @cached |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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 = { |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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.
fd6c596
to
9596b0c
Compare
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 https://github.com/MozillaReality/hubs-blender-exporter/blob/hdr-lightmaps/gather_properties.py#L177 |
There are now several GLTF extensions for alternate image formats like
KHR_texture_basisu
andEXT_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 inExportImage
.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)
: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 ofgather_image
andExportImage
that are generically useful and have the addon call those instead, skippinggather_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