Export ImageTexture node for an image identified in material node tree #39
No reviewers
Labels
No Label
bug
confirmed
duplicate
enhancement
help wanted
High
known issue
Low
Normal
not a task
question
task
v2.4
wontfix
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: extensions/io_scene_x3d#39
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "vmarchetti/io_scene_x3d:image-export"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Re-enables export of an X3D ImageTexture node when an image texture is identified in the material node tree as the base color of a material.
The export routine will also save the image to user disk, even when the image is embedded in blender data as a PackedFile, with no explicit filename
The attached file earth_textureimage.blend can be used to demonstrate the X3D and image file export.
Load it into Blender, invoke the X3D export with default export options.
A file earth_textureimage.x3d and and image file earth_topo.png will be written to disk.
Results can be viewed in an X3D viewer such as Castle Model Viewer
Thanks for the test file, that seems to be working just fine.
However i caught an edge case which should be handled as well: when the image node inside blender is not yet saved to disk, we get following error:
error msg
To sum up, cases to handle:
Features i'd wish for to be included, but i'm open for discussion:
WIP: Export ImageTexture node for an image identified in material node treeto Export ImageTexture node for an image identified in material node treeAt commit 832ed4 the code has passed basic testing, including handling the use-case of an image created within blender. The writing of image files has been rewritten to make use of the existing Blender utilities for evaluate file paths according to the path_mode variable, which can now be set by the user.
Image files are written to a new location only when the path_mode is set to 'COPY' upon export. When path_mode is set to COPY, images are written to s "textures" subfolder which is a file system sibling of the exported x3d file.
Hi Vincent, thanks for the recent changes. I looked through the code, as far as i can tell it looks good. Few minor typos in docstrings and comments (which also could get simplified).
On testing i found a few minor usage issues, which could get addressed:
url='"textures/Untitled.png" "Untitled.png" "D:/Daten/x3d_test/Untitled.png"'
-> copy, strip, absolute combined, should only be copy. This is most likely caused by the legacy algorithm block, as it creates all possible combinations and that full list gets used; from my understanding the filepath_reference should get usedImages and Movies
a movie texture gets saved as ImageTexture node instead of MovieTexture. So either warn the user when a movie file gets detected or handle it correctly@ -259,6 +266,19 @@ def export(file,
# store files to copy
copy_set = set()
# if path_mode is copy, create a temporary folder particular
why is this temporary folder needed? Could you add that to the comment?
@ -654,3 +622,1 @@
fw(ident_step + 'scale="%.6f %.6f"\n' % (sca_x, sca_y))
fw(ident_step + 'rotation="%.6f"\n' % rot)
fw(ident_step + '/>\n')
# this is a location at which we should
Could you shorten the comment down? For example "Find and write ImageTexture and potentially TextureTransform if present" reads a bit better in my opinion
@ -1268,2 +1236,3 @@
fw(ident_step + "url='%s'\n" % ' '.join(['"%s"' % escape(f) for f in images]))
if has_packed_file or not imageTextureNode.image.filepath :
# assume that if the data has a packed file or it
comment is a bit tricky to read, maybe simplify: "Write only the filename if image is packed or has no filepath provided"
@ -1270,0 +1261,4 @@
image_urls = [filepath_ref]
else:
image_urls = [image_filename]
logger.warn("dangling image url %r" % [image_filename])
warn is deprecated, use warning instead
@ -1270,0 +1263,4 @@
image_urls = [image_filename]
logger.warn("dangling image url %r" % [image_filename])
else:
# this is the legacy algorithm for determining
Could also get simplified: "Legacy Algorithm for preparing and normalizing image filepaths: Get the full path, reference path (e.g. relative), filename"
@ -1519,0 +1553,4 @@
# for svRecord in imageInMaterial.imagesToSave():
I personally prefer to not leave dead code in, as it gets outdated quickly and no one remembers what is was used for. So ideally either remove or explain in a short leading comment the purpose of this commented out code block
@ -0,0 +1,114 @@
# SPDX-FileCopyrightText: 2011-2024 Blender Foundation
Not sure about the copyright. Technically this is no longer property of bf, so if you want, you can put in your name (or Web3D Consortium as you're a member there) and current year
@ -0,0 +30,4 @@
def _find_node_by_idname(nodes, idname):
"""
nodes a sequence of Nodes, idname a string
a few typos here
@ -0,0 +41,4 @@
See https://docs.blender.org/api/current/bpy.types.ShaderNode.html for list of
ShaderNode subclasses
if 0 targets are found, returns None
the output as code is not really needed in the docstring, but if you want simplify: Returns None if no targets are found or returns the first found target
@ -0,0 +73,4 @@
bsdf_principled = _find_node_by_idname(
[ndlink.from_node for ndlink in material_output.inputs.get(SURFACE_ATTRIBUTE).links],
_ShaderNodeTypes.BSDF_PRINCIPLED)
if bsdf_principled is None : return None
for better readability put the return into it's own line
@ -0,0 +79,4 @@
image_texture = _find_node_by_idname(
[ndlink.from_node for ndlink in bsdf_principled.inputs.get(BASE_COLOR_ATTRIBUTE).links],
_ShaderNodeTypes.IMAGE_TEXTURE )
if image_texture is None: return None
for better readability put the return into it's own line
@ -0,0 +83,4 @@
_logger.debug("located image texture node %r" % image_texture)
return image_texture
## x3d_supported_file_extension = {"PNG" : ".png","JPEG" : ".jpg"}
dead code
I am not able to reproduce the reported fault:
I follow the procedure in the comment above from 9 Nov 2024 10:27 AM EST #39 (comment)
creating a new image on the initial cube; at no point do I save the image to file or try to pack it; the Shader Editor before I export is attached.
Upon export to X3D with Path Mode = COPY an image file is written to textures/florence.png
Sorry, my bad. Indeed it's working for usnaved textures. The image is in the textures folder and the url correctly set. It's the comparison software "FreeWRL 4.0" that is having issues reading image textures from relative subdfolders as it seems (I'll see if the issue is already reported somewhere then). Thank you for testing and commenting.
By the way, also the current blender x3d importer has trouble with loading images generated by the x3d exporter. Will have to look into it, wether it's the importers fault or the exporter.
Regarding the blender importer: As it seems, it's a format issue. The exporter produces
url='"textures\Untitled.png"'
, the importer expectsurl="textures\Untitled.png"
. Should this be fixed in your export script or in the existing import script?url='"textures\Untitled.png"' is the correct syntax for this X3D property, which is an MFString type, list of strings. The form expected by the importer is a common mistake. I propose to keep the exported output strictly correct and the import tolerant of the other form. I can submit the issue and prepare the patch
Alright, thanks for the feedback. A patch would be greatly appreciated for the import script; a issue is not really needed for that (i usually only create them if it's a backlog task or needs more info). You can push the patch directly to main or include it with this PR.
Regarding the issue with freewrl: Dug found out, it's the backslash which caused the issue. Changing it to a foreslash works. Double backslash would also work, but most likely only for certain os/software.
I tested a bit further: Seems like the unsaved image part is broken, as the url of an unsaved image with option copy looks like this
url='"textures\Untitled123.png"'
but if the image was loaded from disk, it looks like thisurl='"textures/Untitled123.png"'
(i removed the other urls as they shouldn't be there in the first place). Note the difference between backslash (wrong) and foreslash (correct).Can you replicate and fix this?
Recent commits have improved the comments, removed the ashes of dead code.
The file path written to the URL property now follows the logic of the bpy_extras.io_utils.path_reference utility and only one such file path is included. For the Path Mode = COPY case the url and location of copied file are in the same folder as the exported X3D and the url is a relative path to the image.
The limitation that Image Textures defined inside of Node groups are not identified is documented in the docstring to the imageTexture_in_material function
After the backslash issue is fixed (the cases for disk texture auto, absolute, relative path mode are pending), the PR can get merged and a new extension version published after the patch for the MFString is in.
Another minor issue that tickles me, is that an unsaved image gets saved in all modes except copy as
url='"."'
. This crashes other software like freewrl. The crash of other software is not really our concern, however "." ain't really correct either. Fyi the crash is caused with any ImageTexture node without url values, so full node removal would most likely be best. At current state, i'll count it as known rare issue, thus fixing would not really be mandatory.I have added a warning message sent to logging in the case you mentioned of an unsaved image being exported with the Path Mode set to something other than COPY. I agree that it would be best not to include the X3D ImageTexture node at all in this case, and if or when we make the code upgrade to use an xml library, such as xml.etree.ElementTree, to generate the xml output this would be appropriate. In the current code we are sort of streaming the xml text piece by piece and it's awkward to cancel out of a node output.
As for the backslash vs fore slash issue, I propose that the current code that, in COPY mode, refers to the image file as a relatiive file path not in a subdirectory, and the copied image file is saved to the same folder as the exported X3D files, sidesteps the problem by avoiding slashed altogether.
In general, I tried to simplify the X3D export code by relying on the logic of the bpy_extras.io_utils.path_reference utility, code visible at https://projects.blender.org/blender/blender/src/branch/main/scripts/modules/bpy_extras/io_utils.py , to create the file paths according to path_mode. That code uses os.path.join and so the slash/backslash choice will depend on whether the code is being run in Windows OS. Our options for further solving the slash/backslash problem would be
The path_reference utility, and the Path Mode concept is, I think, intended for a use case of content creators working within a shared OS and file system. I suggest that the option of using COPY into the same folder as exported X3D gives content creators the basics to deploy their content outside that shared-filesystem framework, probably with a different tool
Thus rare known issue. Will fix it if someone complains about it, but i highly doubt this will happen. Another fix despite removing the node would be to fallback to COPY in case the path is empty.
Maybe path mode COPY should even be the default option as files usally get sent elsewhere afterwards?
The issue here unfortunately is, that other software (like freewrl) even in windows versions don't handle the regular backslash. Thus the current path output by bpy_extras.io_utils.path_reference is not usable in at least some other x3d software. Foreslash and double backslash would be allowed here. The blender x3d importer can handle all three cases.
I saw that commit already and although it fixes the copy case, the auto(relative)/absolute/relative path options are still affected. For COPY the subdir choice is more or less a individual preference.
I think this would be the easiest and best option here by simply replacing
\
with/
.If this poses an issue, maybe exposing an option fore-/backslash to the user could help in aiding further issues
Thanks for the contribution, will merge soon.