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

First attempt at taxonomy::implicit_item #3827

Open
wants to merge 6 commits into
base: v0.8.0
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/ifcgeom/AbstractKernel.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ namespace ifcopenshell { namespace geometry { namespace kernels {
virtual bool convert_impl(const taxonomy::surface_curve_sweep::ptr, IfcGeom::ConversionResults&) { throw std::runtime_error("Not implemented"); }
virtual bool convert_impl(const taxonomy::loft::ptr, IfcGeom::ConversionResults&) { throw std::runtime_error("Not implemented"); }
virtual bool convert_impl(const taxonomy::collection::ptr, IfcGeom::ConversionResults&);
virtual bool convert_impl(const taxonomy::piecewise_function::ptr item, IfcGeom::ConversionResults& cs) { return convert(item->evaluate(), cs); }


/*
virtual void set_offset(const std::array<double, 3> &p_offset);
Expand Down
23 changes: 17 additions & 6 deletions src/ifcgeom/mapping/IfcCompositeCurve.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ using namespace ifcopenshell::geometry;

taxonomy::ptr mapping::map_impl(const IfcSchema::IfcCompositeCurve* inst) {
auto loop = taxonomy::make<taxonomy::loop>();
auto pwf = taxonomy::make<taxonomy::piecewise_function>();

#ifdef SCHEMA_HAS_IfcSegment
// 4x3
Expand Down Expand Up @@ -66,18 +67,28 @@ taxonomy::ptr mapping::map_impl(const IfcSchema::IfcCompositeCurve* inst) {
}
#ifdef SCHEMA_HAS_IfcCurveSegment
else if (segment->as<IfcSchema::IfcCurveSegment>()) {
// @todo check that we don't get a mixture of implicit and explicit definitions
auto crv = map(segment->as<IfcSchema::IfcCurveSegment>());
for (auto& s : taxonomy::cast<taxonomy::loop>(crv)->children) {
loop->children.push_back(s);
if (crv->kind() == taxonomy::LOOP) {
for (auto& s : taxonomy::cast<taxonomy::loop>(crv)->children) {
loop->children.push_back(s);
}
} else if (crv->kind() == taxonomy::PIECEWISE_FUNCTION) {
auto seg = taxonomy::cast<taxonomy::piecewise_function>(crv);
pwf->spans.insert(pwf->spans.end(), seg->spans.begin(), seg->spans.end());
}
}
#endif
}

aggregate_of_instance::ptr profile = inst->data().getInverse(&IfcSchema::IfcProfileDef::Class(), -1);
const bool force_close = profile && profile->size() > 0;
loop->closed = force_close;
return loop;
if (pwf->spans.empty()) {
aggregate_of_instance::ptr profile = inst->data().getInverse(&IfcSchema::IfcProfileDef::Class(), -1);
const bool force_close = profile && profile->size() > 0;
loop->closed = force_close;
return loop;
} else {
return pwf;
}
}

/*
Expand Down
143 changes: 98 additions & 45 deletions src/ifcgeom/mapping/IfcCurveSegment.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,32 +27,11 @@ using namespace ifcopenshell::geometry;

#include <boost/mpl/vector.hpp>
#include <boost/mpl/for_each.hpp>
#include <boost/math/quadrature/trapezoidal.hpp>
Copy link
Contributor

Choose a reason for hiding this comment

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

Boost math also has constants like pi

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed. Thanks.


// @todo use std::numbers::pi when upgrading to C++ 20
#define PI 3.1415926535897932384626433832795


namespace
{
// trapezoid rule integration
// @todo is there a well established math library we can use instead of
// creating our own integrator?
double integrate(double a, double b, unsigned n, std::function<double(double)> fn)
{
double area = 0;
double h = (b - a) / n;
for (auto i = 1; i <= n; i++)
{
auto x1 = a + h * (i - 1);
auto x2 = a + h * i;
auto f1 = fn(x1);
auto f2 = fn(x2);
area += h * (f1 + f2) / 2.0;
}
return area;
}
}

typedef boost::mpl::vector<
IfcSchema::IfcLine
#ifdef SCHEMA_HAS_IfcClothoid
Expand All @@ -65,19 +44,25 @@ typedef boost::mpl::vector<
, IfcSchema::IfcCircle
> curve_seg_types;

enum segment_type_t {
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of this, coupled with the if-else if-else blocks for each IfcCurve type, would it be cleaner to have three different map functions for IfcCurveSegment? map_horizontal(), map_vertical(), map_cant(). These would get called from IfcCompositeCurve, IfcGradiantCurve, and IfcSegmentedReferenceCurve, respectively.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would be okay with it as it, but admittedly at this point I'm just hanging on for dear life trying to keep up with the expert-level C++ exchanges between you & @aothms 🤣

Copy link
Contributor

Choose a reason for hiding this comment

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

Continuing to think about this, the original approach is better than my idea. My idea would have, for each curve type, three implementations in three different locations. @aothms Original idea has everything for each curve type together. That approach is more clear and maintainable.

Copy link
Member Author

Choose a reason for hiding this comment

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

I had the same doubts myself. I would have also preferred a more statically typed set of three tuples of types for example. I think the approach now is likely to result in the least lines of code, but it's indeed unclear what is implemented and supported and potentially risky in the sense that a parent curve can be used in an interpretation that's not supported. Combined with returning Eigen::VectorXd there is actually very little typing here to prevent issues. I would propose to keep as is while prototyping, but I think we might have to change it in the future.

ST_HORIZONTAL, ST_VERTICAL, ST_CANT
};

class curve_segment_evaluator {
private:
double length_unit_;
double start_;
double length_;
segment_type_t segment_type_;
IfcSchema::IfcCurve* curve_;

std::optional<std::function<Eigen::Vector3d(double)>> eval_;
std::optional<std::function<Eigen::VectorXd(double)>> eval_;

public:
// First constructor, takes parameters from IfcCurveSegment
curve_segment_evaluator(double length_unit, IfcSchema::IfcCurve* curve, IfcSchema::IfcCurveMeasureSelect* st, IfcSchema::IfcCurveMeasureSelect* le)
curve_segment_evaluator(double length_unit, segment_type_t segment_type, IfcSchema::IfcCurve* curve, IfcSchema::IfcCurveMeasureSelect* st, IfcSchema::IfcCurveMeasureSelect* le)
: length_unit_(length_unit)
, segment_type_(segment_type)
, curve_(curve)
{
// @todo in IFC4X3_ADD2 this needs to be length measure
Expand Down Expand Up @@ -146,7 +131,7 @@ class curve_segment_evaluator {
// // transform point into clothoid's coodinate system
// auto x = xl * cos(theta) - yl * sin(theta) + Cx;
// auto y = xl * sin(theta) + yl * cos(theta) + Cy;
// return Eigen::Vector3d(x, y, 0.0);
// return Eigen::VectorXd(x, y, 0.0);
// };
// }
//#endif
Expand Down Expand Up @@ -185,21 +170,26 @@ class curve_segment_evaluator {
auto Cy = C->as<IfcSchema::IfcCartesianPoint>()->Coordinates()[1];

eval_ = [L, Cx, Cy, theta, signX, fnX, signY, fnY](double u) {
using boost::math::quadrature::trapezoidal;

// integration limits, integrate from a to b
// from 8.9.3.19.1, integration limits are 0.0 to u where u is a normalized parameter
auto a = 0.0;
auto b = fabs(u / L);

auto n = 10; // use 10 steps in the numeric integration
// @todo where to plug this in?
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is the 4th argument to trapezoidal

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think so. the docs say:

Since the routine is adaptive, step sizes are halved continuously until a tolerance is reached. In order to control this tolerance, simply call the routine with an additional argument

https://live.boost.org/doc/libs/1_68_0/libs/math/doc/html/math_toolkit/trapezoidal.html

So it's not just a fixed increment, but some limit to the number of refinement iterations. So conceptually it'd be more like 2^10 (big question mark). That's why I was a bit afraid to plug it in, but the default is 12. So I think we can just keep the defaults.

// auto n = 10; // use 10 steps in the numeric integration

auto xl = signX(u)*integrate(a, b, n, fnX);
auto yl = signY(u)*integrate(a, b, n, fnY);
auto xl = signX(u)*trapezoidal(fnX, a, b);
auto yl = signY(u)*trapezoidal(fnY, a, b);

// transform point into clothoid's coodinate system
auto x = xl * cos(theta) - yl * sin(theta) + Cx;
auto y = xl * sin(theta) + yl * cos(theta) + Cy;
return Eigen::Vector3d(x, y, 0.0);
};
Eigen::VectorXd vec(3);
vec << x, y, 0.0;
return vec;
};
}

// Clothoid using numerical integration
Expand Down Expand Up @@ -290,8 +280,10 @@ class curve_segment_evaluator {
// transform point into circle's coodinate system
auto x = xl * cos(theta) - yl * sin(theta) + Cx;
auto y = xl * sin(theta) + yl * cos(theta) + Cy;
return Eigen::Vector3d(x, y, 0.0);
};
Eigen::VectorXd vec;
vec << x, y, 0.0;
return vec;
};
}

void operator()(IfcSchema::IfcPolyline* pl)
Expand Down Expand Up @@ -361,8 +353,10 @@ class curve_segment_evaluator {

auto [u_start, u_end, compare] = iter->first;
auto [x,y] = (iter->second)(u - u_start); // (u - u_start) is distance from start of this segment of the polyline
return Eigen::Vector3d(x, y, 0);
};
Eigen::VectorXd vec;
vec << x, y, 0;
return vec;
};
}

void operator()(IfcSchema::IfcLine* l) {
Expand All @@ -376,11 +370,27 @@ class curve_segment_evaluator {
auto dx = dr[0] / m;
auto dy = dr[1] / m;

eval_ = [px, py, dx, dy](double u) {
auto x = px + u * dx;
auto y = py + u * dy;
return Eigen::Vector3d(x, y, 0);
if (segment_type_ == ST_HORIZONTAL) {
Copy link
Contributor

Choose a reason for hiding this comment

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

The treatment of line assumes the line is either in the xy plane for horizontal or the sz plane for vertical. I don't think the ifc specification requires this.

CT 4.1.7.1.1.4 Alignment Geometry- Segments seems to require that the ParentCurve be 3D

sz plane = developed elevation view of the alignment profile. s = distance along curve, z = elevation.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah thanks. That's exactly what I was beginning to wonder earlier on. If we don't have any guidance on this in the spec it's really an omission that needs to be corrected. Luckily we also have the validation service where such constraints can be incrementally rolled out (cc @civilx64)

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if such constraints are required but they would make implementation much easier and rational.

Consider a line in 3D space. A single IfcLine entity could be used for both the horizontal and vertical definitions. The projection of the line onto the xy and sz planes defines the horizontal and vertical geometry. This is essentially a diagonal line through opposite corners of a cube. That's fairly easy to deal with.

Now consider a clothoid in an arbitrary plane in 3D space. I really don't want to figure out its projections onto the xy and sz planes.

Additionally, I doubt the resulting geometry is actually used in practice since roadway and rail design predates computers - our predecessors engineers likely didn't make these calculations be hand.


eval_ = [px, py, dx, dy](double u) {
auto x = px + u * dx;
auto y = py + u * dy;
Eigen::VectorXd vec;
vec << x, y, 0;
return vec;
};

} else if (segment_type_ == ST_VERTICAL) {

eval_ = [px, py, dx, dy](double u) {
// @todo this can't be correct
auto z = u * dy;
Copy link
Contributor

Choose a reason for hiding this comment

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

auto z = py + u*dy;

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks

Eigen::VectorXd vec;
vec << z;
return vec;
};

}
}

// Take the boost::type value from mpl::for_each and test it against our curve instance
Expand All @@ -392,7 +402,7 @@ class curve_segment_evaluator {
}

// Then, with function populated based on IfcCurve subtype, we can evaluate to points
Eigen::Vector3d operator()(double u) {
Eigen::VectorXd operator()(double u) {
if (eval_) {
return (*eval_)((u + start_) * length_unit_);
} else {
Expand All @@ -403,23 +413,65 @@ class curve_segment_evaluator {
double length() const {
return length_;
}

const std::optional<std::function<Eigen::VectorXd(double)>>& evaluation_function() const {
return eval_;
}
};

taxonomy::ptr mapping::map_impl(const IfcSchema::IfcCurveSegment* inst) {
// @todo fixed number of segments or fixed interval?
// @todo placement
// @todo figure out what to do with the zero length segments at the end of compound curves

static int NUM_SEGMENTS = 64;
curve_segment_evaluator cse(length_unit_, inst->ParentCurve(), inst->SegmentStart(), inst->SegmentLength());
boost::mpl::for_each<curve_seg_types, boost::type<boost::mpl::_>>(std::ref(cse));
bool is_horizontal = false;
bool is_vertical = false;
bool is_cant = false;

std::vector<taxonomy::point3::ptr> polygon;
{
aggregate_of_instance::ptr segment_owners = inst->data().getInverse(&IfcSchema::IfcCompositeCurve::Class(), 0);
if (segment_owners) {
for (auto& cc : *segment_owners) {
if (cc->as<IfcSchema::IfcSegmentedReferenceCurve>()) {
is_cant = true;
} else if (cc->as<IfcSchema::IfcGradientCurve>()) {
is_vertical = true;
} else {
is_horizontal = true;
}
}
}
}

if ((is_horizontal + is_vertical + is_cant) != 1) {
// We have to choose the correct functor based on usage. We can't
// support multiple, because we don't know the caller at this point.
return nullptr;
}

auto segment_type = is_horizontal ? ST_HORIZONTAL : is_vertical ? ST_VERTICAL : ST_CANT;

curve_segment_evaluator cse(length_unit_, segment_type, inst->ParentCurve(), inst->SegmentStart(), inst->SegmentLength());
boost::mpl::for_each<curve_seg_types, boost::type<boost::mpl::_>>(std::ref(cse));

auto fn = *cse.evaluation_function();
auto length = cse.length();

// @todo - for some reason this isn't working, the matrix gets all messed up
//const auto& transformation_matrix = taxonomy::cast<taxonomy::matrix4>(map(inst->Placement()))->ccomponents();
auto transformation_matrix = taxonomy::cast<taxonomy::matrix4>(map(inst->Placement()))->ccomponents();

auto fn_transformed = [fn, transformation_matrix](double u) {
return transformation_matrix * fn(u);
};

// @todo it might be suboptimal that we no longer have the spans now
auto pwf = taxonomy::make<taxonomy::piecewise_function>();
pwf->spans.push_back({ length, fn_transformed });
return pwf;

/*
static int NUM_SEGMENTS = 64;
std::vector<taxonomy::point3::ptr> polygon;

auto length = cse.length();
if (0.001 < fabs(length))
{
Expand All @@ -433,6 +485,7 @@ taxonomy::ptr mapping::map_impl(const IfcSchema::IfcCurveSegment* inst) {
}

return polygon_from_points(polygon);
*/
}

#endif
76 changes: 76 additions & 0 deletions src/ifcgeom/mapping/IfcGradientCurve.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
/********************************************************************************
* *
* This file is part of IfcOpenShell. *
* *
* IfcOpenShell is free software: you can redistribute it and/or modify *
* it under the terms of the Lesser GNU General Public License as published by *
* the Free Software Foundation, either version 3.0 of the License, or *
* (at your option) any later version. *
* *
* IfcOpenShell is distributed in the hope that it will be useful, *
* but WITHOUT ANY WARRANTY; without even the implied warranty of *
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the *
* Lesser GNU General Public License for more details. *
* *
* You should have received a copy of the Lesser GNU General Public License *
* along with this program. If not, see <http://www.gnu.org/licenses/>. *
* *
********************************************************************************/

#include "mapping.h"
#define mapping POSTFIX_SCHEMA(mapping)
using namespace ifcopenshell::geometry;

#ifdef SCHEMA_HAS_IfcGradientCurve

taxonomy::ptr mapping::map_impl(const IfcSchema::IfcGradientCurve* inst) {
auto horizontal = taxonomy::cast<taxonomy::piecewise_function>(map(inst->BaseCurve()));
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a way to get horizontal from a cache if it was previously mapped for a Footprint/Curve2D case or the case where multiple alignments use the same horizontal layout but different vertical profiles?

Copy link
Member Author

Choose a reason for hiding this comment

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

I will work on caching next. It's trivial, it's just a std::map<uint, taxonomy::item::ptr>. With again the caveat that we should be a bit careful considering immutability.

Copy link
Contributor

Choose a reason for hiding this comment

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

What if the map function returns a shared pointer to a constant taxonomy item? std::shared_ptr

The map function creates the taxonomy::item and all other places in the code are consumers of the taxonomy::item. Do consumers have a need to alter the item? Probably not.

This would enforce immutability.

Copy link
Member Author

Choose a reason for hiding this comment

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

Do consumers have a need to alter the item? Probably not.

There are one or two cases that still need to be worked out with orientation on things like IfcOrientedEdge, where the consumer does indeed alter item (minimally). (In OpenCascade they have their own shared pointer / handle, but orientation is basically stored on the pointer, not in the data.)

auto vertical = taxonomy::make<taxonomy::piecewise_function>();

auto segments = inst->Segments();

for (auto& segment : *segments) {
if (segment->as<IfcSchema::IfcCurveSegment>()) {
// @todo check that we don't get a mixture of implicit and explicit definitions
auto crv = map(segment->as<IfcSchema::IfcCurveSegment>());
if (crv->kind() == taxonomy::PIECEWISE_FUNCTION) {
auto seg = taxonomy::cast<taxonomy::piecewise_function>(crv);
vertical->spans.insert(vertical->spans.end(), seg->spans.begin(), seg->spans.end());
} else {
Logger::Error("Unsupported");
return nullptr;
}
} else {
Logger::Error("Unsupported");
return nullptr;
}
}

// @todo does this really make sense?
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, this is typically how it's handled when there is a horizontal and vertical definition.

This doesn't work for CT 4.1.7.1.1.4 Alignment Geometry- Segments because it seems to require that the ParentCurve be 3D. There isn't independent horizontal and vertical components for this CT.

Copy link
Contributor

Choose a reason for hiding this comment

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

We are making another assumption here - sort of - maybe it's just a distinction we need to be mindful of.

For CT Alignment Geometry Segment, the representation is on the IfcCurveSegment whereas the other CTs put the representation on the IfcAlignment.

Copy link
Member Author

Choose a reason for hiding this comment

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

because it seems to require that the ParentCurve be 3D

Where do you see this constraint encoded? I think that would be an error. I briefly looked and IfcShapeRepresentation.RepresentationType=Segment seems to just select IfcCurveSegment without any constraints on the parent curve.

IFC is a community where clear choices are seldomly made. So I think the idea is you do everything. Individual representations for the segments is the most likely thing missing from these aspects. But you're right in the sense that due to the IFC schema flexibility, you never really know.

I've updated my diagram.

instance-diagram dot

dot code
digraph {

node [shape=rect]

IfcAlignment -> IfcRelNests_0[dir=back]
IfcRelNests_0 -> { IfcAlignmentHorizontal, IfcAlignmentVertical, IfcAlignmentCant }

IfcAlignmentHorizontal -> IfcRelNests_1[dir=back]
IfcRelNests_1 -> { IfcAlignmentSegment_00, IfcAlignmentSegment_01, IfcAlignmentSegment_02 } [minlen=5]

IfcAlignmentVertical -> IfcRelNests_2[dir=back]
IfcRelNests_2 -> { IfcAlignmentSegment_10, IfcAlignmentSegment_11, IfcAlignmentSegment_12 } [minlen=3]

IfcAlignmentCant -> IfcRelNests_3[dir=back]
IfcRelNests_3 -> { IfcAlignmentSegment_20, IfcAlignmentSegment_21, IfcAlignmentSegment_22 } [minlen=1]

IfcAlignmentSegment_00 -> IfcAlignmentHorizontalSegment_0
IfcAlignmentSegment_01 -> IfcAlignmentHorizontalSegment_1
IfcAlignmentSegment_02 -> IfcAlignmentHorizontalSegment_2

IfcAlignmentSegment_10 -> IfcAlignmentVerticalSegment_0
IfcAlignmentSegment_11 -> IfcAlignmentVerticalSegment_1
IfcAlignmentSegment_12 -> IfcAlignmentVerticalSegment_2

IfcAlignmentSegment_20 -> IfcAlignmentCantSegment_0
IfcAlignmentSegment_21 -> IfcAlignmentCantSegment_1
IfcAlignmentSegment_22 -> IfcAlignmentCantSegment_2



IfcAlignmentCantSegment_0       [label=IfcAlignmentCantSegment]
IfcAlignmentCantSegment_1       [label=IfcAlignmentCantSegment]
IfcAlignmentCantSegment_2       [label=IfcAlignmentCantSegment]
IfcAlignmentHorizontalSegment_0 [label=IfcAlignmentHorizontalSegment]
IfcAlignmentHorizontalSegment_1 [label=IfcAlignmentHorizontalSegment]
IfcAlignmentHorizontalSegment_2 [label=IfcAlignmentHorizontalSegment]
IfcAlignmentSegment_00          [label=IfcAlignmentSegment]
IfcAlignmentSegment_01          [label=IfcAlignmentSegment]
IfcAlignmentSegment_02          [label=IfcAlignmentSegment]
IfcAlignmentSegment_10          [label=IfcAlignmentSegment]
IfcAlignmentSegment_11          [label=IfcAlignmentSegment]
IfcAlignmentSegment_12          [label=IfcAlignmentSegment]
IfcAlignmentSegment_20          [label=IfcAlignmentSegment]
IfcAlignmentSegment_21          [label=IfcAlignmentSegment]
IfcAlignmentSegment_22          [label=IfcAlignmentSegment]
IfcAlignmentVerticalSegment_0   [label=IfcAlignmentVerticalSegment]
IfcAlignmentVerticalSegment_1   [label=IfcAlignmentVerticalSegment]
IfcAlignmentVerticalSegment_2   [label=IfcAlignmentVerticalSegment]
IfcRelNests_0                   [label=N, shape=circle]
IfcRelNests_1                   [label=N, shape=circle]
IfcRelNests_2                   [label=N, shape=circle]
IfcRelNests_3                   [label=N, shape=circle]


IfcAlignment -> IfcProductDefinitionShape           
IfcProductDefinitionShape -> IfcShapeRepresentation 
IfcShapeRepresentation -> IfcSegmentedReferenceCurve

IfcAlignment -> spacing1                 [style=invis]
spacing1 -> IfcProductDefinitionShape    [style=invis]
IfcProductDefinitionShape -> spacing2    [style=invis]
spacing2 -> IfcShapeRepresentation       [style=invis]
IfcShapeRepresentation -> spacing3       [style=invis]
spacing3 -> IfcSegmentedReferenceCurve   [style=invis]


{ IfcAlignment IfcProductDefinitionShape IfcShapeRepresentation IfcSegmentedReferenceCurve spacing1 spacing2 spacing3 rank=same }

IfcSegmentedReferenceCurve -> {  IfcCurveSegment_20, IfcCurveSegment_21, IfcCurveSegment_22  } [minlen=5]


IfcSegmentedReferenceCurve -> IfcGradientCurve

IfcGradientCurve -> {  IfcCurveSegment_10, IfcCurveSegment_11, IfcCurveSegment_12  }  [minlen=6]

IfcGradientCurve -> IfcCompositeCurve

IfcCompositeCurve -> {  IfcCurveSegment_00, IfcCurveSegment_01, IfcCurveSegment_02  } [minlen=7]

spacing1 [style=invis, width=2]
spacing2 [style=invis, width=2]
spacing3 [style=invis, width=2]


edge [constraint=false]

IfcAlignmentSegment_20 -> IfcCurveSegment_20 [style=dashed, label=Representation]
IfcAlignmentSegment_21 -> IfcCurveSegment_21 [style=dashed, label=Representation]
IfcAlignmentSegment_22 -> IfcCurveSegment_22 [style=dashed, label=Representation]
IfcAlignmentSegment_10 -> IfcCurveSegment_10 [style=dashed, label=Representation]
IfcAlignmentSegment_11 -> IfcCurveSegment_11 [style=dashed, label=Representation]
IfcAlignmentSegment_12 -> IfcCurveSegment_12 [style=dashed, label=Representation]
IfcAlignmentSegment_00 -> IfcCurveSegment_00 [style=dashed, label=Representation]
IfcAlignmentSegment_01 -> IfcCurveSegment_01 [style=dashed, label=Representation]
IfcAlignmentSegment_02 -> IfcCurveSegment_02 [style=dashed, label=Representation]

IfcCurveSegment_20 [label=IfcCurveSegment]
IfcCurveSegment_21 [label=IfcCurveSegment]
IfcCurveSegment_22 [label=IfcCurveSegment]
IfcCurveSegment_10 [label=IfcCurveSegment]
IfcCurveSegment_11 [label=IfcCurveSegment]
IfcCurveSegment_12 [label=IfcCurveSegment]
IfcCurveSegment_00 [label=IfcCurveSegment]
IfcCurveSegment_01 [label=IfcCurveSegment]
IfcCurveSegment_02 [label=IfcCurveSegment]

}

Copy link
Contributor

Choose a reason for hiding this comment

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

Where do you see this constraint encoded?

Perhaps I over stated my assumption. There are 4 CTs, the first 3 decompose the 3D space into two-2D representations that are combined to get a 3D result. I assumed the 4th CT, Segments, models the 3D space directly. The ParentCurve could be in a 2D horizontal xy plane, but this would be the Horizontal CT. Not sure if it make sense that ParentCurve be in a vertical sz plane without a horizontal component.

I'm not sure what use case was imagined for the 4th CT but to me it looks like it would apply for as-built survey data which would be 3D points or segments

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, I interpreted this differently. In my view the segment geometry is not really meant to be interesting in itself, but more to provide a more granular coupling between the business logic and the geometry. So the concept template just expresses that and doesn't really relate to a usecase. For surveying usecases it is possible to express an IfcAlignment representation simply as as 3d polyline.

(Or another possible top-level representation item for alignment: https://ifc43-docs.standards.buildingsmart.org/IFC/RELEASE/IFC4x3/HTML/lexical/IfcOffsetCurveByDistances.htm)

auto composition = [horizontal, vertical](double u) {
auto xy = horizontal->evaluate(u);
auto z = vertical->evaluate(u);
Eigen::VectorXd vec(3);
vec << xy(0), xy(1), z(0);
return vec;
};

// @todo where do we get the startdistalong from @civilx64's code?
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you get it from segment.start and segment.length?

Copy link
Contributor

@civilx64 civilx64 Sep 30, 2023

Choose a reason for hiding this comment

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

https://github.com/civilx64/IfcOpenShell/blob/c263b4f50ebbdcdacc5d3f548c561a1fb24dcd13/src/ifcopenshell-python/ifcopenshell/ifcgeom/IfcAlignmentHorizontal.py#L206

When iterating segments I keep a private variable _length which is incremented by segment.length for each segment.

_length is short for "length along the alignment up to the beginning of this particular segment".

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, I wasn't very clear, this was mostly a note to myself. This was about:

https://github.com/IfcOpenShell/IfcOpenShell/pull/3148/files#diff-4b6d2c91bd1d5ed55b20ad2d6023a46e93c7bdd011e3ecf4727c172e5ae436ecR325

afbeelding

On the business logic side we have StartDistAlong on vertical. But on the geometry side for the first vertical segment we have:

afbeelding

three parameters that can be used to independently change the start point of the vertical wrt to the horizontal:

  • The vertical curve start point (line Pnt in this case)
  • SegmentStart which is an offset over the vertical curve
  • Segment placement, which can be
    • linear placement over horizontal:
      • same horizontal (good, IfcPointByDistanceExpression.DistanceAlong becomes essentially StartDistAlong on vertical)
      • different horizontal (invalid?)
    • localplacement:
      • same as horizontal (can be ignored)
      • different from horizontal (in theory we could project the localplacement position onto the curve to find a length along the curve?)

I'm also really confused about the placement altogether. You can introduce gradient by means of IfcLine.Dir but also with the placement X-axis. Which dimensionality is a vertical alignment curve to begin with? Is it a curve in the XY-plane with that maps $X -&gt; Y$ or $u -&gt; Y$.

The attribute description of Placement is some really cryptic prose:

Placement in the context of the curve using this segment. As insertion point SegmentStart is the reference point of Placement. RefDirection of Placement also specifies the sense of the trimmed segment of ParentCurve. RefDirection is bound to the paramettrization sense of the segment.

std::array<taxonomy::piecewise_function::ptr, 2> both = { horizontal , vertical };
double min_length = std::numeric_limits<double>::infinity();
for (auto i = 0; i < 2; ++i) {
double l = 0;
for (auto& s : both[i]->spans) {
l += s.first;
}
if (l < min_length) {
min_length = l;
}
}

auto pwf = taxonomy::make<taxonomy::piecewise_function>();
pwf->spans.push_back({ min_length, composition });
return pwf;
}

#endif
5 changes: 4 additions & 1 deletion src/ifcgeom/mapping/mapping.i
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,9 @@ BIND(IfcEdge);
BIND(IfcEdgeLoop);
BIND(IfcPolyline);
BIND(IfcPolyLoop);
#ifdef SCHEMA_HAS_IfcGradientCurve
BIND(IfcGradientCurve);
Copy link
Contributor

Choose a reason for hiding this comment

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

@aothms This is a little problematic. IfcGradientCurve is a IfcCompositeCurve so the item gets processed twice and the result for IfcCompositeCurve is returned. When processed as IfCompositeCurve the IfcCurveSegment are the vertical segments.

I have some code in my driver program that maps a IfcGradientCurve but gets back the implicit_item for the vertical components.

if (item->as<Ifc4x3_add1::IfcGradientCurve>())
{
	auto mapped_item = ifcopenshell::geometry::taxonomy::cast<ifcopenshell::geometry::taxonomy::implicit_item>(mapping->map(item->as<Ifc4x3_add1::IfcGradientCurve>()));
	auto loop = ifcopenshell::geometry::taxonomy::cast<ifcopenshell::geometry::taxonomy::loop>(mapped_item->evaluate());
...

Reversing the order of BIND(IfcGradientCurve) and BIND(IfcCompositeCurve) fixes the problem, but I don't think it is the best solution.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh sorry. Yes, that could be addressed like this: https://github.com/IfcOpenShell/IfcOpenShell/pull/3827/files#r1347391406

It's something I introduced in the implementation of caching. I could no longer use the return statement anymore (because writing to cache is at the bottom of the function). So now it "falls through" to the other. That's why my suggestion of the addition of !item && helps circumvent that (well at least in the cases the mapping is successful in assigning something to item).

Copy link
Contributor

Choose a reason for hiding this comment

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

@aothms This is still a little problematic. If BIND(IfcCompositeCurve) is listed before BIND(IfcGradientCurve) the IfcGradientCurve is treated as an IfcCompositeCurve and the IfcGradientCurve code never gets called.

Is there a function similar to inst->as<IfcSchema::T>() that tests if the most derived type of an instance is equal to a specific type?

For IfcGradientCurve the test for IfcCompositeCurve would be false since the most derived type is IfcGradientCurve.

Listing the BIND statements for the more derived types before the parent types solves the problem, but this seems like a brittle and problematic solution.

Copy link
Member Author

Choose a reason for hiding this comment

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

Listing the BIND statements for the more derived types before the parent types solves the problem, but this seems like a brittle and problematic solution.

I agree, but in quite a few cases we do depend on matching also inherited types:

// IfcFacetedBrep included
// IfcAdvancedBrep included
// IfcFacetedBrepWithVoids included
// IfcAdvancedBrepWithVoids included
BIND(IfcManifoldSolidBrep);

// IfcFaceSurface included
// IfcAdvancedFace included in case of IFC4
BIND(IfcFace);

On the other hand in many other cases it's also problematic because in the inheritance hierarchy of the newly added IFC geometry types, they don't seem to consider liskov-substitution in determining whether entities are supposed to be subtypes, but instead rather pragmatically try to reuse some attribute definitions.

So maybe we need to enhance the BIND() macro with an option to selectively specify whether inheritance is considered to have the best of both worlds?

We could check equality of &inst->declaration() == &Ifc2x3::IfcWall::Class(). This disregards inheritance (and is also more efficient because it doesn't rely on RTTI and virtual inheritance).

#endif
BIND(IfcCompositeCurve);
BIND(IfcTrimmedCurve);
BIND(IfcArbitraryOpenProfileDef);
Expand Down Expand Up @@ -150,4 +153,4 @@ BIND(IfcStyledItem); // -> style

#ifdef SCHEMA_HAS_IfcCurveSegment
BIND(IfcCurveSegment);
#endif
#endif