trase icon indicating copy to clipboard operation
trase copied to clipboard

Adding attributes to tags in svg backend

Open fcooper8472 opened this issue 6 years ago • 3 comments

I would like to implement some helper to make this kind of thing look less nasty (easier to read, reason about, and maintain):

m_out << "x=\"" << min[0] << "\" y=\"" << min[1] << "\" width=\"" << delta[0] << "\" height=\"" << delta[1] << "\" ";

There are several possible options.

1. Function template, something like:

template<typename T>
std::string att(const std::string& name, const T val)
{
  std::stringstream ss;
  ss << std::setprecision(5) << name << "=\"" << val << "\" ";
  return ss.str();
}

so that the above example becomes

m_out << att("x", min[0]) << att("y", min[1]) << att("width", delta[0]) << att("height", delta[1]); 

This looks quite nice, and is certainly much easier to read. But, the overhead of generating lots of stringstream objects and returning strings is high (~2x slower). Speed probably isn't a massive concern at this stage, but let's avoid massive slowdowns if there's a better option.

2. As above, but sending straight to m_out:

I don't like this option at all, because the example becomes:

att("x", min[0]); att("y", min[1]); att("width", delta[0]); att("height", delta[1]); 

It's super unclear at a glance what's happening and doesn't have any flexibility at all.

3. A small class for gathering attributes:

struct attribs {
  std::stringstream ss;
  attribs& add(const std::string& name, const float val) {
    ss << std::setprecision(5) << name << "=\"" << val << "\" ";
    return *this;
  }
  /* overload << */
};

The example becomes

attribs att;
att.add("x", min[0]).add("y", min[1]).add("width", delta[0]).add("height", delta[1]);
m_out << att;

This has a greatly reduced cost vs 1, is flexible, but perhaps not the most elegant.

@martinjrobins Any other ideas for how this might look?

fcooper8472 avatar Jul 13 '18 22:07 fcooper8472

  1. looks the best out of those options, here are a couple of suggestions to increase speed, would any of these work?:
  • using snprintf to write to a char buffer, then just return that buffer. This would definitely be faster but perhaps not as safe
  • write it as a function object, perhaps if you re-use the stringstream object it will be faster?:
struct att {
std::stringstream ss;

att() {  ss << std::setprecision(5); }

template<typename T>
std::string operator()(const std::string& name, const T val)
{
  ss.str("");
  ss << name << "=\"" << val << "\" ";
  return ss.str();
}
};

martinjrobins avatar Jul 14 '18 06:07 martinjrobins

@martinjrobins your struct is actually slightly faster than mine, with nicer semantics too :+1:.

Using snprintf is a fair bit faster still:

xigxzblxankjfav4jp5a4f2dray

But, I really don't like it as a solution:

  • It would be a pain to dynamically alter precision
  • It wouldn't work for custom arguments with a << operator, like RGBA

I think I'll go ahead and implement something like your suggestion and we can see what it looks like.

fcooper8472 avatar Jul 14 '18 09:07 fcooper8472

I agree, I don't think using snprintf is worth the extra speed.

martinjrobins avatar Jul 14 '18 09:07 martinjrobins