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

Lens flare cleanup and colorization #2927

Merged
38 changes: 33 additions & 5 deletions gazebo/rendering/LensFlare.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@ namespace gazebo
namespace rendering
{
/// \brief We'll create an instance of this class for each camera, to be
/// used to inject lens flare uniforms and time (for animating flare)
/// in each render call.
/// used to inject lens flare uniforms in each render call.
class LensFlareCompositorListener
: public Ogre::CompositorInstance::Listener
{
Expand Down Expand Up @@ -71,6 +70,13 @@ namespace gazebo
this->scale = _scale;
}

/// \brief Set the color of lens flare.
/// \param[in] _color Color of lens flare
public: void SetColor(const ignition::math::Vector3d _color)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pass by const ref here as well. For consistency with gazebo coding style, can you put & next to variable name, i.e. &_color?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

{
this->color = _color;
}

/// \brief Callback that OGRE will invoke for us on each render call
/// \param[in] _passID OGRE material pass ID.
/// \param[in] _mat Pointer to OGRE material.
Expand All @@ -96,9 +102,6 @@ namespace gazebo
pass->getFragmentProgramParameters();
GZ_ASSERT(!params.isNull(), "Null OGRE material GPU parameters");

// used for animating flare
params->setNamedConstant("time", static_cast<Ogre::Real>(
common::Time::GetWallTime().Double()));
// for adjusting aspect ratio of flare
params->setNamedConstant("viewport",
Ogre::Vector3(static_cast<double>(this->camera->ViewportWidth()),
Expand Down Expand Up @@ -132,6 +135,8 @@ namespace gazebo
params->setNamedConstant("lightPos", Conversions::Convert(pos));
params->setNamedConstant("scale",
static_cast<Ogre::Real>(lensFlareScale));
params->setNamedConstant("color",
Ogre::Vector3(this->color.X(), this->color.Y(), this->color.Z()));
}

/// \brief Get the lens flare position and scale for a normal camera
Expand Down Expand Up @@ -337,6 +342,10 @@ namespace gazebo

/// \brief Scale of lens flare.
private: double scale = 1.0;

/// \brief Color of lens flare.
private: ignition::math::Vector3d color
= ignition::math::Vector3d(1.0, 1.0, 1.0);
};

/// \brief Private data class for LensFlare
Expand Down Expand Up @@ -372,6 +381,10 @@ namespace gazebo

/// \brief Scale of lens flare.
public: double lensFlareScale = 1.0;

/// \brief Color of lens flare.
public: ignition::math::Vector3d lensFlareColor
= ignition::math::Vector3d(1.0, 1.0, 1.0);
};
}
}
Expand Down Expand Up @@ -414,6 +427,8 @@ void LensFlare::SetCamera(CameraPtr _camera)
LensFlareCompositorListener(this->dataPtr->camera, nullptr));
this->dataPtr->lensFlareCompositorListener->SetScale(
this->dataPtr->lensFlareScale);
this->dataPtr->lensFlareCompositorListener->SetColor(
this->dataPtr->lensFlareColor);

this->dataPtr->lensFlareInstance =
Ogre::CompositorManager::getSingleton().addCompositor(
Expand Down Expand Up @@ -441,6 +456,19 @@ void LensFlare::SetScale(const double _scale)
}
}

//////////////////////////////////////////////////
void LensFlare::SetColor(const ignition::math::Vector3d& _color)
{
// lensFlareColor is intentionally not clamped so the user can work in
// HDR color spaces or be artistic.
this->dataPtr->lensFlareColor = _color;
if (this->dataPtr->lensFlareCompositorListener)
{
this->dataPtr->lensFlareCompositorListener->SetColor(
this->dataPtr->lensFlareColor);
}
}

//////////////////////////////////////////////////
void LensFlare::Update()
{
Expand Down
4 changes: 4 additions & 0 deletions gazebo/rendering/LensFlare.hh
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ namespace gazebo
/// \param[in] _scale Scale of lens flare
public: void SetScale(const double _scale);

/// \brief Set the color of lens flare.
/// \param[in] _color Color of lens flare
public: void SetColor(const ignition::math::Vector3d& _color);

/// \brief Update function to search light source
private: void Update();

Expand Down
31 changes: 5 additions & 26 deletions media/materials/programs/camera_lens_flare_fs.glsl
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// The input texture, which is set up by the Ogre Compositor infrastructure.
uniform sampler2D RT;
uniform sampler2D noiseRGBA;

uniform float time;
uniform vec3 viewport;

// light pos in clip space
Expand All @@ -11,16 +9,8 @@ uniform vec3 lightPos;
// lens flare scale
uniform float scale;

float noise(float t)
{
// 256 is the size of noiseRGBA texture
return texture2D(noiseRGBA, vec2(t, 0.0) / vec2(256.0)).x;
}

float noise(vec2 t)
{
return texture2D(noiseRGBA,(t + vec2(time)) / vec2(256.0)).x;
}
// lens flare color
uniform vec3 color;

vec3 lensflare(vec2 uv,vec2 pos)
{
Expand All @@ -30,11 +20,8 @@ vec3 lensflare(vec2 uv,vec2 pos)

float ang = atan(main.y, main.x);
float dist = length(main); dist = pow(dist,.1);
float n = noise(vec2((ang-time/9.0)*16.0,dist*32.0));

float f0 = 1.0/(length(uv-pos)*16.0/scale+1.0);

f0 = f0+f0*(sin((ang+time/18.0 + noise(abs(ang)+n/2.0)*2.0)*12.0)*.1+dist*.1+.8);
float f0 = 2.0/(length(uv-pos)*16.0/scale+1.0);
Copy link
Contributor Author

@mogumbo mogumbo Jan 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed 1.0 to 2.0 here. After removing the legacy animated effect (which was causing ugly visual artifacts) and cc() (which had almost no effect on color), I boosted this number to restore the LensFlare's original appearance.

In my opinion, we should not do this because it makes the LensFlare look washed out and maintains a very big hotspot over the light source. Those features would never have been there if the broken animated effect had never been included.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so do you recommend 1.0 or 2.0 here?

Copy link
Contributor Author

@mogumbo mogumbo Mar 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer 1.0 because the legacy animation effect made the whole lens flare look overdone. I think a more realistic lens flare effect would have excluded it. The only reason to keep 2.0 is so that appearance does not change significantly.


float f2 = max(1.0/(1.0+32.0*pow(length(uvd+0.8*pos),2.0)),.0)*00.25;
float f22 = max(1.0/(1.0+32.0*pow(length(uvd+0.85*pos),2.0)),.0)*00.23;
Expand Down Expand Up @@ -67,13 +54,6 @@ vec3 lensflare(vec2 uv,vec2 pos)
return c;
}

// color modifier
vec3 cc(vec3 color, float factor, float factor2)
{
float w = color.x+color.y+color.z;
return mix(color,vec3(w)*factor,w*factor2);
}

void main()
{
// return if light is behind the view
Expand All @@ -97,9 +77,8 @@ void main()
pos.x *= aspect;

// compute lens flare
vec3 color = vec3(1.4,1.2,1.0)*lensflare(uv, pos.xy);
color = cc(color,.5,.1);
vec3 finalColor = color * lensflare(uv, pos.xy);

// apply lens flare
gl_FragColor = texture2D(RT, gl_TexCoord[0].xy) + vec4(color, 1.0);
gl_FragColor = texture2D(RT, gl_TexCoord[0].xy) + vec4(finalColor, 1.0);
}
15 changes: 5 additions & 10 deletions media/materials/scripts/gazebo.material
Original file line number Diff line number Diff line change
Expand Up @@ -1545,23 +1545,22 @@ material Gazebo/WideLensMap
}


vertex_program Gazebo/CameraLesnFlareVS glsl
vertex_program Gazebo/CameraLensFlareVS glsl
{
source camera_lens_flare_vs.glsl
}

fragment_program Gazebo/CameraLesnFlareFS glsl
fragment_program Gazebo/CameraLensFlareFS glsl
{
source camera_lens_flare_fs.glsl

default_params
{
param_named RT int 0
param_named noiseRGBA int 1
param_named time float 0.0
param_named viewport float3 0.0 0.0 0.0
param_named lightPos float3 0 0 0
param_named scale float 1.0
param_named color float3 1.4 1.2 1.0
}
}

Expand All @@ -1571,19 +1570,15 @@ material Gazebo/CameraLensFlare
{
pass
{
vertex_program_ref Gazebo/CameraLesnFlareVS { }
fragment_program_ref Gazebo/CameraLesnFlareFS { }
vertex_program_ref Gazebo/CameraLensFlareVS { }
fragment_program_ref Gazebo/CameraLensFlareFS { }

texture_unit RT
{
tex_coord_set 0
tex_address_mode border
filtering linear linear linear
}
texture_unit noiseRGBA
{
texture noise_rgba.png
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

}
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion media/materials/scripts/lens_flare.compositor
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ compositor CameraLensFlare/Default
technique
{
// Temporary textures
texture rt0 target_width target_height PF_A8R8G8B8
texture rt0 target_width target_height PF_FLOAT32_RGB
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @scpeters - this is somewhat related to issue #2928. We could just change the internal texture format directly if we don't want to have a new sdformat param to configure it.

Copy link
Contributor Author

@mogumbo mogumbo Jan 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both issues would benefit from an sdf param because most cases can work with a smaller, integer texture. Always using a FLOAT32 texture should work for every case but often waste graphics memory.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm working on a custom SDFormat parameter in scpeters@8e88380, just need to finish writing the test

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm ready to approve all parts of this pull request except for this line. Can we revert it and come back to it later, perhaps in the context of #2928?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Reverted to PF_A8R8G8B8


target rt0
{
Expand Down
32 changes: 32 additions & 0 deletions plugins/LensFlareSensorPlugin.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ namespace gazebo

/// \brief Lens flare scale
public: double scale = 1.0;

/// \brief Lens flare color
public: ignition::math::Vector3d color
= ignition::math::Vector3d(1.4, 1.2, 1.0);
};
}

Expand Down Expand Up @@ -75,6 +79,11 @@ void LensFlareSensorPlugin::Load(sensors::SensorPtr _sensor,
gzerr << "Lens flare scale must be greater than 0" << std::endl;
}

if (_sdf->HasElement("color"))
{
this->dataPtr->color = _sdf->Get<ignition::math::Vector3d>("color");
}

sensors::CameraSensorPtr cameraSensor =
std::dynamic_pointer_cast<sensors::CameraSensor>(_sensor);

Expand All @@ -101,6 +110,28 @@ void LensFlareSensorPlugin::Load(sensors::SensorPtr _sensor,
return;
}

/////////////////////////////////////////////////
void LensFlareSensorPlugin::SetScale(const double _scale)
{
this->dataPtr->scale = _scale;

for (auto flare : this->dataPtr->lensFlares)
{
flare->SetScale(_scale);
}
}

/////////////////////////////////////////////////
void LensFlareSensorPlugin::SetColor(const ignition::math::Vector3d& _color)
{
this->dataPtr->color = _color;

for (auto flare : this->dataPtr->lensFlares)
{
flare->SetColor(_color);
}
}

/////////////////////////////////////////////////
void LensFlareSensorPlugin::AddLensFlare(rendering::CameraPtr _camera)
{
Expand All @@ -111,5 +142,6 @@ void LensFlareSensorPlugin::AddLensFlare(rendering::CameraPtr _camera)
lensFlare.reset(new rendering::LensFlare);
lensFlare->SetCamera(_camera);
lensFlare->SetScale(this->dataPtr->scale);
lensFlare->SetColor(this->dataPtr->color);
this->dataPtr->lensFlares.push_back(lensFlare);
}
13 changes: 12 additions & 1 deletion plugins/LensFlareSensorPlugin.hh
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,10 @@ namespace gazebo
/// \brief Plugin that adds lens flare effect to a camera or multicamera
/// sensor
/// The plugin has the following optional parameter:
/// <scale> Scale of lens flare. Must be greater than 0
/// <scale> Scale of lens flare. Must be greater than 0
/// <color> Color of lens flare.
/// \todo A potentially useful feature would be an option for contantly
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

contantly -> constantly?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

/// updating the flare color to match the light source color.
class GAZEBO_VISIBLE LensFlareSensorPlugin : public SensorPlugin
{
/// \brief Constructor.
Expand All @@ -41,6 +44,14 @@ namespace gazebo
public: virtual void Load(sensors::SensorPtr _sensor,
sdf::ElementPtr _sdf);

/// \brief Set the scale of lens flare.
/// \param[in] _scale Scale of lens flare
public: void SetScale(const double _scale);

/// \brief Set the color of lens flare.
/// \param[in] _color Color of lens flare
public: void SetColor(const ignition::math::Vector3d& _color);

/// \brief Add lens flare effect to a camera
/// \param[in] _camera Camera to add the lens flare effect to.
private: void AddLensFlare(rendering::CameraPtr _camera);
Expand Down