Commit: 47983ccec5bad1cceb34183aa00fcf31d500649e Author: Bastien Montagne Date: Wed Feb 19 20:28:31 2014 +0100 Branches: temp-units-refactor https://developer.blender.org/rB47983ccec5bad1cceb34183aa00fcf31d500649e
Fix T38722: Adding units in Imperial setting results in inconsistent values Now always check for a default unit, and evaluate the whole expression in this "unit space". Not an ideal solution, but should handle most cases nicely (we can't address all possible corner cases anyway). Note default unit is searched in current string first (bigger unit of current system wins), then in previous string. Note this also replaces ',' by '+' in default separation between units, helps solving issues with parenthesis (e.g. (1'1")*2.5 would fail in existing code)! Reviewers: campbellbarton Differential Revision: https://developer.blender.org/D340 =================================================================== M source/blender/blenkernel/intern/unit.c M tests/python/bl_pyapi_units.py =================================================================== diff --git a/source/blender/blenkernel/intern/unit.c b/source/blender/blenkernel/intern/unit.c index f7024de..868b7a1 100644 --- a/source/blender/blenkernel/intern/unit.c +++ b/source/blender/blenkernel/intern/unit.c @@ -493,11 +493,11 @@ static const char *unit_find_str(const char *str, const char *substr) * * "1m1cm+2mm" - Original value * "1*1#1*0.01#+2*0.001#" - Replace numbers - * "1*1,1*0.01 +2*0.001 " - Add comma's if ( - + * / % ^ < > ) not found in between + * "1*1+1*0.01 +2*0.001 " - Add add signs if ( + - * / | & ~ < > ^ ! = % ) not found in between * */ -/* not too strict, (- = * /) are most common */ +/* not too strict, (+ - * /) are most common */ static bool ch_is_op(char op) { switch (op) { @@ -597,8 +597,8 @@ static int unit_find(const char *str, bUnitDef *unit) * 10.1km -> 10.1*1000.0 * ...will be resolved by python. * - * values will be split by a comma's - * 5'2" -> 5*0.3048, 2*0.0254 + * values will be split by an add sign + * 5'2" -> 5*0.3048 + 2*0.0254 * * str_prev is optional, when valid it is used to get a base unit when none is set. * @@ -608,7 +608,8 @@ int bUnit_ReplaceString(char *str, int len_max, const char *str_prev, double sca { bUnitCollection *usys = unit_get_system(system, type); - bUnitDef *unit; + bUnitDef *unit = NULL, *default_unit; + double scale_pref_corr = scale_pref; char str_tmp[TEMP_STR_SIZE]; int changed = 0; @@ -619,15 +620,54 @@ int bUnit_ReplaceString(char *str, int len_max, const char *str_prev, double sca /* make lowercase */ BLI_ascii_strtolower(str, len_max); + /* Try to find a default unit from current string. + * This allows us to handle cases like 2 + 2mm, people would expect to get 4mm, not 2.002m! + * Note this does not handle corner cases like 2 + 2cm + 1 + 2.5mm... We can't support everything. */ + /* see which units the new value has */ + for (unit = usys->units; unit->name; unit++) { + if (unit_find(str, unit)) + break; + } + /* Else, try to infer the default unit from the previous string. */ + if (str_prev && (unit == NULL || unit->name == NULL)) { + /* see which units the original value had */ + for (unit = usys->units; unit->name; unit++) { + if (unit_find(str_prev, unit)) + break; + } + } + /* Else, fall back to default unit. */ + if (unit == NULL || unit->name == NULL) { + unit = unit_default(usys); + } + default_unit = unit; + + /* We apply the default unit to the whole expression (default unit is now the reference '1.0' one). */ + scale_pref_corr *= default_unit->scalar; + + /* Apply the default unit on the whole expression, this allows to handle nasty cases like '2+2in'. */ + if (BLI_snprintf(str_tmp, sizeof(str_tmp), "(%s)*%.9g", str, default_unit->scalar) < sizeof(str_tmp)) { + strncpy(str, str_tmp, len_max); + } + else { + /* BLI_snprintf would not fit into str_tmp, cant do much in this case + * check for this because otherwise bUnit_ReplaceString could call its self forever */ + return 0; + } + for (unit = usys->units; unit->name; unit++) { /* in case there are multiple instances */ - while (unit_replace(str, len_max, str_tmp, scale_pref, unit)) + while (unit_replace(str, len_max, str_tmp, scale_pref_corr, unit)) changed = true; } unit = NULL; { /* try other unit systems now, so we can evaluate imperial when metric is set for eg. */ + /* Note that checking other systems at that point means we do not support their units as 'default' one. + * In other words, when in metrics, typing '2+2in' will give 2 meters 2 inches, not 4 inches. + * I do think this is the desired behavior! + */ bUnitCollection *usys_iter; int system_iter; @@ -638,7 +678,7 @@ int bUnit_ReplaceString(char *str, int len_max, const char *str_prev, double sca for (unit = usys_iter->units; unit->name; unit++) { int ofs = 0; /* in case there are multiple instances */ - while ((ofs = unit_replace(str + ofs, len_max - ofs, str_tmp, scale_pref, unit))) + while ((ofs = unit_replace(str + ofs, len_max - ofs, str_tmp, scale_pref_corr, unit))) changed = true; } } @@ -647,35 +687,9 @@ int bUnit_ReplaceString(char *str, int len_max, const char *str_prev, double sca } unit = NULL; - if (changed == 0) { - /* no units given so infer a unit from the previous string or default */ - if (str_prev) { - /* see which units the original value had */ - for (unit = usys->units; unit->name; unit++) { - if (unit_find(str_prev, unit)) - break; - } - } - - if (unit == NULL || unit->name == NULL) - unit = unit_default(usys); - - /* add the unit prefix and re-run, use brackets in case there was an expression given */ - if (BLI_snprintf(str_tmp, sizeof(str_tmp), "(%s)%s", str, unit->name) < sizeof(str_tmp)) { - strncpy(str, str_tmp, len_max); - return bUnit_ReplaceString(str, len_max, NULL, scale_pref, system, type); - } - else { - /* BLI_snprintf would not fit into str_tmp, cant do much in this case - * check for this because otherwise bUnit_ReplaceString could call its self forever */ - return 0; - } - - } - - /* replace # with commas when there is no operator between it and the next number + /* replace # with add sign when there is no operator between it and the next number * - * "1*1# 3*100# * 3" -> "1 *1, 3 *100 * 3" + * "1*1# 3*100# * 3" -> "1*1+ 3*100 * 3" * * */ { @@ -691,7 +705,8 @@ int bUnit_ReplaceString(char *str, int len_max, const char *str_prev, double sca if (*ch == ' ' || *ch == '\t') { /* do nothing */ } - else if (ch_is_op(*ch) || *ch == ',') { /* found an op, no need to insert a ',' */ + else if (ch_is_op(*ch) || ELEM(*ch, ',', ')')) { + /* found an op, comma or closing parenthesis, no need to insert a '+' */ op_found = 1; break; } @@ -701,7 +716,7 @@ int bUnit_ReplaceString(char *str, int len_max, const char *str_prev, double sca } } - *str_found++ = op_found ? ' ' : ','; + *str_found++ = op_found ? ' ' : '+'; } } diff --git a/tests/python/bl_pyapi_units.py b/tests/python/bl_pyapi_units.py index 478adef..a09a25e 100644 --- a/tests/python/bl_pyapi_units.py +++ b/tests/python/bl_pyapi_units.py @@ -23,7 +23,10 @@ class UnitsTesting(unittest.TestCase): ('METRIC', 'LENGTH', "33.3dm", "1", 0.1), ('IMPERIAL', 'LENGTH', "33.3cm", "1", 0.3048), # ref unit is not in IMPERIAL system, default to feet... ('IMPERIAL', 'LENGTH', "33.3ft", "1\"", 0.0254), # unused ref unit, since one is given already! - #('IMPERIAL', 'LENGTH', "", "1+1ft", 0.3048 * 2), # Will fail with current code! + ('IMPERIAL', 'LENGTH', "", "1+1ft", 0.3048 * 2), # default unit taken from current string (feet). + ('METRIC', 'LENGTH', "", "1+1ft", 1.3048), # no metric units, we default to meters. + ('IMPERIAL', 'LENGTH', "", "3+1in+1ft", 0.3048 * 4 + 0.0254), # bigger unit becomes default one! + ('IMPERIAL', 'LENGTH', "", "(3+1)in+1ft", 0.3048 + 0.0254 * 4), ) # From 'internal' Blender value to user-friendly printing _______________________________________________ Bf-blender-cvs mailing list [email protected] http://lists.blender.org/mailman/listinfo/bf-blender-cvs
