Skip to content

Commit dc4efce

Browse files
refactor: performance and safety improvements
1 parent a672f65 commit dc4efce

File tree

6 files changed

+70
-67
lines changed

6 files changed

+70
-67
lines changed

source/matplot/backend/backend_interface.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ namespace matplot {
4141
class MATPLOT_EXPORTS backend_interface {
4242
/// Virtual functions you can override to create any backend
4343
public:
44+
virtual ~backend_interface() noexcept = default;
4445
/// \brief True if backend is in interactive mode
4546
/// One backends might support both interactive and
4647
/// non-interactive mode.
@@ -213,7 +214,7 @@ namespace matplot {
213214
/// This is useful when tracing the gnuplot commands
214215
/// and when generating a gnuplot file.
215216
virtual void include_comment(const std::string &text);
216-
};
217+
}; // class backend_interface
217218
} // namespace backend
218219

219220
} // namespace matplot

source/matplot/backend/gnuplot.cpp

Lines changed: 48 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
#include <matplot/util/common.h>
88
#include <matplot/util/popen.h>
99
#include <iostream>
10-
#include <regex>
10+
#include <charconv>
1111
#include <thread>
1212
#include <cstring>
1313
#include <cstdlib>
@@ -317,13 +317,28 @@ namespace matplot::backend {
317317
}
318318
}
319319

320+
/// returns the next word in text after prefix terminated with white space.
321+
static std::string_view word_after(std::string_view text, std::string_view prefix)
322+
{
323+
auto res = text.substr(0,0);
324+
if (auto b = text.find(prefix); b != std::string_view::npos) {
325+
b += prefix.length();
326+
while (b < text.length() && std::isspace(text[b]))
327+
++b; // skip white space before word
328+
auto e = b;
329+
while (e < text.length() && !std::isspace(text[e]))
330+
++e; // scan until white space or end
331+
res = text.substr(b, e-b);
332+
}
333+
return res;
334+
}
335+
320336
std::string gnuplot::default_terminal_type() {
321337
static std::string terminal_type;
322338
const bool dont_know_term_type = terminal_type.empty();
323339
if (dont_know_term_type) {
324340
terminal_type = run_and_get_output("gnuplot -e \"show terminal\" 2>&1");
325-
terminal_type = std::regex_replace(terminal_type,
326-
std::regex("[^]*terminal type is ([^ ]+)[^]*"), "$1");
341+
terminal_type = word_after(terminal_type, "terminal type is ");
327342
const bool still_dont_know_term_type = terminal_type.empty();
328343
if (still_dont_know_term_type) {
329344
terminal_type = "qt";
@@ -334,54 +349,45 @@ namespace matplot::backend {
334349

335350
bool gnuplot::terminal_is_available(std::string_view term) {
336351
std::string msg = run_and_get_output("gnuplot -e \"set terminal " +
337-
std::string(term.data()) + "\" 2>&1");
352+
std::string{term} + "\" 2>&1");
338353
return msg.empty();
339354
}
340355

356+
template <typename T>
357+
void convert_to(std::string_view text, T& value) {
358+
std::from_chars(text.data(), text.data() + text.length(), value);
359+
}
360+
341361
std::tuple<int, int, int> gnuplot::gnuplot_version() {
342-
static std::tuple<int, int, int> version{0, 0, 0};
343-
const bool dont_know_gnuplot_version_yet =
344-
version == std::tuple<int, int, int>({0, 0, 0});
345-
if (dont_know_gnuplot_version_yet) {
346-
std::string version_str =
347-
run_and_get_output("gnuplot --version 2>&1");
348-
std::string version_major = std::regex_replace(
349-
version_str,
350-
std::regex("[^]*gnuplot (\\d+)\\.\\d+ patchlevel \\d+ *"),
351-
"$1");
352-
std::string version_minor = std::regex_replace(
353-
version_str,
354-
std::regex("[^]*gnuplot \\d+\\.(\\d+) patchlevel \\d+ *"),
355-
"$1");
356-
std::string version_patch = std::regex_replace(
357-
version_str,
358-
std::regex("[^]*gnuplot \\d+\\.\\d+ patchlevel (\\d+) *"),
359-
"$1");
360-
try {
361-
std::get<0>(version) = std::stoi(version_major);
362-
} catch (...) {
363-
std::get<0>(version) = 0;
364-
}
365-
try {
366-
std::get<1>(version) = std::stoi(version_minor);
367-
} catch (...) {
368-
std::get<1>(version) = 0;
369-
}
370-
try {
371-
std::get<2>(version) = std::stoi(version_patch);
372-
} catch (...) {
373-
std::get<2>(version) = 0;
374-
}
375-
const bool still_dont_know_gnuplot_version =
376-
version == std::tuple<int, int, int>({0, 0, 0});
377-
if (still_dont_know_gnuplot_version) {
378-
// assume it's 5.2.6 by convention
379-
version = std::tuple<int, int, int>({5, 2, 6});
362+
constexpr auto version_zero = std::make_tuple(0, 0, 0);
363+
static auto version = version_zero;
364+
if (version == version_zero) { // unknown version
365+
const auto version_str = run_and_get_output("gnuplot --version 2>&1");
366+
// gnuplot version_str example: "5.2 patchlevel 6"
367+
const auto major_minor = word_after(version_str, "gnuplot"); // "5.2"
368+
const auto minor = word_after(major_minor, "."); // "2"
369+
const auto patch = word_after(version_str, "patchlevel"); // "6"
370+
if (!major_minor.empty() && !minor.empty() && !patch.empty()) {
371+
convert_to(major_minor, std::get<0>(version));
372+
convert_to(minor, std::get<1>(version));
373+
convert_to(patch, std::get<2>(version));
380374
}
375+
if (version == version_zero) // still unknown
376+
version = {5, 2, 6}; // assume by convention
381377
}
382378
return version;
383379
}
384380

381+
bool gnuplot::gnuplot_includes_legends() {
382+
return gnuplot_version() >= std::make_tuple(5, 2, 6);
383+
}
384+
bool gnuplot::gnuplot_has_wall_option() {
385+
return gnuplot_version() >= std::make_tuple(5, 5, 0);
386+
}
387+
bool gnuplot::gnuplot_supports_keyentry() {
388+
return gnuplot_version() >= std::make_tuple(5, 2, 6);
389+
}
390+
385391
bool gnuplot::terminal_has_title_option(const std::string &t) {
386392
SV_CONSTEXPR std::string_view whitelist[] = {
387393
"qt", "aqua", "caca", "canvas", "windows", "wxt", "x11"};

source/matplot/backend/gnuplot.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,9 @@ namespace matplot::backend {
4848
static std::string default_terminal_type();
4949
static bool terminal_is_available(std::string_view);
5050
static std::tuple<int, int, int> gnuplot_version();
51+
static bool gnuplot_includes_legends();
52+
static bool gnuplot_has_wall_option();
53+
static bool gnuplot_supports_keyentry();
5154
static bool terminal_has_title_option(const std::string &t);
5255
static bool terminal_has_size_option(const std::string &t);
5356
static bool terminal_has_position_option(const std::string &t);

source/matplot/core/axes_type.cpp

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -426,7 +426,7 @@ namespace matplot {
426426
run_command("set polar");
427427
}
428428
auto set_or_unset_axis = [this](class axis_type &ax,
429-
std::string axis_name,
429+
const std::string &axis_name,
430430
bool minor_ticks = false) {
431431
// cb is the only axis we don't unset if tics are empty
432432
// r-axis labels should still be handled even if axis is invisible since we use the grid
@@ -695,10 +695,7 @@ namespace matplot {
695695
// Gnuplot version needs to be 5.2.6+ for keyentry
696696
bool ok = true;
697697
if (parent_->backend_->consumes_gnuplot_commands()) {
698-
if (backend::gnuplot::gnuplot_version() <
699-
std::tuple<int, int, int>{5, 2, 6}) {
700-
ok = false;
701-
}
698+
ok = backend::gnuplot::gnuplot_supports_keyentry();
702699
}
703700
if (legend_ == nullptr || !legend_->visible() || !ok) {
704701
run_command("set key off");
@@ -917,9 +914,7 @@ namespace matplot {
917914
static bool msg_shown_once = false;
918915
// Gnuplot version needs to be 5.2.6+ for keyentry
919916
if (parent_->backend_->consumes_gnuplot_commands()) {
920-
std::tuple<int, int, int> v =
921-
backend::gnuplot::gnuplot_version();
922-
if (v < std::tuple<int, int, int>{5, 2, 6}) {
917+
if (backend::gnuplot::gnuplot_includes_legends()) {
923918
if (!msg_shown_once) {
924919
std::cerr
925920
<< "You need gnuplot 5.2.6+ to include legends"
@@ -2746,9 +2741,9 @@ namespace matplot {
27462741
}
27472742

27482743
std::vector<function_line_handle>
2749-
axes_type::fplot(std::vector<function_line::function_type> equations,
2750-
std::array<double, 2> x_range,
2751-
std::vector<std::string> line_specs) {
2744+
axes_type::fplot(const std::vector<function_line::function_type> &equations,
2745+
const std::array<double, 2> &x_range,
2746+
const std::vector<std::string> &line_specs) {
27522747
axes_silencer temp_silencer_{this};
27532748
std::vector<function_line_handle> res;
27542749
auto it_line_specs = line_specs.begin();
@@ -2765,9 +2760,9 @@ namespace matplot {
27652760
}
27662761

27672762
std::vector<function_line_handle>
2768-
axes_type::fplot(std::vector<function_line::function_type> equations,
2769-
std::vector<double> x_range,
2770-
std::vector<std::string> line_specs) {
2763+
axes_type::fplot(const std::vector<function_line::function_type>& equations,
2764+
const std::vector<double>& x_range,
2765+
const std::vector<std::string>& line_specs) {
27712766
return this->fplot(equations, to_array<2>(x_range), line_specs);
27722767
}
27732768

source/matplot/core/axes_type.h

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -979,15 +979,15 @@ namespace matplot {
979979

980980
/// Lambda function line plot - list of functions
981981
std::vector<function_line_handle>
982-
fplot(std::vector<function_line::function_type> equations,
983-
std::array<double, 2> x_range = {-5, 5},
984-
std::vector<std::string> line_specs = {});
982+
fplot(const std::vector<function_line::function_type> &equations,
983+
const std::array<double, 2> &x_range = {-5, 5},
984+
const std::vector<std::string> &line_specs = {});
985985

986986
/// Lambda function line plot - list of functions and line specs
987987
std::vector<function_line_handle>
988-
fplot(std::vector<function_line::function_type> equations,
989-
std::vector<double> x_range,
990-
std::vector<std::string> line_specs = {});
988+
fplot(const std::vector<function_line::function_type> &equations,
989+
const std::vector<double> &x_range,
990+
const std::vector<std::string> &line_specs = {});
991991

992992
using implicit_function_type = std::function<double(double, double)>;
993993

source/matplot/core/figure_type.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -633,9 +633,7 @@ namespace matplot {
633633
void figure_type::run_window_color_command() {
634634
// In gnuplot 5.5 we have the wall function to set the axes color
635635
// with a rectangle workaround, which does not work well for 3d.
636-
static const auto v = backend::gnuplot::gnuplot_version();
637-
const bool has_wall_option =
638-
std::get<0>(v) > 5 || (std::get<0>(v) == 5 && std::get<1>(v) >= 5);
636+
const bool has_wall_option = backend::gnuplot::gnuplot_has_wall_option();
639637
// So we only plot the default background if it's not 3d or version is
640638
// higher than 5.5. Otherwise, gnuplot won't be able to set the axes
641639
// colors.
@@ -895,4 +893,4 @@ namespace matplot {
895893

896894
bool figure_type::should_close() { return backend_->should_close(); }
897895

898-
} // namespace matplot
896+
} // namespace matplot

0 commit comments

Comments
 (0)