Skip to content

Commit 6497f59

Browse files
committed
Refactoring pass
This commit bundles a few general C++ code quality improvements. - Replaced almost all use of ``const std::string &`` as string passing convention by the more general ``std::string_view``. - Complete removal of ``unordered_map`` from header files. Maps are replaced either by vectors (when few elements are expected) or ``tsl::map`` using the pIMPL design pattern to keep those internals out of header files. This should not impact outside users of the API. - Bumped macOS version requirement to 10.11 (Big Sur, EOL Sep 2023) - Common header files are included once in ``common.h``
1 parent af76188 commit 6497f59

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

50 files changed

+591
-470
lines changed

CMakeLists.txt

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
cmake_minimum_required (VERSION 3.13..3.18)
2-
set(CMAKE_OSX_DEPLOYMENT_TARGET 10.14)
2+
3+
set(CMAKE_OSX_DEPLOYMENT_TARGET 11.0) # macOS Big Sur (released Nov 2020, EOL Sep 2023)
34

45
# Extract project version from source
56
file(STRINGS "${CMAKE_CURRENT_SOURCE_DIR}/include/nanogui/common.h"
@@ -453,6 +454,11 @@ if (NOT NANOGUI_SKIP_STB_IMAGE_IMPLEMENTATION)
453454
PRIVATE -DNVG_STB_IMAGE_IMPLEMENTATION)
454455
endif()
455456

457+
target_include_directories(nanogui
458+
PRIVATE
459+
${CMAKE_CURRENT_SOURCE_DIR}/ext/nanobind/ext/robin_map/include
460+
)
461+
456462
target_include_directories(nanogui
457463
PUBLIC
458464
$<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>
@@ -510,6 +516,11 @@ else()
510516
target_link_libraries(nanogui PUBLIC ${NANOGUI_LIBS})
511517
endif()
512518

519+
if (MSVC)
520+
# warning C4324: "...": structure was padded due to alignment specifier
521+
target_compile_options(nanogui PRIVATE /wd4324)
522+
endif()
523+
513524
target_link_libraries(nanogui PRIVATE nfd)
514525

515526
# Auto-detect GLES include directory on Raspberry PI

include/nanogui/button.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,13 +53,13 @@ class NANOGUI_EXPORT Button : public Widget {
5353
* \param icon
5454
* The icon to display with this Button. See \ref nanogui::Button::mIcon.
5555
*/
56-
Button(Widget *parent, const std::string &caption = "Untitled", int icon = 0);
56+
Button(Widget *parent, std::string_view caption = "Untitled", int icon = 0);
5757

5858
/// Returns the caption of this Button.
59-
const std::string &caption() const { return m_caption; }
59+
std::string_view caption() const { return m_caption; }
6060

6161
/// Sets the caption of this Button.
62-
void set_caption(const std::string &caption) { m_caption = caption; }
62+
void set_caption(std::string_view caption) { m_caption = caption; }
6363

6464
/// Returns the background color of this Button.
6565
const Color &background_color() const { return m_background_color; }

include/nanogui/checkbox.h

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -45,22 +45,22 @@ class NANOGUI_EXPORT CheckBox : public Widget {
4545
* \ref nanogui::CheckBox::mPushed for the difference between "pushed"
4646
* and "checked".
4747
*/
48-
CheckBox(Widget *parent, const std::string &caption = "Untitled",
48+
CheckBox(Widget *parent, std::string_view caption = "Untitled",
4949
const std::function<void(bool)> &callback = std::function<void(bool)>());
5050

51-
/// The caption of this CheckBox.
52-
const std::string &caption() const { return m_caption; }
51+
/// The caption of this check box.
52+
std::string_view caption() const { return m_caption; }
5353

54-
/// Sets the caption of this CheckBox.
55-
void set_caption(const std::string &caption) { m_caption = caption; }
54+
/// Sets the caption of this check box
55+
void set_caption(std::string_view caption) { m_caption = caption; }
5656

57-
/// Whether or not this CheckBox is currently checked.
57+
/// Return whether or not this widget is currently checked.
5858
const bool &checked() const { return m_checked; }
5959

60-
/// Sets whether or not this CheckBox is currently checked.
60+
/// Sets whether or not this widget is currently checked
6161
void set_checked(const bool &checked) { m_checked = checked; }
6262

63-
/// Whether or not this CheckBox is currently pushed. See \ref nanogui::CheckBox::m_pushed.
63+
/// Whether or not this widget is currently pushed. See \ref nanogui::CheckBox::m_pushed.
6464
const bool &pushed() const { return m_pushed; }
6565
void set_pushed(const bool &pushed) { m_pushed = pushed; }
6666

@@ -81,6 +81,7 @@ class NANOGUI_EXPORT CheckBox : public Widget {
8181
protected:
8282
/// The caption text of this CheckBox.
8383
std::string m_caption;
84+
8485
/**
8586
* Internal tracking variable to distinguish between mouse click and release.
8687
* \ref nanogui::CheckBox::m_callback is only called upon release. See

include/nanogui/colorpicker.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,16 +67,16 @@ class NANOGUI_EXPORT ColorPicker : public PopupButton {
6767
void set_color(const Color& color);
6868

6969
/// The current caption of the \ref nanogui::ColorPicker::m_pick_button.
70-
const std::string &pick_button_caption() { return m_pick_button->caption(); }
70+
std::string_view pick_button_caption() { return m_pick_button->caption(); }
7171

7272
/// Sets the current caption of the \ref nanogui::ColorPicker::m_pick_button.
73-
void set_pick_button_caption(const std::string &caption) { m_pick_button->set_caption(caption); }
73+
void set_pick_button_caption(std::string_view caption) { m_pick_button->set_caption(caption); }
7474

7575
/// The current caption of the \ref nanogui::ColorPicker::m_reset_button.
76-
const std::string &reset_button_caption() { return m_reset_button->caption(); }
76+
std::string_view reset_button_caption() { return m_reset_button->caption(); }
7777

7878
/// Sets the current caption of the \ref nanogui::ColorPicker::m_reset_button.
79-
void set_reset_button_caption(const std::string &caption) { m_reset_button->set_caption(caption); }
79+
void set_reset_button_caption(std::string_view caption) { m_reset_button->set_caption(caption); }
8080
protected:
8181
/// The "fast" callback executed when the ColorWheel has changed.
8282
std::function<void(const Color &)> m_callback;

include/nanogui/common.h

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,15 @@
1515

1616
#pragma once
1717

18-
#include <stdint.h>
19-
20-
#include <algorithm>
18+
#include <cstdint>
2119
#include <array>
20+
#include <utility>
2221
#include <functional>
2322
#include <string>
23+
#include <string_view>
2424
#include <vector>
25+
#include <stdexcept>
26+
#include <cassert>
2527

2628
#define NANOGUI_VERSION_MAJOR 0
2729
#define NANOGUI_VERSION_MINOR 2
@@ -349,7 +351,7 @@ extern NANOGUI_EXPORT std::vector<std::pair<int, std::string>>
349351
/// Convenience function for instanting a PNG icon from the application's data segment (via bin2c)
350352
#define nvgImageIcon(ctx, name) nanogui::__nanogui_get_image(ctx, #name, name##_png, name##_png_size)
351353
/// Helper function used by nvg_image_icon
352-
extern NANOGUI_EXPORT int __nanogui_get_image(NVGcontext *ctx, const std::string &name,
354+
extern NANOGUI_EXPORT int __nanogui_get_image(NVGcontext *ctx, std::string_view name,
353355
uint8_t *data, uint32_t size);
354356

355357
static const size_t DITHER_MATRIX_SIZE = 8;

include/nanogui/formhelper.h

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ class FormHelper {
128128

129129
/// Add a new top-level window
130130
Window *add_window(const Vector2i &pos,
131-
const std::string &title = "Untitled") {
131+
std::string_view title = "Untitled") {
132132
assert(m_screen);
133133
m_window = new Window(m_screen, title);
134134
m_layout = new AdvancedGridLayout({10, 0, 10, 0}, {});
@@ -141,7 +141,7 @@ class FormHelper {
141141
}
142142

143143
/// Add a new group that may contain several sub-widgets
144-
Label *add_group(const std::string &caption) {
144+
Label *add_group(std::string_view caption) {
145145
Label* label = new Label(m_window, caption, m_group_font_name, m_group_font_size);
146146
if (m_layout->row_count() > 0)
147147
m_layout->append_row(m_pre_group_spacing); /* Spacing */
@@ -153,7 +153,7 @@ class FormHelper {
153153

154154
/// Add a new data widget controlled using custom getter/setter functions
155155
template <typename Type> detail::FormWidget<Type> *
156-
add_variable(const std::string &label, const std::function<void(const Type &)> &setter,
156+
add_variable(std::string_view label, const std::function<void(const Type &)> &setter,
157157
const std::function<Type()> &getter, bool editable = true) {
158158
Label *label_w = new Label(m_window, label, m_label_font_name, m_label_font_size);
159159
auto widget = new detail::FormWidget<Type>(m_window);
@@ -180,7 +180,7 @@ class FormHelper {
180180

181181
/// Add a new data widget that exposes a raw variable in memory
182182
template <typename Type> detail::FormWidget<Type> *
183-
add_variable(const std::string &label, Type &value, bool editable = true) {
183+
add_variable(std::string_view label, Type &value, bool editable = true) {
184184
return add_variable<Type>(label,
185185
[&](const Type & v) { value = v; },
186186
[&]() -> Type { return value; },
@@ -189,7 +189,7 @@ class FormHelper {
189189
}
190190

191191
/// Add a button with a custom callback
192-
Button *add_button(const std::string &label, const std::function<void()> &cb) {
192+
Button *add_button(std::string_view label, const std::function<void()> &cb) {
193193
Button *button = new Button(m_window, label);
194194
button->set_callback(cb);
195195
button->set_fixed_height(25);
@@ -201,7 +201,7 @@ class FormHelper {
201201
}
202202

203203
/// Add an arbitrary (optionally labeled) widget to the layout
204-
void add_widget(const std::string &label, Widget *widget) {
204+
void add_widget(std::string_view label, Widget *widget) {
205205
m_layout->append_row(0);
206206
if (label == "") {
207207
m_layout->set_anchor(widget, AdvancedGridLayout::Anchor(1, m_layout->row_count()-1, 3, 1));
@@ -237,16 +237,16 @@ class FormHelper {
237237
Vector2i fixed_size() { return m_fixed_size; }
238238

239239
/// The font name being used for group headers.
240-
const std::string &group_font_name() const { return m_group_font_name; }
240+
std::string_view group_font_name() const { return m_group_font_name; }
241241

242242
/// Sets the font name to be used for group headers.
243-
void set_group_font_name(const std::string &name) { m_group_font_name = name; }
243+
void set_group_font_name(std::string_view name) { m_group_font_name = name; }
244244

245245
/// The font name being used for labels.
246-
const std::string &label_font_name() const { return m_label_font_name; }
246+
std::string_view label_font_name() const { return m_label_font_name; }
247247

248248
/// Sets the font name being used for labels.
249-
void set_label_font_name(const std::string &name) { m_label_font_name = name; }
249+
void set_label_font_name(std::string_view name) { m_label_font_name = name; }
250250

251251
/// The size of the font being used for group headers.
252252
int group_font_size() const { return m_group_font_size; }

include/nanogui/graph.h

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,16 +23,16 @@ NAMESPACE_BEGIN(nanogui)
2323
*/
2424
class NANOGUI_EXPORT Graph : public Widget {
2525
public:
26-
Graph(Widget *parent, const std::string &caption = "Untitled");
26+
Graph(Widget *parent, std::string_view caption = "Untitled");
2727

28-
const std::string &caption() const { return m_caption; }
29-
void set_caption(const std::string &caption) { m_caption = caption; }
28+
std::string_view caption() const { return m_caption; }
29+
void set_caption(std::string_view caption) { m_caption = caption; }
3030

31-
const std::string &header() const { return m_header; }
32-
void set_header(const std::string &header) { m_header = header; }
31+
std::string_view header() const { return m_header; }
32+
void set_header(std::string_view header) { m_header = header; }
3333

34-
const std::string &footer() const { return m_footer; }
35-
void set_footer(const std::string &footer) { m_footer = footer; }
34+
std::string_view footer() const { return m_footer; }
35+
void set_footer(std::string_view footer) { m_footer = footer; }
3636

3737
const Color &background_color() const { return m_background_color; }
3838
void set_background_color(const Color &background_color) { m_background_color = background_color; }

include/nanogui/label.h

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,18 +26,18 @@ NAMESPACE_BEGIN(nanogui)
2626
*/
2727
class NANOGUI_EXPORT Label : public Widget {
2828
public:
29-
Label(Widget *parent, const std::string &caption,
30-
const std::string &font = "sans", int font_size = -1);
29+
Label(Widget *parent, std::string_view caption,
30+
std::string_view font = "sans", int font_size = -1);
3131

3232
/// Get the label's text caption
33-
const std::string &caption() const { return m_caption; }
33+
std::string_view caption() const { return m_caption; }
3434
/// Set the label's text caption
35-
void set_caption(const std::string &caption) { m_caption = caption; }
35+
void set_caption(std::string_view caption) { m_caption = caption; }
3636

3737
/// Set the currently active font (2 are available by default: 'sans' and 'sans-bold')
38-
void set_font(const std::string &font) { m_font = font; }
38+
void set_font(std::string_view font) { m_font = font; }
3939
/// Get the currently active font
40-
const std::string &font() const { return m_font; }
40+
std::string_view font() const { return m_font; }
4141

4242
/// Get the label color
4343
Color color() const { return m_color; }

include/nanogui/layout.h

Lines changed: 16 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,6 @@
1717

1818
#include <nanogui/object.h>
1919
#include <nanogui/vector.h>
20-
#include <unordered_map>
21-
22-
#include <stdexcept>
23-
#include <vector>
2420

2521
NAMESPACE_BEGIN(nanogui)
2622

@@ -419,39 +415,37 @@ class NANOGUI_EXPORT AdvancedGridLayout : public Layout {
419415
/// Creates an AdvancedGridLayout with specified columns, rows, and margin.
420416
AdvancedGridLayout(const std::vector<int> &cols = {}, const std::vector<int> &rows = {}, int margin = 0);
421417

418+
/// Destructor
419+
virtual ~AdvancedGridLayout();
420+
422421
/// The margin of this AdvancedGridLayout.
423-
int margin() const { return m_margin; }
422+
int margin() const;
424423
/// Sets the margin of this AdvancedGridLayout.
425-
void set_margin(int margin) { m_margin = margin; }
424+
void set_margin(int margin);
426425

427426
/// Return the number of cols
428-
int col_count() const { return (int) m_cols.size(); }
427+
int col_count() const;
429428

430429
/// Return the number of rows
431-
int row_count() const { return (int) m_rows.size(); }
430+
int row_count() const;
432431

433432
/// Append a row of the given size (and stretch factor)
434-
void append_row(int size, float stretch = 0.f) { m_rows.push_back(size); m_row_stretch.push_back(stretch); }
433+
void append_row(int size, float stretch = 0.f);
435434

436435
/// Append a column of the given size (and stretch factor)
437-
void append_col(int size, float stretch = 0.f) { m_cols.push_back(size); m_col_stretch.push_back(stretch); }
436+
void append_col(int size, float stretch = 0.f);
438437

439438
/// Set the stretch factor of a given row
440-
void set_row_stretch(int index, float stretch) { m_row_stretch.at(index) = stretch; }
439+
void set_row_stretch(int index, float stretch);
441440

442441
/// Set the stretch factor of a given column
443-
void set_col_stretch(int index, float stretch) { m_col_stretch.at(index) = stretch; }
442+
void set_col_stretch(int index, float stretch);
444443

445444
/// Specify the anchor data structure for a given widget
446-
void set_anchor(const Widget *widget, const Anchor &anchor) { m_anchor[widget] = anchor; }
445+
void set_anchor(const Widget *widget, const Anchor &anchor);
447446

448447
/// Retrieve the anchor data structure for a given widget
449-
Anchor anchor(const Widget *widget) const {
450-
auto it = m_anchor.find(widget);
451-
if (it == m_anchor.end())
452-
throw std::runtime_error("Widget was not registered with the grid layout!");
453-
return it->second;
454-
}
448+
Anchor anchor(const Widget *widget) const;
455449

456450
/* Implementation of the layout interface */
457451

@@ -466,24 +460,9 @@ class NANOGUI_EXPORT AdvancedGridLayout : public Layout {
466460
void compute_layout(NVGcontext *ctx, const Widget *widget,
467461
std::vector<int> *grid) const;
468462

469-
protected:
470-
/// The columns of this AdvancedGridLayout.
471-
std::vector<int> m_cols;
472-
473-
/// The rows of this AdvancedGridLayout.
474-
std::vector<int> m_rows;
475-
476-
/// The stretch for each column of this AdvancedGridLayout.
477-
std::vector<float> m_col_stretch;
478-
479-
/// The stretch for each row of this AdvancedGridLayout.
480-
std::vector<float> m_row_stretch;
481-
482-
/// The mapping of widgets to their specified anchor points.
483-
std::unordered_map<const Widget *, Anchor> m_anchor;
484-
485-
/// The margin around this AdvancedGridLayout.
486-
int m_margin;
463+
private:
464+
struct Impl;
465+
Impl *p;
487466
};
488467

489468
NAMESPACE_END(nanogui)

include/nanogui/messagedialog.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,10 @@ class NANOGUI_EXPORT MessageDialog : public Window {
3030
Warning
3131
};
3232

33-
MessageDialog(Widget *parent, Type type, const std::string &title = "Untitled",
34-
const std::string &message = "Message",
35-
const std::string &button_text = "OK",
36-
const std::string &alt_button_text = "Cancel", bool alt_button = false);
33+
MessageDialog(Widget *parent, Type type, std::string_view title = "Untitled",
34+
std::string_view message = "Message",
35+
std::string_view button_text = "OK",
36+
std::string_view alt_button_text = "Cancel", bool alt_button = false);
3737

3838
Label *message_label() { return m_message_label; }
3939
const Label *message_label() const { return m_message_label; }

0 commit comments

Comments
 (0)