Sukender | 31 May 2012 11:14
Picon
Favicon
Gravatar

ReaderWriter X reworked

Hi Robert,

Here is another patch, and an (unrelated) question...

Let start with the question: I told you (once upon a time) about double precision Tesselator and
TesselateVisitor. You roughly told me "wait for tesselation to be replaced with Delaunay". However the
tesselation hasn't changed much since then. May I submit my code then? It may be thrown away when Delaunay
will be there, but at least double precision tessellation will be working :) We use it daily (or such) and it
works nicely.

And now the patch. It is about .X:
- Brand new writer for .x format. Yes, the format is deprecated but some still use it. Current
implementation has limitations (only supports one texture, for instance) but works.
- Reader
  - Added checks, warnings, and crash guards
  - Now handles "FrameTransformMatrix" element (kind of osg::MatrixTransform)
  - Fixed CW / CCW for faces
  - Better handling of materials (handles meshes with no materials, fixed reading of transparent textures...)
  - Handling triangles and polygons differently
  - Reader can now create indexed geometries
  - And more...
The reader clearly doesn't read 100% of .x files, but handling is way better now.

Sorry for the huge amount of changes, but these are the result of many commits from a colleague, and giving
you these commits separately won't help you merge the code as the "1 commit = 1 feature" rule was not
followed at all...

Cheers,

Sukender
(Continue reading)

Ulrich Hertlein | 14 Jun 2012 15:52
Picon
Favicon

Re: ReaderWriter X reworked

Hi Sukender,

I had a quick look at the patch and checked that it still works with the various .x files
I have flying around.

One thing I did notice in the code is that you're passing 'const std::string' in some
places.  For the sake of efficiency these chould be replaced with 'const std::string&' to
avoid making a copy every time.  Same for passing 'osg::Vec', these should always be
passed as const-ref unless they are modified or returned.

Maybe this is something you could fix and resubmit?  Apart from that I'd say it's good to go.

Just my $0.02

Cheers,
/ulrich

On 31/05/12 11:14 , Sukender wrote:
> And now the patch. It is about .X:
> - Brand new writer for .x format. Yes, the format is deprecated but some still use it. Current
implementation has limitations (only supports one texture, for instance) but works.
> - Reader
>   - Added checks, warnings, and crash guards
>   - Now handles "FrameTransformMatrix" element (kind of osg::MatrixTransform)
>   - Fixed CW / CCW for faces
>   - Better handling of materials (handles meshes with no materials, fixed reading of transparent textures...)
>   - Handling triangles and polygons differently
>   - Reader can now create indexed geometries
>   - And more...
> The reader clearly doesn't read 100% of .x files, but handling is way better now.
(Continue reading)

Sukender | 2 Jul 2012 21:03
Picon
Favicon
Gravatar

Re: [osg-submissions] ReaderWriter X reworked

Hi Ulrich,

I also spotted the missing '&'. You're absolutely right.
Well, if you already fixed them you can submit ^^... or else I'll do (but sincerely I don't know when...)

Cheers,

Sukender

----- Mail original -----
De: "Ulrich Hertlein" <u.hertlein <at> sandbox.de>
À: "OpenSceneGraph Submissions" <osg-submissions <at> lists.openscenegraph.org>
Cc: "Sukender" <suky0001 <at> free.fr>
Envoyé: Jeudi 14 Juin 2012 15:52:30
Objet: Re: [osg-submissions] ReaderWriter X reworked

Hi Sukender,

I had a quick look at the patch and checked that it still works with the various .x files
I have flying around.

One thing I did notice in the code is that you're passing 'const std::string' in some
places.  For the sake of efficiency these chould be replaced with 'const std::string&' to
avoid making a copy every time.  Same for passing 'osg::Vec', these should always be
passed as const-ref unless they are modified or returned.

Maybe this is something you could fix and resubmit?  Apart from that I'd say it's good to go.

Just my $0.02

(Continue reading)

Ulrich Hertlein | 8 Sep 2012 05:57
Picon
Favicon

Re: ReaderWriter X reworked

Hi Sukender, hi Robert,

After using this patch on a fresh checkout it doesn't build for me on OS X 10.8 with clang++:

Linking CXX shared module ../../../lib/osgPlugins-3.1.3/osgdb_x.so
Undefined symbols for architecture x86_64:
  "VTT for WriterNodeVisitorDirectX", referenced from:
      WriterNodeVisitorDirectX::WriterNodeVisitorDirectX(std::ostream&, bool, bool,
std::string, std::string) in ReaderWriterDirectX.cpp.o
      WriterNodeVisitorDirectX::~WriterNodeVisitorDirectX() in ReaderWriterDirectX.cpp.o
  "vtable for WriterNodeVisitorDirectX", referenced from:
      WriterNodeVisitorDirectX::WriterNodeVisitorDirectX(std::ostream&, bool, bool,
std::string, std::string) in ReaderWriterDirectX.cpp.o
  NOTE: a missing vtable usually means the first non-inline virtual member function has no
definition.
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [lib/osgPlugins-3.1.3/osgdb_x.so] Error 1
make[1]: *** [src/osgPlugins/x/CMakeFiles/osgdb_x.dir/all] Error 2
make: *** [all] Error 2

This is something I've seen (and fixed) before but don't remember what the fix was.

At that time I also found that the plugin isn't able to read files generated by itself.

Given this last issue and some coding issues (e.g. passing std::string copies instead of
references) maybe Sukender could address these and re-submit?

Cheers,
/ulrich
(Continue reading)


Gmane