juce-framework / JUCE

JUCE is an open-source cross-platform C++ application framework for desktop and mobile applications, including VST, VST3, AU, AUv3, LV2 and AAX audio plug-ins.
https://juce.com
Other
6.62k stars 1.75k forks source link

[Bug]: UnitTests fail on x86-32 #1466

Open umlaeute opened 2 days ago

umlaeute commented 2 days ago

Detailed steps on how to reproduce the bug

What is the expected behaviour?

I expect JUCE to work i386 (aka x86 or x86-32)

If this architecture is not supported (like powerpc #1464, and presumably s390/x), it would be great if you could specify the actually supported architectures.

Operating systems

Linux

What versions of the operating systems?

Debian/sid

Architectures

32-bit

Stacktrace

Time / Time: 1 test failure
approximatelyEqual<long double> / Tolerances scale with the values being compared: 1 test failure
OSCTimeTag class / Conversion to/from JUCE Time: 2 test failures

Plug-in formats (if applicable)

No response

Plug-in host applications (DAWs) (if applicable)

No response

Testing on the develop branch

The bug is present on the develop branch

Code of Conduct

umlaeute commented 2 days ago

afaict, the failing Time/Time (epoch wrap, when using local time) and approximatelyEqual<long double> / Tolerances are inherent to the i386, and cannot be easily fixed.

However, the failing OSCTimeTag tests are based on a wrong assumption in the code at https://github.com/juce-framework/JUCE/blob/5179f4e720d8406ebd1b5401c86aea8da6cc83c9/modules/juce_core/time/juce_RelativeTime.cpp#L52

The test tests with numSeconds=1.234 which cannot be represented in ieee754 floats. instead the actual value is 1.2339999675750732421875. Multiplying this with 1000 and naively casting to int64 will result in 1233 rather than the expected 1234. The actual behaviour is depending on the rounding-mode of the CPU, but I think it is an error to simply assume that it will round upwards.

For a discussion, see https://stackoverflow.com/questions/79131400

I'd suggest to use std::llround() instead.