extensions/io_scene_x3d
2
1
Fork 4
Code Issues 14 Pull Requests 2 Projects Releases Wiki Activity

Export ImageTexture node for an image identified in material node tree #39

Open
Vincent Marchetti wants to merge 7 commits from vmarchetti/io_scene_x3d:image-export into main
pull from: vmarchetti/io_scene_x3d:image-export
merge into: extensions:main

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.
Conversation 9 Commits 7 Files Changed 3 +263 -82
Vincent Marchetti commented 2024-11-09 01:35:10 +01:00
Collaborator
Copy Link

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

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
earth_textureimage.blend
1009 KiB
👍 1
Vincent Marchetti added 1 commit 2024-11-09 01:35:11 +01:00 
properly write the ImageTexture node even when there is no
explicit file name (as the case when image is read from a glTF file)
and save the image file to disk
Vincent Marchetti commented 2024-11-09 12:08:35 +01:00
Author
Collaborator
Copy Link

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

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](https://castle-engine.io/castle-model-viewer)
Cedric Steiert commented 2024-11-09 16:27:05 +01:00
Collaborator
Copy Link

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 
Traceback (most recent call last):
  File "__init__.py", line 346, in execute
    return export_x3d.save(context, **keywords)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "export_x3d.py", line 1716, in save
    export(file,
  File "export_x3d.py", line 1656, in export
    image_filepath = os.path.join(base_dst, svRecord.url)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<frozen ntpath>", line 147, in join
  File "<frozen genericpath>", line 152, in _check_arg_types
TypeError: join() argument must be str, bytes, or os.PathLike object, not 'NoneType'
To replicate, create a new blend file, on the default cube in the shader node editor add a new image texture node and create a new image (press new), don't save to disk. A bit further testing, this seems to be happening to files on disk as well, files need to be packed into the blender file by the user manually (File > External Data > Pack Resources) to work properly. This should be fixed.

To sum up, cases to handle:

  • (done) packed files
  • files on disk
  • files not yet saved
  • image texture node is inside a NodeGroup: currently the image won't get detected. If possible add support for that, if not document as known limitation
  • movie files: Ideally handle them properly or message the user that this ain't supported.

Features i'd wish for to be included, but i'm open for discussion:

  • Path Mode selector in exporter ui (like fbx exporter) to copy image, use absolute/relative path, pack to subdirectory
  • Don't overwrite copied image: Currently the image gets resaved to the output location at each export, i'd suggest to first check if file exists. Additionaly a new ui property "Overwrite/Replace Resource Files" could be exposed. In my opinion that IO operation can get saved, as images usually don't change too often. Alternatively (this is only applicable for files on disk), the modification stamp could get checked first.
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: <details> <summary>error msg</summary> ``` Traceback (most recent call last): File "__init__.py", line 346, in execute return export_x3d.save(context, **keywords) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "export_x3d.py", line 1716, in save export(file, File "export_x3d.py", line 1656, in export image_filepath = os.path.join(base_dst, svRecord.url) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "<frozen ntpath>", line 147, in join File "<frozen genericpath>", line 152, in _check_arg_types TypeError: join() argument must be str, bytes, or os.PathLike object, not 'NoneType' ``` </details> To replicate, create a new blend file, on the default cube in the shader node editor add a new image texture node and create a new image (press new), don't save to disk. A bit further testing, this seems to be happening to files on disk as well, files need to be packed into the blender file by the user manually (File > External Data > Pack Resources) to work properly. This should be fixed. **To sum up, cases to handle:** - (done) packed files - files on disk - files not yet saved - image texture node is inside a NodeGroup: currently the image won't get detected. If possible add support for that, if not document as known limitation - movie files: Ideally handle them properly or message the user that this ain't supported. **Features i'd wish for to be included, but i'm open for discussion:** - Path Mode selector in exporter ui (like fbx exporter) to copy image, use absolute/relative path, pack to subdirectory - Don't overwrite copied image: Currently the image gets resaved to the output location at each export, i'd suggest to first check if file exists. Additionaly a new ui property "Overwrite/Replace Resource Files" could be exposed. In my opinion that IO operation can get saved, as images usually don't change too often. Alternatively (this is only applicable for files on disk), the modification stamp could get checked first.
Vincent Marchetti added 1 commit 2024-11-16 00:03:57 +01:00 
to be used in the export. For purposes of development the default setting
in the UI is set to COPY, this is contrary to other default value for path_mode.
It may be appropriate to set the default UI setting to AUTO upon merging into
release code.

Added logging messages to export_x3d.py. The intent here is to extend the copy_set
mechanism to handle cases where the image file does not exist on disk at time of
export.
Vincent Marchetti added 5 commits 2024-11-18 21:27:07 +01:00 
implementing search through material node-tree
Not ready for pull-request
The way it was done prevents path_mode from being changed
visible when needed
Vincent Marchetti changed title from WIP: Export ImageTexture node for an image identified in material node tree to Export ImageTexture node for an image identified in material node tree 2024-11-18 21:31:06 +01:00
Vincent Marchetti commented 2024-11-18 21:36:06 +01:00
Author
Collaborator
Copy Link

At 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.

At 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.
Vincent Marchetti requested review from Cedric Steiert 2024-11-18 23:49:56 +01:00
Cedric Steiert requested changes 2024-11-19 16:58:43 +01:00
Cedric Steiert left a comment
Collaborator
Copy Link

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:

  • (edit: seems like an issue with subfolders in freewrl) newly created unsaved image does get saved on export , but not getting displayed.
  • image texture inside a node group still does not get detected and in the code i see no comment declaring it as a technical limitation
  • url is not like setting: example for Filepath Copy 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 used
  • despite the export section being named Images 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
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: - (edit: seems like an issue with subfolders in freewrl) newly created unsaved image does get saved on export , but not getting displayed. - image texture inside a node group still does not get detected and in the code i see no comment declaring it as a technical limitation - url is not like setting: example for Filepath Copy `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 used - despite the export section being named `Images 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
source/export_x3d.py
@ -259,6 +266,19 @@ def export(file,
# store files to copy
copy_set = set()
# if path_mode is copy, create a temporary folder particular
Cedric Steiert commented 2024-11-19 16:20:38 +01:00
Collaborator
Copy Link

why is this temporary folder needed? Could you add that to the comment?

why is this temporary folder needed? Could you add that to the comment?
source/export_x3d.py
@ -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
Cedric Steiert commented 2024-11-19 16:22:30 +01:00
Collaborator
Copy Link

Could you shorten the comment down? For example "Find and write ImageTexture and potentially TextureTransform if present" reads a bit better in my opinion

Could you shorten the comment down? For example "Find and write ImageTexture and potentially TextureTransform if present" reads a bit better in my opinion
source/export_x3d.py
@ -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
Cedric Steiert commented 2024-11-19 16:26:39 +01:00
Collaborator
Copy Link

comment is a bit tricky to read, maybe simplify: "Write only the filename if image is packed or has no filepath provided"

comment is a bit tricky to read, maybe simplify: "Write only the filename if image is packed or has no filepath provided"
source/export_x3d.py
@ -1270,0 +1261,4 @@
image_urls = [filepath_ref]
else:
image_urls = [image_filename]
logger.warn("dangling image url %r" % [image_filename])
Cedric Steiert commented 2024-11-19 16:16:42 +01:00
Collaborator
Copy Link

warn is deprecated, use warning instead

warn is deprecated, use warning instead
source/export_x3d.py
@ -1270,0 +1263,4 @@
image_urls = [image_filename]
logger.warn("dangling image url %r" % [image_filename])
else:
# this is the legacy algorithm for determining
Cedric Steiert commented 2024-11-19 16:36:17 +01:00
Collaborator
Copy Link

Could also get simplified: "Legacy Algorithm for preparing and normalizing image filepaths: Get the full path, reference path (e.g. relative), filename"

Could also get simplified: "Legacy Algorithm for preparing and normalizing image filepaths: Get the full path, reference path (e.g. relative), filename"
source/export_x3d.py
@ -1519,0 +1553,4 @@
# for svRecord in imageInMaterial.imagesToSave():
Cedric Steiert commented 2024-11-19 16:30:56 +01:00
Collaborator
Copy Link

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

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
source/material_node_search.py
@ -0,0 +1,114 @@
# SPDX-FileCopyrightText: 2011-2024 Blender Foundation
Cedric Steiert commented 2024-11-19 16:38:15 +01:00
Collaborator
Copy Link

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

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
source/material_node_search.py
@ -0,0 +30,4 @@
def _find_node_by_idname(nodes, idname):
"""
nodes a sequence of Nodes, idname a string
Cedric Steiert commented 2024-11-19 16:39:48 +01:00
Collaborator
Copy Link

a few typos here

a few typos here
source/material_node_search.py
@ -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
Cedric Steiert commented 2024-11-19 16:41:51 +01:00
Collaborator
Copy Link

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

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
source/material_node_search.py
@ -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
Cedric Steiert commented 2024-11-19 16:43:54 +01:00
Collaborator
Copy Link

for better readability put the return into it's own line

for better readability put the return into it's own line
source/material_node_search.py
@ -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
Cedric Steiert commented 2024-11-19 16:44:01 +01:00
Collaborator
Copy Link

for better readability put the return into it's own line

for better readability put the return into it's own line
source/material_node_search.py
@ -0,0 +83,4 @@
_logger.debug("located image texture node %r" % image_texture)
return image_texture
## x3d_supported_file_extension = {"PNG" : ".png","JPEG" : ".jpg"}
Cedric Steiert commented 2024-11-19 16:38:51 +01:00
Collaborator
Copy Link

dead code

dead code
Vincent Marchetti commented 2024-11-21 13:19:50 +01:00
Author
Collaborator
Copy Link

I am not able to reproduce the reported fault:

  • newly created unsaved image does not get saved on export in any mode,

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

I am not able to reproduce the reported fault: - newly created unsaved image does not get saved on export in any mode, I follow the procedure in the comment above from 9 Nov 2024 10:27 AM EST https://projects.blender.org/extensions/io_scene_x3d/pulls/39#issuecomment-1335727 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
blender_screenshot_20241121_0710.png
1.6 MiB
blender_screenshot_20241121_0710.png
Cedric Steiert commented 2024-11-21 13:39:37 +01:00
Collaborator
Copy Link

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.

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.
Cedric Steiert commented 2024-11-21 13:47:21 +01:00
Collaborator
Copy Link

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.

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.
Cedric Steiert commented 2024-11-21 16:02:54 +01:00
Collaborator
Copy Link

Regarding the blender importer: As it seems, it's a format issue. The exporter produces url='"textures\Untitled.png"', the importer expects url="textures\Untitled.png". Should this be fixed in your export script or in the existing import script?

Regarding the blender importer: As it seems, it's a format issue. The exporter produces `url='"textures\Untitled.png"'`, the importer expects `url="textures\Untitled.png"`. Should this be fixed in your export script or in the existing import script?
Vincent Marchetti commented 2024-11-21 17:12:06 +01:00
Author
Collaborator
Copy Link

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

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
Cedric Steiert commented 2024-11-21 17:28:31 +01:00
Collaborator
Copy Link

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.

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.
This pull request can be merged automatically.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.
View command line instructions.

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u image-export:vmarchetti-image-export
git checkout vmarchetti-image-export
Sign in to join this conversation.
Reviewers
No reviewers
Cedric Steiert
Labels
Clear labels   
bug

Something is not working

  
confirmed

The issue is confirmed.

  
duplicate

This issue or pull request already exists

  
enhancement

New feature

  
help wanted

Need some help

  
High

Priority high: This issue should be worked on soon, as it breaks things drastically

  
known issue

A known technical limitation. It may get fixed eventually, but no support expected.

  
Low

Priority low: Could be worked on, but is of no need

  
Normal

Priority normal: Not a high issue, still relevant

  
not a task

just some content

  
question

More information is needed

  
task

Tasks that should be worked on

  
v2.4

Tasks included for 2.4 release

  
wontfix

This won't be fixed

No Label
bug
confirmed
duplicate
enhancement
help wanted
High
known issue
Low
Normal
not a task
question
task
v2.4
wontfix
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: extensions/io_scene_x3d#39
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?