Expanded X3D import addon #44758

New Issue

Seva Alekseyev (NIH) · 2015-05-19T00:13:29+02:00

2015-05-19 00:13:29 +02:00

{F177117}

The X3D import in Blender 2.74 is rather limited. We've expanded it to support a larger subset of the standard.

All the code changes are in import_x3d.py , and it should go into C:\Program Files\Blender Foundation\Blender%VERSION%\scripts\addons\io_scene_x3d (on Windows) or into /usr/local/blender-$VERSION/$VERSION /scripts/addons/io_scene_x3d (on Linux).

I've been going by the standard as outlined here: http:*www.web3d.org/documents/specifications/19775-1/V3.3/index.html . When not sure, I'd compare to a reference implementation, X3DOM ( http:*www.x3dom.org/ ).

It supports:

all geometry nodes from Rendering
all geometry nodes from Geometry3D
ImageTexture (including primitives)
TextureTransform (needs careful testing)
all lamp nodes from Lighting
Viewpoint

The UI was left intact from the existing implementation.

{F177117} The X3D import in Blender 2.74 is rather limited. We've expanded it to support a larger subset of the standard. All the code changes are in import_x3d.py , and it should go into C:\Program Files\Blender Foundation\Blender\%VERSION%\scripts\addons\io_scene_x3d (on Windows) or into /usr/local/blender-$VERSION/$VERSION /scripts/addons/io_scene_x3d (on Linux). I've been going by the standard as outlined here: http:*www.web3d.org/documents/specifications/19775-1/V3.3/index.html . When not sure, I'd compare to a reference implementation, X3DOM ( http:*www.x3dom.org/ ). It supports: - all geometry nodes from Rendering - all geometry nodes from Geometry3D - ImageTexture (including primitives) - TextureTransform (needs careful testing) - all lamp nodes from Lighting - Viewpoint The UI was left intact from the existing implementation.

Seva Alekseyev (NIH) commented

2015-05-19 00:13:29 +02:00

Changed status to: 'Open'

Seva Alekseyev (NIH) commented

2015-05-19 00:13:29 +02:00

Added subscriber: @sevaa

Seva Alekseyev (NIH) commented

2015-05-19 16:09:48 +02:00

This comment was removed by @sevaa

*This comment was removed by @sevaa*

Sergey Sharybin commented

2015-05-19 18:20:30 +02:00

Added subscribers: @mont29, @Sergey

Sergey Sharybin commented

2015-05-19 18:20:30 +02:00

@mont29, think you might be interested in having a look? :)

Seva Alekseyev (NIH) commented

2015-05-19 18:59:08 +02:00

Added PixelTexture support to match functionality with the patch #39594{F177571}

Added PixelTexture support to match functionality with the patch #39594{[F177571](https://archive.blender.org/developer/F177571/import_x3d.py)}

Odd Gunnar Østensen commented

2015-05-19 19:56:01 +02:00

Added subscriber: @oddgoest

Odd Gunnar Østensen commented

2015-05-19 19:56:01 +02:00

Thank You, it works fine :)

Bastien Montagne commented

2015-05-19 20:00:59 +02:00

@Sergey never did anything with x3d… :/

@sevaa: do you have a set of nice ref files to check import behavior?

@Sergey never did anything with x3d… :/ @sevaa: do you have a set of nice ref files to check import behavior?

Seva Alekseyev (NIH) commented

2015-05-19 20:23:27 +02:00

It was the same file that I'd edit over and over and over. :) Also, I have some tool-generated files that caused this whole effort to begin with. But those, while big, have a pretty poor code coverage. So no, I don't have a file library that I could share, sorry.

There's a big library of stock X3Ds at http://www.web3d.org/x3d/content/examples/Vrml2.0Sourcebook/index.html
Not all features that are showcased there are supported by my code. Fog, Text, Sensors, for example, are not. From a cursory look at the book, the models from the following chapters should display: 3, 5-7, 10-11, partially 13, 14-18, 20-22, 26, 29.

Some of those gaps I might address at some point. Lacking feedback from the community, it's hard to prioritize.

On Mac, the file goes into:
/Applications/blender.app/Contents/Resources/$VERSION/scripts/addons/io_scene_x3d

It was the same file that I'd edit over and over and over. :) Also, I have some tool-generated files that caused this whole effort to begin with. But those, while big, have a pretty poor code coverage. So no, I don't have a file library that I could share, sorry. There's a big library of stock X3Ds at http://www.web3d.org/x3d/content/examples/Vrml2.0Sourcebook/index.html Not all features that are showcased there are supported by my code. Fog, Text, Sensors, for example, are not. From a cursory look at the book, the models from the following chapters should display: 3, 5-7, 10-11, partially 13, 14-18, 20-22, 26, 29. Some of those gaps I might address at some point. Lacking feedback from the community, it's hard to prioritize. On Mac, the file goes into: /Applications/blender.app/Contents/Resources/$VERSION/scripts/addons/io_scene_x3d

Campbell Barton was assigned by Sergey Sharybin

2015-05-20 09:54:21 +02:00

Sergey Sharybin commented

2015-05-20 09:54:21 +02:00

@mont29, woops, my mistake. It's actually cambo :)

Campbell Barton commented

2015-05-21 01:31:23 +02:00

Checked F177571,

@sevaa, This is quite an involved patch. while I can check on a basic level. the implications of all your changes are hard to predict and would really be best shown to work with test files.

First pass review

no need for global material_cache, just define at top of file and clear() after use.
for f in bpymesh.uv_textures.new().data, in this case better to assign the layer a var name since its used immediately after.
some pep8 style conventions aren't followed (no big deal, but should resolve)

Checked [F177571](https://archive.blender.org/developer/F177571/import_x3d.py), @sevaa, This is quite an involved patch. while I can check on a basic level. the implications of all your changes are hard to predict and would really be best shown to work with test files. First pass review - no need for `global material_cache`, just define at top of file and `clear()` after use. - `for f in bpymesh.uv_textures.new().data`, in this case better to assign the layer a var name since its used immediately after. - some pep8 style conventions aren't followed (no big deal, but should resolve)

Campbell Barton commented

2015-05-21 02:01:42 +02:00

Update, did some tests and this file fails with an exception: (Bear.x3d)
https://savage.nps.edu/svn/repos/nps/list/Savage/AircraftFixedWing/Bear-Russia/

I've committed your update to a branch:
https://developer.blender.org/rBA5e72555f61bbc75d239872dfe26d2d57c7290702

And added you as a committer to:
https://developer.blender.org/diffusion/BA

You'll have to upload SSH keys so you can push:
http://wiki.blender.org/index.php/Dev:Doc/Tools/Git#SSH_Keys

Would you be able to look into this? (and ideally commit a fix to the temp-x3d_import-#44758 branch)

Update, did some tests and this file fails with an exception: (Bear.x3d) https://savage.nps.edu/svn/repos/nps/list/Savage/AircraftFixedWing/Bear-Russia/ I've committed your update to a branch: https://developer.blender.org/rBA5e72555f61bbc75d239872dfe26d2d57c7290702 And added you as a committer to: https://developer.blender.org/diffusion/BA You'll have to upload SSH keys so you can push: http://wiki.blender.org/index.php/Dev:Doc/Tools/Git#SSH_Keys ---- Would you be able to look into this? (and ideally commit a fix to the `temp-x3d_import-#44758` branch)

Seva Alekseyev (NIH) commented

2015-05-21 16:53:50 +02:00

I've got the git branch and I'm working.

Judging by http://pep8online.com/, the file was not PEP8 compliant to begin with.

I've got the git branch and I'm working. Judging by http://pep8online.com/, the file was not PEP8 compliant to begin with.

Campbell Barton commented

2015-05-22 04:00:08 +02:00

@sevaa, re pep8,

We allow lines up to 120 width as an exception (many of the warnings are about this)

Also the pep8 checking tool has changed over time, it warns about things it used not to (when x3d importer was written).

For the purpose of checking pep8 for this script, run:

  pep8 import_x3d.py --ignore=E501,E402,E265,E711,E266

@sevaa, re pep8, We allow lines up to 120 width as an exception (many of the warnings are about this) Also the pep8 checking tool has changed over time, it warns about things it used not to (when x3d importer was written). For the purpose of checking pep8 for this script, run: ``` pep8 import_x3d.py --ignore=E501,E402,E265,E711,E266 ```

Seva Alekseyev (NIH) commented

2015-05-22 04:14:00 +02:00

I've already brought it in line with stock pep8 (with 76 char lines). Lots of refactoring of legacy code was in order :) It was pretty stifling. Oh well, what's done is done. However, I'm afraid I broke something during the refactoring, so I'm retesting everything all over again.

One notable exception - the module has import declarations for bpy stuff in the middle, so that bpy agnostic code is above bpy aware code. Pep8 doesn't like that. I'm leaving those in place; there must've been a reason.

Also, I'm putting together a bunch of reference images, aiming for maximum code coverage. Regarding those, do you have a preferred reference implementation of X3D that you'd be comparing against? I do, it's X3DOM.

I've already brought it in line with stock pep8 (with 76 char lines). Lots of refactoring of legacy code was in order :) It was pretty stifling. Oh well, what's done is done. However, I'm afraid I broke something during the refactoring, so I'm retesting everything all over again. One notable exception - the module has import declarations for bpy stuff in the middle, so that bpy agnostic code is above bpy aware code. Pep8 doesn't like that. I'm leaving those in place; there must've been a reason. Also, I'm putting together a bunch of reference images, aiming for maximum code coverage. Regarding those, do you have a preferred reference implementation of X3D that you'd be comparing against? I do, it's X3DOM.

Campbell Barton commented

2015-05-22 04:21:17 +02:00

Please don't make large changes outside your own additions, it makes the code much harder to review.

If you want to check code style changes don't impact on real output, you can diff the AST. http://stackoverflow.com/a/28598913/432509 (own modified AST pretty-printer).

X3DOM seems fine as a reference.

Please don't make large changes outside your own additions, it makes the code much harder to review. If you want to check code style changes don't impact on real output, you can diff the AST. http://stackoverflow.com/a/28598913/432509 (own modified AST pretty-printer). X3DOM seems fine as a reference.

Seva Alekseyev (NIH) commented

2015-05-22 04:25:25 +02:00

The legacy code had some other PEP8 issues, not just line length. For now, I've left those alone.

As for X3DOM, it doesn't like the filesystem; it likes the Web. Were I to share my ref files, you'd need a kind of a Web framework to display them in the browser from the Web server. I can put one together and upload to some Web space I control, too. Would that help?

The legacy code had some other PEP8 issues, not just line length. For now, I've left those alone. As for X3DOM, it doesn't like the filesystem; it likes the Web. Were I to share my ref files, you'd need a kind of a Web framework to display them in the browser from the Web server. I can put one together and upload to some Web space I control, too. Would that help?

Seva Alekseyev (NIH) commented

2015-05-22 22:57:51 +02:00

I've pushed a commit to the branch. I can see my commit by the explicit link, but it doesn't show up in the branch history. Did I do everything right?

I've uploaded my reference X3Ds in an easy to browse/download format (via X3DOM) at http://www.jishop.com/temp/x3d/

All the polygonal geometries are covered. Elevations and Extrusions are not supported in X3DOM all that well; the former freezes, the latter misdisplays. TriangleFanSets are not supported at all.

I've pushed a commit to the branch. I can see my commit by the explicit link, but it doesn't show up in the branch history. Did I do everything right? I've uploaded my reference X3Ds in an easy to browse/download format (via X3DOM) at http://www.jishop.com/temp/x3d/ All the polygonal geometries are covered. Elevations and Extrusions are not supported in X3DOM all that well; the former freezes, the latter misdisplays. TriangleFanSets are not supported at all.

Campbell Barton commented

2015-05-23 06:04:51 +02:00

@sevaa, somehow you managed to commit Windows style \r\n line endings.
This destroyed all history and rewrote the entire file.

We should have docs to explain how to setup GIT on windows. I think you have to set native line endings.
Please check on changing this setting locally and see if you can find what went wrong.

I had to delete the temp-x3d_import-#44758 branch and re-create it with \n line endings.

Before pushing check your diff... if it rewrites the entire file this is a sign something went wrong.

Fixed commit:
https://developer.blender.org/rBA57a82943aa02b9048017adeda4695d073a6c99e0

You'll have to reset your branch to an older revision before pulling.

@sevaa, somehow you managed to commit Windows style `\r\n` line endings. This destroyed all history and rewrote the entire file. We should have docs to explain how to setup GIT on windows. I think you have to set native line endings. Please check on changing this setting locally and see if you can find what went wrong. I had to delete the ` temp-x3d_import-#44758` branch and re-create it with `\n` line endings. Before pushing check your diff... if it rewrites the entire file this is a sign something went wrong. Fixed commit: https://developer.blender.org/rBA57a82943aa02b9048017adeda4695d073a6c99e0 You'll have to reset your branch to an older revision before pulling.

Campbell Barton commented

2015-05-23 06:21:02 +02:00

Checked with some more test files and since F177571 (also changes after in branch temp-x3d_import-#44758). this file fails to load.

It doesn't have any errors, but no geometry loads.

https://savage.nps.edu/Savage/AircraftFixedWing/B52H-StrategicBomber-UnitedStates/viewpointSlideshow.html

Checked with some more test files and since [F177571](https://archive.blender.org/developer/F177571/import_x3d.py) (also changes after in branch `temp-x3d_import-#44758`). this file fails to load. It doesn't have any errors, but no geometry loads. https://savage.nps.edu/Savage/AircraftFixedWing/B52H-StrategicBomber-UnitedStates/viewpointSlideshow.html

Seva Alekseyev (NIH) commented

2015-05-23 20:44:56 +02:00

Sorry for the CRLF issue and thanks for fixing. I'll re-clone the whole local repository just in case, and I recall Git for Windows has a setting for line ending translation.

I'll take a look at the bomber model.

Sorry for the CRLF issue and thanks for fixing. I'll re-clone the whole local repository just in case, and I recall Git for Windows has a setting for line ending translation. I'll take a look at the bomber model.

Seva Alekseyev (NIH) commented

2015-05-24 17:21:25 +02:00

Found a bug, fixed, committed, please check the CRLFs (Git should've fixed them). As expected, the bug was in the code path that I didn't cover in my dummy models; none of those had explicit normals 'cause I was in a hurry to get something out by the end of Friday. :)

I've tested on several more aircraft models from the Savage site. Aircraft models look so much better with a background :) But I'm in a feature lock.

When the model has a light, it appears all black in Texture mode in Blender, not sure if that's my bug or Blender's feature. You can see that on the Hercules model; if you delete the only spotlight, the textures show up as expected. Also, the semitransparent cabin in the Euro Fighter model doesn't show up as semitransparent in Blender, no matter what I do. The material shows up right in the Properties, but the 3D view doesn't reflect that.

Found a bug, fixed, committed, please check the CRLFs (Git should've fixed them). As expected, the bug was in the code path that I didn't cover in my dummy models; none of those had explicit normals 'cause I was in a hurry to get something out by the end of Friday. :) I've tested on several more aircraft models from the Savage site. Aircraft models look so much better with a background :) But I'm in a feature lock. When the model has a light, it appears all black in Texture mode in Blender, not sure if that's my bug or Blender's feature. You can see that on the Hercules model; if you delete the only spotlight, the textures show up as expected. Also, the semitransparent cabin in the Euro Fighter model doesn't show up as semitransparent in Blender, no matter what I do. The material shows up right in the Properties, but the 3D view doesn't reflect that.

Campbell Barton commented

2015-05-29 03:57:52 +02:00

@sevaa thanks for checking on fixes for issues I reported. Id like to do one final test - batch import many files and check that no new errors are introduced - and that meshes don't load in empty for example., if the importer manages to pass that, the patch can be moved to master after 2.75 release.

@sevaa thanks for checking on fixes for issues I reported. Id like to do one final test - *batch import many files and check that no new errors are introduced - and that meshes don't load in empty for example.*, if the importer manages to pass that, the patch can be moved to master after 2.75 release.

Seva Alekseyev (NIH) commented

2015-05-29 05:06:25 +02:00

Sure, go ahead. Note the latest commit; it was not about geometry, but still.

Any comments regarding the lighting/transparency troubles, please? You know Blender better than I do...

Sure, go ahead. Note the latest commit; it was not about geometry, but still. Any comments regarding the lighting/transparency troubles, please? You know Blender better than I do...

Aaron Carlisle commented

2015-06-01 00:59:26 +02:00

Added subscriber: @Blendify

Sebastian Ullrich commented

2015-08-10 11:41:35 +02:00

Added subscriber: @souljedi

Seva Alekseyev (NIH) commented

2015-08-17 21:14:11 +02:00

I see this one wasn't published with 2.75a... May I ask why not?

Aaron Carlisle commented

2015-08-17 21:18:47 +02:00

2.75a was bug fixes only no new features or anything are aloud

Seva Alekseyev (NIH) commented

2015-08-17 21:19:34 +02:00

Thanks, got it.

Campbell Barton removed their assignment 2015-09-19 14:13:18 +02:00

Bastien Montagne was assigned by Campbell Barton

2015-09-19 14:13:18 +02:00

Campbell Barton commented

2015-09-19 14:13:18 +02:00

Added subscriber: @ideasman42

Campbell Barton commented

2015-09-19 14:13:18 +02:00

Since this patch was submitted, 2 changes were made to X3D import that make the merge fail.

@sevaa or @mont29, could one of you look into resolving the conflict?

The branch is temp-x3d_import-#44758.

Since this patch was submitted, 2 changes were made to X3D import that make the merge fail. - 7051ad3028 - 1b1ad4665b @sevaa or @mont29, could one of you look into resolving the conflict? The branch is `temp-x3d_import-#44758`.

Bastien Montagne commented

2015-09-19 16:15:04 +02:00

@ideasman42 did the merge, hopefully did not break anything… That patch is huge. :|

Bastien Montagne commented

2015-09-19 17:00:47 +02:00

@sevaa are you still active/interested in getting that patch into Blender?

Seva Alekseyev (NIH) commented

2015-09-20 04:36:50 +02:00

Yes, absolutely. What should I do at this juncture? I guess I could test the import module against real models...

Bastien Montagne commented

2015-09-20 08:53:23 +02:00

Well, merge has been done already, also fixed some issue in mesh importing code (just look at the branch's log).

What we need now is to test as much .x3d (and.wrml) files as possible (already done a few from https://savage.nps.edu/Savage), if OK think in one week or two (once 2.76 is definitively published) we can merge this back in master.

And would be nice if you could check current code to see whether merge was OK or not (think it's OK, but it was rather big conflict, so…).

Well, merge has been done already, also fixed some issue in mesh importing code (just look at [the branch's log](https://developer.blender.org/diffusion/BA/history/temp-x3d_import-#44758/)). What we need now is to test as much .x3d (and.wrml) files as possible (already done a few from https://savage.nps.edu/Savage), if OK think in one week or two (once 2.76 is definitively published) we can merge this back in master. And would be nice if you could check current code to see whether merge was OK or not (think it's OK, but it was rather big conflict, so…).

Seva Alekseyev (NIH) commented

2015-09-21 16:43:29 +02:00

Maybe I was looking on the wrong place, but the merge seemed rather limited to me; just some checks for null objects. This one, right? https://developer.blender.org/rBAef984a976590fd619c35e898cbcc16cf9f947645

Anyway, I've fetched the latest and I'm testing against my cache of X3Ds...

Maybe I was looking on the wrong place, but the merge seemed rather limited to me; just some checks for null objects. This one, right? https://developer.blender.org/rBAef984a976590fd619c35e898cbcc16cf9f947645 Anyway, I've fetched the latest and I'm testing against my cache of X3Ds...

Bastien Montagne commented

2015-09-21 16:47:38 +02:00

sigh no, previous commit, as stated in commit message… https://developer.blender.org/rBA9733e526cf6c325e8b0e687825817841b50a9246

*sigh* no, previous commit, as stated in commit message… https://developer.blender.org/rBA9733e526cf6c325e8b0e687825817841b50a9246

Seva Alekseyev (NIH) commented

2015-09-21 16:52:59 +02:00

That one looks pretty sensible to me, too. I trust the VRML parsing code changes were there for a reason; the X3D-not-VRML geometry bits that I've been working on are pretty much intact. My test models are all X3D anyway.

Bastien Montagne commented

2015-09-21 17:22:27 +02:00

Ok, then will do some more tests in comming days, and then once 2.76 is out for good we can merge into master. :)

Seva Alekseyev (NIH) commented

2015-09-21 17:27:18 +02:00

FYI, my test models, save for Savage and a few reference ones, are coming from here: http://3dprint.nih.gov/

Seva Alekseyev (NIH) commented

2015-09-24 22:06:15 +02:00

I've identified one pretty exotic use case where the importer misbehaves. X3DOM handles those cases. Shall I fix and commit, or would you rather have code freeze?

Bastien Montagne commented

2015-09-24 22:27:39 +02:00

No, feel free to commit, this is a branch so no need to bother about release and freeze here ;)

Seva Alekseyev (NIH) commented

2015-09-24 22:47:12 +02:00

Fixed, committed, enjoy. Anything on your tests?

EDIT: my commit doesn't show up for some reason... Could be latency, could be my messup :(

Fixed, committed, enjoy. Anything on your tests? EDIT: my commit doesn't show up for some reason... Could be latency, could be my messup :(

Seva Alekseyev (NIH) commented

2015-09-29 18:47:52 +02:00

Any update re:testing?

Bastien Montagne commented

2015-09-29 20:13:34 +02:00

Sorry, no, did not had time for that last week (2.76 is not yet out anyway ;) ).

Bastien Montagne commented

2015-10-15 12:49:21 +02:00

Changed status from 'Open' to: 'Resolved'

Bastien Montagne closed this issue

2015-10-15 12:49:21 +02:00

Bastien Montagne commented

2015-10-15 12:49:21 +02:00

Made some more testing, everything looks fine so far, so merged it to master (44cc56c927) - real testing can only happen when users use it anyway…

@sevaa please commit any further fix directly to master, I deleted the working branch anyway (and do not hesitate to first get review on phabdiff first in case of big/potentially dangerous changes, if any. :)

Made some more testing, everything looks fine so far, so merged it to master (44cc56c927) - real testing can only happen when users use it anyway… @sevaa please commit any further fix directly to master, I deleted the working branch anyway (and do not hesitate to first get review on [phabdiff](https://developer.blender.org/maniphest/task/create/?project=3&type=Patch) first in case of big/potentially dangerous changes, if any. :)

Seva Alekseyev (NIH) commented

2015-10-23 18:24:21 +02:00

Found a subtle bug. I'll commit to master.

Also, support for Text nodes would greatly help testing - annotating models would be helpful. I think I'll implement some support while I'm at it.

Found a subtle bug. I'll commit to master. Also, support for Text nodes would greatly help testing - annotating models would be helpful. I think I'll implement some support while I'm at it.

Seva Alekseyev (NIH) commented

2015-10-27 17:29:50 +01:00

Hi all,

now that the patch is committed to master, what are my time constraints, please? I'm currently discussing some fine points of the X3D standard with the Web3D people, this may cause another round of changes. What's your code freeze policy, if any?

Hi all, now that the patch is committed to master, what are my time constraints, please? I'm currently discussing some fine points of the X3D standard with the Web3D people, this may cause another round of changes. What's your code freeze policy, if any?

Sign in to join this conversation.