Page MenuHome

Expanded X3D import addon
Closed, ResolvedPublic

Description

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

Details

Type
Patch

Event Timeline

Sergey Sharybin (sergey) triaged this task as Normal priority.May 19 2015, 6:20 PM

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

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

@Sergey Sharybin (sergey) never did anything with x3d… :/

@Seva Alekseyev (NIH) (sevaa): do you have a set of nice ref files to check import behavior?

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

Checked F177571,

@Seva Alekseyev (NIH) (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 (campbellbarton) lowered the priority of this task from Normal to Incomplete.May 21 2015, 2:01 AM

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-T44758 branch)

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

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

@Seva Alekseyev (NIH) (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

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.

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.

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?

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.

@Seva Alekseyev (NIH) (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-T44758 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.

Checked with some more test files and since F177571 (also changes after in branch temp-x3d_import-T44758). 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

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.

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.

@Seva Alekseyev (NIH) (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.

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 (Blendify) raised the priority of this task from Incomplete to Normal.Jun 1 2015, 12:59 AM

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

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

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

@Seva Alekseyev (NIH) (sevaa) or @Bastien Montagne (mont29), could one of you look into resolving the conflict?

The branch is temp-x3d_import-T44758.

@Campbell Barton (campbellbarton) did the merge, hopefully did not break anything… That patch is huge. :|

@Seva Alekseyev (NIH) (sevaa) are you still active/interested in getting that patch into Blender?

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

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…).

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

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.

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

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

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?

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

Fixed, committed, enjoy. Anything on your tests?

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

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

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

@Seva Alekseyev (NIH) (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. :)

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.

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?