From 6fdaee3cfef1ab79f8cd4ac4844c0341860f95c0 Mon Sep 17 00:00:00 2001
From: Filip Sykala <filip.sykala@prusa3d.cz>
Date: Mon, 7 Mar 2022 12:16:24 +0100
Subject: [PATCH] Fix data type of collection

Fix warnings
../src/libslic3r/Emboss.cpp:135:24: warning: comparison of integer expressions of different signedness: 'int' and 'const unsigned int' [-Wsign-compare]
../src/libslic3r/Emboss.cpp:653:20: warning: comparison of integer expressions of different signedness: 'int' and 'unsigned int' [-Wsign-compare]
../src/slic3r/GUI/Gizmos/GLGizmoEmboss.cpp:2057:12: warning: unused variable 'count_icons' [-Wunused-variable]
../src/slic3r/GUI/Gizmos/GLGizmoEmboss.cpp:2058:12: warning: unused variable 'count_states' [-Wunused-variable]
---
 src/libslic3r/Emboss.cpp                  |  4 ++--
 src/libslic3r/Emboss.hpp                  |  2 +-
 src/libslic3r/Format/3mf.cpp              |  2 +-
 src/libslic3r/TextConfiguration.hpp       |  2 +-
 src/slic3r/GUI/Gizmos/GLGizmoEmboss.cpp   | 12 ++++++------
 src/slic3r/Utils/FontListSerializable.cpp | 12 ++++++++++++
 src/slic3r/Utils/FontListSerializable.hpp |  1 +
 tests/libslic3r/test_emboss.cpp           | 12 +++++++++---
 8 files changed, 33 insertions(+), 14 deletions(-)

diff --git a/src/libslic3r/Emboss.cpp b/src/libslic3r/Emboss.cpp
index 8e7d40259..d60cf0f74 100644
--- a/src/libslic3r/Emboss.cpp
+++ b/src/libslic3r/Emboss.cpp
@@ -130,7 +130,7 @@ std::optional<Emboss::Glyph> Private::get_glyph(
         return glyph_item->second;
     
     if (!font_info_opt.has_value()) {
-        int font_index = font_prop.collection_number.has_value()?
+        unsigned int font_index = font_prop.collection_number.has_value()?
             *font_prop.collection_number : 0;
         if (font_index >= font.count) return {};
         font_info_opt  = Private::load_font_info(font.data->data(), font_index);
@@ -648,7 +648,7 @@ void Emboss::apply_transformation(const FontProp &font_prop,
     }
 }
 
-bool Emboss::is_italic(FontFile &font, int font_index)
+bool Emboss::is_italic(FontFile &font, unsigned int font_index)
 {
     if (font_index >= font.count) return false;
     std::optional<stbtt_fontinfo> font_info_opt = Private::load_font_info(font.data->data(), font_index);
diff --git a/src/libslic3r/Emboss.hpp b/src/libslic3r/Emboss.hpp
index fd3b8c8aa..e6ba022b4 100644
--- a/src/libslic3r/Emboss.hpp
+++ b/src/libslic3r/Emboss.hpp
@@ -168,7 +168,7 @@ public:
     /// <param name="font">Selector of font</param>
     /// <param name="font_index">Index of font in collection</param>
     /// <returns>True when the font description contains italic/obligue otherwise False</returns>
-    static bool is_italic(FontFile &font, int font_index = 0);
+    static bool is_italic(FontFile &font, unsigned int font_index);
 
     /// <summary>
     /// Project 2d point into space
diff --git a/src/libslic3r/Format/3mf.cpp b/src/libslic3r/Format/3mf.cpp
index bd870cf8c..745d1310a 100644
--- a/src/libslic3r/Format/3mf.cpp
+++ b/src/libslic3r/Format/3mf.cpp
@@ -3365,7 +3365,7 @@ std::optional<TextConfiguration> TextConfigurationSerialization::read(const char
     if (std::fabs(angle) > std::numeric_limits<float>::epsilon())
         fp.angle = angle;
     int collection_number = get_attribute_value_int(attributes, num_attributes, COLLECTION_NUMBER_ATTR);
-    if (collection_number > 0) fp.collection_number = collection_number;
+    if (collection_number > 0) fp.collection_number = static_cast<unsigned int>(collection_number);
 
     fp.size_in_mm = get_attribute_value_float(attributes, num_attributes, LINE_HEIGHT_ATTR);
     fp.emboss = get_attribute_value_float(attributes, num_attributes, DEPTH_ATTR);
diff --git a/src/libslic3r/TextConfiguration.hpp b/src/libslic3r/TextConfiguration.hpp
index 160493453..98822e639 100644
--- a/src/libslic3r/TextConfiguration.hpp
+++ b/src/libslic3r/TextConfiguration.hpp
@@ -45,7 +45,7 @@ struct FontProp
 
     // Parameter for True Type Font collections
     // Select index of font in collection
-    std::optional<int> collection_number;
+    std::optional<unsigned int> collection_number;
 
     //enum class Align {
     //    left,
diff --git a/src/slic3r/GUI/Gizmos/GLGizmoEmboss.cpp b/src/slic3r/GUI/Gizmos/GLGizmoEmboss.cpp
index 7624de78c..a17f28a3e 100644
--- a/src/slic3r/GUI/Gizmos/GLGizmoEmboss.cpp
+++ b/src/slic3r/GUI/Gizmos/GLGizmoEmboss.cpp
@@ -1224,8 +1224,9 @@ void GLGizmoEmboss::draw_style_list() {
         m_font_manager     = FontManager(m_imgui->get_glyph_ranges());
         FontList font_list = create_default_font_list();
         m_font_manager.add_fonts(font_list);
-        // TODO: What to do when default fonts are not loadable?
+        // TODO: What to do when NO one default font is loadable?
         bool success = m_font_manager.load_first_valid_font();
+        assert(success);
         select_stored_font_item();
         process();
     }else if (ImGui::IsItemHovered()) 
@@ -1608,7 +1609,7 @@ void GLGizmoEmboss::draw_advanced()
         ", unitPerEm=" + std::to_string(font_file->unit_per_em) + 
         ", cache(" + std::to_string(cache_size) + " glyphs)";
     if (font_file->count > 1) { 
-        int collection = font_prop.collection_number.has_value() ?
+        unsigned int collection = font_prop.collection_number.has_value() ?
             *font_prop.collection_number : 0;
         ff_property += ", collect=" + std::to_string(collection+1) + "/" + std::to_string(font_file->count);
     }
@@ -1821,8 +1822,10 @@ bool GLGizmoEmboss::choose_font_by_wxdialog()
     }
 
     // fix dynamic creation of italic font
+    const auto& cn = m_font_manager.get_font_prop().collection_number;
+    unsigned int font_collection = cn.has_value() ? *cn : 0;
     if (WxFontUtils::is_italic(wx_font) &&
-        !Emboss::is_italic(*m_font_manager.get_font_file())) {
+        !Emboss::is_italic(*m_font_manager.get_font_file(), font_collection)) {
         m_font_manager.get_font_item().prop.skew = 0.2;
     }
     return true;
@@ -2053,9 +2056,6 @@ void GLGizmoEmboss::draw_icon(IconType icon, IconState state)
     if ((icons_texture_id == 0) || (tex_width <= 1) || (tex_height <= 1))
         return;
     ImTextureID tex_id = (void *) (intptr_t) (GLuint) icons_texture_id;
-
-    size_t count_icons  = static_cast<size_t>(IconType::_count);
-    size_t count_states = 3; // activable | hovered | disabled
     int start_x = static_cast<unsigned>(state) * (icon_width + 1) + 1,
         start_y = static_cast<unsigned>(icon) * (icon_width + 1) + 1;
 
diff --git a/src/slic3r/Utils/FontListSerializable.cpp b/src/slic3r/Utils/FontListSerializable.cpp
index 920b91ad3..802cc2185 100644
--- a/src/slic3r/Utils/FontListSerializable.cpp
+++ b/src/slic3r/Utils/FontListSerializable.cpp
@@ -50,6 +50,18 @@ bool FontListSerializable::read(const std::map<std::string, std::string>& sectio
     return true;
 }
 
+bool FontListSerializable::read(const std::map<std::string, std::string>& section, const std::string& key, std::optional<unsigned int>& value){
+    auto item = section.find(key);
+    if (item == section.end()) return false;
+    const std::string &data = item->second;
+    if (data.empty()) return false;
+    int value_ = std::atoi(data.c_str());
+    if (value_ <= 0) return false;
+
+    value = static_cast<unsigned int>(value_);
+    return true;
+}
+
 bool FontListSerializable::read(const std::map<std::string, std::string>& section, const std::string& key, std::optional<float>& value){
     auto item = section.find(key);
     if (item == section.end()) return false;
diff --git a/src/slic3r/Utils/FontListSerializable.hpp b/src/slic3r/Utils/FontListSerializable.hpp
index a47ef149c..607efef85 100644
--- a/src/slic3r/Utils/FontListSerializable.hpp
+++ b/src/slic3r/Utils/FontListSerializable.hpp
@@ -39,6 +39,7 @@ private:
     // TODO: move to app config like read from section
     static bool read(const std::map<std::string, std::string>& section, const std::string& key, float& value);
     static bool read(const std::map<std::string, std::string>& section, const std::string& key, std::optional<int>& value);
+    static bool read(const std::map<std::string, std::string>& section, const std::string& key, std::optional<unsigned int>& value);
     static bool read(const std::map<std::string, std::string>& section, const std::string& key, std::optional<float>& value);
 };
 } // namespace Slic3r
diff --git a/tests/libslic3r/test_emboss.cpp b/tests/libslic3r/test_emboss.cpp
index 2844c233f..14b723807 100644
--- a/tests/libslic3r/test_emboss.cpp
+++ b/tests/libslic3r/test_emboss.cpp
@@ -104,7 +104,6 @@ std::string get_font_filepath() {
 TEST_CASE("Read glyph C shape from font, stb library calls ONLY", "[Emboss]") {
     std::string font_path = get_font_filepath();
     char  letter   = 'C';
-    float flatness = 2.;
     
     // Read  font file
     FILE *file = fopen(font_path.c_str(), "rb");
@@ -221,13 +220,16 @@ TEST_CASE("triangle intersection", "[]")
 #include <iostream>
 #include <filesystem>
 namespace fs = std::filesystem;
-TEST_CASE("Italic check", "[]") 
+// Check function Emboss::is_italic that exist some italic and some non-italic font.
+TEST_CASE("Italic check", "[Emboss]") 
 {  
     std::queue<std::string> dir_paths;
 #ifdef _WIN32
     dir_paths.push("C:/Windows/Fonts");
 #elif defined(__linux__)
     dir_paths.push("/usr/share/fonts");
+//#elif defined(__APPLE__)
+//    dir_paths.push("//System/Library/Fonts");
 #endif
     bool exist_italic = false;
     bool exist_non_italic = false;
@@ -247,10 +249,14 @@ TEST_CASE("Italic check", "[]")
             std::string path_str = act_path.u8string();
             auto        font_opt = Emboss::create_font_file(path_str.c_str());
             if (font_opt == nullptr) continue;
-            if (Emboss::is_italic(*font_opt))
+
+            unsigned int collection_number = 0;
+            if (Emboss::is_italic(*font_opt, collection_number))
                 exist_italic = true;
             else
                 exist_non_italic = true;
+
+            if (exist_italic && exist_non_italic) break;
             //std::cout << ((Emboss::is_italic(*font_opt)) ? "[yes] " : "[no ] ") << entry.path() << std::endl;
         }
     }