Opened 5 years ago

Closed 16 months ago

Last modified 10 months ago

#10625 closed enhancement (fixed)

Allow to use IEC 754 format for doubles wxDataInputStream/wxDataOutputStream

Reported by: net147 Owned by:
Priority: low Milestone: 3.0.0
Component: base Version: stable-latest
Keywords: IEEE wxDataStream IEC-754 Cc: net147@…
Blocked By: Blocking:
Patch: yes

Description

You can only read/write 80-bit IEEE extended floats using wxDataInputStream/wxDataOutputStream.

I propose we add ability to read/write standard 32-bit and 64-bit IEEE float and double and make this the default when using << and >> operators. 80-bit IEEE extended floats can still be used with the functions ReadIeeeExtended and WriteIeeeExtended.

Attachments (1)

datstrm_float_double_r59108.patch download (11.2 KB) - added by net147 5 years ago.
32-bit and 64-bit floating point support.

Download all attachments as: .zip

Change History (21)

comment:1 Changed 5 years ago by net147

  • Cc net147@… added

comment:2 Changed 5 years ago by net147

On further thought, it would probably be a good idea to add a property (e.g. IeeeExtended) and have it set to true by default to maintain compatibility with the current behaviour. The property will control whether to use 80-bit IEEE extended floating point for the <<, >> operators and {Read,Write}{Float,Double}. {Read/Write}IeeeExtended could then be removed.

Changed 5 years ago by net147

32-bit and 64-bit floating point support.

comment:3 Changed 5 years ago by net147

I've updated the patch to leave the current float and double handling the same by allow disabling use of 80-bit IEEE extended floating point format when reading/writing through the use a new UseIeeeExtended method which causes floats to be read/written as 32-bit IEEE single precision and doubles to be read/written as 64-bit IEEE double precision.

comment:4 Changed 5 years ago by vadz

  • Keywords IEEE wxDataStream added

But the streams produced in this way wouldn't be portable between platforms, would they? IMO this is a big advantage of the current code outweighing its disadvantages (what are they BTW?).

So I'm really not sure if we want to change this.

comment:5 Changed 5 years ago by net147

This doesn't change the current behaviour. It only adds additional functionality (needs to be explicitly enabled by disabling IeeeExtended property). I've tested the code and confirms it read/writes correct big endian floats/doubles for Sun Sparc and little endian floats/doubles for Intel P4. A lot of file formats use this data type and I think that 32-bit and 64-bit floats/doubles are a lot more common than 80-bit extended precision floats (because 80-bit extended float don't align well with 32-bit and 64-bit).

comment:6 Changed 5 years ago by net147

Because this doesn't change default behaviour, this doesn't create any disadvantage but gives the advantage of allowing easy reading/writing of common 32-bit float and 64-bit double in use in existing binary formats. The developer has to consciously disable the use of portable IEEE extended precision floats to utilise this which I think gives more advantage than disadvantage. It also gives an alternative for users that don't want to accept the license terms for the IEEE extended float code.

comment:7 Changed 5 years ago by net147

Any objections to this patch? The patch only adds additional functionality without changing the existing behaviour.

comment:8 follow-up: Changed 5 years ago by vadz

  • Milestone set to 2.9.1
  • Status changed from new to confirmed

What bothers me a little with this patch is that it implicitly assumes that the hardware uses IEEE single/double precision format for float/double as -- AFAICS -- you simply dump memory contents to the disk. But then to be honest I don't know of any machine which doesn't use IEEE 754 nowadays so it probably doesn't matter in practice...

So I'll apply this patch if nobody objects to it in near future, thanks for the nice patch, as usual. The only thing which it would be nice to add to it would be some unit tests.

TIA!

comment:9 in reply to: ↑ 8 Changed 5 years ago by net147

Replying to vadz:

What bothers me a little with this patch is that it implicitly assumes that the hardware uses IEEE single/double precision format for float/double as -- AFAICS -- you simply dump memory contents to the disk. But then to be honest I don't know of any machine which doesn't use IEEE 754 nowadays so it probably doesn't matter in practice...

The patch doesn't explicitly use the memory representation of float and double but uses wxFloat32 and wxFloat64. I guess those map to the same type as float and double on most if not all platforms though.

comment:10 Changed 5 years ago by juliansmart

  • Status changed from confirmed to infoneeded_new

If I've read the comments correctly, this still produces files that are not portable between different-endian systems. Doesn't that conflict with the stated aim of the class (to write data portable)? If it really isn't portable, we need to flag it very heavily in the documentation. But I don't know if we even want to encourage or even let people write unportable data files using a 'portable data' class...

comment:11 Changed 5 years ago by net147

  • Status changed from infoneeded_new to new

The files are portable between different endian systems as long as:

  • wxFloat32 is 32-bit IEEE float
  • wxFloat64 is 64-bit IEEE float

include/wx/defs.h states that they are.
It swaps the byte order where needed which is the same as for integers.

I've tested it between Windows on Intel x86 (little endian) and Solaris on Sun Sparc (big endian).
It seems to work fine.

Though it only works if the endianness of IEEE 754 on the system is the same endianness as integers on the system. I haven't used any system where this isn't the case.

A test to verify the portability would be useful but I don't have much experience with the internals of IEEE 754.

comment:12 Changed 5 years ago by net147

I think a good idea for testing float endianness and portability would be:

  • Create a set of IEEE 754 floats in binary
  • Decode them using the system in both original and reversed order to determine the endianness on the system and verify that they are the same endianness as integers
  • Verify the resulting float value to ensure they are decoded to the correct value

The QDataStream class in QT also seems to do the same thing as this patch.
It just dumps the float or double from memory and swaps the byte order if needed according to system endianness (it assumes endianness is same for integers and float/double).

comment:13 Changed 5 years ago by jatupper

Just a comment: different IEEE 754 environments can handle / encode NaN payloads differently. (Direct bit propagation doesn't imply preservation of semantics.)

comment:14 Changed 4 years ago by vadz

  • Keywords IEC-754 added
  • Milestone 2.9.1 deleted
  • Priority changed from normal to low
  • Status changed from new to confirmed
  • Summary changed from Can't read/write standard float/double using wxDataInputStream/wxDataOutputStream to Allow to use IEC 754 format for doubles wxDataInputStream/wxDataOutputStream

This is not critical for 2.9.1 but it would still be nice to decide what do we do with this patch. I'd probably apply it as it's true that it doesn't change anything by default and the added behaviour can be useful for interoperability.

My only change would be to rename UseIeeeExtended() to UseIEEE754() (or UseIEC754()?) with opposite meaning as it seems more logical to have a function which would change the default when called with true argument and not false one.

I also wonder if we shouldn't use IEC 754 by default when wxUSE_APPLE_IEEE==0?

Final remark is that we really, really need a unit test for this. If you could please update your patch to include it, it would be great.

TIA!

comment:15 Changed 2 years ago by vadz

  • Milestone set to 3.0

We need to decide what to do with this one before 3.0. Any opinions about it?

comment:16 Changed 16 months ago by VZ

(In [73933]) Extract common parts of wxData{In,Out}putStream in a common base class.

No real changes, just put BigEndianOrdered() and SetConv() methods and the
corresponding fields in a common wxDataStreamBase class instead of duplicating
them in wxDataInputStream and wxDataOutputStream.

This will make it simpler to add more features common to both classes in the
future, see #10625.

comment:17 Changed 16 months ago by VZ

(In [73934]) No real changes, just rename double variables to "d".

Don't use "i" or "f" for double variable names, this is confusing, especially
when we do it inconsistently.

See #10625.

comment:18 Changed 16 months ago by VZ

  • Resolution set to fixed
  • Status changed from confirmed to closed

(In [73938]) Add IEEE 754 single/double precision support to wxDataStream classes.

Allow to optionally raed/write float/double values in IEEE 754 single/double
precision formats, respectively, instead of always using the extended
precision format for both of them.

This makes the code more flexible, allowing for better interoperability with
the other programs, and also allows to implement floating point functions in
these classes even when wxUSE_APPLE_IEEE is turned off (as can be the case
because of the licencing concerns for the code in extended.c).

Closes #10625.

comment:19 Changed 11 months ago by net147

You can use my name "Jonathan Liu" in changes.txt instead of net147.

comment:20 Changed 10 months ago by VZ

(In [75067]) Use correct contributor name for Jonathan Liu.

Correct the changelog entry added by r73938, see #10625.

Note: See TracTickets for help on using tickets.