Opened 19 years ago

Last modified 19 years ago

#661 closed defect (invalid)

Fields added not found in output files

Reported by: mielket@… Owned by: warmerdam
Priority: high Milestone:
Component: OGR_SF Version: unspecified
Severity: normal Keywords:
Cc:

Description

When I add fields to a feature they are not part of the data representation 
(tested with GML and SHP), though they are being dumped with DumpReadable.

I tried everything to get it to work. 

Here is the code:
____

#include "..\gdal-1.2.2\ogr\ogrsf_frmts\ogrsf_frmts.h"

void CSynOptXDoc::OnFileExportSpatial() 
{
    OGRRegisterAll();

	OGRDataSource *poDS;

	OGRSFDriverRegistrar *poR = OGRSFDriverRegistrar::GetRegistrar();
	OGRSFDriver          *poDriver = NULL;
	int                  iDriver;

	for( iDriver = 0;
		 iDriver < poR->GetDriverCount() && poDriver == NULL;
		 iDriver++ )
	{
		//if( EQUAL(poR->GetDriver(iDriver)->GetName(),"ESRI 
Shapefile") )
		if( EQUAL(poR->GetDriver(iDriver)->GetName(),"GML") )
		{
			poDriver = poR->GetDriver(iDriver);
		}
	}

	if (!poDriver)
	{
		CString cs;
		CString csDriverName = "GML";
		cs.Format("%s driver is not available.", csDriverName);
		AfxMessageBox(cs.GetBuffer(0));
		return;
	}

	if( !poDriver->TestCapability( ODrCCreateDataSource ) )
	{
		CString cs;
		CString csDriverName = "GML";
		cs.Format("%s driver does not support data source creation.", 
csDriverName);
		AfxMessageBox(cs.GetBuffer(0));
		return;
	}

	//if(!(poDS = poDriver->CreateDataSource
("D:\\work\\SampleData\\Export.shp"))) return;
	if(!(poDS = poDriver->CreateDataSource
("D:\\work\\SampleData\\Export.gml"))) return;
    
	if(!(poDS->TestCapability(ODsCCreateLayer))) return;
	
	CSynLayer  *pLayerIterate = m_layers;

	// constuct feature definition
	OGRFeatureDefn *poFD = new OGRFeatureDefn("Rectangle");
	poFD->SetGeomType(wkbLineString);

	// construct attribute field definition
	CTime i;	
	int n_max = 0;	
	CSynOptXDoc *pDoc = this;
	int n;
	for (i = pDoc->m_starttime, n = 0; i < pDoc->m_endtime; i += pDoc-
>m_interval, n++)
	{
		CString cs = i.Format("%H_%M");	// time as field name
		OGRFieldDefn oFLD(cs.GetBuffer(0), OFTReal);
		oFLD.Set(cs.GetBuffer(0), OFTReal, 0, 0, OJUndefined);
		OGRField oField; oField.Real = 0.0;
		oFLD.SetDefault(&oField);
		poFD->AddFieldDefn(&oFLD);
	}
	n_max = n;
FILE *file;
file = fopen("C:\\llll.txt", "w");
	while (pLayerIterate)
	{
		// papszOptions = CSLSetNameValue( papszOptions, "DIM", "2" );

		OGRLayer *poL = poDS->CreateLayer(pLayerIterate->name.GetBuffer
(0),  
									
	NULL,  
									
	wkbUnknown,  
									
	NULL);
		
		
		CSynObject *psObjIterate = pLayerIterate->objects;
		while (psObjIterate)
		{
			if (psObjIterate->vertex_size == 2)
			{
				// construct geometry
				OGRLinearRing oLR;
				oLR.addPoint(psObjIterate->vertex[0].x, -
psObjIterate->vertex[0].y);
				oLR.addPoint(psObjIterate->vertex[0].x, -
psObjIterate->vertex[1].y);
				oLR.addPoint(psObjIterate->vertex[1].x, -
psObjIterate->vertex[1].y);
				oLR.addPoint(psObjIterate->vertex[1].x, -
psObjIterate->vertex[0].y);
				oLR.addPoint(psObjIterate->vertex[0].x, -
psObjIterate->vertex[0].y);
				OGRFeature *poF = new OGRFeature(poFD);
				poF->SetGeometryDirectly(&oLR);

				// construct temporal activity values in 
attribute table
				CSynTime *pt = psObjIterate->times;
				while (pt)
				{
					CString cs = pt->time.Format("%H_%M");
	
					n = poFD->GetFieldIndex(cs.GetBuffer
(0));		// retrieve field index by time field name
					if (n >= 0) poF->SetField(n, pt-
>activity);

					pt = pt->next;
				}
poF->DumpReadable(file);  
				poL->CreateFeature(poF);
			}
				
			psObjIterate = psObjIterate->next;
		}

		pLayerIterate = pLayerIterate->next;
	}

	OGRErr ret = poDS->SyncToDisk();
fclose(file);	
	n = poDS->GetLayerCount(); // just for debugging	
	n = n - 0;

	delete poFD;

	delete poDS;
}
______

And here is some dumped output:

______


OGRFeature(Rectangle):-1
  00_00 (Real) = (null)
  01_00 (Real) = (null)
  02_00 (Real) = (null)
  03_00 (Real) = (null)
  04_00 (Real) = (null)
  05_00 (Real) = (null)
  06_00 (Real) = (null)
  07_00 (Real) = 0
  08_00 (Real) = 740
  09_00 (Real) = 740
  10_00 (Real) = 950
  11_00 (Real) = 950
  12_00 (Real) = 360
  13_00 (Real) = 950
  14_00 (Real) = 950
  15_00 (Real) = 910
  16_00 (Real) = 910
  17_00 (Real) = 735
  18_00 (Real) = 735
  19_00 (Real) = 100
  20_00 (Real) = 100
  21_00 (Real) = 0
  22_00 (Real) = (null)
  23_00 (Real) = (null)
  LINEARRING (403 -420,403 -484,437 -484,437 -420,403 -420)
______

The same as GML (without the attribute fields):
______

  <gml:featureMember>
    <Studierendenzahl_gesamt__Uni__Dez._5_ fid="10">
      <ogr:geometryProperty><gml:LinearRing><gml:coordinates>403,-420 403,-484 
437,-484 437,-420 403,-
420</gml:coordinates></gml:LinearRing></ogr:geometryProperty>
    </Studierendenzahl_gesamt__Uni__Dez._5_>
  </gml:featureMember>
_______

Can you help?

Change History (14)

comment:1 by warmerdam, 19 years ago

Thomas, 

You are doing this:

 o Create datasource. 
 o Create OGRFeatureDefn (Rectangle)
 o Add fields to OGRFeatureDefn.
 o Create OGRLayer.
 o Loop creating features of the OGRFeatureDefn "Rectangle" and write to layer.

You need to:
 o Create datasource
 o Create Layer.
 o Create new fields on layer with CreateField(). 
 o Fetch back OGRFeatureDefn from Layer. 
 o Loop creating features of the OGRFeatureDefn from the layer and write them. 

You are not allowed to call OGRLayer::CreateFeature() except with features
created against the OGRFeatureDefn of the same layer.  I am going to leave
this bug open till I actually enforce this condition in the CreateFeature()
call somehow. You aren't the first person to make this mistake because the
docs are fuzzy, and the library isn't helping you identify the error. 



comment:2 by mielket@…, 19 years ago

Sorry, Frank, to bother you again. But I now begin to become relly confused 
after trying to reproduce your suggested steps:

 o Create datasource
 o Create Layer.

Ok.

 o Create new fields on layer with CreateField(). 

I cannot find a member called CreateField() in any class. I only can guess 
that you mean CreateFeature(). But if I don't call it in the loop, what is the 
right method to serialize my features?

 o Fetch back OGRFeatureDefn from Layer. 

How is OGRFeatureDefn connected to the Layer? I cannot find any clue for it 
and the documentation suggests that OGRFeatureDefn is separated from OGRLayer 
in order to reuse OGRFeatureDefns for different layers, for example.

 o Loop creating features of the OGRFeatureDefn from the layer and write them. 

If I guessed right two steps ago, CreateFeature() isn't the right method to 
create features.

Thanks for your help.

comment:3 by warmerdam, 19 years ago

Thomas,  

I do indeed mean OGRLayer::CreateField(). 

I was going to post the URL to the docs, but I am shocked to see CreateField()
is not listed in the online documentation!

The method looks like this:

    virtual OGRErr      CreateField( OGRFieldDefn *poField,
                                     int bApproxOK = TRUE );

So you would prpeare an OGRFieldDefn and then call CreateField() to add
it to a layer.  When you have added all the fields, fetch the OGRFeatureDefn
back from the layer and use that for creating OGRFeature() objects.  Once
the attributes and geometry are set on the OGRFeature use
OGRLayer::CreateFeature() to write the feature out to the file. 



comment:4 by warmerdam, 19 years ago

Added docs at:

  http://ogr.maptools.org/classOGRLayer.html#a17

comment:5 by mielket@…, 19 years ago

With CreateField the fields are now added to the datasource, which is a 
progress. But they contain no values.

To me, the last secret now is how to "fetch back" the field definition from 
the layer. I re-read the documentation and took a look at the OGRLayer class 
declaration but found no member such as "GetFeatureDefn" or "m_poFeatureDefn".

I searched for OGRFeatureDefn and found that GetLayerDefn() returned such an 
object and used it instead of the genuine feature definition (without effect 
in the output datasource). But an OGRFeatureDefn is never associated with the 
layer. How can I give a field definition to the layer and get a feature 
definition? Wouldn't it make more sense to have the CreateField in the 
FeatureDefn and have a SetFeatureDefn member for the layer? It seems, I still 
don't understand the philosophy.

comment:6 by warmerdam, 19 years ago

GetLayerDefn() is the correct method.  Why do you say you used that instead of
the *genuine* feature definition?  This methods returns a pointer to the real
internal OGRFeatureDefn for the layer. 

Perhaps you should attach (or email) me your current code. 

comment:7 by mielket@…, 19 years ago

Maybe 'primordial' FeadureDefn is the better word. It's because I don't trust 
my own code as it is not clear to me how and when the FeatureDefn comes to be 
associated with the layer. Is it really only CreateField() which connects the 
FeatureDefn to the layer or the first call of CreateFeature?

This is my current code:
_____

FILE *file;
file = fopen("C:\\llll.txt", "w");
	while (pLayerIterate)
	{
		// papszOptions = CSLSetNameValue( papszOptions, "DIM", "2" );

		OGRLayer *poL = poDS->CreateLayer(pLayerIterate->name.GetBuffer
(0),  
									
	NULL,  
									
	wkbUnknown,  
									
	NULL);
		
		// constuct feature definition
		OGRFeatureDefn *poFD = new OGRFeatureDefn("Rectangle");
		poFD->SetGeomType(wkbLineString);
		// construct attribute field definition
		CTime i;	
		int n_max = 0;	
		CSynOptXDoc *pDoc = this;
		int n;

		for (i = pDoc->m_starttime, n = 0; i < pDoc->m_endtime; i += 
pDoc->m_interval, n++)
		{
			CString cs = i.Format("%H_%M");	// time as field name
			OGRFieldDefn oFLD(cs.GetBuffer(0), OFTReal);
			oFLD.Set(cs.GetBuffer(0), OFTReal, 0, 0, OJUndefined);
			OGRField oField; oField.Real = 0.0;
			oFLD.SetDefault(&oField);
			poFD->AddFieldDefn(&oFLD);

			poL->CreateField(&oFLD);
		}
		n_max = n;
		
		CSynObject *psObjIterate = pLayerIterate->objects;
		while (psObjIterate)
		{
			if (psObjIterate->vertex_size == 2)
			{

				// construct geometry
				OGRLinearRing oLR;
				oLR.addPoint(psObjIterate->vertex[0].x, -
psObjIterate->vertex[0].y);
				oLR.addPoint(psObjIterate->vertex[0].x, -
psObjIterate->vertex[1].y);
				oLR.addPoint(psObjIterate->vertex[1].x, -
psObjIterate->vertex[1].y);
				oLR.addPoint(psObjIterate->vertex[1].x, -
psObjIterate->vertex[0].y);
				oLR.addPoint(psObjIterate->vertex[0].x, -
psObjIterate->vertex[0].y);
				OGRFeature *poF = new OGRFeature(poL-
>GetLayerDefn());
				poF->SetGeometryDirectly(&oLR);
				poL->CreateFeature(poF);

				// construct temporal activity values in 
attribute table
				CSynTime *pt = psObjIterate->times;
				while (pt)
				{
					CString cs = pt->time.Format("%H_%M");
	
					n = poFD->GetFieldIndex(cs.GetBuffer
(0));		// retrieve field index by time field name
					if (n >= 0) poF->SetField(n, pt-
>activity);

					pt = pt->next;
				}
poF->DumpReadable(file);  
			}
				
			psObjIterate = psObjIterate->next;
		}

		delete poFD;

		pLayerIterate = pLayerIterate->next;
	}

	OGRErr ret = poDS->SyncToDisk();
fclose(file);	

	delete poDS;
}
_____

The SetField() seems to have no effect. The fields associated with each 
feature are all empty.

comment:8 by warmerdam, 19 years ago

Thomas, 

I appologise, I see I have still not been clear. 

1) You should not be explicitly creating an OGRFeatureDefn at all. 

2) You cannot modify the GeomType of a layer after it has been created.  Ensure
   you pass the correct geomtype in the CreateLayer() call. 

3) You never call AddFieldDefn(), that happens internally. You just call
   poL->CreateField(). 

4) You have to set the field values on your created feature before you call
   OGRLayer::CreateFeature().  At that point the layer copies all the 
   information from your template feature and applies it to the database. 

5) You can't call OGRFeature::SetGeometryDirectly() with a stack based geometry.
   Use SetGeometry().  The SetGeometryDirectly() takes ownership of the passed
   object but this doesn't work properly for stack based objects.  Only ones
   dynamically created with new. 

6) It isn't really legal to write pure OGRLinearRing geometries.  If you want
   a polygon you need to encapsulate it in a Polygon object.  Linear rings
   are not a first class object in the OpenGIS Simple Feature model.  They are
   a sort of secondary item only allowed for use in the composition of polygons.

I have placed a reworked copy of your code on my web server since Bugzilla 
is substantially corrupting the code. 

 http://home.gdal.org/~warmerda/rework.cpp. 

I see now that I clearly need to write a tutorial on writing layers via
OGR.  The reference docs lead people down the wrong road quite easily.

comment:9 by mielket@…, 19 years ago

Thank you very much, Frank, at first for your patience! The code now works 
fine. I just had to put a poL->GetLayerDefn() before the GetFieldIndex(). Of 
course you may use this codepiece as basis for the tutorial. I think code 
examples are most times more explanatory than abstract descriptions. It helped 
me alot to learn from the ogr2ogr tool when figuring out how to handle the 
datasource and drivers, for example.

I hope to do the import on my own and not need to bother you again.

comment:10 by mielket@…, 19 years ago

Hmm, GML works perfect now but ESRI Shape files neither contain geometries nor 
field values, which is a step backwards. I will investigate more tomorrow.

comment:11 by mielket@…, 19 years ago

This is what I found:

The features are not being written to the shape files unless I revoke your 
code where you added the OGRPolygon to comply with the simple features model.
____

    // construct geometry
    OGRLinearRing oLR;
    oLR.addPoint(psObjIterate->vertex[0].x, -psObjIterate->vertex[0].y);
    oLR.addPoint(psObjIterate->vertex[0].x, -psObjIterate->vertex[1].y);
    oLR.addPoint(psObjIterate->vertex[1].x, -psObjIterate->vertex[1].y);
    oLR.addPoint(psObjIterate->vertex[1].x, -psObjIterate->vertex[0].y);
    oLR.addPoint(psObjIterate->vertex[0].x, -psObjIterate->vertex[0].y);

//    OGRPolygon   oPolygon;
//    oPolygon.addRing( &oLR );

//    poF->SetGeometry( &oPolygon );
    poF->SetGeometry( &oLR );
____

Any ideas?

comment:12 by warmerdam, 19 years ago

Thomas, 

Ah yes, this is because the geometry type is defined to be wkbLineString.
If you really want linestrings then omitting the polygon stuff, and using
an OGRLineString instead of an OGRLinearRing would be appropriate.  If you 
want it to be interepreted as a polygon, you should change the geom type to 
wkbPolygon and keep my OGRPolygon wrapper. 

To summarize, I am leaving this bug open for now to mark the need for an 
OGR "writing" tutorial. 

comment:13 by warmerdam, 19 years ago

Closing, I beieve Thomas has things working.

comment:14 by warmerdam, 19 years ago

I should also note that a tutorial for the OGR C++ API was added to the OGR
web site. 

Note: See TracTickets for help on using tickets.