Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DepthCameraTest.DepthCameraBoxes/ogre2 failing on macOS #369

Closed
jacobperron opened this issue Jul 26, 2021 · 7 comments · Fixed by #785
Closed

DepthCameraTest.DepthCameraBoxes/ogre2 failing on macOS #369

jacobperron opened this issue Jul 26, 2021 · 7 comments · Fixed by #785
Labels
bug Something isn't working 🏰 citadel Ignition Citadel 🏢 edifice Ignition Edifice macOS macOS support tests Broken or missing tests / testing infra

Comments

@jacobperron
Copy link

Environment

  • OS Version: macOS
  • Source or binary build?
    Source (latest Citadel 9341eb2 and Ediface 44a1948)

Description

Example ign-rendering5 failure: https://build.osrfoundation.org/job/ignition_rendering-ci-ign-rendering5-homebrew-amd64/17/testReport/(root)/DepthCamera_DepthCameraTest/DepthCameraBoxes_ogre2/

Example ign-rendering3 failure: https://build.osrfoundation.org/job/ignition_rendering-ci-ign-rendering3-homebrew-amd64/44/testReport/junit/(root)/DepthCamera_DepthCameraTest/DepthCameraBoxes_ogre2/

Output

The output is long and repeats). Here is the first bunch of output before it starts repeating:

53: [ RUN      ] DepthCamera/DepthCameraTest.DepthCameraBoxes/ogre2
53: [Err] [Ogre2RenderEngine.cc:432] Unable to find Ogre Plugin[/usr/local/opt/ogre2.1/lib/OGRE-2.1/RenderSystem_GL3Plus]. Rendering will not be possible.Make sure you have installed OGRE properly.
53: /Users/jenkins/workspace/ignition_rendering-ci-ign-rendering3-homebrew-amd64/ign-rendering/test/integration/depth_camera.cc:254: Failure
53: Expected: (mb) > (0u), actual: 0 vs 0
53: /Users/jenkins/workspace/ignition_rendering-ci-ign-rendering3-homebrew-amd64/ign-rendering/test/integration/depth_camera.cc:455: Failure
53: Expected: (b) > (0u), actual: 0 vs 0
53: /Users/jenkins/workspace/ignition_rendering-ci-ign-rendering3-homebrew-amd64/ign-rendering/test/integration/depth_camera.cc:456: Failure
53: Expected equality of these values:
53:   255u
53:     Which is: 255
53:   a
53:     Which is: 0
@jacobperron jacobperron added bug Something isn't working 🏰 citadel Ignition Citadel macOS macOS support 🏢 edifice Ignition Edifice labels Jul 26, 2021
@darksylinc
Copy link
Contributor

darksylinc commented Jul 31, 2021

Ugh.

This bug was supposed to be fixed with #339

The root of the problem is that the value on memory in floating point representation was very close to 0 (e.g. blue = 0xff while everything else including alpha is 0x00 is 3.57331e-43f) so GPU was flushing it to 0.0f. Full analysis on #332

This is probably HW / driver specific so full info on your OS version and HW model is appreciated

@chapulina chapulina added the tests Broken or missing tests / testing infra label Aug 2, 2021
@chapulina
Copy link
Contributor

These tests ran in one of our CI machines: https://build.osrfoundation.org/computer/mac-t1000.catalina/

Maybe 🧑‍🌾 @scpeters can help digging into why it's still failing on that machine.

@scpeters
Copy link
Member

These tests ran in one of our CI machines: https://build.osrfoundation.org/computer/mac-t1000.catalina/

Yes, it only seems to fail on that machine. Here is some info about the operating system and hardware:

Some info from brew config

CPU: octa-core 64-bit ivybridge
Clang: 12.0.0 build 1200
Git: 2.32.0 => /usr/local/bin/git
Curl: 7.64.1 => /usr/bin/curl
macOS: 10.15.7-x86_64
CLT: 12.0.0.32.29
Xcode: N/A

Hardware Overview from system profiler

  Model Name:	Mac mini
  Model Identifier:	Macmini6,2
  Processor Name:	Quad-Core Intel Core i7
  Processor Speed:	2.3 GHz
  Number of Processors:	1
  Total Number of Cores:	4
  L2 Cache (per Core):	256 KB
  L3 Cache:	6 MB
  Hyper-Threading Technology:	Enabled
  Memory:	8 GB
  Boot ROM Version:	421.0.0.0.0
  SMC Version (system):	2.8f1

Graphics/Displays from System Profiler

Intel HD Graphics 4000:

  Chipset Model:	Intel HD Graphics 4000
  Type:	GPU
  Bus:	Built-In
  VRAM (Dynamic, Max):	1536 MB
  Vendor:	Intel
  Device ID:	0x0166
  Revision ID:	0x0009
  Metal:	Supported, feature set macOS GPUFamily1 v4

@jacobperron
Copy link
Author

@darksylinc
Copy link
Contributor

I forgot to comment on this ticket:

Intel HD Graphics 4000

This GPU has multiple HW bugs which drivers have to workaround. I'm not surprised it randomly fails on macOS on this HW given that Apple isn't paying much attention to GL drivers.

Unfortunately this bug is nearly unfixable for that system. Perhaps a full rewrite on how data is stored could fix it (i.e. stop reinterpret casting ints to floats, that'd be a great start); but there are other bugs in Apple's GL implementation as well when dealing with UINT/SINT formats (vs UNORM/SNORM formats)

@iche033 iche033 mentioned this issue Sep 13, 2021
7 tasks
@Blast545
Copy link
Contributor

👨‍🌾 This test and #253 started failing recently on Citadel.

Here it was working fine: ignition_rendering-ci-ign-rendering3-homebrew-amd64#72
Here ignition_rendering-ci-ign-rendering3-homebrew-amd64#73/ started failing DepthCamera_DepthCameraTest/DepthCameraBoxes_ogre2
And here started failing ThermalCameraBoxes_ogre2 ignition_rendering-ci-ign-rendering3-homebrew-amd64#74/

The problem may be triggered by new versions of the dependency packages for the job. For now we'll disable the tests on Citadel (Backporting #405) and attach the logs of these jobs to this issue so they can be investigated later.

FYI: @Crola1702

@darksylinc
Copy link
Contributor

darksylinc commented Dec 24, 2022

This bug will be fixed for good in Garden once PR #785 gets merged.

It may be possible to backport the changes to Fortress.

Update: PR #797 is a backport to Fortress.

Update 2: This bug cannot be fixed in Citadel, and Edifice is already EOL. AFAICT this ticket should be closed after the PRs are merged.

iche033 added a commit that referenced this issue Dec 28, 2022
This PR adds a heightmap test.

Almost all issues in #546 are covered.

Only Thermal Camera is not covered.

Fixes #332 (again) for macOS and possibly any OS/HW/Driver combination.
Fixes #369

Signed-off-by: Matias N. Goldberg <[email protected]>
Co-authored-by: Ian Chen <[email protected]>
iche033 pushed a commit that referenced this issue Dec 28, 2022
Fixes #332 (again) for macOS and possibly any OS/HW/Driver combination.
Fixes #369

For a very long time macOS (and possibly other OS/HW/Driver combination) would fail the tests because the shader code was doing (simplified, in C to convey the explanation):

```
uint32_t valueToStore = r << 24 | g << 16 | b << 8 | a;
float output;
output = (float*)&valueToStore;
return output;
```

In ticket #332 I made a deep analysis and found out that given the wrong conditions, the bit pattern would result in either a denormal or a NaN in floating point; which risks being flushed to 0 (or set all types of NaN to the same type bit pattern of NaN).

Although the ticket was closed and tagged as "fixed", on some OS/HW/Driver combinations (e.g. macOS + Metal) this error would reappear.

This isn't just a mere test failure. This is an actual bug. Running DepthCamera in macOS could result in wrong readings.

This PR fixes the problem for good by using RGBA32_UINT instead of RGBA32_FLOAT; and having uints be reinterpreted to floats and back when needed; which is well-defined and much more stable as far as GPU shader compilers go.

The same fix is also part of PR #785 which targets Garden and includes other changes.

This PR is a backport specific for Fortress.

Signed-off-by: Matias N. Goldberg <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working 🏰 citadel Ignition Citadel 🏢 edifice Ignition Edifice macOS macOS support tests Broken or missing tests / testing infra
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants