Skip to content

Commit

Permalink
Merge pull request #1432 from openscad/issue1407
Browse files Browse the repository at this point in the history 
Fix #1407 by storing vector values as heap objects (ValuePtr) rather than stack objects(Value)
  • Loading branch information
@kintel
kintel committed Sep 14, 2015
2 parents ed637eb + c8097fd commit 7f9e7f9
Show file tree
Hide file tree
Showing 18 changed files with 324 additions and 292 deletions.
20 changes: 10 additions & 10 deletions src/Preferences.cc
View file
Expand Up @@ -32,6 +32,7 @@
#include <QSettings>
#include <QStatusBar>
#include <boost/algorithm/string.hpp>
#include <boost/foreach.hpp>
#include "GeometryCache.h"
#include "AutoUpdater.h"
#include "feature.h"
Expand All @@ -49,7 +50,7 @@ Q_DECLARE_METATYPE(Feature *);
class SettingsReader : public Settings::Visitor
{
QSettings settings;
const Value getValue(const Settings::SettingsEntry& entry, const std::string& value) const {
Value getValue(const Settings::SettingsEntry& entry, const std::string& value) const {
std::string trimmed_value(value);
boost::trim(trimmed_value);

Expand Down Expand Up @@ -101,7 +102,7 @@ class SettingsWriter : public Settings::Visitor
settings.remove(key);
PRINTDB("SettingsWriter D: %s", key.toStdString().c_str());
} else {
Value value = s->get(entry);
const Value &value = s->get(entry);
settings.setValue(key, QString::fromStdString(value.toString()));
PRINTDB("SettingsWriter W: %s = '%s'", key.toStdString().c_str() % value.toString().c_str());
}
Expand Down Expand Up @@ -689,18 +690,17 @@ void Preferences::updateGUI()
void Preferences::initComboBox(QComboBox *comboBox, const Settings::SettingsEntry& entry)
{
comboBox->clear();
Value::VectorType vector = entry.range().toVector();
for (Value::VectorType::iterator it = vector.begin();it != vector.end();it++) {
QString val = QString::fromStdString((*it)[0].toString());
std::string text((*it)[1].toString());
QString qtext = QString::fromStdString(gettext(text.c_str()));
// Range is a vector of 2D vectors: [[name, value], ...]
BOOST_FOREACH(const ValuePtr &v, entry.range().toVector()) {
QString val = QString::fromStdString(v[0]->toString());
QString qtext = QString::fromStdString(gettext(v[1]->toString().c_str()));
comboBox->addItem(qtext, val);
}
}

void Preferences::initSpinBox(QSpinBox *spinBox, const Settings::SettingsEntry& entry)
{
Value::RangeType range = entry.range().toRange();
RangeType range = entry.range().toRange();
spinBox->setMinimum(range.begin_value());
spinBox->setMaximum(range.end_value());
}
Expand All @@ -709,13 +709,13 @@ void Preferences::updateComboBox(QComboBox *comboBox, const Settings::SettingsEn
{
Settings::Settings *s = Settings::Settings::inst();

Value value = s->get(entry);
const Value &value = s->get(entry);
QString text = QString::fromStdString(value.toString());
int idx = comboBox->findData(text);
if (idx >= 0) {
comboBox->setCurrentIndex(idx);
} else {
Value defaultValue = entry.defaultValue();
const Value &defaultValue = entry.defaultValue();
QString defaultText = QString::fromStdString(defaultValue.toString());
int defIdx = comboBox->findData(defaultText);
if (defIdx >= 0) {
Expand Down
6 changes: 3 additions & 3 deletions src/builtin.cc
View file
Expand Up @@ -101,9 +101,9 @@ Builtins::Builtins()
this->globalscope.assignments.push_back(Assignment("$t", boost::shared_ptr<Expression>(new ExpressionConst(ValuePtr(0.0)))));

Value::VectorType zero3;
zero3.push_back(Value(0.0));
zero3.push_back(Value(0.0));
zero3.push_back(Value(0.0));
zero3.push_back(ValuePtr(0.0));
zero3.push_back(ValuePtr(0.0));
zero3.push_back(ValuePtr(0.0));
ValuePtr zero3val(zero3);
this->globalscope.assignments.push_back(Assignment("$vpt", boost::shared_ptr<Expression>(new ExpressionConst(zero3val))));
this->globalscope.assignments.push_back(Assignment("$vpr", boost::shared_ptr<Expression>(new ExpressionConst(zero3val))));
Expand Down
12 changes: 6 additions & 6 deletions src/cgaladv.cc
View file
Expand Up @@ -89,17 +89,17 @@ AbstractNode *CgaladvModule::instantiate(const Context *ctx, const ModuleInstant
node->newsize << 0,0,0;
if ( ns->type() == Value::VECTOR ) {
const Value::VectorType &vs = ns->toVector();
if ( vs.size() >= 1 ) node->newsize[0] = vs[0].toDouble();
if ( vs.size() >= 2 ) node->newsize[1] = vs[1].toDouble();
if ( vs.size() >= 3 ) node->newsize[2] = vs[2].toDouble();
if ( vs.size() >= 1 ) node->newsize[0] = vs[0]->toDouble();
if ( vs.size() >= 2 ) node->newsize[1] = vs[1]->toDouble();
if ( vs.size() >= 3 ) node->newsize[2] = vs[2]->toDouble();
}
ValuePtr autosize = c.lookup_variable("auto");
node->autosize << false, false, false;
if ( autosize->type() == Value::VECTOR ) {
const Value::VectorType &va = autosize->toVector();
if ( va.size() >= 1 ) node->autosize[0] = va[0].toBool();
if ( va.size() >= 2 ) node->autosize[1] = va[1].toBool();
if ( va.size() >= 3 ) node->autosize[2] = va[2].toBool();
if ( va.size() >= 1 ) node->autosize[0] = va[0]->toBool();
if ( va.size() >= 2 ) node->autosize[1] = va[1]->toBool();
if ( va.size() >= 3 ) node->autosize[2] = va[2]->toBool();
}
else if ( autosize->type() == Value::BOOL ) {
node->autosize << autosize->toBool(),autosize->toBool(),autosize->toBool();
Expand Down
2 changes: 1 addition & 1 deletion src/color.cc
View file
Expand Up @@ -229,7 +229,7 @@ AbstractNode *ColorModule::instantiate(const Context *ctx, const ModuleInstantia
ValuePtr v = c.lookup_variable("c");
if (v->type() == Value::VECTOR) {
for (size_t i = 0; i < 4; i++) {
node->color[i] = i < v->toVector().size() ? v->toVector()[i].toDouble() : 1.0;
node->color[i] = i < v->toVector().size() ? v->toVector()[i]->toDouble() : 1.0;
if (node->color[i] > 1)
PRINTB_NOCACHE("WARNING: color() expects numbers between 0.0 and 1.0. Value of %.1f is too large.", node->color[i]);
}
Expand Down
40 changes: 20 additions & 20 deletions src/control.cc
View file
Expand Up @@ -63,7 +63,7 @@ class ControlModule : public AbstractModule

static const EvalContext* getLastModuleCtx(const EvalContext *evalctx);

static AbstractNode* getChild(const Value &value, const EvalContext* modulectx);
static AbstractNode* getChild(const ValuePtr &value, const EvalContext* modulectx);

private: // data
Type type;
Expand All @@ -78,15 +78,15 @@ void ControlModule::for_eval(AbstractNode &node, const ModuleInstantiation &inst
ValuePtr it_values = evalctx->getArgValue(l, ctx);
Context c(ctx);
if (it_values->type() == Value::RANGE) {
Value::RangeType range = it_values->toRange();
boost::uint32_t steps = range.nbsteps();
if (steps >= 10000) {
PRINTB("WARNING: Bad range parameter in for statement: too many elements (%lu).", steps);
} else {
for (Value::RangeType::iterator it = range.begin();it != range.end();it++) {
c.set_variable(it_name, ValuePtr(*it));
for_eval(node, inst, l+1, &c, evalctx);
}
RangeType range = it_values->toRange();
boost::uint32_t steps = range.nbsteps();
if (steps >= 10000) {
PRINTB("WARNING: Bad range parameter in for statement: too many elements (%lu).", steps);
} else {
for (RangeType::iterator it = range.begin();it != range.end();it++) {
c.set_variable(it_name, ValuePtr(*it));
for_eval(node, inst, l+1, &c, evalctx);
}
}
}
else if (it_values->type() == Value::VECTOR) {
Expand Down Expand Up @@ -133,17 +133,17 @@ const EvalContext* ControlModule::getLastModuleCtx(const EvalContext *evalctx)
}

// static
AbstractNode* ControlModule::getChild(const Value& value, const EvalContext* modulectx)
AbstractNode* ControlModule::getChild(const ValuePtr &value, const EvalContext* modulectx)
{
if (value.type()!=Value::NUMBER) {
if (value->type()!=Value::NUMBER) {
// Invalid parameter
// (e.g. first child of difference is invalid)
PRINTB("WARNING: Bad parameter type (%s) for children, only accept: empty, number, vector, range.", value.toString());
PRINTB("WARNING: Bad parameter type (%s) for children, only accept: empty, number, vector, range.", value->toString());
return NULL;
}
double v;
if (!value.getDouble(v)) {
PRINTB("WARNING: Bad parameter type (%s) for children, only accept: empty, number, vector, range.", value.toString());
if (!value->getDouble(v)) {
PRINTB("WARNING: Bad parameter type (%s) for children, only accept: empty, number, vector, range.", value->toString());
return NULL;
}

Expand Down Expand Up @@ -223,28 +223,28 @@ AbstractNode *ControlModule::instantiate(const Context* /*ctx*/, const ModuleIns
// one (or more ignored) parameter
ValuePtr value = evalctx->getArgValue(0);
if (value->type() == Value::NUMBER) {
return getChild(*value, modulectx);
return getChild(value, modulectx);
}
else if (value->type() == Value::VECTOR) {
AbstractNode* node = new AbstractNode(inst);
const Value::VectorType& vect = value->toVector();
foreach (const Value::VectorType::value_type& vectvalue, vect) {
foreach (const ValuePtr &vectvalue, vect) {
AbstractNode* childnode = getChild(vectvalue,modulectx);
if (childnode==NULL) continue; // error
node->children.push_back(childnode);
}
return node;
}
else if (value->type() == Value::RANGE) {
Value::RangeType range = value->toRange();
RangeType range = value->toRange();
boost::uint32_t steps = range.nbsteps();
if (steps >= 10000) {
PRINTB("WARNING: Bad range parameter for children: too many elements (%lu).", steps);
return NULL;
}
AbstractNode* node = new AbstractNode(inst);
for (Value::RangeType::iterator it = range.begin();it != range.end();it++) {
AbstractNode* childnode = getChild(Value(*it),modulectx); // with error cases
for (RangeType::iterator it = range.begin();it != range.end();it++) {
AbstractNode* childnode = getChild(ValuePtr(*it),modulectx); // with error cases
if (childnode==NULL) continue; // error
node->children.push_back(childnode);
}
Expand Down
4 changes: 2 additions & 2 deletions src/dxfdim.cc
View file
Expand Up @@ -200,8 +200,8 @@ ValuePtr builtin_dxf_cross(const Context *ctx, const EvalContext *evalctx)
double x = x1 + ua*(x2 - x1);
double y = y1 + ua*(y2 - y1);
Value::VectorType ret;
ret.push_back(Value(x));
ret.push_back(Value(y));
ret.push_back(ValuePtr(x));
ret.push_back(ValuePtr(y));
return dxf_cross_cache[key] = ValuePtr(ret);
}
}
Expand Down
27 changes: 14 additions & 13 deletions src/expr.cc
View file
Expand Up @@ -41,12 +41,12 @@ namespace {
Value::VectorType flatten(Value::VectorType const& vec) {
int n = 0;
for (unsigned int i = 0; i < vec.size(); i++) {
assert(vec[i].type() == Value::VECTOR);
n += vec[i].toVector().size();
assert(vec[i]->type() == Value::VECTOR);
n += vec[i]->toVector().size();
}
Value::VectorType ret; ret.reserve(n);
for (unsigned int i = 0; i < vec.size(); i++) {
std::copy(vec[i].toVector().begin(),vec[i].toVector().end(),std::back_inserter(ret));
std::copy(vec[i]->toVector().begin(),vec[i]->toVector().end(),std::back_inserter(ret));
}
return ret;
}
Expand Down Expand Up @@ -357,7 +357,7 @@ ExpressionConst::ExpressionConst(const ValuePtr &val) : const_value(val)

ValuePtr ExpressionConst::evaluate(const class Context *) const
{
return ValuePtr(this->const_value);
return this->const_value;
}

void ExpressionConst::print(std::ostream &stream) const
Expand All @@ -380,12 +380,12 @@ ValuePtr ExpressionRange::evaluate(const Context *context) const
ValuePtr v2 = this->second->evaluate(context);
if (v2->type() == Value::NUMBER) {
if (this->children.size() == 2) {
Value::RangeType range(v1->toDouble(), v2->toDouble());
RangeType range(v1->toDouble(), v2->toDouble());
return ValuePtr(range);
} else {
ValuePtr v3 = this->third->evaluate(context);
if (v3->type() == Value::NUMBER) {
Value::RangeType range(v1->toDouble(), v2->toDouble(), v3->toDouble());
RangeType range(v1->toDouble(), v2->toDouble(), v3->toDouble());
return ValuePtr(range);
}
}
Expand All @@ -409,7 +409,8 @@ ValuePtr ExpressionVector::evaluate(const Context *context) const
{
Value::VectorType vec;
BOOST_FOREACH(const Expression *e, this->children) {
vec.push_back(*(e->evaluate(context)));
ValuePtr tmpval = e->evaluate(context);
vec.push_back(tmpval);
}
return ValuePtr(vec);
}
Expand Down Expand Up @@ -545,7 +546,7 @@ ValuePtr ExpressionLc::evaluate(const Context *context) const
if (this->second->isListComprehension()) {
return this->second->evaluate(context);
} else {
vec.push_back((*this->second->evaluate(context)));
vec.push_back(this->second->evaluate(context));
}
}
return ValuePtr(vec);
Expand All @@ -561,26 +562,26 @@ ValuePtr ExpressionLc::evaluate(const Context *context) const
Context c(context);

if (it_values->type() == Value::RANGE) {
Value::RangeType range = it_values->toRange();
RangeType range = it_values->toRange();
boost::uint32_t steps = range.nbsteps();
if (steps >= 1000000) {
PRINTB("WARNING: Bad range parameter in for statement: too many elements (%lu).", steps);
} else {
for (Value::RangeType::iterator it = range.begin();it != range.end();it++) {
for (RangeType::iterator it = range.begin();it != range.end();it++) {
c.set_variable(it_name, ValuePtr(*it));
vec.push_back((*this->first->evaluate(&c)));
vec.push_back(this->first->evaluate(&c));
}
}
}
else if (it_values->type() == Value::VECTOR) {
for (size_t i = 0; i < it_values->toVector().size(); i++) {
c.set_variable(it_name, it_values->toVector()[i]);
vec.push_back((*this->first->evaluate(&c)));
vec.push_back(this->first->evaluate(&c));
}
}
else if (it_values->type() != Value::UNDEFINED) {
c.set_variable(it_name, it_values);
vec.push_back((*this->first->evaluate(&c)));
vec.push_back(this->first->evaluate(&c));
}
if (this->first->isListComprehension()) {
return ValuePtr(flatten(vec));
Expand Down

0 comments on commit 7f9e7f9

Please sign in to comment.