[feat] Apply @jfkthame reviews

src/hb-aat-layout-feat-table.hh

@@ -84,14 +84,13 @@ struct FeatureName
 <<<<<<< HEAD
 =======
   inline unsigned int get_settings (const feat                     *feat,
-				    hb_bool_t                      *is_exclusive,
+				    hb_aat_feature_setting_t       *default_setting,
 				    unsigned int                    start_offset,
 				    unsigned int                   *records_count,
 				    hb_aat_feature_option_record_t *records_buffer) const
   {
     bool exclusive = featureFlags & Exclusive;
     bool not_default = featureFlags & NotDefault;
-    if (is_exclusive) *is_exclusive = exclusive;
     const UnsizedArrayOf<SettingName>& settings = feat+settingTable;
     unsigned int len = 0;
     unsigned int settings_count = nSettings;
@@ -100,15 +99,19 @@ struct FeatureName
       len = MIN (settings_count - start_offset, *records_count);
       for (unsigned int i = 0; i < len; i++)
       {
-	records_buffer[i].is_default = exclusive && not_default &&
-				       i + start_offset == (featureFlags & IndexMask);
 	records_buffer[i].name_id = settings[start_offset + i].nameIndex;
 	records_buffer[i].setting = settings[start_offset + i].setting;
       }
-      if (exclusive && !not_default && start_offset == 0 && len != 0)
-        records_buffer[0].is_default = true;
     }
-    if (is_exclusive) *is_exclusive = exclusive;
+    if (default_setting)
+    {
+      if (exclusive)
+      {
+	if (settings_count && !not_default) *default_setting = settings[0].setting;
+	else if (not_default) *default_setting = featureFlags & IndexMask;
+      }
+      else *default_setting = HB_AAT_FEATURE_NO_DEFAULT_INDEX;
+    }
     if (records_count) *records_count = len;
     return settings_count;
   }
@@ -151,14 +154,14 @@ struct feat
     return feature ? *feature : Null (FeatureName);
   }
 
-  inline unsigned int get_settings (hb_aat_feature_type_t           key,
-				    hb_bool_t                      *is_exclusive,
+  inline unsigned int get_settings (hb_aat_feature_type_t           type,
+				    hb_aat_feature_setting_t       *default_setting, /* OUT.     May be NULL. */
 				    unsigned int                    start_offset,
-				    unsigned int                   *records_count,
+				    unsigned int                   *records_count,   /* IN/OUT.  May be NULL. */
 				    hb_aat_feature_option_record_t *records_buffer) const
   {
-    return get_feature (key).get_settings (this, is_exclusive, start_offset,
-					   records_count, records_buffer);
+    return get_feature (type).get_settings (this, default_setting,
+					    start_offset, records_count, records_buffer);
   }
 
   inline bool sanitize (hb_sanitize_context_t *c) const

src/hb-aat-layout.cc

@@ -308,12 +308,13 @@ _hb_aat_language_get (hb_face_t *face,
 
 /**
  * hb_aat_get_feature_settings:
- * @face:           a font face.
- * @identifier:     aat feature id you are querying.
- * @is_exclusive:   (out):    is only one of the features can be enabled.
- * @start_offset:   start offset, if you are iterating
- * @records_count:  (inout): gets input buffer size, puts number of filled one
- * @records_buffer: (out):  buffer of records
+ * @face:            a font face.
+ * @identifier:      aat feature id you are querying.
+ * @default_setting: (out): default value for the type. If it is HB_AAT_FEATURE_NO_DEFAULT_INDEX
+ *                          means non is default and it is not exclusive also.
+ * @start_offset:    start offset, if you are iterating
+ * @records_count:   (inout): gets input buffer size, puts number of filled one
+ * @records_buffer:  (out):  buffer of records
  *
  * Returns: Total number of records available for the feature.
  *
@@ -322,11 +323,11 @@ _hb_aat_language_get (hb_face_t *face,
 unsigned int
 hb_aat_get_feature_settings (hb_face_t                      *face,
 			     hb_aat_feature_type_t           identifier,
-			     hb_bool_t                      *is_exclusive,
+			     hb_aat_feature_setting_t       *default_setting, /* OUT.     May be NULL. */
 			     unsigned int                    start_offset,
-			     unsigned int                   *records_count, /* IN/OUT.  May be NULL. */
-			     hb_aat_feature_option_record_t *records_buffer /* OUT.     May be NULL. */)
+			     unsigned int                   *records_count,   /* IN/OUT.  May be NULL. */
+			     hb_aat_feature_option_record_t *records_buffer   /* OUT.     May be NULL. */)
 {
-  return _get_feat (face).get_settings (identifier, is_exclusive, start_offset,
-					records_count, records_buffer);
+  return _get_feat (face).get_settings (identifier, default_setting,
+					start_offset, records_count, records_buffer);
 }

src/hb-aat-layout.h

@@ -57,18 +57,22 @@ typedef uint16_t hb_aat_feature_setting_t;
  **/
 typedef struct hb_aat_feature_option_record_t
 {
-  hb_bool_t is_default;
   hb_aat_feature_setting_t setting;
   hb_ot_name_id_t name_id;
 } hb_aat_feature_option_record_t;
 
+/*
+ * Since: REPLACEME
+ */
+#define HB_AAT_FEATURE_NO_DEFAULT_INDEX ((hb_aat_feature_setting_t) -1)
+
 HB_EXTERN unsigned int
 hb_aat_get_feature_settings (hb_face_t                      *face,
-			     hb_aat_feature_type_t           identifier,
-			     hb_bool_t                      *is_exclusive,
+			     hb_aat_feature_type_t           type,
+			     hb_aat_feature_setting_t       *default_setting, /* OUT.     May be NULL. */
 			     unsigned int                    start_offset,
-			     unsigned int                   *records_count, /* IN/OUT.  May be NULL. */
-			     hb_aat_feature_option_record_t *records_buffer /* OUT.     May be NULL. */);
+			     unsigned int                   *records_count,   /* IN/OUT.  May be NULL. */
+			     hb_aat_feature_option_record_t *records_buffer   /* OUT.     May be NULL. */);
 
 HB_END_DECLS
 

test/api/test-aat-layout.c

@@ -31,49 +31,55 @@
 static void
 test_aat_get_feature_settings (void)
 {
-  hb_bool_t is_exclusive;
+  hb_aat_feature_setting_t default_setting;
   hb_aat_feature_option_record_t records[3];
   unsigned int count = 3;
 
   hb_face_t *face = hb_test_open_font_file ("fonts/aat-feat.ttf");
 
-  g_assert_cmpuint (4, ==, hb_aat_get_feature_settings (face, 18, &is_exclusive,
+  g_assert_cmpuint (4, ==, hb_aat_get_feature_settings (face, 18, &default_setting,
 							0, &count, records));
   g_assert_cmpuint (3, ==, count);
-  g_assert (is_exclusive);
+  g_assert_cmpuint (0, ==, default_setting);
 
-  g_assert_cmpuint (1, ==, records[0].is_default);
   g_assert_cmpuint (0, ==, records[0].setting);
   g_assert_cmpuint (294, ==, records[0].name_id);
 
-  g_assert_cmpuint (0, ==, records[1].is_default);
   g_assert_cmpuint (1, ==, records[1].setting);
   g_assert_cmpuint (295, ==, records[1].name_id);
 
-  g_assert_cmpuint (0, ==, records[2].is_default);
   g_assert_cmpuint (2, ==, records[2].setting);
   g_assert_cmpuint (296, ==, records[2].name_id);
 
   count = 3;
-  g_assert_cmpuint (4, ==, hb_aat_get_feature_settings (face, 18, &is_exclusive,
+  g_assert_cmpuint (4, ==, hb_aat_get_feature_settings (face, 18, &default_setting,
 							3, &count, records));
   g_assert_cmpuint (1, ==, count);
-  g_assert (is_exclusive);
+  g_assert_cmpuint (0, ==, default_setting);
 
-  g_assert_cmpuint (0, ==, records[0].is_default);
   g_assert_cmpuint (3, ==, records[0].setting);
   g_assert_cmpuint (297, ==, records[0].name_id);
 
+
+  count = 1;
+  g_assert_cmpuint (1, ==, hb_aat_get_feature_settings (face, 14, &default_setting,
+							0, &count, records));
+  g_assert_cmpuint (1, ==, count);
+  g_assert_cmpuint (HB_AAT_FEATURE_NO_DEFAULT_INDEX, ==, default_setting);
+
+  g_assert_cmpuint (8, ==, records[0].setting);
+  g_assert_cmpuint (308, ==, records[0].name_id);
+
+
   count = 100;
-  g_assert_cmpuint (0, ==, hb_aat_get_feature_settings (face, 32, &is_exclusive,
+  g_assert_cmpuint (0, ==, hb_aat_get_feature_settings (face, 32, NULL,
 							0, &count, records));
   g_assert_cmpuint (0, ==, count);
-  g_assert (!is_exclusive);
 
   hb_face_destroy (face);
 
   hb_face_t *sbix = hb_test_open_font_file ("fonts/chromacheck-sbix.ttf");
-  g_assert_cmpuint (0, ==, hb_aat_get_feature_settings (face, 100, &is_exclusive,
+  g_assert_cmpuint (0, ==, hb_aat_get_feature_settings (sbix, 100, NULL,
 							0, &count, records));
   hb_face_destroy (sbix);
 }