Hi Wang,

Wow, that was a quick turn around.  I've begun a code review, and so
far am impressed as how much code you've been able to share between
the binary + ascii implementations.  I do wonder about the performance
impact on binary reading that there will be due to adding all the if
statements that are required for ascii support.  Have you performacne
profiled the new plugin against your previous one or the .ive plugin?

In reviewing the wrappers I do wonder if we might be able to wrap the
individual properties as objects in their own right, with each of
these mini classes handling the reading and writing of that specific
property, then the classes wrappers just containers for this property
wrapper classes.  I experimented with such an approach in
VirtualPlanetBuilder's src/vpb/BuildOptionsIO.cpp.  This approach
extends the standard .osg format in a very compact way that is far
less error prone and verbose that usual DotOsgWrappers.  This approach
does use lots of templates and macros though.  To get a taste have a
look at how I wrapped the BuildOptions class that has dozens of
separate properties of different types:

    BuildOptionsLookUps()
    {
        BuildOptions prototype;

        ADD_STRING_PROPERTY(Directory);
        ADD_BOOL_PROPERTY(OutputTaskDirectories);
        ADD_STRING_PROPERTY(DestinationTileBaseName);
        ADD_STRING_PROPERTY(DestinationTileExtension);
        ADD_STRING_PROPERTY(DestinationImageExtension);
        ADD_BOOL_PROPERTY(PowerOfTwoImages);
        ADD_STRING_PROPERTY(ArchiveName);
        ADD_STRING_PROPERTY(IntermediateBuildName);
        ADD_STRING_PROPERTY(LogFileName);
        ADD_STRING_PROPERTY(TaskFileName);
        ADD_STRING_PROPERTY(CommentString);

        ADD_ENUM_PROPERTY_TWO_VALUES(DatabaseType, LOD_DATABASE,
PagedLOD_DATABASE)
        ADD_ENUM_PROPERTY_THREE_VALUES(GeometryType, HEIGHT_FIELD,
POLYGONAL, TERRAIN)
        ADD_ENUM_PROPERTY_THREE_VALUES(MipMappingMode, NO_MIP_MAPPING,
MIP_MAPPING_HARDWARE,MIP_MAPPING_IMAGERY)

        {
            AEP(TextureType);
            AEV(RGB_24);
            AEV(RGBA);
            AEV(RGB_16);
            AEV(RGBA_16);
            AEV(RGB_S3TC_DXT1);
            AEV(RGBA_S3TC_DXT1);
            AEV(RGBA_S3TC_DXT3);
            AEV(RGBA_S3TC_DXT5);
            AEV(ARB_COMPRESSED);
            AEV(COMPRESSED_TEXTURE);
            AEV(COMPRESSED_RGBA_TEXTURE);
        }

        ADD_UINT_PROPERTY(MaximumTileImageSize);
        ADD_UINT_PROPERTY(MaximumTileTerrainSize);

        ADD_FLOAT_PROPERTY(MaximumVisibleDistanceOfTopLevel);
        ADD_FLOAT_PROPERTY(RadiusToMaxVisibleDistanceRatio);
        ADD_FLOAT_PROPERTY(VerticalScale);
        ADD_FLOAT_PROPERTY(SkirtRatio);
        ADD_UINT_PROPERTY(ImageryQuantization);
        ADD_BOOL_PROPERTY(ImageryErrorDiffusion);
        ADD_FLOAT_PROPERTY(MaxAnisotropy);

        ADD_BOOL_PROPERTY(BuildOverlays);
        ADD_BOOL_PROPERTY(ReprojectSources);
        ADD_BOOL_PROPERTY(GenerateTiles);
        ADD_BOOL_PROPERTY(ConvertFromGeographicToGeocentric);
        ADD_BOOL_PROPERTY(UseLocalTileTransform);
        ADD_BOOL_PROPERTY(SimplifyTerrain);
        ADD_BOOL_PROPERTY(DecorateGeneratedSceneGraphWithCoordinateSystemNode);
        ADD_BOOL_PROPERTY(DecorateGeneratedSceneGraphWithMultiTextureControl);
        ADD_BOOL_PROPERTY(WriteNodeBeforeSimplification);

        ADD_VEC4_PROPERTY(DefaultColor);

        ADD_BOOL_PROPERTY(UseInterpolatedImagerySampling);
        ADD_BOOL_PROPERTY(UseInterpolatedTerrainSampling);

        ADD_STRING_PROPERTY(DestinationCoordinateSystem);
        ADD_STRING_PROPERTY(DestinationCoordinateSystemFormat);
        ADD_DOUBLE_PROPERTY(RadiusPolar);
        ADD_DOUBLE_PROPERTY(RadiusEquator);

        _serializerList.push_back(new GeospatialExtentsSerializer<BuildOptions>(
                "DestinationExtents",
                prototype.getDestinationExtents(),
                &BuildOptions::getDestinationExtents,
                &BuildOptions::setDestinationExtents));

        ADD_UINT_PROPERTY(MaximumNumOfLevels);

        ADD_UINT_PROPERTY(DistributedBuildSplitLevel);
        ADD_UINT_PROPERTY(DistributedBuildSecondarySplitLevel);
        ADD_BOOL_PROPERTY(RecordSubtileFileNamesOnLeafTile);
        ADD_BOOL_PROPERTY(GenerateSubtile);
        ADD_UINT_PROPERTY(SubtileLevel);
        ADD_UINT_PROPERTY(SubtileX);
        ADD_UINT_PROPERTY(SubtileY);

        { AEP(NotifyLevel); AEV(ALWAYS); AEV(FATAL); AEV(WARN);
AEV(NOTICE); AEV(INFO); AEV(DEBUG_INFO); AEV(DEBUG_FP); }

        ADD_BOOL_PROPERTY(DisableWrites);

        ADD_FLOAT_PROPERTY(NumReadThreadsToCoresRatio);
        ADD_FLOAT_PROPERTY(NumWriteThreadsToCoresRatio);

        ADD_STRING_PROPERTY(BuildOptionsString);
        ADD_STRING_PROPERTY(WriteOptionsString);

        { AEP(LayerInheritance); AEV(INHERIT_LOWEST_AVAILABLE);
AEV(INHERIT_NEAREST_AVAILABLE); AEV(NO_INHERITANCE); }

        ADD_BOOL_PROPERTY(AbortTaskOnError);
        ADD_BOOL_PROPERTY(AbortRunOnError);

        { AEP2(DefaultImageLayerOutputPolicy, LayerOutputPolicy);
AEV(INLINE); AEV(EXTERNAL_LOCAL_DIRECTORY);
AEV(EXTERNAL_SET_DIRECTORY); }
        { AEP2(DefaultElevationLayerOutputPolicy, LayerOutputPolicy);
AEV(INLINE); AEV(EXTERNAL_LOCAL_DIRECTORY);
AEV(EXTERNAL_SET_DIRECTORY); }

        { AEP2(OptionalImageLayerOutputPolicy, LayerOutputPolicy);
AEV(INLINE); AEV(EXTERNAL_LOCAL_DIRECTORY);
AEV(EXTERNAL_SET_DIRECTORY); }
        { AEP2(OptionalElevationLayerOutputPolicy, LayerOutputPolicy);
AEV(INLINE); AEV(EXTERNAL_LOCAL_DIRECTORY);
AEV(EXTERNAL_SET_DIRECTORY); }

        _serializerList.push_back(new SetSerializer<BuildOptions,
BuildOptions::OptionalLayerSet,
BuildOptions::OptionalLayerSet::const_iterator>(
                "OptionalLayerSet",
                prototype.getOptionalLayerSet(),
                &BuildOptions::getOptionalLayerSet,
                &BuildOptions::setOptionalLayerSet));

        ADD_UINT_PROPERTY(RevisionNumber);
    }


The above provides all the read and write support, with the properties
being access via Type CLASS::getPROPERTYNAME() and void
CLASS::setPROPERTYNAME(Type) methods, with macro's adding the
get's/set's and types automatically.

The VPB code doesn't support binary and ascii, and I haven't profiled
how expensive it is in memory or speed, for ascii support I would
expect it be pretty competitive to the standard OSG ascii, but for
binary I guess it would fare less well w.r.t .ive.

I do wonder if for the new binary/ascii format we might be adopt such
as scheme as the above.  it's certainly easier to write and
potentially automate than the normal .osg wrappers, .ive
implementation or your new proposed codes.  Perhaps merging the two
schemes might be possible, get the best of both worlds.

Robert.
_______________________________________________
osg-users mailing list
osg-users@lists.openscenegraph.org
http://lists.openscenegraph.org/listinfo.cgi/osg-users-openscenegraph.org

Reply via email to