Skip to content

Commit 2f2e29f

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 2f2e29f

Some content is hidden

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

52 files changed

+602
-485
lines changed

.github/workflows/build.yml

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -44,15 +44,8 @@ jobs:
4444
-DNANOGUI_BUILD_SHARED=${{ env.BUILD_SHARED }} `
4545
"-DPython_EXECUTABLE=$(python -c 'import sys; print(sys.executable)')"
4646
47-
- name: Configure (Linux)
48-
if: runner.os == 'Linux'
49-
run: |
50-
cmake -S . -B build \
51-
-DNANOGUI_BUILD_SHARED=${{ env.BUILD_SHARED }} \
52-
"-DPython_EXECUTABLE=$(python3 -c 'import sys; print(sys.executable)')"
53-
54-
- name: Configure (macOS)
55-
if: runner.os == 'macOS'
47+
- name: Configure (Linux/macOS)
48+
if: runner.os == 'Linux' || runner.os == 'macOS'
5649
run: |
5750
cmake -S . -B build \
5851
-DNANOGUI_BUILD_SHARED=${{ env.BUILD_SHARED }} \

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

README.rst

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ available on PyPI_.
2222
**Note**: This repository contains an improved port of the original NanoGUI_.
2323
The most visible change to developers is that it no longer relies on Eigen or
2424
Enoki and ships with its own (absolutely minimal) vector library. Additionally,
25-
the the repository here incorporates the following changes:
25+
the repository here incorporates the following changes:
2626

2727
1. A different set of naming conventions is used for function and variable
2828
names that feels more natural in a mixed C++ & Python environment.
@@ -37,20 +37,23 @@ the the repository here incorporates the following changes:
3737
NanoGUI includes generic wrappers around shaders and textures that work for
3838
all of these frameworks.
3939

40-
3. The event loop is much more conservative by default and only issues redraw
40+
3. Cross-platform (macOS, Wayland, Windows) support for HDR displays and
41+
extended color spaces. See `example_hdr`
42+
43+
4. The event loop is much more conservative by default and only issues redraw
4144
calls when explicitly requested by an event callback.
4245

43-
4. Python integration: the library comes with a ``pip``-compatible ``setup.py``
46+
5. Python integration: the library comes with a ``pip``-compatible ``setup.py``
4447
installation script.
4548

46-
5. WebAssembly code generation works out of the box (requires Emscripten),
49+
6. WebAssembly code generation works out of the box (requires Emscripten),
4750
enabling powerful UI development for the web. See Tekari_ for an example of
4851
such an application.
4952

50-
6. Significantly revamped tab widget (supports right-click context menus,
53+
7. Significantly revamped tab widget (supports right-click context menus,
5154
draggable, and closeable tabs) and image view widget.
5255

53-
7. The Entypo_ icon font has been replaced by FontAwesome_ (v5.10.1).
56+
8. The Entypo_ icon font has been replaced by FontAwesome_ (v5.10.1).
5457

5558
.. _NanoVG: https://github.com/memononen/NanoVG
5659
.. _nanobind: https://github.com/wjakob/nanobind

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; }

0 commit comments

Comments
 (0)