From 990164b12b4efe1f36f3396e40f08ffde3f54406 Mon Sep 17 00:00:00 2001 From: Adam Kewley Date: Fri, 22 Nov 2024 11:15:07 +0100 Subject: [PATCH] Finish applying resharper suggestions to oscar/ --- .../Documents/MeshImporter/OpenSimBridge.cpp | 1 + .../Documents/Model/UndoableModelActions.cpp | 1 + .../Platform/OpenSimCreatorApp.cpp | 2 +- .../UI/MeshImporter/MeshImporterTab.cpp | 2 +- .../UI/Shared/CoordinateEditorPanel.cpp | 2 +- src/OpenSimCreator/UI/Shared/MainMenu.cpp | 2 +- src/oscar/UI/Popups/StandardPopup.cpp | 2 +- src/oscar/UI/Widgets/GuiRuler.cpp | 20 ++--- src/oscar/UI/oscimgui.cpp | 12 +-- src/oscar/UI/oscimgui.h | 22 +++--- src/oscar/UI/ui_context.cpp | 8 +- src/oscar/UI/ui_graphics_backend.cpp | 76 +++++++++---------- src/oscar/UI/ui_graphics_backend.h | 2 - src/oscar/Utils.h | 3 +- src/oscar/Utils/Algorithms.h | 5 +- src/oscar/Utils/CStringView.h | 9 ++- src/oscar/Utils/CircularBuffer.h | 10 +-- src/oscar/Utils/ClonePtr.h | 8 +- src/oscar/Utils/Concepts.h | 1 - src/oscar/Utils/CopyOnUpdPtr.h | 4 +- src/oscar/Utils/DefaultConstructOnCopy.h | 24 +++--- src/oscar/Utils/EnumHelpers.h | 2 +- src/oscar/Utils/FileChangePoller.cpp | 8 +- src/oscar/Utils/FilesystemHelpers.cpp | 11 +-- src/oscar/Utils/FilesystemHelpers.h | 7 +- src/oscar/Utils/LifetimedPtr.h | 2 +- src/oscar/Utils/NullStreambuf.h | 4 +- src/oscar/Utils/Perf.cpp | 2 - src/oscar/Utils/Perf.h | 1 - src/oscar/Utils/PerfMeasurement.h | 2 - src/oscar/Utils/PerfMeasurementMetadata.h | 1 - src/oscar/Utils/ScopeGuard.h | 8 +- src/oscar/Utils/SharedPreHashedString.h | 10 +-- src/oscar/Utils/Spsc.h | 16 ++-- src/oscar/Utils/StringHelpers.cpp | 28 +++---- src/oscar/Utils/StringHelpers.h | 14 ++-- src/oscar/Utils/StringName.cpp | 10 +-- src/oscar/Utils/StringName.h | 5 +- src/oscar/Utils/SynchronizedValue.h | 7 +- src/oscar/Utils/TemporaryFile.cpp | 17 ++--- src/oscar/Utils/TemporaryFile.h | 3 +- src/oscar/Utils/TemporaryFileParameters.h | 5 +- src/oscar/Utils/TransparentStringHasher.h | 12 +-- src/oscar/Utils/UID.h | 2 +- src/oscar/Utils/UndoRedo.cpp | 4 +- src/oscar/Utils/UndoRedo.h | 8 +- src/oscar/Variant/Variant.cpp | 6 +- src/oscar/Variant/Variant.h | 4 +- src/oscar/Variant/VariantType.cpp | 1 - 49 files changed, 193 insertions(+), 223 deletions(-) diff --git a/src/OpenSimCreator/Documents/MeshImporter/OpenSimBridge.cpp b/src/OpenSimCreator/Documents/MeshImporter/OpenSimBridge.cpp index 77a652f0d..863ba04ac 100644 --- a/src/OpenSimCreator/Documents/MeshImporter/OpenSimBridge.cpp +++ b/src/OpenSimCreator/Documents/MeshImporter/OpenSimBridge.cpp @@ -41,6 +41,7 @@ #include #include #include +#include #include #include diff --git a/src/OpenSimCreator/Documents/Model/UndoableModelActions.cpp b/src/OpenSimCreator/Documents/Model/UndoableModelActions.cpp index a02848669..30ecd378d 100644 --- a/src/OpenSimCreator/Documents/Model/UndoableModelActions.cpp +++ b/src/OpenSimCreator/Documents/Model/UndoableModelActions.cpp @@ -56,6 +56,7 @@ #include #include #include +#include #include #include diff --git a/src/OpenSimCreator/Platform/OpenSimCreatorApp.cpp b/src/OpenSimCreator/Platform/OpenSimCreatorApp.cpp index 961b31429..ee4344082 100644 --- a/src/OpenSimCreator/Platform/OpenSimCreatorApp.cpp +++ b/src/OpenSimCreator/Platform/OpenSimCreatorApp.cpp @@ -13,8 +13,8 @@ #include #include #include -#include #include +#include #include #include #include diff --git a/src/OpenSimCreator/UI/MeshImporter/MeshImporterTab.cpp b/src/OpenSimCreator/UI/MeshImporter/MeshImporterTab.cpp index 09cceb245..5c6f93f7a 100644 --- a/src/OpenSimCreator/UI/MeshImporter/MeshImporterTab.cpp +++ b/src/OpenSimCreator/UI/MeshImporter/MeshImporterTab.cpp @@ -58,8 +58,8 @@ #include #include #include +#include #include -#include #include #include diff --git a/src/OpenSimCreator/UI/Shared/CoordinateEditorPanel.cpp b/src/OpenSimCreator/UI/Shared/CoordinateEditorPanel.cpp index 734ab1328..c7682d949 100644 --- a/src/OpenSimCreator/UI/Shared/CoordinateEditorPanel.cpp +++ b/src/OpenSimCreator/UI/Shared/CoordinateEditorPanel.cpp @@ -15,8 +15,8 @@ #include #include #include +#include #include -#include #include #include diff --git a/src/OpenSimCreator/UI/Shared/MainMenu.cpp b/src/OpenSimCreator/UI/Shared/MainMenu.cpp index 404fd1dc6..8d126bc19 100644 --- a/src/OpenSimCreator/UI/Shared/MainMenu.cpp +++ b/src/OpenSimCreator/UI/Shared/MainMenu.cpp @@ -47,7 +47,7 @@ osc::MainMenuFileTab::MainMenuFileTab(Widget& parent) : ) } { - rgs::sort(exampleOsimFiles, is_filename_lexographically_greater_than); + rgs::sort(exampleOsimFiles, is_filename_lexicographically_greater_than); } void osc::MainMenuFileTab::onDraw(IModelStatePair* maybeModel) diff --git a/src/oscar/UI/Popups/StandardPopup.cpp b/src/oscar/UI/Popups/StandardPopup.cpp index 775488372..18bd66857 100644 --- a/src/oscar/UI/Popups/StandardPopup.cpp +++ b/src/oscar/UI/Popups/StandardPopup.cpp @@ -106,7 +106,7 @@ bool osc::StandardPopup::impl_begin_popup() // // else, do nothing - the popup's position will be determined // by other means (unlike a modal, which usually takes control - // of the screen and, therefore, should proabably be centered + // of the screen and, therefore, should probably be centered // in it) if (maybe_position_) { ui::set_next_panel_pos( diff --git a/src/oscar/UI/Widgets/GuiRuler.cpp b/src/oscar/UI/Widgets/GuiRuler.cpp index fde86b003..e1969b0f8 100644 --- a/src/oscar/UI/Widgets/GuiRuler.cpp +++ b/src/oscar/UI/Widgets/GuiRuler.cpp @@ -45,8 +45,8 @@ void osc::GuiRuler::on_draw( const float circle_radius = 5.0f; const float line_thickness = 3.0f; - ui::DrawListView drawlist = ui::get_panel_draw_list(); - const auto draw_tooltip_with_bg = [&drawlist, &text_background_color, &text_color](const Vec2& pos, CStringView tooltip_text) + ui::DrawListView draw_list = ui::get_panel_draw_list(); + const auto draw_tooltip_with_bg = [&draw_list, &text_background_color, &text_color](const Vec2& pos, CStringView tooltip_text) { const Vec2 text_size = ui::calc_text_size(tooltip_text); const float background_padding = 5.0f; @@ -56,19 +56,19 @@ void osc::GuiRuler::on_draw( {pos - background_padding}, {pos + text_size + background_padding}, }; - drawlist.add_rect_filled(background_rect, text_background_color, edge_rounding); - drawlist.add_text(pos, text_color, tooltip_text); + draw_list.add_rect_filled(background_rect, text_background_color, edge_rounding); + draw_list.add_text(pos, text_color, tooltip_text); }; if (state_ == State::WaitingForFirstPoint) { if (not maybe_mouseover) { // not mousing over anything - drawlist.add_circle_filled(Circle{mouse_pos, circle_radius}, circle_moused_over_nothing_color); + draw_list.add_circle_filled(Circle{mouse_pos, circle_radius}, circle_moused_over_nothing_color); return; } else { // mousing over something - drawlist.add_circle_filled(Circle{mouse_pos, circle_radius}, circle_color); + draw_list.add_circle_filled(Circle{mouse_pos, circle_radius}, circle_color); if (ui::is_mouse_released(ui::MouseButton::Left)) { state_ = State::WaitingForSecondPoint; @@ -88,9 +88,9 @@ void osc::GuiRuler::on_draw( const Vec2 line_midpoint = (start_screenpos + end_screenpos) / 2.0f; const float line_world_length = length(maybe_mouseover->worldspace_location - start_world_pos_); - drawlist.add_circle_filled({start_screenpos, circle_radius}, circle_color); - drawlist.add_line(start_screenpos, end_screenpos, line_color, line_thickness); - drawlist.add_circle_filled({end_screenpos, circle_radius}, circle_color); + draw_list.add_circle_filled({start_screenpos, circle_radius}, circle_color); + draw_list.add_line(start_screenpos, end_screenpos, line_color, line_thickness); + draw_list.add_circle_filled({end_screenpos, circle_radius}, circle_color); // label the line's length { @@ -104,7 +104,7 @@ void osc::GuiRuler::on_draw( } } else { - drawlist.add_circle_filled({start_screenpos, circle_radius}, circle_color); + draw_list.add_circle_filled({start_screenpos, circle_radius}, circle_color); } } } diff --git a/src/oscar/UI/oscimgui.cpp b/src/oscar/UI/oscimgui.cpp index 6a132d7bb..e4fd4be4e 100644 --- a/src/oscar/UI/oscimgui.cpp +++ b/src/oscar/UI/oscimgui.cpp @@ -947,9 +947,9 @@ float osc::ui::get_cursor_pos_x() return ImGui::GetCursorPosX(); } -void osc::ui::set_cursor_pos(Vec2 p) +void osc::ui::set_cursor_pos(Vec2 pos) { - ImGui::SetCursorPos(p); + ImGui::SetCursorPos(pos); } void osc::ui::set_cursor_pos_x(float local_x) @@ -1440,7 +1440,7 @@ void osc::ui::DrawList::render_to(RenderTexture& target) c.set_view_matrix_override(identity()); { - // project screenspace overlays into NDC + // project screen-space overlays into NDC float L = 0.0f; float R = static_cast(target.dimensions().x); float T = 0.0f; @@ -2348,7 +2348,7 @@ bool osc::ui::draw_float_circular_slider( // figure out whether the user is (temporarily) editing the slider as an input text box bool temporary_text_input_active = temporary_text_input_allowed and ImGui::TempInputIsActive(id); if (not temporary_text_input_active) { - // tabbing or double clicking the slider temporarily transforms it into an input box + // tabbing or double-clicking the slider temporarily transforms it into an input box const bool clicked = is_hovered and ui::is_mouse_clicked(MouseButton::Left, ui::ID{id}); const bool double_clicked = (is_hovered and g.IO.MouseClickedCount[0] == 2 and ImGui::TestKeyOwner(ImGuiKey_MouseLeft, id)); const bool make_active = (clicked or double_clicked or g.NavActivateId == id); @@ -2686,7 +2686,7 @@ std::optional osc::ui::Gizmo::draw_to( const Mat4& view_matrix, const Mat4& projection_matrix, const Rect& screenspace_rect, - ImDrawList* drawlist) + ImDrawList* draw_list) { if (operation_ == GizmoOperation::None) { return std::nullopt; // disabled @@ -2706,7 +2706,7 @@ std::optional osc::ui::Gizmo::draw_to( dimensions_of(screenspace_rect).x, dimensions_of(screenspace_rect).y ); - ImGuizmo::SetDrawlist(drawlist); + ImGuizmo::SetDrawlist(draw_list); ImGuizmo::AllowAxisFlip(false); // user's didn't like this feature in UX sessions // use rotation from the parent, translation from station diff --git a/src/oscar/UI/oscimgui.h b/src/oscar/UI/oscimgui.h index bce0265e8..83baed2d9 100644 --- a/src/oscar/UI/oscimgui.h +++ b/src/oscar/UI/oscimgui.h @@ -9,7 +9,6 @@ #include #include #include -#include #include #include #include @@ -17,7 +16,6 @@ #include #include #include -#include #include #include @@ -573,14 +571,14 @@ namespace osc::ui // applies "dark" theme to current UI context void apply_dark_theme(); - // updates a polar comera's rotation, position, etc. from UI keyboard input state + // updates a polar camera's rotation, position, etc. from UI keyboard input state bool update_polar_camera_from_keyboard_inputs( PolarPerspectiveCamera&, const Rect& viewport_rect, std::optional maybe_scene_aabb ); - // updates a polar comera's rotation, position, etc. from UI input state (all) + // updates a polar camera's rotation, position, etc. from UI input state (all) bool update_polar_camera_from_all_inputs( PolarPerspectiveCamera&, const Rect& viewport_rect, @@ -592,7 +590,7 @@ namespace osc::ui EulerAngles& ); - // returns the UI content region available in screenspace as a `Rect` + // returns the UI content region available in screen-space as a `Rect` Rect content_region_avail_as_screen_rect(); // draws a texture within the 2D UI @@ -641,7 +639,7 @@ namespace osc::ui Vec2 dimensions ); - // returns the screenspace bounding rectangle of the last-drawn item + // returns the screen-space bounding rectangle of the last-drawn item Rect get_last_drawn_item_screen_rect(); // hittest the last-drawn item in the UI @@ -924,8 +922,8 @@ namespace osc::ui const Rect& screenspace_rect ); - // same as `draw`, but draws to the foreground drawlist, rather than the - // drawlist of the currently active panel + // same as `draw`, but draws to the foreground draw list, rather than the + // draw list of the currently active panel std::optional draw_to_foreground( Mat4& model_matrix, // edited in-place const Mat4& view_matrix, @@ -949,7 +947,7 @@ namespace osc::ui const Mat4& view_matrix, const Mat4& projection_matrix, const Rect& screenspace_rect, - ImDrawList* drawlist + ImDrawList* draw_list ); UID id_; @@ -1231,7 +1229,7 @@ namespace osc::ui DragToolFlags = DragToolFlags::Default ); - // draws a draggable vertical guide line at an x-value in the plot area + // draws a draggable vertical guideline at an x-value in the plot area bool drag_line_x( int id, double* x, @@ -1240,7 +1238,7 @@ namespace osc::ui DragToolFlags = DragToolFlags::Default ); - // draws a draggable horizontal guide line at a y-value in the plot area + // draws a draggable horizontal guideline at a y-value in the plot area bool drag_line_y( int id, double* y, @@ -1249,7 +1247,7 @@ namespace osc::ui DragToolFlags = DragToolFlags::Default ); - // draws a tag on the x axis at the specified x value + // draws a tag on the x-axis at the specified x value void tag_x(double x, const Color&, bool round = false); // returns `true` if the plot area in the current plot is hovered diff --git a/src/oscar/UI/ui_context.cpp b/src/oscar/UI/ui_context.cpp index d19397ebc..79f96f4b3 100644 --- a/src/oscar/UI/ui_context.cpp +++ b/src/oscar/UI/ui_context.cpp @@ -422,7 +422,7 @@ static void ImGui_ImplSDL2_UpdateMouseData() if (io.BackendFlags & ImGuiBackendFlags_HasMouseHoveredViewport) { ImGuiID mouse_viewport_id = 0; if (SDL_Window* sdl_mouse_window = SDL_GetWindowFromID(bd->MouseWindowID)) { - if (ImGuiViewport* mouse_viewport = ImGui::FindViewportByPlatformHandle(cpp20::bit_cast(sdl_mouse_window))) { + if (const ImGuiViewport* mouse_viewport = ImGui::FindViewportByPlatformHandle(cpp20::bit_cast(sdl_mouse_window))) { mouse_viewport_id = mouse_viewport->ID; } } @@ -500,7 +500,7 @@ static void ImGui_ImplOscar_NewFrame(App& app) } // Our io.AddMouseViewportEvent() calls will only be valid when not capturing. - // Technically speaking testing for 'bd->MouseButtonsDown == 0' would be more rygorous, but testing for payload reduces noise and potential side-effects. + // Technically speaking testing for 'bd->MouseButtonsDown == 0' would be more rigorous, but testing for payload reduces noise and potential side effects. if (bd.MouseCanReportHoveredViewport and ImGui::GetDragDropPayload() == nullptr) { io.BackendFlags |= ImGuiBackendFlags_HasMouseHoveredViewport; } @@ -568,10 +568,10 @@ void osc::ui::context::init(App& app) #ifdef EMSCRIPTEN io.IniFilename = nullptr; #else - float dpi_scale_factor = [&]() + const float dpi_scale_factor = [&]() { // if the user explicitly enabled high_dpi_mode... - if (auto v = app.get_config().find_value("experimental_feature_flags/high_dpi_mode"); v and *v) { + if (const auto v = app.get_config().find_value("experimental_feature_flags/high_dpi_mode"); v and *v) { return app.main_window_dpi() / 96.0f; } else { diff --git a/src/oscar/UI/ui_graphics_backend.cpp b/src/oscar/UI/ui_graphics_backend.cpp index a1d3ab8fc..73ed0c461 100644 --- a/src/oscar/UI/ui_graphics_backend.cpp +++ b/src/oscar/UI/ui_graphics_backend.cpp @@ -23,7 +23,6 @@ #include #include #include -#include #include #include #include @@ -39,17 +38,14 @@ #include #include #include -#include -#include #include namespace graphics = osc::graphics; -namespace cpp20 = osc::cpp20; using namespace osc; namespace { - constexpr CStringView c_VertexShader = R"( + constexpr CStringView c_ui_vertex_shader_src = R"( #version 330 core uniform mat4 uProjMat; @@ -69,7 +65,7 @@ namespace } )"; - constexpr CStringView c_FragmentShader = R"( + constexpr CStringView c_ui_fragment_shader_src = R"( #version 330 core uniform sampler2D uTexture; @@ -101,7 +97,7 @@ namespace ImGuiIO& io = ImGui::GetIO(); uint8_t* pixel_data = nullptr; - Vec2i dims{}; + Vec2i dims; io.Fonts->GetTexDataAsRGBA32(&pixel_data, &dims.x, &dims.y); io.Fonts->SetTexID(to_imgui_texture_id(texture_id)); const size_t num_bytes = static_cast(dims.x)*static_cast(dims.y)*static_cast(4); @@ -117,7 +113,7 @@ namespace return rv; } - // create a lookup table that maps sRGB color bytes to linear-space color bytes + // Returns a lookup table that maps sRGB color bytes to linear-space color bytes std::array create_srgb_to_linear_lut() { std::array rv{}; @@ -132,29 +128,29 @@ namespace const std::array& get_srgc_to_linear_lut_singleton() { - static const std::array s_LUT = create_srgb_to_linear_lut(); - return s_LUT; + static const std::array s_srgb_to_linear_lut = create_srgb_to_linear_lut(); + return s_srgb_to_linear_lut; } - void convert_draw_data_from_srgb_to_linear(ImDrawList& drawlist) + void convert_draw_data_from_srgb_to_linear(ImDrawList& draw_list) { const std::array& lut = get_srgc_to_linear_lut_singleton(); - for (ImDrawVert& v : drawlist.VtxBuffer) { - const auto rSRGB = static_cast((v.col >> IM_COL32_R_SHIFT) & 0xFF); - const auto gSRGB = static_cast((v.col >> IM_COL32_G_SHIFT) & 0xFF); - const auto bSRGB = static_cast((v.col >> IM_COL32_B_SHIFT) & 0xFF); - const auto aSRGB = static_cast((v.col >> IM_COL32_A_SHIFT) & 0xFF); + for (ImDrawVert& v : draw_list.VtxBuffer) { + const auto r_srgb = static_cast((v.col >> IM_COL32_R_SHIFT) & 0xFF); + const auto g_srgb = static_cast((v.col >> IM_COL32_G_SHIFT) & 0xFF); + const auto b_srgb = static_cast((v.col >> IM_COL32_B_SHIFT) & 0xFF); + const auto alpha = static_cast((v.col >> IM_COL32_A_SHIFT) & 0xFF); - const uint8_t rLinear = lut[rSRGB]; - const uint8_t gLinear = lut[gSRGB]; - const uint8_t bLinear = lut[bSRGB]; + const uint8_t r_linear = lut[r_srgb]; + const uint8_t g_linear = lut[g_srgb]; + const uint8_t b_linear = lut[b_srgb]; v.col = - static_cast(rLinear) << IM_COL32_R_SHIFT | - static_cast(gLinear) << IM_COL32_G_SHIFT | - static_cast(bLinear) << IM_COL32_B_SHIFT | - static_cast(aSRGB) << IM_COL32_A_SHIFT; + static_cast(r_linear) << IM_COL32_R_SHIFT | + static_cast(g_linear) << IM_COL32_G_SHIFT | + static_cast(b_linear) << IM_COL32_B_SHIFT | + static_cast(alpha) << IM_COL32_A_SHIFT; } } @@ -170,10 +166,10 @@ namespace UID font_texture_id; Texture2D font_texture = create_font_texture(font_texture_id); - Material ui_material{Shader{c_VertexShader, c_FragmentShader}}; + Material ui_material{Shader{c_ui_vertex_shader_src, c_ui_fragment_shader_src}}; Camera camera; Mesh mesh; - ankerl::unordered_dense::map> texures_allocated_this_frame = {{font_texture_id, font_texture}}; + ankerl::unordered_dense::map> textures_allocated_this_frame = {{font_texture_id, font_texture}}; }; // Backend data stored in io.BackendRendererUserData to allow support for multiple Dear ImGui contexts @@ -188,7 +184,7 @@ namespace } } - void setup_camera_view_matrix(ImDrawData& draw_data, Camera& camera) + void setup_camera_view_matrix(const ImDrawData& draw_data, Camera& camera) { // Our visible imgui space lies from draw_data->DisplayPos (top left) to draw_data->DisplayPos+data_data->DisplaySize (bottom right). DisplayPos is (0,0) for single viewport apps. const float L = draw_data.DisplayPos.x; @@ -231,8 +227,8 @@ namespace const Vec2 maxflip{clip_max.x, (draw_data.FramebufferScale.y * draw_data.DisplaySize.y) - clip_min.y}; bd.camera.set_scissor_rect(Rect{minflip, maxflip}); - // setup submesh description - const size_t idx = mesh.num_submesh_descriptors(); + // setup sub-mesh description + const size_t sub_mesh_index = mesh.num_submesh_descriptors(); mesh.push_submesh_descriptor(SubMeshDescriptor{ draw_command.IdxOffset, draw_command.ElemCount, @@ -240,11 +236,11 @@ namespace draw_command.VtxOffset }); - if (const auto* texture = lookup_or_nullptr(bd.texures_allocated_this_frame, to_uid(draw_command.GetTexID()))) { + if (const auto* texture = lookup_or_nullptr(bd.textures_allocated_this_frame, to_uid(draw_command.GetTexID()))) { std::visit(Overload{ [&bd](const auto& texture) { bd.ui_material.set("uTexture", texture); }, }, *texture); - graphics::draw(mesh, identity(), bd.ui_material, bd.camera, std::nullopt, idx); + graphics::draw(mesh, identity(), bd.ui_material, bd.camera, std::nullopt, sub_mesh_index); bd.camera.render_to_screen(); } } @@ -284,8 +280,8 @@ namespace mesh.set_indices({draw_list.IdxBuffer.Data, static_cast(draw_list.IdxBuffer.size())}, {MeshUpdateFlag::DontRecalculateBounds, MeshUpdateFlag::DontValidateIndices}); // iterate through command buffer - for (const ImDrawCmd& cmd : draw_list.CmdBuffer) { - render_draw_command(bd, draw_data, draw_list, mesh, cmd); + for (const ImDrawCmd& draw_command : draw_list.CmdBuffer) { + render_draw_command(bd, draw_data, draw_list, mesh, draw_command); } mesh.clear(); } @@ -295,8 +291,8 @@ namespace { OscarImguiBackendData* bd = get_backend_data(); OSC_ASSERT(bd != nullptr && "no oscar ImGui renderer backend was available to shutdown - this is a developer error"); - UID uid = bd->texures_allocated_this_frame.try_emplace(UID{}, texture).first->first; - return to_imgui_texture_id(uid); + const UID texture_uid = bd->textures_allocated_this_frame.try_emplace(UID{}, texture).first->first; + return to_imgui_texture_id(texture_uid); } } @@ -334,18 +330,18 @@ void osc::ui::graphics_backend::on_start_new_frame() OscarImguiBackendData* bd = get_backend_data(); OSC_ASSERT(bd != nullptr && "no oscar ImGui renderer backend was available to shutdown - this is a developer error"); - bd->texures_allocated_this_frame.clear(); - bd->texures_allocated_this_frame.try_emplace(bd->font_texture_id, bd->font_texture); // (so that all lookups can hit the same LUT) + bd->textures_allocated_this_frame.clear(); + bd->textures_allocated_this_frame.try_emplace(bd->font_texture_id, bd->font_texture); // (so that all lookups can hit the same LUT) } -void osc::ui::graphics_backend::render(ImDrawData* drawData) +void osc::ui::graphics_backend::render(ImDrawData* draw_data) { OscarImguiBackendData* bd = get_backend_data(); OSC_ASSERT(bd != nullptr && "no oscar ImGui renderer backend was available to shutdown - this is a developer error"); - setup_camera_view_matrix(*drawData, bd->camera); - for (int n = 0; n < drawData->CmdListsCount; ++n) { - render_drawlist(*bd, *drawData, *drawData->CmdLists[n]); + setup_camera_view_matrix(*draw_data, bd->camera); + for (int n = 0; n < draw_data->CmdListsCount; ++n) { + render_drawlist(*bd, *draw_data, *draw_data->CmdLists[n]); } } diff --git a/src/oscar/UI/ui_graphics_backend.h b/src/oscar/UI/ui_graphics_backend.h index 1c82c0eb8..7b2a86dca 100644 --- a/src/oscar/UI/ui_graphics_backend.h +++ b/src/oscar/UI/ui_graphics_backend.h @@ -1,7 +1,5 @@ #pragma once -#include - namespace osc { class RenderTexture; } namespace osc { class Texture2D; } diff --git a/src/oscar/Utils.h b/src/oscar/Utils.h index f1d06c100..0ada49419 100644 --- a/src/oscar/Utils.h +++ b/src/oscar/Utils.h @@ -44,6 +44,5 @@ #include #include #include -// TODO: reenable once MacOS gets pmr::memory_resource support -// #include +// #include // TODO: reenable once MacOS gets pmr::memory_resource support #include diff --git a/src/oscar/Utils/Algorithms.h b/src/oscar/Utils/Algorithms.h index 34d3e935d..e13d12517 100644 --- a/src/oscar/Utils/Algorithms.h +++ b/src/oscar/Utils/Algorithms.h @@ -3,7 +3,6 @@ #include #include -#include #include #include #include @@ -65,7 +64,7 @@ namespace osc constexpr std::ranges::range_reference_t at(R&& r, std::ranges::range_size_t pos) { if (pos < std::ranges::size(r)) { - return r[pos]; + return std::forward(r)[pos]; } else { throw std::out_of_range{"out of bounds index given to a container"}; @@ -133,7 +132,7 @@ namespace osc return static_cast(nullptr); } - // returns `true` if both `lhs` and `rhs` can be sucessfully `dynamic_cast`ed to `Downcasted` and compare equal + // returns `true` if both `lhs` and `rhs` can be successfully `dynamic_cast`ed to `Downcasted` and compare equal template requires std::equality_comparable and diff --git a/src/oscar/Utils/CStringView.h b/src/oscar/Utils/CStringView.h index b1400d27d..d71a75b05 100644 --- a/src/oscar/Utils/CStringView.h +++ b/src/oscar/Utils/CStringView.h @@ -42,6 +42,7 @@ namespace osc constexpr CStringView(CStringView&&) noexcept = default; constexpr CStringView& operator=(const CStringView&) = default; constexpr CStringView& operator=(CStringView&&) noexcept = default; + constexpr ~CStringView() noexcept = default; constexpr size_type size() const { @@ -104,9 +105,9 @@ namespace osc } } - inline std::string to_string(const CStringView& cstrv) + inline std::string to_string(const CStringView& cstring_view) { - return std::string{cstrv}; + return std::string{cstring_view}; } std::ostream& operator<<(std::ostream&, const CStringView&); @@ -116,8 +117,8 @@ namespace osc template<> struct std::hash final { - size_t operator()(const osc::CStringView& cstrv) const + size_t operator()(const osc::CStringView& cstring_view) const noexcept { - return std::hash{}(cstrv); + return std::hash{}(cstring_view); } }; diff --git a/src/oscar/Utils/CircularBuffer.h b/src/oscar/Utils/CircularBuffer.h index 6088bbf8b..8c49daf6c 100644 --- a/src/oscar/Utils/CircularBuffer.h +++ b/src/oscar/Utils/CircularBuffer.h @@ -106,12 +106,10 @@ namespace osc { difference_type im = (i % N); - if (im > offset_) - { + if (im > offset_) { offset_ = N - (im - offset_); } - else - { + else { offset_ = offset_ - im; } @@ -266,7 +264,7 @@ namespace osc // // be careful here: the data is type-punned from a sequence of bytes // so that the backing storage does not have a strict requirement of - // having to contain redundant default-constrcuted elements + // having to contain redundant default-constructed elements constexpr void clear() { @@ -384,7 +382,7 @@ namespace osc // - the above constraints imply that the number of "live" // elements in storage is N-1, because `end_offset_` will modulo // spin into position 0 once it is equal to `N` (this is - // in constrast to non-circular storage, where `end_offset_` + // in contrast to non-circular storage, where `end_offset_` // typically points one past the end of the storage range) // // - this behavior makes the implementation simpler, because diff --git a/src/oscar/Utils/ClonePtr.h b/src/oscar/Utils/ClonePtr.h index 9631f9720..f2f00100b 100644 --- a/src/oscar/Utils/ClonePtr.h +++ b/src/oscar/Utils/ClonePtr.h @@ -36,10 +36,10 @@ namespace osc // constructs a `ClonePtr` by `clone`ing `src` ClonePtr(const ClonePtr& src) : value_{src.value_ ? src.value_->clone() : nullptr} {} - // constructs `ClonePtr` by transferring ownership from an rvalue + // constructs `ClonePtr` by transferring ownership from a rvalue ClonePtr(ClonePtr&&) noexcept = default; - // constructs `ClonePtr` by transferring ownership from an rvalue (with conversion) + // constructs `ClonePtr` by transferring ownership from a rvalue (with conversion) template requires std::convertible_to::pointer, pointer> and @@ -124,7 +124,7 @@ namespace osc return value_; } - typename std::add_lvalue_reference::type operator*() const noexcept(noexcept(*std::declval())) + std::add_lvalue_reference_t operator*() const noexcept(noexcept(*std::declval())) { return *value_; } @@ -168,7 +168,7 @@ namespace osc template struct std::hash> final { - size_t operator()(const osc::ClonePtr& p) const + size_t operator()(const osc::ClonePtr& p) const noexcept { return std::hash(p.get()); } diff --git a/src/oscar/Utils/Concepts.h b/src/oscar/Utils/Concepts.h index 15cb7a70d..fe958ca71 100644 --- a/src/oscar/Utils/Concepts.h +++ b/src/oscar/Utils/Concepts.h @@ -4,7 +4,6 @@ #include #include #include -#include #include #include #include diff --git a/src/oscar/Utils/CopyOnUpdPtr.h b/src/oscar/Utils/CopyOnUpdPtr.h index 4236b5c09..327a57a8a 100644 --- a/src/oscar/Utils/CopyOnUpdPtr.h +++ b/src/oscar/Utils/CopyOnUpdPtr.h @@ -1,7 +1,5 @@ #pragma once -#include - #include #include #include @@ -88,7 +86,7 @@ namespace osc template struct std::hash> final { - size_t operator()(const osc::CopyOnUpdPtr& cow) const + size_t operator()(const osc::CopyOnUpdPtr& cow) const noexcept { return std::hash>{}(cow.ptr_); } diff --git a/src/oscar/Utils/DefaultConstructOnCopy.h b/src/oscar/Utils/DefaultConstructOnCopy.h index bd3955e91..61249f81b 100644 --- a/src/oscar/Utils/DefaultConstructOnCopy.h +++ b/src/oscar/Utils/DefaultConstructOnCopy.h @@ -12,19 +12,19 @@ namespace osc template requires std::constructible_from - DefaultConstructOnCopy(Args&& ...args) : - m_Value{std::forward(args)...} + explicit DefaultConstructOnCopy(Args&& ...args) : + value_{std::forward(args)...} {} DefaultConstructOnCopy(const DefaultConstructOnCopy&) : - m_Value{} + value_{} {} DefaultConstructOnCopy(DefaultConstructOnCopy&&) noexcept = default; DefaultConstructOnCopy& operator=(const DefaultConstructOnCopy&) { - m_Value = T{}; // exception safety: construct it then move-assign it + value_ = T{}; // exception safety: construct it then move-assign it return *this; } @@ -32,16 +32,16 @@ namespace osc ~DefaultConstructOnCopy() noexcept = default; - T* operator->() { return &m_Value; } - const T* operator->() const { return &m_Value; } - T& operator*() { return m_Value; } - const T& operator*() const { return m_Value; } - T* get() { return m_Value; } - const T* get() const { return m_Value; } + T* operator->() { return &value_; } + const T* operator->() const { return &value_; } + T& operator*() { return value_; } + const T& operator*() const { return value_; } + T* get() { return value_; } + const T* get() const { return value_; } - void reset() { m_Value = T{}; } + void reset() { value_ = T{}; } private: - T m_Value; + T value_; }; } diff --git a/src/oscar/Utils/EnumHelpers.h b/src/oscar/Utils/EnumHelpers.h index 2f587fd85..a23e12c4e 100644 --- a/src/oscar/Utils/EnumHelpers.h +++ b/src/oscar/Utils/EnumHelpers.h @@ -36,7 +36,7 @@ namespace osc static_assert(sizeof...(TEnumOptions) == num_options()); }; - // returns the value of `v` casted to a `size_t` (i.e. `v` should probably satisfy + // returns the value of `v` cast to a `size_t` (i.e. `v` should probably satisfy // `DenselyPackedOptionsEnum` for this to work template constexpr size_t to_index(TEnum v) diff --git a/src/oscar/Utils/FileChangePoller.cpp b/src/oscar/Utils/FileChangePoller.cpp index 7eaedcd0a..86bc02dc5 100644 --- a/src/oscar/Utils/FileChangePoller.cpp +++ b/src/oscar/Utils/FileChangePoller.cpp @@ -35,14 +35,14 @@ osc::FileChangePoller::FileChangePoller( bool osc::FileChangePoller::change_detected(const std::string& path) { if (path.empty() or path == c_model_no_backing_file_sentinel) { - // has no, or a senteniel, path - do no checks + // has no, or a sentinel, path - do no checks return false; } - auto now = std::chrono::system_clock::now(); + const auto now = std::chrono::system_clock::now(); if (now < next_polling_time_) { - // to soon to poll again + // too soon to poll again return false; } @@ -53,7 +53,7 @@ bool osc::FileChangePoller::change_detected(const std::string& path) return false; } - auto modification_time = std::filesystem::last_write_time(path); + const auto modification_time = std::filesystem::last_write_time(path); next_polling_time_ = now + delay_between_checks_; if (modification_time == file_last_modification_time_) { diff --git a/src/oscar/Utils/FilesystemHelpers.cpp b/src/oscar/Utils/FilesystemHelpers.cpp index 83200b1df..2eb047926 100644 --- a/src/oscar/Utils/FilesystemHelpers.cpp +++ b/src/oscar/Utils/FilesystemHelpers.cpp @@ -1,15 +1,12 @@ #include "FilesystemHelpers.h" -#include #include #include -#include #include -#include #include #include -#include +#include #include void osc::for_each_file_with_extensions_recursive( @@ -77,15 +74,15 @@ std::vector osc::find_files_recursive( return rv; } -bool osc::is_filename_lexographically_greater_than(const std::filesystem::path& p1, const std::filesystem::path& p2) +bool osc::is_filename_lexicographically_greater_than(const std::filesystem::path& p1, const std::filesystem::path& p2) { return is_string_case_insensitive_greater_than(p1.filename().string(), p2.filename().string()); } bool osc::is_subpath(const std::filesystem::path& dir, const std::filesystem::path& path) { - auto num_dir_components = std::distance(dir.begin(), dir.end()); - auto num_path_components = std::distance(path.begin(), path.end()); + const auto num_dir_components = std::distance(dir.begin(), dir.end()); + const auto num_path_components = std::distance(path.begin(), path.end()); if (num_path_components < num_dir_components) { return false; diff --git a/src/oscar/Utils/FilesystemHelpers.h b/src/oscar/Utils/FilesystemHelpers.h index d5249310d..95a3a7244 100644 --- a/src/oscar/Utils/FilesystemHelpers.h +++ b/src/oscar/Utils/FilesystemHelpers.h @@ -3,14 +3,13 @@ #include #include #include -#include #include #include namespace osc { // calls `consumer` with each file recursively found in `root` that ends with - // any of the provied `extensions` + // any of the provided `extensions` void for_each_file_with_extensions_recursive( const std::filesystem::path& root, const std::function& consumer, @@ -35,10 +34,10 @@ namespace osc const std::filesystem::path& root ); - // returns true if `p1` is lexographically greater than `p2`, ignoring case + // returns true if `p1` is lexicographically greater than `p2`, ignoring case // // e.g. "b" > "a", "B" > "a" (this isn't true if case-sensitive) - bool is_filename_lexographically_greater_than(const std::filesystem::path& p1, const std::filesystem::path& p2); + bool is_filename_lexicographically_greater_than(const std::filesystem::path& p1, const std::filesystem::path& p2); // returns true if `path` is within `dir` (non-recursive) bool is_subpath(const std::filesystem::path& dir, const std::filesystem::path& path); diff --git a/src/oscar/Utils/LifetimedPtr.h b/src/oscar/Utils/LifetimedPtr.h index 56bd0f03d..7dff11b78 100644 --- a/src/oscar/Utils/LifetimedPtr.h +++ b/src/oscar/Utils/LifetimedPtr.h @@ -17,7 +17,7 @@ namespace osc // without having to invasively add reference counters etc. to the things being // managed // - // note: `LifetimedPtr` isn't threadsafe in the same way that (e.g.) `std::weak_ptr` + // note: `LifetimedPtr` isn't thread-safe in the same way that (e.g.) `std::weak_ptr` // is. Because there's no way to `lock` a raw pointer, this class is susceptible // to (e.g.) checking the lifetime, followed by accessing the object while the // owning thread is destructing it diff --git a/src/oscar/Utils/NullStreambuf.h b/src/oscar/Utils/NullStreambuf.h index 5b82b3679..76c012c80 100644 --- a/src/oscar/Utils/NullStreambuf.h +++ b/src/oscar/Utils/NullStreambuf.h @@ -19,14 +19,14 @@ namespace osc return num_chars_written_ > 0; } protected: - int overflow(int c) final + int overflow(int c) override { setp(dummy_buffer_.data(), dummy_buffer_.data() + dummy_buffer_.size()); ++num_chars_written_; return (c == traits_type::eof() ? char_type{} : c); } - std::streamsize xsputn(const char_type*, std::streamsize count) final + std::streamsize xsputn(const char_type*, std::streamsize count) override { num_chars_written_ += count; return count; diff --git a/src/oscar/Utils/Perf.cpp b/src/oscar/Utils/Perf.cpp index e0ff3b88b..ada6d78af 100644 --- a/src/oscar/Utils/Perf.cpp +++ b/src/oscar/Utils/Perf.cpp @@ -5,8 +5,6 @@ #include -#include -#include #include #include #include diff --git a/src/oscar/Utils/Perf.h b/src/oscar/Utils/Perf.h index 615eee5dd..8d2efa3f5 100644 --- a/src/oscar/Utils/Perf.h +++ b/src/oscar/Utils/Perf.h @@ -4,7 +4,6 @@ #include #include -#include #include #include diff --git a/src/oscar/Utils/PerfMeasurement.h b/src/oscar/Utils/PerfMeasurement.h index 86394d521..da0d4d318 100644 --- a/src/oscar/Utils/PerfMeasurement.h +++ b/src/oscar/Utils/PerfMeasurement.h @@ -5,9 +5,7 @@ #include #include -#include #include -#include namespace osc { diff --git a/src/oscar/Utils/PerfMeasurementMetadata.h b/src/oscar/Utils/PerfMeasurementMetadata.h index f5536ca50..a43f2bc54 100644 --- a/src/oscar/Utils/PerfMeasurementMetadata.h +++ b/src/oscar/Utils/PerfMeasurementMetadata.h @@ -2,7 +2,6 @@ #include -#include #include #include diff --git a/src/oscar/Utils/ScopeGuard.h b/src/oscar/Utils/ScopeGuard.h index da59b87d8..d51bc031b 100644 --- a/src/oscar/Utils/ScopeGuard.h +++ b/src/oscar/Utils/ScopeGuard.h @@ -5,11 +5,11 @@ namespace osc { - template + template class ScopeGuard final { public: - explicit ScopeGuard(Dtor&& destructor) : - destructor_{std::forward(destructor)} + explicit ScopeGuard(Destructor&& destructor) : + destructor_{std::forward(destructor)} {} ScopeGuard(const ScopeGuard&) = delete; ScopeGuard(ScopeGuard&&) noexcept = delete; @@ -21,6 +21,6 @@ namespace osc } private: - Dtor destructor_; + Destructor destructor_; }; } diff --git a/src/oscar/Utils/SharedPreHashedString.h b/src/oscar/Utils/SharedPreHashedString.h index 1688b5564..fec2dc2ac 100644 --- a/src/oscar/Utils/SharedPreHashedString.h +++ b/src/oscar/Utils/SharedPreHashedString.h @@ -48,7 +48,7 @@ namespace osc // // - use `StringName` in larger multi-level systems that use, copy, and hash a lot of // potentially-longer strings in associative lookups. E.g. a large application - // containing a graphics system, which binds materialkeys to shader uniforms on the GPU; + // containing a graphics system, which binds material keys to shader uniforms on the GPU; // a mid-level UI controller, which maintains a datamodel containing material key-value // pairs; and a high-level UI, which binds those pairs to a nodegraph UI (i.e. despite // each tier having an independent responsibility, there's likely to be overlaps in @@ -67,7 +67,7 @@ namespace osc using const_reverse_iterator = std::string_view::const_reverse_iterator; explicit SharedPreHashedString() : - SharedPreHashedString{SharedPreHashedString::static_default_instance()} + SharedPreHashedString{static_default_instance()} {} explicit SharedPreHashedString(std::string_view str) @@ -275,17 +275,17 @@ namespace osc private: friend struct std::hash; - const SharedPreHashedString& static_default_instance() + static const SharedPreHashedString& static_default_instance() { static SharedPreHashedString s_default_instance{std::string_view{}}; return s_default_instance; } - // internal datastructure that's placed at the front of the dynamically-allocated + // internal data structure that's placed at the front of the dynamically-allocated // allocated memory block struct Metadata final { - Metadata(std::string_view str) noexcept : + explicit Metadata(std::string_view str) noexcept : size{str.size()}, hash{std::hash{}(str)} {} diff --git a/src/oscar/Utils/Spsc.h b/src/oscar/Utils/Spsc.h index cdcd9e427..52ceae3c1 100644 --- a/src/oscar/Utils/Spsc.h +++ b/src/oscar/Utils/Spsc.h @@ -30,7 +30,7 @@ namespace osc::spsc // queue mutex std::mutex mutex_; - // queue != empty condvar for worker + // queue != empty condition variable for worker std::condition_variable condition_variable_; // the queue @@ -57,7 +57,7 @@ namespace osc::spsc template friend std::pair, Receiver> channel(); - Sender(std::shared_ptr> impl) : + explicit Sender(std::shared_ptr> impl) : impl_{std::move(impl)} { ++impl_->num_senders_; @@ -99,7 +99,7 @@ namespace osc::spsc template friend std::pair, Receiver> channel(); - Receiver(std::shared_ptr> impl) : + explicit Receiver(std::shared_ptr> impl) : impl_{std::move(impl)} { ++impl_->num_receivers_; @@ -150,10 +150,10 @@ namespace osc::spsc return not impl_->message_queue_.empty() or impl_->num_senders_ == 0; }); - // the condvar woke up (non-spuriously), either: + // the condition variable woke up (non-spuriously), either: // // - there's something in the queue (return it) - // - the sender hung up (return nullopt) + // - the sender hung up (return `std::nullopt`) if (not impl_->message_queue_.empty()) { std::optional rv{std::move(impl_->message_queue_.back())}; @@ -171,7 +171,7 @@ namespace osc::spsc } }; - // create a new threadsafe spsc channel (sender + receiver) + // create a new thread-safe spsc channel (sender + receiver) template std::pair, Receiver> channel() { @@ -181,7 +181,7 @@ namespace osc::spsc // SPSC worker: single-producer single-consumer worker abstraction // - // encapsulates a worker background thread with threadsafe communication + // encapsulates a worker background thread with thread-safe communication // channels template class Worker { @@ -202,7 +202,7 @@ namespace osc::spsc spsc::Sender sender, Func message_processor) { - // continously try to receive input messages and + // continuously try to receive input messages and // respond to them one-by-one while (not sender.is_receiver_hung_up()) { std::optional message = receiver.receive(); diff --git a/src/oscar/Utils/StringHelpers.cpp b/src/oscar/Utils/StringHelpers.cpp index 371a7c39b..06cbb76c2 100644 --- a/src/oscar/Utils/StringHelpers.cpp +++ b/src/oscar/Utils/StringHelpers.cpp @@ -26,9 +26,9 @@ namespace '0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'a', 'b', 'c', 'd', 'e', 'f' }); - std::string to_lowercase(std::string_view sv) + std::string to_lowercase(std::string_view str) { - std::string cpy{sv}; + std::string cpy{str}; rgs::transform(cpy, cpy.begin(), [](std::string::value_type c) { return static_cast(std::tolower(c)); @@ -99,7 +99,7 @@ bool osc::is_valid_identifier(std::string_view sv) ( '_' == c ) or ('a' <= c && c <= 'z'); }; - const auto is_valid_nonfirst_character_of_identifier = [](std::string_view::value_type c) + const auto is_valid_non_first_character_of_identifier = [](std::string_view::value_type c) { return ('0' <= c && c <= '9') or @@ -116,13 +116,13 @@ bool osc::is_valid_identifier(std::string_view sv) return false; } - return rgs::all_of(sv.begin() + 1, sv.end(), is_valid_nonfirst_character_of_identifier); + return rgs::all_of(sv.begin() + 1, sv.end(), is_valid_non_first_character_of_identifier); } std::string_view osc::strip_whitespace(std::string_view sv) { - const std::string_view::const_iterator front = rgs::find_if_not(sv, ::isspace); - const std::string_view::const_iterator back = rgs::find_if_not(sv.rbegin(), std::string_view::const_reverse_iterator{front}, ::isspace).base(); + const auto front = rgs::find_if_not(sv, ::isspace); + const auto back = rgs::find_if_not(sv.rbegin(), std::string_view::const_reverse_iterator{front}, ::isspace).base(); return {sv.data() + std::distance(sv.begin(), front), static_cast(std::distance(front, back))}; } @@ -160,10 +160,10 @@ std::string osc::truncate_with_ellipsis(std::string_view v, size_t max_length) return std::string{v}; } - std::string_view substr = v.substr(0, max(0_z, static_cast(max_length)-3)); + std::string_view substring = v.substr(0, max(0_z, static_cast(max_length)-3)); std::string rv; - rv.reserve(substr.length() + 3); - rv = substr; + rv.reserve(substring.length() + 3); + rv = substring; rv += "..."; return rv; } @@ -198,17 +198,17 @@ std::optional osc::try_parse_hex_chars_as_byte(char a, char b) // you might be wondering why we aren't using a library function, it's // because: // - // - std::stringstream is a sledge-hammer that will try its best to parse - // alsorts of `n`-lengthed strings as hex strings, so users of this + // - `std::stringstream` is a sledgehammer that will try its best to parse + // all sorts of `n`-length strings as hex strings, so users of this // function would need to know the edge-cases // - // - std::strtol is closer, in that it supports parsing base16 strings etc. + // - `std::strtol` is closer, in that it supports parsing base16 strings etc. // but it _also_ handles things such as plus/minus signs, the `0x`, octal, // etc. // - // - std::from_chars, is the savior from std::strtol, but _also_ has special + // - `std::from_chars`, is the savior from `std::strtol`, but _also_ has special // parsing behavior (e.g. the test suite for this function found that it - // is effectively a wrapper around std::strol in terms of behavior + // is effectively a wrapper around `std::strol` in terms of behavior` // // and all this particular function needs to do is map strings like '00' to // 0x0, 'ff' to 255, etc. diff --git a/src/oscar/Utils/StringHelpers.h b/src/oscar/Utils/StringHelpers.h index 2aa14eda5..a7f005b47 100644 --- a/src/oscar/Utils/StringHelpers.h +++ b/src/oscar/Utils/StringHelpers.h @@ -20,7 +20,7 @@ namespace osc // returns true if `sv` contains `substr` (case-insensitive) bool contains_case_insensitive(std::string_view sv, std::string_view substr); - // returns true if `b` is lexographically greater than `a`, ignoring case + // returns true if `b` is lexicographically greater than `a`, ignoring case bool is_string_case_insensitive_greater_than(std::string_view a, std::string_view b); // returns true if `a` is equal to `b` (case-insensitive) @@ -47,7 +47,7 @@ namespace osc // number, internally using something like `std::strtof` (which depends // on C locale - careful) // - // returns the resulting float if sucessful, or `std::nullopt` if it fails + // returns the resulting float if successful, or `std::nullopt` if it fails // // the reason this function exists is because, at time of writing, C++'s // `std::from_chars` function isn't implemented in Mac OSX @@ -60,7 +60,7 @@ namespace osc // of `sv` exceeds `max_length` std::string truncate_with_ellipsis(std::string_view sv, size_t max_length); - // returns the end of the string between the last occurance of `delimiter` and + // returns the end of the string between the last occurrence of `delimiter` and // the end of `sv`, or `sv` if `delimiter` does not occur within `sv`. std::string_view substring_after_last(std::string_view sv, std::string_view::value_type delimiter); @@ -89,14 +89,14 @@ namespace osc template requires OutputStreamable> - std::string join(R&& r, std::string_view delimeter) + std::string join(R&& r, std::string_view delimiter) { std::stringstream ss; - std::string_view prefix_delim; + std::string_view prefix_delimiter; for (auto&& el : r) { - ss << prefix_delim; + ss << prefix_delimiter; ss << el; - prefix_delim = delimeter; + prefix_delimiter = delimiter; } return std::move(ss).str(); } diff --git a/src/oscar/Utils/StringName.cpp b/src/oscar/Utils/StringName.cpp index 0674c2487..8afe2c5cb 100644 --- a/src/oscar/Utils/StringName.cpp +++ b/src/oscar/Utils/StringName.cpp @@ -14,15 +14,15 @@ namespace { using FastStringLookup = ankerl::unordered_dense::set>; - SynchronizedValue& get_global_lookup() + SynchronizedValue& get_global_lut() { - static SynchronizedValue s_lookup; - return s_lookup; + static SynchronizedValue s_string_name_global_lut; + return s_string_name_global_lut; } } osc::StringName::StringName(std::string_view sv) : - SharedPreHashedString{*get_global_lookup().lock()->emplace(sv).first} + SharedPreHashedString{*get_global_lut().lock()->emplace(sv).first} {} osc::StringName::~StringName() noexcept @@ -36,5 +36,5 @@ osc::StringName::~StringName() noexcept } // else: clear it from the global table - get_global_lookup().lock()->erase(static_cast(*this)); + get_global_lut().lock()->erase(static_cast(*this)); } diff --git a/src/oscar/Utils/StringName.h b/src/oscar/Utils/StringName.h index 6a392c3ed..9a0d9de27 100644 --- a/src/oscar/Utils/StringName.h +++ b/src/oscar/Utils/StringName.h @@ -1,6 +1,5 @@ #pragma once -#include #include #include @@ -64,8 +63,8 @@ namespace osc template<> struct std::hash final { - size_t operator()(const osc::StringName& sn) const + size_t operator()(const osc::StringName& string_name) const noexcept { - return std::hash{}(sn); + return std::hash{}(string_name); } }; diff --git a/src/oscar/Utils/SynchronizedValue.h b/src/oscar/Utils/SynchronizedValue.h index fd5957244..64eb0d50d 100644 --- a/src/oscar/Utils/SynchronizedValue.h +++ b/src/oscar/Utils/SynchronizedValue.h @@ -9,7 +9,7 @@ namespace osc { - // represents a `T` value that can only be accessed via a mutexed guard + // represents a `T` value that can only be accessed via a mutex guard template< typename T, typename TDefaultGuard = std::lock_guard @@ -23,17 +23,14 @@ namespace osc template requires std::constructible_from explicit SynchronizedValue(Args&&... args) : - value_mutex_{}, value_{std::forward(args)...} {} SynchronizedValue(const SynchronizedValue& other) : - value_mutex_{}, value_{*other.lock()} {} SynchronizedValue(SynchronizedValue&& tmp) noexcept : - value_mutex_{}, value_{std::move(tmp).value()} {} @@ -53,6 +50,8 @@ namespace osc return *this; } + ~SynchronizedValue() noexcept = default; + T value() && { const auto guard = std::lock_guard{value_mutex_}; diff --git a/src/oscar/Utils/TemporaryFile.cpp b/src/oscar/Utils/TemporaryFile.cpp index e3bd13906..ffe87950e 100644 --- a/src/oscar/Utils/TemporaryFile.cpp +++ b/src/oscar/Utils/TemporaryFile.cpp @@ -1,22 +1,20 @@ #include "TemporaryFile.h" +#include #include #include #include #include -#include -#include -#include #include using namespace osc; osc::TemporaryFile::TemporaryFile(const TemporaryFileParameters& params) { - auto p = mkstemp(params.suffix, params.prefix); - absolute_path_ = std::move(p.second); - handle_ = std::move(p.first); + auto [stream, path] = mkstemp(params.suffix, params.prefix); + absolute_path_ = std::move(path); + handle_ = std::move(stream); } osc::TemporaryFile::TemporaryFile(TemporaryFile&& tmp) noexcept : @@ -24,6 +22,7 @@ osc::TemporaryFile::TemporaryFile(TemporaryFile&& tmp) noexcept : handle_{std::move(tmp.handle_)}, should_delete_{std::exchange(tmp.should_delete_, false)} {} + TemporaryFile& osc::TemporaryFile::operator=(TemporaryFile&& tmp) noexcept { using std::swap; @@ -37,6 +36,7 @@ TemporaryFile& osc::TemporaryFile::operator=(TemporaryFile&& tmp) noexcept swap(should_delete_, tmp.should_delete_); return *this; } + osc::TemporaryFile::~TemporaryFile() noexcept { if (should_delete_) { @@ -46,9 +46,8 @@ osc::TemporaryFile::~TemporaryFile() noexcept try { std::filesystem::remove(absolute_path_); } - catch (const std::filesystem::filesystem_error&) { - // swallow it: we're in a destructor, so the best we could hope for - // is to report it to the log or similar + catch (const std::filesystem::filesystem_error& ex) { + log_error("error closing a temporary file, this could be a sign of operating system issues: %s", ex.what()); } } } diff --git a/src/oscar/Utils/TemporaryFile.h b/src/oscar/Utils/TemporaryFile.h index c1f7ec8a0..624c49a97 100644 --- a/src/oscar/Utils/TemporaryFile.h +++ b/src/oscar/Utils/TemporaryFile.h @@ -4,7 +4,6 @@ #include #include -#include namespace osc { @@ -14,7 +13,7 @@ namespace osc // // - The file is created in the operating system's temporary directory, or `dir`, throwing otherwise // - The name of the temporary file begins with `prefix`, ends with `suffix`, and the characters between - // those two are are chosen to result in a new, unique, filename, throwing otherwise + // those two are chosen to result in a new, unique, filename, throwing otherwise // - The file will be deleted from the filesystem upon destruction of the `TemporaryFile` object class TemporaryFile final { public: diff --git a/src/oscar/Utils/TemporaryFileParameters.h b/src/oscar/Utils/TemporaryFileParameters.h index b34ae8da6..48212f5ed 100644 --- a/src/oscar/Utils/TemporaryFileParameters.h +++ b/src/oscar/Utils/TemporaryFileParameters.h @@ -1,15 +1,12 @@ #pragma once -#include -#include -#include #include namespace osc { // parameters for constructing a `TemporaryFile` // - // designed for designated initializer compatibility: `TemporaryFile tmpfile({ .suffix = ".obj" });` + // designed for designated initializer compatibility: `TemporaryFile temporary_file({ .suffix = ".obj" });` struct TemporaryFileParameters final { // a prefix that will be prepended to the dynamic portion of the temporary file name (e.g. `${prefix}XXXXXX${suffix}`) diff --git a/src/oscar/Utils/TransparentStringHasher.h b/src/oscar/Utils/TransparentStringHasher.h index 84975ee10..21736dbea 100644 --- a/src/oscar/Utils/TransparentStringHasher.h +++ b/src/oscar/Utils/TransparentStringHasher.h @@ -14,23 +14,23 @@ namespace osc using is_transparent = void; // C++20, `std::unordered_map` uses this - size_t operator()(std::string_view sv) const noexcept + size_t operator()(std::string_view string_view) const noexcept { // if something implicitly converts into a `std::string_view` then it's // eligible for transparent hashing - return std::hash{}(sv); + return std::hash{}(string_view); } - size_t operator()(const SharedPreHashedString& sn) const noexcept + size_t operator()(const SharedPreHashedString& shared_pre_hashed_string) const noexcept { // special case: `SharedPreHashedString`s are pre-hashed - return std::hash{}(sn); + return std::hash{}(shared_pre_hashed_string); } - size_t operator()(const StringName& sn) const noexcept + size_t operator()(const StringName& string_name) const noexcept { // special case: `StringName`s are pre-hashed - return std::hash{}(sn); + return std::hash{}(string_name); } }; } diff --git a/src/oscar/Utils/UID.h b/src/oscar/Utils/UID.h index a9a5692ef..46a9de758 100644 --- a/src/oscar/Utils/UID.h +++ b/src/oscar/Utils/UID.h @@ -77,7 +77,7 @@ namespace osc // lets them be used as associative lookup keys, etc. template<> struct std::hash final { - size_t operator()(const osc::UID& id) const + size_t operator()(const osc::UID& id) const noexcept { return static_cast(id.get()); } diff --git a/src/oscar/Utils/UndoRedo.cpp b/src/oscar/Utils/UndoRedo.cpp index f0a667be7..f7372fa8d 100644 --- a/src/oscar/Utils/UndoRedo.cpp +++ b/src/oscar/Utils/UndoRedo.cpp @@ -61,7 +61,7 @@ void osc::UndoRedoBase::undo_to(size_t pos) // copy any commits between the top of the undo stack, but shallower than // the requested entry, onto the redo stack - copy(undo_.rbegin(), undo_.rbegin() + pos, std::back_inserter(redo_)); + std::copy_n(undo_.rbegin(), pos, std::back_inserter(redo_)); undo_.erase((undo_.rbegin() + pos + 1).base(), undo_.end()); head_ = new_head; @@ -108,7 +108,7 @@ void osc::UndoRedoBase::redo_to(size_t pos) // copy any commits between the top of the redo stack but *before* the // requested entry onto the redo stack - copy(redo_.rbegin(), redo_.rbegin() + pos, std::back_inserter(undo_)); + std::copy_n(redo_.rbegin(), pos, std::back_inserter(undo_)); redo_.erase((redo_.rbegin() + pos + 1).base(), redo_.end()); head_ = new_head; diff --git a/src/oscar/Utils/UndoRedo.h b/src/oscar/Utils/UndoRedo.h index 11e88ad81..1d734d5c1 100644 --- a/src/oscar/Utils/UndoRedo.h +++ b/src/oscar/Utils/UndoRedo.h @@ -15,7 +15,7 @@ // undo/redo algorithm support // // snapshot-based, rather than command-pattern based. Designed to be reference-counted, and -// type-eraseable, so that generic downstream code doesn't necessarily need to know what, +// type-erasable, so that generic downstream code doesn't necessarily need to know what, // or how, the data is actually stored in memory namespace osc { @@ -54,7 +54,7 @@ namespace osc public: template requires std::constructible_from - UndoRedoEntryData(std::string_view message, Args&&... args) : + explicit UndoRedoEntryData(std::string_view message, Args&&... args) : UndoRedoEntryMetadata{std::move(message)}, value_{std::forward(args)...} {} @@ -92,7 +92,7 @@ namespace osc public: template requires std::constructible_from - UndoRedoEntry(std::string_view message, Args&&... args) : + explicit UndoRedoEntry(std::string_view message, Args&&... args) : UndoRedoEntryBase{std::make_shared>(std::move(message), std::forward(args)...)} {} @@ -150,7 +150,7 @@ namespace osc public: template requires std::constructible_from - UndoRedo(Args&&... args) : + explicit UndoRedo(Args&&... args) : UndoRedoBase(UndoRedoEntry{"created document", std::forward(args)...}), scratch_{static_cast&>(head()).value()} {} diff --git a/src/oscar/Variant/Variant.cpp b/src/oscar/Variant/Variant.cpp index eeb875c2f..26b97cd0b 100644 --- a/src/oscar/Variant/Variant.cpp +++ b/src/oscar/Variant/Variant.cpp @@ -234,7 +234,7 @@ std::ostream& osc::operator<<(std::ostream& out, const Variant& variant) return out << to(variant); } -size_t std::hash::operator()(const Variant& variant) const +size_t std::hash::operator()(const Variant& variant) const noexcept { // note: you might be wondering why this isn't `std::hash{}(v.data_)` // @@ -250,9 +250,9 @@ size_t std::hash::operator()(const Variant& variant) const // `hash_of(Variant) == hash_of(std::string) == hash_of(std::string_view) == hash_of(StringName)...` return std::visit(Overload{ - [](const auto& inner) + [](const T& inner) { - return std::hash>>{}(inner); + return std::hash>>{}(inner); }, }, variant.data_); } diff --git a/src/oscar/Variant/Variant.h b/src/oscar/Variant/Variant.h index 88c884e95..746687170 100644 --- a/src/oscar/Variant/Variant.h +++ b/src/oscar/Variant/Variant.h @@ -27,7 +27,7 @@ namespace osc Variant(std::string_view); Variant(const char* ptr) : Variant{std::string_view{ptr}} {} Variant(std::nullopt_t) = delete; - Variant(CStringView cstr) : Variant{std::string_view{cstr}} {} + Variant(CStringView cstring_view) : Variant{std::string_view{cstring_view}} {} Variant(const StringName&); Variant(Vec2); Variant(Vec3); @@ -73,5 +73,5 @@ namespace osc template<> struct std::hash final { - size_t operator()(const osc::Variant&) const; + size_t operator()(const osc::Variant&) const noexcept; }; diff --git a/src/oscar/Variant/VariantType.cpp b/src/oscar/Variant/VariantType.cpp index 550951320..4dee09a85 100644 --- a/src/oscar/Variant/VariantType.cpp +++ b/src/oscar/Variant/VariantType.cpp @@ -5,7 +5,6 @@ #include #include -#include #include std::ostream& osc::operator<<(std::ostream& out, VariantType variant_type)