Enhancements to OGR DXF Driver

[srock]

Answer to the first test, where the angle is missing:

That was my mistake. In all the dxf-files I had tested, there was a DXF-Group-Code of 42, where the actual measurement value was written. Therefore I used this value for the dimension label below the comment

// Do we need to compute the dimension value?

But as I now found in the AutoCAD DXF-reference, this code is only optional. I fixed this problem in the patch "gdal-ogr-dxf-20111212_.patch".

Answer to the second test:

In TranslateINSERT I added:

/* -------------------------------------------------------------------- */
/*      if the subfeature is a text, then adjust                        */
/*		- text angle                                                    */
/*		- text height                                                   */
/*		- text width-factor (Stretch)                                   */
/* -------------------------------------------------------------------- */
            int iType = poSubFeature->GetGeometryRef()->getGeometryType();
            OGRStyleMgr *poStyle = new OGRStyleMgr();
            poStyle->InitFromFeature(poSubFeature);

            if ( wkbFlatten(iType) == wkbPoint && poStyle->GetStyleString()!=NULL )
            {
                const char *str;
                OGRStyleLabel *poLabel = (OGRStyleLabel*) poStyle->GetPart(0);
                GBool bN;
                double dfAngle	 = poLabel->Angle(bN);
                poLabel->SetUnit(OGRSTUGround);
                double dfSize	 = poLabel->Size(bN);
                double dfStretch = poLabel->Stretch(bN);
                if (dfStretch==0.0) dfStretch = 1.0;
                poLabel->SetAngle	(dfAngle   + oTransformer.dfAngle / PI * 180.0);
                poLabel->SetSize	(dfSize    * oTransformer.dfYScale);
                poLabel->SetStretch	(dfStretch * oTransformer.dfXScale / oTransformer.dfYScale);
                poSubFeature->SetStyleString(poLabel->GetStyleString());
                str = poLabel->GetParamStr(OGRSTLabelFontName,bN);
            }

            delete poStyle;

This could be the source of the problem. In the line

poSubFeature->SetStyleString(poLabel->GetStyleString());

I copy the style string. Today I realized that in the method "OGRStyleTool::GetStyleString" of the file "ogrfeaturestyle.cpp" the style string is generated without quotation marks! I'm not sure if this is correct. The solution would be either to remove my change above, or to change the behaviour of GetStyleString. Or maybe there is a better solution for fixing the "subtle things".

[Even Rouault]

This belongs to FrankW's area of expertise, but I did try to read the patch and was stopped by its unreadability as a patch. I guess there must have issues with your text editor messing with end of line characters, or something like that, but if you look at http://trac.osgeo.org/gdal/attachment/ticket/4369/gdal-ogr-dxf-20111205.patch, you'll see that is impossible to know if you have indeed rewritten everything, or if your changes are more local. I also observe strange indentations, perhaps due to the use of tab characters ?

[srock]

Replying to rouault:

This belongs to FrankW's area of expertise, but I did try to read the patch and was stopped by its unreadability as a patch. I guess there must have issues with your text editor messing with end of line characters, or something like that, but if you look at http://trac.osgeo.org/gdal/attachment/ticket/4369/gdal-ogr-dxf-20111205.patch, you'll see that is impossible to know if you have indeed rewritten everything, or if your changes are more local. I also observe strange indentations, perhaps due to the use of tab characters ?

I'm sorry, there was indeed a problem with tab characters and end of line characters. I removed all tab characters and changed the end of line characters to UNIX format. I think now it looks better. I have uploaded the fixed patch again as a replacement for the previous one. I have also corrected some typos in the description above.

[warmerdam]

An impressive set of changes. I'll try to review and integrate them tonight.

I'd like to see additional information on text escape sequences captured as close to the translation code as possible.

[warmerdam]

In ogrdxflayer.cpp I see:

    // Why don't add these fields in "AddStandardFields" ??
    // That seems more consistent to me, so I removed them here.
    /*
    if( !poDS->InlineBlocks() )
    {
        OGRFieldDefn  oScaleField( "BlockScale", OFTRealList );
        poFeatureDefn->AddFieldDefn( &oScaleField );

        OGRFieldDefn  oBlockAngleField( "BlockAngle", OFTReal );
        poFeatureDefn->AddFieldDefn( &oBlockAngleField );
    }
    */

The answer is that these fields are not appropriate in a blocks layer as things were setup. I didn't see any obvious reason why this change was needed as part of your patch so I have reverted it.

[warmerdam]

As I'm trying to work through the ogr_dxf.py test script I'm getting some differences that don't seem kosher.

  TEST: ogr_dxf_8 ... fail
    line 285: Got unexpected style string:
LABEL(f:"Arial",t:"0.0000",p:5,a:43.3,s:2.5g)
instead of:
LABEL(f:"Arial",t:"54.3264",p:5,a:43.3,s:2.5g).

In the above test, it seems as if the angle with which the dimension is supposed to be labelled is lost.

  TEST: ogr_dxf_9 ... fail
    line 323: Got unexpected style string:
LABEL(f:Arial,s:0.5g,t:Text Sample1¿λ
"abc",a:45,c:#000000,p:5,w:1.000000)
instead of:
LABEL(f:"Arial",t:"Text Sample1¿λ
\"abc\"",a:45,s:0.5g,p:5,c:#000000).

In the above difference there seems to have been some decay of the quoting logic for the text in a LABEL.

Any thoughts on these? I have made a few changes. The above mentioned one about blockscale/angle and also I moved PaperSpace and Invisible to the end of AddStandardFields() to avoid renumbering existing fields.

[srock]

Replying to warmerdam:

In ogrdxflayer.cpp I see:

    // Why don't add these fields in "AddStandardFields" ??
    // That seems more consistent to me, so I removed them here.
    /*
    if( !poDS->InlineBlocks() )
    {
        OGRFieldDefn  oScaleField( "BlockScale", OFTRealList );
        poFeatureDefn->AddFieldDefn( &oScaleField );

        OGRFieldDefn  oBlockAngleField( "BlockAngle", OFTReal );
        poFeatureDefn->AddFieldDefn( &oBlockAngleField );
    }
    */

The answer is that these fields are not appropriate in a blocks layer as things were setup. I didn't see any obvious reason why this change was needed as part of your patch so I have reverted it.

No, this part is not needed for the patch, it just confused me that the fields were not added in "AddStandardFields" and I thought that would be a mistake.

[srock]

Replying to warmerdam:

As I'm trying to work through the ogr_dxf.py test script I'm getting some differences that don't seem kosher.

  TEST: ogr_dxf_8 ... fail
    line 285: Got unexpected style string:
LABEL(f:"Arial",t:"0.0000",p:5,a:43.3,s:2.5g)
instead of:
LABEL(f:"Arial",t:"54.3264",p:5,a:43.3,s:2.5g).

In the above test, it seems as if the angle with which the dimension is supposed to be labelled is lost.

  TEST: ogr_dxf_9 ... fail
    line 323: Got unexpected style string:
LABEL(f:Arial,s:0.5g,t:Text Sample1¿λ
"abc",a:45,c:#000000,p:5,w:1.000000)
instead of:
LABEL(f:"Arial",t:"Text Sample1¿λ
\"abc\"",a:45,s:0.5g,p:5,c:#000000).

In the above difference there seems to have been some decay of the quoting logic for the text in a LABEL.

Any thoughts on these? I have made a few changes. The above mentioned one about blockscale/angle and also I moved PaperSpace and Invisible to the end of AddStandardFields() to avoid renumbering existing fields.