iche033
commented
2 years ago the tests on our homebrew CI builds were all failing as well to so we had to switch back to running ogre 1.x tests on macOS in Fortress, #407.
Closed srmainwaring closed 2 years ago
iche033
commented
2 years ago the tests on our homebrew CI builds were all failing as well to so we had to switch back to running ogre 1.x tests on macOS in Fortress, #407.
srmainwaring
commented
2 years ago The segmentation fault should be fixed by this PR: [OSX] Fix segmentation fault when creating a CocoaWindow #225.
There is an issue compiling one of shader pieces on macOS because texelFetch does not have a signature matching the type usampler2D (this in ogre2/src/media/Hlms/Common/GLSL/CrossPlatformSettings_piece_all.glsl. Commenting out the problem definition allows the samples to run but nothing is being rendered (just the clear screen).
I am trying to isolate the cause of the rendering issue - not sure if it's a shader problem or something in the way the ignition-rendering uses Ogre2-2 that is different from the Ogre Samples. The Ogre2 examples: Sample_Tutorial03_DeterministicLoop and Sample_PbsMaterials are both working with the GL3Plus render system, so it should be feasible to get Fortress running on macOS.
iche033
commented
2 years ago great, that sounds promising. We can update our ogre2.2 formula (and probably also the debs) to pull from a new commit upstream when it's ready. Thanks!
srmainwaring
commented
2 years ago We can update our ogre2.2 formula (and probably also the debs) to pull from a new commit upstream when it's ready.
https://github.com/OGRECave/ogre-next/pull/225 has been merged into ogre-next/v2-2 and ogre-next/main. I've checked that cherry-picking it into the commit used by osrf/simulation/ogre2.2.rb (https://github.com/OGRECave/ogre-next/commit/ec3f70cb53a8a7e5d196855d0274ac03f90a2f4c) addresses the segmentation fault (i.e. no other commits are required).
The shader compilation error when running simple_demo ogre2 is this:
GLSL compile log: 100000002PixelShader_ps
ERROR: 0:1118: No matching function for call to texelFetch(usampler2D, int)
ERROR: 0:1125: No matching function for call to texelFetch(usampler2D, int)
ERROR: 0:1128: Use of undeclared identifier 'idx'
ERROR: 0:1131: Use of undeclared identifier 'idx'
ERROR: 0:1133: Use of undeclared identifier 'idx'
ERROR: 0:1134: Use of undeclared identifier 'idx'
ERROR: 0:1137: Use of undeclared identifier 'posAndType'
ERROR: 0:1140: Use of undeclared identifier 'attenuation'
ERROR: 0:1143: Use of undeclared identifier 'attenuation'
ERROR: 0:1143: Use of undeclared identifier 'attenuation'
ERROR: 0:1145: Use of undeclared identifier 'atten'
ERROR: 0:1145: Use of undeclared identifier 'attenuation'
ERROR: 0:1145: Use of undeclared identifier 'attenuation'
ERROR: 0:1151: Use of undeclared identifier 'lightDiffuse'
ERROR: 0:1151: Use of undeclared identifier 'lightSpecular'
ERROR: 0:1152: Use of undeclared identifier 'atten'
ERROR: 0:1157: No matching function for call to texelFetch(usampler2D, int)
ERROR: 0:1164: No matching function for call to texelFetch(usampler2D, int)
ERROR: 0:1167: Use of undeclared identifier 'idx'
ERROR: 0:1170: Use of undeclared identifier 'idx'
ERROR: 0:1172: Use of undeclared identifier 'idx'
ERROR: 0:1173: Use of undeclared identifier 'idx'
ERROR: 0:1175: Use of undeclared identifier 'idx'
ERROR: 0:1177: Use of undeclared identifier 'idx'
ERROR: 0:1179: Use of undeclared identifier 'posAndType'
ERROR: 0:1182: Use of undeclared identifier 'attenuation'
ERROR: 0:1185: Use of undeclared identifier 'attenuation'
ERROR: 0:1185: Use of undeclared identifier 'attenuation'
ERROR: 0:1187: Use of undeclared identifier 'atten'
ERROR: 0:1187: Use of undeclared identifier 'attenuation'
ERROR: 0:1187: Use of undeclared identifier 'attenuation'
ERROR: 0:1195: Use of undeclared identifier 'spotDirection'
ERROR: 0:1197: Use of undeclared identifier 'spotParams'
ERROR: 0:1197: Use of undeclared identifier 'spotParams'
ERROR: 0:1198: Use of undeclared identifier 'spotAtten'
ERROR: 0:1198: Use of undeclared identifier 'spotAtten'
ERROR: 0:1198: Use of undeclared identifier 'spotParams'
ERROR: 0:1199: Use of undeclared identifier 'atten'
ERROR: 0:1199: Use of undeclared identifier 'spotAtten'
ERROR: 0:1203: Use of undeclared identifier 'spotParams'
ERROR: 0:1205: Use of undeclared identifier 'lightDiffuse'
ERROR: 0:1205: Use of undeclared identifier 'lightSpecular'
ERROR: 0:1206: Use of undeclared identifier 'atten'
libc++abi: terminating with uncaught exception of type Ogre::RenderingAPIException: OGRE EXCEPTION(3:RenderingAPIException): Fragment Program 100000002PixelShader_ps failed to compile. See compile log above for details. in GLSLShader::compile at /Users/rhys/Code/ogre/ogre-next2.2/RenderSystems/GL3Plus/src/GLSL/OgreGLSLShader.cpp (line 314)
zsh: abort ./simple_demo ogre2It's a bit odd. I've created a standalone ogre2 sample based off the Sample_PbsMaterial that renders to GLUT (like simple_demo) rather than SDL, and added some custom Unlit and Pbs materials with scripting. It runs as expected using the patched homebrew version of ogre2.2 (see below):
The ign-rendering6 render pass is more complicated than my simple test case, and now includes Hlms/Terra so it will take further analysis to try and isolate the change that is causing the error. Any insights welcomed.
It would be nice to be able to easily switch rendering features on and off to help track down the origins of this type of error - even to the extent of being able to swap out Pbs for a simpler shader.
iche033
commented
2 years ago OGRECave/ogre-next#225 has been merged into ogre-next/v2-2 and ogre-next/main. I've checked that cherry-picking it into the commit used by osrf/simulation/ogre2.2.rb (OGRECave/ogre-next@ec3f70c) addresses the segmentation fault (i.e. no other commits are required).
cool, I also just want to make sure that we don't need any more changes upstream to fix the remaining ign-rendering shader compile issue. Just so that we don't roll out 2 different ogre 2.2 versions in a short time.
I've created a standalone ogre2 sample based off the Sample_PbsMaterial that renders to GLUT (like simple_demo) rather than SDL
thanks for testing that! Just brainstorming ideas:
The ign-rendering6 compositor setup is based on the Postprocessing demo in ogre-next: https://github.com/OGRECave/ogre-next/tree/v2-2/Samples/2.0/Showcase/Postprocessing So maybe try and see if that ogre demo runs?
the shader compile error look like they are related to lights and I noticed that the PbsMaterial demo in ogre samples only have directional and spot lights. So maybe try commenting out the directional and point lights (or the whole scene with just the camera left) in the ign-rendering simple_demo?
and now includes Hlms/Terra
I think the issue is less likely related to Terra as those Hlms are not used if heightmaps are not loaded in the scene. But I could be wrong
srmainwaring
commented
2 years ago cool, I also just want to make sure that we don't need any more changes upstream to fix the remaining ign-rendering shader compile issue.
Agree completely - I'm hoping it's just shaders from here on in rather than the C++ code in Ogre, but it could be residual OpenGL < 4.2 compatibility issues.
2. the shader compile error look like they are related to lights and I noticed that the PbsMaterial demo in ogre samples only have directional and spot lights.
The Sample_Forward3D demo is not working correctly, so there is an issue with the lights shaders. I will probably need help from the Ogre team to track this down, but will keep this issue updated with progress.
Have checked the Sample_Postprocessing demo and it's not working at all (rendering clear colour only) - so it looks like there is still a bit of work required in Ogre to get macOS / GL3+ back up and running well enough to support ignition.
iche033
commented
2 years ago The Sample_Forward3D demo is not working correctly, so there is an issue with the lights shaders. I will probably need help from the Ogre team to track this down, but will keep this issue updated with progress.
cc @darksylinc, do you have some insights on what could be the causing the Forward3D demo to generate the shader errors (see https://github.com/ignitionrobotics/ign-rendering/issues/422#issuecomment-932512412) on macOS?
darksylinc
commented
2 years ago The shader errors were fixed by srmainwaring as he says:
There is an issue compiling one of shader pieces on macOS because
texelFetchdoes not have a signature matching the typeusampler2D(this inogre2/src/media/Hlms/Common/GLSL/CrossPlatformSettings_piece_all.glsl. Commenting out the problem definition allows the samples to run but nothing is being rendered (just the clear screen).
Indeed, the problem is that texelFetching a TBO does not exist in macOS so a workaround is to use a regular texture 2D and use texelFetch on it (which is 2D instead of 1D).
Now as to why the screen is clear after the fix could either be a problem with the way srmainwaring fixed it; it could be a bug in how we workaround the problem (alignment issues for being a 2D texture, integer slot not being set since we rely on layout() on normal GLSL/OpenGL*), a driver bug; or a bug caused by missing functionality. It's hard to guess without trying it.
*There's a relatively high chance this is the problem. I see setTextureReg( PixelShader, "f3dLightList", texUnit++ ) (and "f3dGrid") are being called but we'd have to follow the paths to ensure the slots are properly set.
darksylinc
commented
2 years ago Btw I strongly suggest to place efforts on Metal support instead. I don't see this being a hard endeavour; and getting OpenGL to work on macOS is an uphill battle.
srmainwaring
commented
2 years ago @darksylinc to get rid of the shader compile error I modified CrossPlatformSettings_piece_all.glsl to use:
@property( GL_ARB_texture_buffer_range )
#define bufferFetch1( buffer, idx ) texelFetch( buffer, idx ).x
@else
#define bufferFetch1( buffer, idx ) bufferFetch( buffer, idx ).x
@endwhere bufferFetch is defined in @piece( SetCompatibilityLayer ).
With this the Forward3D sample will run but the lights are not rendering correctly. I think there is something to your suggestion that it's an alignment issue as when the number of lights are increased there are overlapping and incorrectly positioned blocks of lights.
Sample_Postprocessing is running but not rendering anything, and I'm not really sure where to start there.
I appreciate OpenGL is legacy for macOS now, but if you'd be able to help I'd like to try get ignition fortress working as well as ignition edifice is on macOS (which is functional enough for day to day dev work). I can either open an issue on ogre-next or continue the ogre forum thread I started.
Looking to the next version of ignition, I'd very much be in favour of having Metal support as well. I'm hoping that this work on legacy OpenGL can tide things over until then.
srmainwaring
commented
2 years ago The ign-rendering6 compositor setup is based on the Postprocessing demo in ogre-next: https://github.com/OGRECave/ogre-next/tree/v2-2/Samples/2.0/Showcase/Postprocessing So maybe try and see if that ogre demo runs?
@iche033 this may not be the whole story for what's preventing macOS from running fortress, but it explains why I'm not seeing anything rendered: Compositor local textures not working correctly in macOS with GL3+ #231.
iche033
commented
2 years ago thanks for investigating the issue! I was thinking that we could work around the issue by disabling post processing in regular cameras (and just use the PbsMaterial compositor setup) but lots of sensors also use local textures unfortunately. If the local texture issue turns out that it may take some time to fix, we could try to just get a regular camera working for macOs by adding ifdefs in our code for macOS.
srmainwaring
commented
2 years ago no prob. @darksylinc is being very helpful suggesting lines of investigation and I'm sure we'll eventually get to the bottom of it.
On a related note: have been thinking about looking into what would be required to add Metal support to the ogre2 render engine. I realise that the initial thought was the team would not support Metal shaders, but it may turn out to be less effort than trying to maintain OpenGL on macOS. Ogre2 already has mechanisms for creating universal materials. If I was to get the initial port to Metal running would the team support it?
iche033
commented
2 years ago Metal support came up a few times. The main concern was the extra maintenance effort on the shaders down the road but looks like now it's hard to avoid.
A few ideas on the shader situation if we were to add Metal support:
3 will probably take quite a bit of effort to do so I am leaning towards 1 and 2 for now. I don't have experience with SPIR-V Cross so I don't know how well it works. If we're able to go from glsl -> spirv -> metal using this conversion tool, it may be the faster way to get something working.
srmainwaring
commented
2 years ago 3 will probably take quite a bit of effort to do so I am leaning towards 1 and 2 for now. I don't have experience with SPIR-V Cross so I don't know how well it works. If we're able to go from glsl -> spirv -> metal using this conversion tool, it may be the faster way to get something working.
I'd favour option 1 as well, at least initially. There are only about 17 - 18 shaders specific to ignition-rendering that need to be ported to Metal as all the Ogre HlmsPbs and HlmsUnit versions are there already. That doesn't seem too bad. As it's a bit off topic for this issue I'll open a new feature request for Metal support as a place to gather ideas and outline the effort.
srmainwaring
commented
2 years ago Progress update:
The problem in Ogre2 is related to setting up the FBOs for legacy OpenGL. With a partial workaround the ignition-rendering demos will render on macOS, although there are some artefacts. I believe this is arising in some cases because of the order in which the textures are created then attached to FBOs (it looks like a placeholder for a texture may be set in the FBO with the texture creation deferred - which is not supported on macOS) . The good news is that if this is addressed then it should be feasible to have ogre2 GL3+ rending on macOS working again.
simple_demo with incorrect rendering artefacts
thermal_camera
transform_control
ogre2_demo with rendering artefacts
darksylinc
commented
2 years ago Hi!
I'll chime in regarding Metal support:
Indeed adding Metal support should be easy, since it's mostly porting shaders and ignition uses few shaders.
As for how to port shaders (option 1, 2, 3):
Pros:
Cons:
Contrary to what ignition devs believe, "any" shader files are not an extension or special functionality. It's a convention.
90% of the shader syntax is the same. Thus we use macros to abstract some of the differences.
e.g.
// Raw GLSL
vec4 myColour = texture( myTexture, uv );
// Any (macros defined on GLSL)
#define float4 vec4
#define OGRE_Sample texture
float4 myColour = OGRE_Sample( myTexture, uv );The main divergence is that metal provides uniform parameters through a struct that has only local scope while GLSL is via uniform variables with global scope. The convention we use at Ogre is the following:
// GLSL
uniform float myParam;
#define p_myParam myParam
void main()
{
#include "MyShader.any" // uses p_myParam
}
// Metal
struct Params
{
float myParam;
};
#define p_myParam p.myParam
fragment float main_metal( constant Param &p [[buffer(PARAMETER_SLOT)]] )
{
#include "MyShader.any" // uses p_myParam
}Pros:
Cons:
vec4 myVal inside the any file, the OpenGL backend will accept it, but the Metal backend will reject it. This is easy to fix, but what I mean is that it's not guaranteed)I never tried this venue but did hear about it a lot. Epic and Roblox use it.
Pros:
Cons
Possible issues:
uniform float myParam I don't know how it will appear in the Metal version and if it follows Ogre conventions (i.e. placing the constant buffer in the PARAMETER_SLOT which is a macro Ogre defines). If the mismatch is too large, rather than converting the shader and feeding metal code to the Metal backend, Ogre would need to have a SpirvCrossProgram class in the Metal backend that asks the lib how parameters (and other inputs) have been remapped so that it takes care of everything automatically. Depending on how much SPIRV-Cross modifies, this could be super easy, or super hard to support.srmainwaring
commented
2 years ago @darksylinc thanks for the detailed analysis on options for providing Metal support.
I've taken approach 1, at least to start with. This seemed like the least intrusive method with a minimal impact on existing GLSL shaders while getting things up and running. The main change to existing methods is a move to unified vertex and fragment program definitions in the material files.
So far it looks promising - all the ign-rendering shaders now have a Metal version and most of the examples that should run on Metal are now running. There's some odd behaviour where an example that runs in my environment won't complete the last render step on @ahcorde's machine, but we're working through that. The last hurdle is to get the QML demo working, which currently relies on a shared OpenGL context between QML and Ogre. IIRC there is a variant of the Qt declarative example that covers this case for Metal - so that's next, and solving that should provide a path to getting the Scene3d plugin working for ign-gui and ign-gazebo.
As a follow up it might help with maintenance to move to your second proposal, and consolidate into Any shaders. It could be worth converting one shader as an example so the Ignition team can compare approaches and decide what works best for them.
srmainwaring
commented
2 years ago With the exception of the simple_demo_qml example, this is addressed by https://github.com/ignitionrobotics/ign-rendering/pull/463. The QML example can be followed up separately using a similar approach to that proposed in https://github.com/ignitionrobotics/ign-gui/pull/323.
This issue is a placeholder to report that an upstream issue with Ogre2.2 is preventing examples that work for Edifice using Ogre2.1 from running in Fortress.
The examples tested include:
simple_demosimple_demo_qmlogre2_demoThe problem is essentially the same in all cases: the application segmentation faults.
The issue is arising in the macOS OpenGL3Plus windows code in Ogre2.2 located in
ogre-next/RenderSystems/GL3Plus/src/windowing/OSX. It appears that ignition-rendering uses this code in a way that is not tested by running the Ogre2 samples, which do run on the environment described below.Environment
OS Version:
Source or binary build?
mainRendering issue
Rendering system info:
Description
simple_demo ogre2Steps to reproduce
simple_demoexampleOutput