asmaloney / libE57Format

Library for reading & writing the E57 file format
Boost Software License 1.0
137 stars 66 forks source link

Error in writer when intensity value is greater than 0 #206

Closed PascalRunde closed 1 year ago

PascalRunde commented 1 year ago

Within the WriterImpl.cpp at lines 641 and 651 are constructor-calls for "FloatNode", that only use one intensity value and it appears that the value is always set to "minimum" even though they are the maximum value. intbox.set( "intensityMaximum", FloatNode( imf_, 0.0, PrecisionSingle, intensityMax) ); During tests it seem to work fine, because min and max is always 0, but using real data it causes an error.

I suggest the following (lines 636 to 654):

case NumericalNodeType::Float:
            {
               intbox.set( "intensityMinimum",
                           FloatNode( imf_, 0.0, PrecisionSingle, intensityMin,intensityMax ) );
               intbox.set( "intensityMaximum",
                            FloatNode( imf_, 0.0, PrecisionSingle, intensityMin,intensityMax ) );

               break;
            }

            case NumericalNodeType::Double:
            {
               intbox.set( "intensityMinimum",
                           FloatNode( imf_, 0.0, PrecisionDouble, intensityMin,intensityMax ) );
               intbox.set( "intensityMaximum",
                           FloatNode( imf_, 0.0, PrecisionDouble, intensityMax,intensityMax ) );

               break;
            }

I did not investigate this further, but it works fine for my data and doesnt break the tests.

asmaloney commented 1 year ago

I have modified that code since the last release, so let me take a look.

In the meantime, do you have a test you could add to the test suite to show the issue?

Also - are you using the Simple API (e57::Writer)?

PascalRunde commented 1 year ago

I simply copied the Code used in the test "(SimpleReaderData, ColouredCubeFloat)" and added some code I found in test_SimpleWriter.cpp using my real data to read.

I ended up with this code for the writing part:

Random::seed( 42 );

   e57::WriterOptions options;
   options.guid = "Coloured Cube File GUID";

   e57::Writer *writer = nullptr;

   E57_ASSERT_NO_THROW( writer = new e57::Writer( "./RealDataCopy.e57", options ) );

   writer->WriteData3DData( data3DHeader, pointsData );

   delete writer;

I did not create test data within the code.

asmaloney commented 1 year ago

Thanks for catching this Pascal!

BTW your fix is incorrect - it leaves the value as 0.0. intensityMinimum and intensityMaximum need to write the value, not the range. These together make up the intensityLimits structure which define the range. (See section 8.4.19 and tables 13 and 29 in the standard.)

Not sure why they did it this way 🤷 but it is confusing.