material-foundation / material-color-utilities

Color libraries for Material You
Apache License 2.0
1.57k stars 134 forks source link

Missing include for round() usage in tones.cc #108

Open jdapena opened 11 months ago

jdapena commented 11 months ago

Chromium build with GCC/libstdc++ library because of missing include in tones.cc. It should include math.h.

FAILED: obj/third_party/material_color_utilities/material_color_utilities/tones.o 
g++ -MMD -MF obj/third_party/material_color_utilities/material_color_utilities/tones.o.d -DUSE_UDEV -DUSE_AURA=1 -DUSE_GLIB=1 -DUSE_OZONE=1 -DOFFICIAL_BUILD -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D_FORTIFY_SOURCE=2 -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -DNO_UNWIND_TABLES -DNDEBUG -DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -I../.. -Igen -I../../third_party/material_color_utilities/src -I../../third_party/abseil-cpp -Wall -Wno-unused-local-typedefs -Wno-maybe-uninitialized -Wno-deprecated-declarations -Wno-comments -Wno-packed-not-aligned -Wno-missing-field-initializers -Wno-unused-parameter -Wno-psabi -fno-ident -fno-strict-aliasing --param=ssp-buffer-size=4 -fstack-protector -fno-unwind-tables -fno-asynchronous-unwind-tables -fPIC -pipe -pthread -m64 -msse3 -O2 -fdata-sections -ffunction-sections -fno-omit-frame-pointer -gdwarf-4 -g2 -gsplit-dwarf -fvisibility=hidden -Wno-narrowing -Wno-class-memaccess -std=gnu++2a -fno-exceptions -fno-rtti -fvisibility-inlines-hidden -c ../../third_party/material_color_utilities/src/cpp/palettes/tones.cc -o obj/third_party/material_color_utilities/material_color_utilities/tones.o
../../third_party/material_color_utilities/src/cpp/palettes/tones.cc: In member function ‘material_color_utilities::Hct material_color_utilities::TonalPalette::createKeyColor(double, double)’:
../../third_party/material_color_utilities/src/cpp/palettes/tones.cc:70:9: error: ‘round’ was not declared in this scope
   70 |     if (round(chroma) == round(smallest_delta_hct.get_chroma())) {
      |         ^~~~~
jdapena commented 11 months ago

I would contribute the PR with the fix myself, but contributing guidelines are quite explicit about not accepting external contributions (even under Google CLA, I guess?).

dilinger commented 9 months ago

It's not just the missing header; it's also missing the std:: namespace for calls to abs(); at least with an older clang (14) and newer libstdc++ (13). Here's the patch that I wrote, but the CONTRIBUTING text saying they're not looking for external contributions is.. interesting.

--- a/cpp/palettes/tones.cc
+++ b/cpp/palettes/tones.cc
@@ -16,6 +16,8 @@

 #include "cpp/palettes/tones.h"

+#include <cmath>
+
 #include "cpp/cam/cam.h"
 #include "cpp/cam/hct.h"

@@ -55,7 +57,7 @@ Argb TonalPalette::get(double tone) cons
 Hct TonalPalette::createKeyColor(double hue, double chroma) {
   double start_tone = 50.0;
   Hct smallest_delta_hct(hue, chroma, start_tone);
-  double smallest_delta = abs(smallest_delta_hct.get_chroma() - chroma);
+  double smallest_delta = std::abs(smallest_delta_hct.get_chroma() - chroma);
   // Starting from T50, check T+/-delta to see if they match the requested
   // chroma.
   //
@@ -71,13 +73,13 @@ Hct TonalPalette::createKeyColor(double
       return smallest_delta_hct;
     }
     Hct hct_add(hue, chroma, start_tone + delta);
-    double hct_add_delta = abs(hct_add.get_chroma() - chroma);
+    double hct_add_delta = std::abs(hct_add.get_chroma() - chroma);
     if (hct_add_delta < smallest_delta) {
       smallest_delta = hct_add_delta;
       smallest_delta_hct = hct_add;
     }
     Hct hct_subtract(hue, chroma, start_tone - delta);
-    double hct_subtract_delta = abs(hct_subtract.get_chroma() - chroma);
+    double hct_subtract_delta = std::abs(hct_subtract.get_chroma() - chroma);
     if (hct_subtract_delta < smallest_delta) {
       smallest_delta = hct_subtract_delta;
       smallest_delta_hct = hct_subtract;