Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
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
184 changes: 179 additions & 5 deletions avogadro/core/cube.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

#include "molecule.h"
#include "mutex.h"
#include <iostream>

namespace Avogadro::Core {

Expand All @@ -23,21 +24,32 @@ Cube::~Cube()
m_lock = nullptr;
}

std::vector<float>::const_iterator Cube::getRowIter(size_t j, size_t k) const
{
return m_data.cbegin() + m_points.x() * (k * m_points.y() + j);
}


bool Cube::setLimits(const Vector3& min_, const Vector3& max_,
const Vector3i& points)
{
// We can calculate all necessary properties and initialise our data
// Calculate the maximum dimension
int maxPoints = std::max({points.x(), points.y(), points.z()});
Vector3i equalPoints(maxPoints, maxPoints, maxPoints);

// Recalculate spacing based on equal points
Vector3 delta = max_ - min_;
m_spacing =
Vector3(delta.x() / (points.x() - 1), delta.y() / (points.y() - 1),
delta.z() / (points.z() - 1));
m_spacing = Vector3(delta.x() / (equalPoints.x() - 1),

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Okay, I can recheck and could say here's the issue lies.

When I was having different dimensions (as a non-uniform grid), lets say m_points.x() != m_points.y() != m_points.z() it was showing me a discrete mesh. (looking like the parts of the mesh has been seperated).

So I thought to use the equal points as a workaround but it leads to the wrong orientation. Any tip for fixing it? @ghutchis

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Okay, but that doesn't work. Most "cubes" are going to be rectangular solids. They have equal spacing, but different number of points along different axes. Caffeine is a good example here, since it's fairly flat, but kinda a rectangle in the other two dimensions.

CH4 or water aren't great examples because they're roughly cubic.

This code definitely shouldn't change. You can't compute the spacing this way because the number of points will be different along different axes.

I see the weird "mesh" when I revert the changes to this code, but that just means the bug is somewhere else.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes... I got it. Will look into the code again and try to find out the issue tomorrow. Thanks for your reply.

delta.y() / (equalPoints.y() - 1),
delta.z() / (equalPoints.z() - 1));
m_min = min_;
m_max = max_;
m_points = points;
m_points = equalPoints;
m_data.resize(m_points.x() * m_points.y() * m_points.z());
return true;
}


bool Cube::setLimits(const Vector3& min_, const Vector3& max_, float spacing_)
{
Vector3 delta = max_ - min_;
Expand Down Expand Up @@ -194,6 +206,168 @@ float Cube::value(int i, int j, int k) const
return 0.0;
}

std::array<float, 3> Cube::computeGradient(size_t i, size_t j, size_t k) const
{
std::array<std::array<float, 2>, 3> x;
std::array<float, 3> run;

size_t dataIdx = i + (j * m_points.x()) + (k * m_points.x() * m_points.y());

// std::cout << m_spacing[0];
Comment thread Fixed
if (i == 0)
{
x[0][0] = m_data[dataIdx + 1];
x[0][1] = m_data[dataIdx];
run[0] = m_spacing.x();
}
else if (i == (m_points.x() - 1))
{
x[0][0] = m_data[dataIdx];
x[0][1] = m_data[dataIdx - 1];
run[0] = m_spacing.x();
}
else
{
x[0][0] = m_data[dataIdx + 1];
x[0][1] = m_data[dataIdx - 1];
run[0] = 2 * m_spacing.x();
}

if (j == 0)
{
x[1][0] = m_data[dataIdx + m_points.x()];
x[1][1] = m_data[dataIdx];
run[1] = m_spacing.y();
}
else if (j == (m_points.y() - 1))
{
x[1][0] = m_data[dataIdx];
x[1][1] = m_data[dataIdx - m_points.x()];
run[1] = m_spacing.y();
}
else
{
x[1][0] = m_data[dataIdx + m_points.x()];
x[1][1] = m_data[dataIdx - m_points.x()];
run[1] = 2 * m_spacing.y();
}

if (k == 0)
{
x[2][0] = m_data[dataIdx + (m_points.x() * m_points.y())];
x[2][1] = m_data[dataIdx];
run[2] = m_spacing.z();
}
else if (k == (m_points.z() - 1))
{
x[2][0] = m_data[dataIdx];
x[2][1] = m_data[dataIdx - (m_points.x() * m_points.y())];
run[2] = m_spacing.z();
}
else
{
x[2][0] = m_data[dataIdx + (m_points.x() * m_points.y())];
x[2][1] = m_data[dataIdx - (m_points.x() * m_points.y())];
run[2] = 2 * m_spacing.z();
}

std::array<float, 3> ret;

ret[0] = (x[0][1] - x[0][0]) / run[0];
ret[1] = (x[1][1] - x[1][0]) / run[1];
ret[2] = (x[2][1] - x[2][0]) / run[2];

return ret;
}

std::array<std::array<float, 3>, 8>
Cube::getGradCube(size_t i, size_t j, size_t k) const
{
std::array<std::array<float, 3>, 8> grad;

grad[0] = computeGradient(i, j, k);
grad[1] = computeGradient(i + 1, j, k);
grad[2] = computeGradient(i + 1, j + 1, k);
grad[3] = computeGradient(i, j + 1, k);
grad[4] = computeGradient(i, j, k + 1);
grad[5] = computeGradient(i + 1, j, k + 1);
grad[6] = computeGradient(i + 1, j + 1, k + 1);
grad[7] = computeGradient(i, j + 1, k + 1);

return grad;
}

std::array<float, 8> Cube::getValsCube(size_t i, size_t j, size_t k) const
{
std::array<float, 8> vals;

size_t idx = i + (j * m_points.x()) + (k * m_points.x() * m_points.y());
Comment thread Fixed

vals[0] = getData(i, j, k);
vals[1] = getData(i + 1, j, k);
vals[2] = getData(i + 1, j + 1, k);
vals[3] = getData(i, j + 1, k);
vals[4] = getData(i, j, k + 1);
vals[5] = getData(i + 1, j, k + 1);
vals[6] = getData(i + 1, j + 1, k + 1);
vals[7] = getData(i, j + 1, k + 1);

return vals;
}


inline float
Cube::getData(size_t i, size_t j, size_t k) const
{
return m_data[(k * m_points.x() * m_points.y()) + (j * m_points.x()) + i];
}


std::array<std::array<float, 3>, 8> Cube::getPosCube(size_t i, size_t j, size_t k) const
{
std::array<std::array<float, 3>, 8> pos;

// std::cout<< m_spacing.x();
Comment thread Fixed
Comment thread Fixed
float xpos = m_min.x() + i * m_spacing.y();
float ypos = m_min.y() + j * m_spacing.x();
float zpos = m_min.z() + k * m_spacing.z();

pos[0][0] = xpos;
pos[0][1] = ypos;
pos[0][2] = zpos;

pos[1][0] = xpos + m_spacing.x();
pos[1][1] = ypos;
pos[1][2] = zpos;

pos[2][0] = xpos + m_spacing.x();
pos[2][1] = ypos + m_spacing.y();
pos[2][2] = zpos;

pos[3][0] = xpos;
pos[3][1] = ypos + m_spacing.y();
pos[3][2] = zpos;

pos[4][0] = xpos;
pos[4][1] = ypos;
pos[4][2] = zpos + m_spacing.z();

pos[5][0] = xpos + m_spacing.x();
pos[5][1] = ypos;
pos[5][2] = zpos + m_spacing.z();

pos[6][0] = xpos + m_spacing.x();
pos[6][1] = ypos + m_spacing.y();
pos[6][2] = zpos + m_spacing.z();

pos[7][0] = xpos;
pos[7][1] = ypos + m_spacing.y();
pos[7][2] = zpos + m_spacing.z();

// std::cout << "pos" << " " << pos[0][0] << std::endl;
Comment thread Fixed
return pos;
}

float Cube::value(const Vector3i& pos) const
{
unsigned int index =
Expand Down
56 changes: 50 additions & 6 deletions avogadro/core/cube.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "vector.h"

#include <vector>
#include <array> // Include this for std::array

namespace Avogadro {
namespace Core {
Expand All @@ -23,9 +24,7 @@ class Mutex;
/**
* @class Cube cube.h <avogadro/core/cube.h>
* @brief Provide a data structure for regularly spaced 3D grids.
* @author Marcus D. Hanwell

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@author Marcus D. Hanwell
@author Perminder Singh

*/

class AVOGADROCORE_EXPORT Cube
{
public:
Expand Down Expand Up @@ -118,7 +117,7 @@ class AVOGADROCORE_EXPORT Cube
bool setLimits(const Molecule& mol, float spacing, float padding);

/**
* @return Vector containing all the data in a one-dimensional array.
* @return Pointer to the vector containing all the data in a one-dimensional array.
*/
std::vector<float>* data();
const std::vector<float>* data() const;
Expand All @@ -133,6 +132,11 @@ class AVOGADROCORE_EXPORT Cube
*/
bool addData(const std::vector<float>& values);

/**
* @return Const iterator to the beginning of a specific row in the cube data.
*/
std::vector<float>::const_iterator getRowIter(size_t j, size_t k) const;

/**
* @return Index of the point closest to the position supplied.
* @param pos Position to get closest index for.
Expand Down Expand Up @@ -201,7 +205,7 @@ class AVOGADROCORE_EXPORT Cube
* @param value Value to fill the cube with.
*/
void fill(float value);

/**
* Sets all indices in a Z stripe of the cube to the specified value.
* @param i x component of the position.
Expand All @@ -215,12 +219,12 @@ class AVOGADROCORE_EXPORT Cube
);

/**
* @return The minimum value at any point in the Cube.
* @return The minimum value at any point in the Cube.
*/
float minValue() const { return m_minValue; }

/**
* @return The maximum value at any point in the Cube.
* @return The maximum value at any point in the Cube.
*/
float maxValue() const { return m_maxValue; }

Expand All @@ -235,11 +239,51 @@ class AVOGADROCORE_EXPORT Cube
*/
Mutex* lock() const { return m_lock; }

// Add declarations for the functions defined in cube.cpp
/**
* Compute the gradient at a specific point in the cube.
* @param i x index
* @param j y index
* @param k z index
* @return Gradient vector at the specified point.
*/
std::array<float, 3> computeGradient(size_t i, size_t j, size_t k) const;

/**
* Get the values of the eight corners of a cube cell.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not quite sure I understand from the docs here. I've got some index (i, j, k) into the cube. What's the "cell" of the cube? Is it just going to get me the corners of the entire cube? If so, why do I need i, j, k parameters?

* @param i x index
* @param j y index
* @param k z index
* @return Array of values at the eight corners.
*/
std::array<float, 8> getValsCube(size_t i, size_t j, size_t k) const;


std::array<std::array<float, 3>, 8> getGradCube(size_t i, size_t j, size_t k) const;
/**
* Get the data value at the specified indices.
* @param i x index
* @param j y index
* @param k z index
* @return Value at the specified indices.
*/
float getData(size_t i, size_t j, size_t k) const;

/**
* Get the positions of the eight corners of a cube cell.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Again, what's the "cell" here? Are these the min, max of the corners of the cube? If so, why do you need the i, j, k?

* @param i x index
* @param j y index
* @param k z index
* @return Array of positions at the eight corners.
*/
std::array<std::array<float, 3>, 8> getPosCube(size_t i, size_t j, size_t k) const;

protected:
std::vector<float> m_data;
Vector3 m_min, m_max, m_spacing;
Vector3i m_points;
float m_minValue, m_maxValue;
// Removed conflicting 'data' member variable
std::string m_name;
Type m_cubeType;
Mutex* m_lock;
Expand Down
14 changes: 14 additions & 0 deletions avogadro/core/mesh.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,20 @@ const Vector3f* Mesh::vertex(int n) const
return &(m_vertices[n * 3]);
}


bool Mesh::setTriangles(const std::vector<std::array<size_t, 3> >& values)
{
tris.clear();
tris = values;
return true;
}

const std::vector<std::array<size_t, 3> >& Mesh::triangles() const
{
return tris;
}


bool Mesh::setVertices(const Core::Array<Vector3f>& values)
{
m_vertices.clear();
Expand Down
13 changes: 9 additions & 4 deletions avogadro/core/mesh.h
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,10 @@ class AVOGADROCORE_EXPORT Mesh
*/
const Vector3f* vertex(int n) const;

bool setTriangles(const std::vector<std::array<size_t, 3> >& values);
const std::vector<std::array<size_t, 3> >& triangles() const;


/**
* Clear the vertices vector and assign new values.
*/
Expand All @@ -141,10 +145,10 @@ class AVOGADROCORE_EXPORT Mesh
/**

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I still don't understand why you're removing numNormals() which seems like a useful method.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

But i was unable to find where we were using numnormals() in our actual code?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We're not, currently. But since this is a core class, it's also API for future development.

* @return The number of normals.
*/
unsigned int numNormals() const
{
return static_cast<unsigned int>(m_normals.size());
}
// unsigned int numNormals() const
// {
// return static_cast<unsigned int>(m_normals.size());
// }
Comment thread
github-advanced-security[bot] marked this conversation as resolved.
Fixed

/**
* @return Pointer to the first normal of the specified triangle.
Expand Down Expand Up @@ -228,6 +232,7 @@ class AVOGADROCORE_EXPORT Mesh
Core::Array<Vector3f> m_vertices;
Core::Array<Vector3f> m_normals;
Core::Array<Color3f> m_colors;
std::vector<std::array<size_t, 3> > tris;
std::string m_name;
bool m_stable;
float m_isoValue;
Expand Down
Loading