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

Fix selection buffer crash after resizing window #446

Merged
merged 32 commits into from
Dec 1, 2021
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
806f9e8
fix selection buffer crash due to resize and incorrect selections
iche033 Oct 1, 2021
5692211
Merge branch 'ign-rendering6' into fix_selection_resize
iche033 Oct 1, 2021
cd54161
Merge branch 'ign-rendering6' into fix_selection_resize
chapulina Oct 2, 2021
8350c99
test updating full selection buffer texture
iche033 Oct 2, 2021
361f8a7
reenable visual at test
iche033 Oct 2, 2021
ff4ddb6
fix codecheck
iche033 Oct 2, 2021
f045550
Merge branch 'main' into fix_selection_resize2
iche033 Oct 2, 2021
8e13cc6
testing rgb no depth, full buffer
iche033 Oct 2, 2021
ba2bd43
use 1x1 buffer, still no depth data
iche033 Oct 2, 2021
a1d1726
prnt scaling factor
iche033 Oct 2, 2021
251c21c
disable device ratio
iche033 Oct 2, 2021
3867a0c
reenable depth and utils test
iche033 Oct 2, 2021
e31b06c
disable utils test, add visual at test after resize
iche033 Oct 2, 2021
adf0a3e
test texelfetch
iche033 Oct 3, 2021
2a1d370
back to full buffer
iche033 Oct 3, 2021
5010e1a
print ogre log
iche033 Oct 7, 2021
9809ed5
codecheck
iche033 Oct 7, 2021
36fd8b8
fixing selection buffer mat script
iche033 Oct 7, 2021
bbbd0b6
try 1x1 buffer again
iche033 Oct 7, 2021
a1edad4
revert some test changes
iche033 Oct 7, 2021
9d7a685
Merge branch 'ign-rendering6' into fix_selection_resize
iche033 Oct 19, 2021
7b41056
Merge branch 'fix_selection_resize2' into fix_selection_resize
iche033 Oct 19, 2021
bfacea0
uncomment tests
iche033 Oct 19, 2021
e03538b
Merge branch 'fix_selection_resize' of github.com:ignitionrobotics/ig…
iche033 Oct 19, 2021
ce66356
update scaling factor
iche033 Oct 20, 2021
91a9914
merge from ign-rendering6
iche033 Nov 2, 2021
7f2e1ad
Merge branch 'ign-rendering6' into fix_selection_resize
chapulina Nov 9, 2021
97d5869
Merge branch 'ign-rendering6' into fix_selection_resize
iche033 Nov 11, 2021
ab5b397
fix removing selection mat
iche033 Nov 11, 2021
b0fd1c7
update screenScalingFactor
iche033 Nov 11, 2021
4739072
minor tweak
iche033 Nov 11, 2021
dc985f5
Merge branch 'ign-rendering6' into fix_selection_resize
iche033 Nov 30, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion ogre/src/OgreCamera.cc
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,8 @@ VisualPtr OgreCamera::VisualAt(const ignition::math::Vector2i
}
}

float ratio = screenScalingFactor();
// float ratio = screenScalingFactor();
float ratio = 1.0f;
chapulina marked this conversation as resolved.
Show resolved Hide resolved
ignition::math::Vector2i mousePos(
static_cast<int>(std::rint(ratio * _mousePos.X())),
static_cast<int>(std::rint(ratio * _mousePos.Y())));
Expand Down
4 changes: 2 additions & 2 deletions ogre2/src/Ogre2Camera.cc
Original file line number Diff line number Diff line change
Expand Up @@ -259,11 +259,11 @@ VisualPtr Ogre2Camera::VisualAt(const ignition::math::Vector2i &_mousePos)
this->ImageWidth(), this->ImageHeight());
}

float ratio = screenScalingFactor();
// float ratio = screenScalingFactor();
float ratio = 1.0f;
chapulina marked this conversation as resolved.
Show resolved Hide resolved
ignition::math::Vector2i mousePos(
static_cast<int>(std::rint(ratio * _mousePos.X())),
static_cast<int>(std::rint(ratio * _mousePos.Y())));

Ogre::Item *ogreItem = this->selectionBuffer->OnSelectionClick(
mousePos.X(), mousePos.Y());

Expand Down
4 changes: 4 additions & 0 deletions ogre2/src/Ogre2RayQuery.cc
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,10 @@ RayQueryResult Ogre2RayQuery::ClosestPoint()
//////////////////////////////////////////////////
RayQueryResult Ogre2RayQuery::ClosestPointBySelectionBuffer()
{
// update selection buffer dimension in case window is resized
this->dataPtr->camera->SelectionBuffer()->SetDimensions(
this->dataPtr->camera->ImageWidth(), this->dataPtr->camera->ImageHeight());

RayQueryResult result;
Ogre::Item *ogreItem = nullptr;
math::Vector3d point;
Expand Down
88 changes: 60 additions & 28 deletions ogre2/src/Ogre2SelectionBuffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ class ignition::rendering::Ogre2SelectionBufferPrivate
/// into a render target or render texture.
public: Ogre::CompositorWorkspace *ogreCompositorWorkspace = nullptr;

/// \brief Name of the compositor workspace definition
public: std::string ogreCompWorkspaceDefName;

/// \brief The selection buffer material
public: Ogre::MaterialPtr selectionMaterial;
};
Expand Down Expand Up @@ -181,10 +184,27 @@ void Ogre2SelectionBuffer::Update()
/////////////////////////////////////////////////
void Ogre2SelectionBuffer::DeleteRTTBuffer()
{
if (this->dataPtr->selectionMaterial)
// \todo Delete material?
// This causes an ogre assertion error about unlinking renderables
// when the RTT buffer is created again
// if (this->dataPtr->selectionMaterial)
// {
// Ogre::MaterialManager::getSingleton().remove(
// this->dataPtr->selectionMaterial->getName());
// this->dataPtr->selectionMaterial.setNull();
// }
chapulina marked this conversation as resolved.
Show resolved Hide resolved

if (this->dataPtr->ogreCompositorWorkspace)
{
Ogre::MaterialManager::getSingleton().remove(
this->dataPtr->selectionMaterial->getName());
// TODO(ahcorde): Remove the workspace. Potential leak here
this->dataPtr->ogreCompMgr->removeWorkspace(
this->dataPtr->ogreCompositorWorkspace);

this->dataPtr->ogreCompMgr->removeWorkspaceDefinition(
this->dataPtr->ogreCompWorkspaceDefName);
this->dataPtr->ogreCompMgr->removeNodeDefinition(
this->dataPtr->ogreCompWorkspaceDefName + "/Node");
this->dataPtr->ogreCompositorWorkspace = nullptr;
}

auto engine = Ogre2RenderEngine::Instance();
Expand All @@ -209,8 +229,11 @@ void Ogre2SelectionBuffer::CreateRTTBuffer()
Ogre::TextureFlags::RenderToTexture,
Ogre::TextureTypes::Type2D);
this->dataPtr->renderTexture->setResolution(1, 1);
// this->dataPtr->renderTexture->setResolution(this->dataPtr->width,
// this->dataPtr->height);
this->dataPtr->renderTexture->setNumMipmaps(1u);
this->dataPtr->renderTexture->setPixelFormat(Ogre::PFG_RGBA32_FLOAT);
// this->dataPtr->renderTexture->setPixelFormat(Ogre::PFG_RGBA8_UNORM);

this->dataPtr->renderTexture->scheduleTransitionTo(
Ogre::GpuResidency::Resident);
Expand All @@ -222,11 +245,18 @@ void Ogre2SelectionBuffer::CreateRTTBuffer()
// The SelectionBuffer material is defined in script
// (selection_buffer.material).
std::string matSelectionName = "SelectionBuffer";
Ogre::MaterialPtr matSelection =
std::string matSelectionCloneName =
this->dataPtr->camera->getName() + "_" + matSelectionName;
this->dataPtr->selectionMaterial =
Ogre::MaterialManager::getSingleton().getByName(matSelectionName);
this->dataPtr->selectionMaterial = matSelection->clone(
this->dataPtr->camera->getName() + "_" + matSelectionName);
this->dataPtr->selectionMaterial->load();
if (this->dataPtr->selectionMaterial.isNull())
{
Ogre::MaterialPtr matSelection =
Ogre::MaterialManager::getSingleton().getByName(matSelectionName);
this->dataPtr->selectionMaterial = matSelection->clone(
matSelectionCloneName);
this->dataPtr->selectionMaterial->load();
}
Ogre::Pass *p = this->dataPtr->selectionMaterial->getTechnique(0)->getPass(0);
Ogre::GpuProgramParametersSharedPtr psParams =
p->getFragmentProgramParameters();
Expand All @@ -252,13 +282,15 @@ void Ogre2SelectionBuffer::CreateRTTBuffer()

// create compositor workspace for rendering
// Setup the selection buffer compositor.
const Ogre::String workspaceName = "SelectionBufferWorkspace" +
this->dataPtr->ogreCompWorkspaceDefName = "SelectionBufferWorkspace" +
this->dataPtr->camera->getName();

std::string nodeSpaceDefName =
this->dataPtr->ogreCompWorkspaceDefName + "/Node";

Ogre::CompositorNodeDef *nodeDef =
this->dataPtr->ogreCompMgr->addNodeDefinition(
"AutoGen " + Ogre::IdString(workspaceName +
"/Node").getReleaseText());
nodeSpaceDefName);
Ogre::TextureDefinitionBase::TextureDefinition *depthTexDef =
nodeDef->addTextureDefinition("depthTexture");
depthTexDef->textureType = Ogre::TextureTypes::Type2D;
Expand Down Expand Up @@ -346,15 +378,16 @@ void Ogre2SelectionBuffer::CreateRTTBuffer()
}

Ogre::CompositorWorkspaceDef *workDef =
this->dataPtr->ogreCompMgr->addWorkspaceDefinition(workspaceName);
this->dataPtr->ogreCompMgr->addWorkspaceDefinition(
this->dataPtr->ogreCompWorkspaceDefName);
workDef->connectExternal(0, nodeDef->getName(), 0);

this->dataPtr->ogreCompositorWorkspace =
this->dataPtr->ogreCompMgr->addWorkspace(
this->dataPtr->scene->OgreSceneManager(),
this->dataPtr->renderTexture,
this->dataPtr->selectionCamera,
workspaceName,
this->dataPtr->ogreCompWorkspaceDefName,
false);
}

Expand All @@ -369,14 +402,6 @@ void Ogre2SelectionBuffer::SetDimensions(
this->dataPtr->height = _height;

this->DeleteRTTBuffer();

if (this->dataPtr->ogreCompositorWorkspace)
{
// TODO(ahcorde): Remove the workspace. Potential leak here
this->dataPtr->ogreCompMgr->removeWorkspace(
this->dataPtr->ogreCompositorWorkspace);
}

this->CreateRTTBuffer();
}
/////////////////////////////////////////////////
Expand All @@ -398,15 +423,23 @@ bool Ogre2SelectionBuffer::ExecuteQuery(const int _x, const int _y,
if (!this->dataPtr->camera)
return false;

// check camera has valid projection matrix
// There could be nan values if camera was resized
Ogre::Matrix4 projectionMatrix =
this->dataPtr->camera->getProjectionMatrix();
if (projectionMatrix.getTrans().isNaN() ||
projectionMatrix.extractQuaternion().isNaN())
return false;

const unsigned int targetWidth = this->dataPtr->width;
const unsigned int targetHeight = this->dataPtr->height;

if (_x < 0 || _y < 0 || _x >= static_cast<int>(targetWidth)
|| _y >= static_cast<int>(targetHeight))
return false;

// // 1x1 selection buffer, adapted from rviz
// // http://docs.ros.org/indigo/api/rviz/html/c++/selection__manager_8cpp.html
// 1x1 selection buffer, adapted from rviz
// http://docs.ros.org/indigo/api/rviz/html/c++/selection__manager_8cpp.html
unsigned int width = 1;
unsigned int height = 1;
float x1 = static_cast<float>(_x) /
Expand All @@ -425,10 +458,10 @@ bool Ogre2SelectionBuffer::ExecuteQuery(const int _x, const int _y,
transMatrix[0][3] -= x1+x2;
transMatrix[1][3] += y1+y2;
Ogre::Matrix4 customProjectionMatrix =
scaleMatrix * transMatrix * this->dataPtr->camera->getProjectionMatrix();

this->dataPtr->selectionCamera->setCustomProjectionMatrix(true,
customProjectionMatrix);
scaleMatrix * transMatrix *
this->dataPtr->camera->getProjectionMatrix();
this->dataPtr->selectionCamera->setCustomProjectionMatrix(true,
customProjectionMatrix);

this->dataPtr->selectionCamera->setPosition(
this->dataPtr->camera->getDerivedPosition());
Expand All @@ -440,9 +473,8 @@ bool Ogre2SelectionBuffer::ExecuteQuery(const int _x, const int _y,

Ogre::Image2 image;
image.convertFromTexture(this->dataPtr->renderTexture, 0, 0);

Ogre::ColourValue pixel = image.getColourAt(0, 0, 0, 0);
// Ogre::ColourValue pixel = image.getColourAt(_x, _y, 0, 0);

float color = pixel[3];
uint32_t *rgba = reinterpret_cast<uint32_t *>(&color);
unsigned int r = *rgba >> 24 & 0xFF;
Expand Down
4 changes: 3 additions & 1 deletion ogre2/src/media/materials/programs/selection_buffer_fs.glsl
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ in block

uniform sampler2D colorTexture;
uniform sampler2D depthTexture;
uniform vec4 colorTexResolution;

out vec4 fragColor;

Expand Down Expand Up @@ -58,7 +59,8 @@ void main()
point = vec3(inf);

// color
vec4 color = texture(colorTexture, inPs.uv0);
vec4 color = texelFetch(colorTexture,
ivec2(inPs.uv0 * colorTexResolution.xy), 0);

float rgba = packFloat(color);

Expand Down
2 changes: 2 additions & 0 deletions ogre2/src/media/materials/scripts/selection_buffer.material
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ fragment_program selection_buffer_fs glsl
{
param_named colorTexture int 0
param_named depthTexture int 1

param_named_auto colorTexResolution texture_size 0
}
}

Expand Down
9 changes: 0 additions & 9 deletions src/Utils_TEST.cc
Original file line number Diff line number Diff line change
Expand Up @@ -121,15 +121,6 @@ void UtilTest::ClickToScene(const std::string &_renderEngine)
root->AddChild(camera);
camera->Update();

if (_renderEngine == "ogre2")
{
// tests using selection buffer fail on CI, see issue #170
// https://github.com/ignitionrobotics/ign-rendering/issues/170
igndbg << "Selection buffer based screenToScene test is disabled in "
<< _renderEngine << "." << std::endl;
return;
}

// API without RayQueryResult and default max distance
result = screenToScene(centerClick, camera, rayQuery, rayResult);

Expand Down
31 changes: 23 additions & 8 deletions test/integration/camera.cc
Original file line number Diff line number Diff line change
Expand Up @@ -187,14 +187,6 @@ void CameraTest::VisualAt(const std::string &_renderEngine)
<< _renderEngine << std::endl;
return;
}
else if (_renderEngine == "ogre2")
{
// VisualAt tests fail on CI, see issue #170
// https://github.com/ignitionrobotics/ign-rendering/issues/170
igndbg << "VisualAt test is disabled in " << _renderEngine << "."
<< std::endl;
return;
}

// create and populate scene
RenderEngine *engine = rendering::engine(_renderEngine);
Expand Down Expand Up @@ -285,6 +277,29 @@ void CameraTest::VisualAt(const std::string &_renderEngine)
}
}

// change camera size
camera->SetImageWidth(1200);
camera->SetImageHeight(800);

// render a few frames
for (auto i = 0; i < 30; ++i)
{
camera->Update();
}

// test that VisualAt still works after resize
{
unsigned int x = 300u;
auto vis = camera->VisualAt(math::Vector2i(x, camera->ImageHeight() / 2));
EXPECT_NE(nullptr, vis) << "X: " << x;
if (vis)
{
EXPECT_EQ("sphere", vis->Name());
}
}



// Clean up
engine->DestroyScene(scene);
rendering::unloadEngine(engine->Name());
Expand Down
8 changes: 0 additions & 8 deletions test/integration/scene.cc
Original file line number Diff line number Diff line change
Expand Up @@ -154,14 +154,6 @@ void SceneTest::VisualAt(const std::string &_renderEngine)
<< _renderEngine << std::endl;
return;
}
else if (_renderEngine == "ogre2")
{
// VisualAt tests fail on CI, see issue #170
// https://github.com/ignitionrobotics/ign-rendering/issues/170
igndbg << "VisualAt test is disabled in " << _renderEngine << "."
<< std::endl;
return;
}

// create and populate scene
RenderEngine *engine = rendering::engine(_renderEngine);
Expand Down