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)! This would break if someone uses py ops with lower precedence than '+' (like bitwise operations, and comparison), but these are not expected usecase here anyway. Reviewers: campbellbarton Differential Revision: https://developer.blender.org/D340
This commit is contained in:
parent
21da42bd7a
commit
8535b9bd15
|
@ -493,11 +493,11 @@ static const char *unit_find_str(const char *str, const char *substr)
|
||||||
*
|
*
|
||||||
* "1m1cm+2mm" - Original value
|
* "1m1cm+2mm" - Original value
|
||||||
* "1*1#1*0.01#+2*0.001#" - Replace numbers
|
* "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)
|
static bool ch_is_op(char op)
|
||||||
{
|
{
|
||||||
switch (op) {
|
switch (op) {
|
||||||
|
@ -589,6 +589,34 @@ static int unit_find(const char *str, bUnitDef *unit)
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
static bUnitDef *unit_detect_from_str(bUnitCollection *usys, const char *str, const char *str_prev)
|
||||||
|
{
|
||||||
|
/* Try to find a default unit from current or previous 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. */
|
||||||
|
bUnitDef *unit = NULL;
|
||||||
|
|
||||||
|
/* 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);
|
||||||
|
}
|
||||||
|
|
||||||
|
return unit;
|
||||||
|
}
|
||||||
|
|
||||||
/* make a copy of the string that replaces the units with numbers
|
/* make a copy of the string that replaces the units with numbers
|
||||||
* this is used before parsing
|
* this is used before parsing
|
||||||
* This is only used when evaluating user input and can afford to be a bit slower
|
* This is only used when evaluating user input and can afford to be a bit slower
|
||||||
|
@ -597,8 +625,8 @@ static int unit_find(const char *str, bUnitDef *unit)
|
||||||
* 10.1km -> 10.1*1000.0
|
* 10.1km -> 10.1*1000.0
|
||||||
* ...will be resolved by python.
|
* ...will be resolved by python.
|
||||||
*
|
*
|
||||||
* values will be split by a comma's
|
* values will be split by an add sign
|
||||||
* 5'2" -> 5*0.3048, 2*0.0254
|
* 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.
|
* str_prev is optional, when valid it is used to get a base unit when none is set.
|
||||||
*
|
*
|
||||||
|
@ -608,7 +636,8 @@ int bUnit_ReplaceString(char *str, int len_max, const char *str_prev, double sca
|
||||||
{
|
{
|
||||||
bUnitCollection *usys = unit_get_system(system, type);
|
bUnitCollection *usys = unit_get_system(system, type);
|
||||||
|
|
||||||
bUnitDef *unit;
|
bUnitDef *unit = NULL, *default_unit;
|
||||||
|
double scale_pref_base = scale_pref;
|
||||||
char str_tmp[TEMP_STR_SIZE];
|
char str_tmp[TEMP_STR_SIZE];
|
||||||
int changed = 0;
|
int changed = 0;
|
||||||
|
|
||||||
|
@ -619,15 +648,35 @@ int bUnit_ReplaceString(char *str, int len_max, const char *str_prev, double sca
|
||||||
/* make lowercase */
|
/* make lowercase */
|
||||||
BLI_ascii_strtolower(str, len_max);
|
BLI_ascii_strtolower(str, len_max);
|
||||||
|
|
||||||
|
/* Try to find a default unit from current or previous string. */
|
||||||
|
default_unit = unit_detect_from_str(usys, str, str_prev);
|
||||||
|
|
||||||
|
/* We apply the default unit to the whole expression (default unit is now the reference '1.0' one). */
|
||||||
|
scale_pref_base *= 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++) {
|
for (unit = usys->units; unit->name; unit++) {
|
||||||
/* in case there are multiple instances */
|
/* 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_base, unit))
|
||||||
changed = true;
|
changed = true;
|
||||||
}
|
}
|
||||||
unit = NULL;
|
unit = NULL;
|
||||||
|
|
||||||
{
|
{
|
||||||
/* try other unit systems now, so we can evaluate imperial when metric is set for eg. */
|
/* 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;
|
bUnitCollection *usys_iter;
|
||||||
int system_iter;
|
int system_iter;
|
||||||
|
|
||||||
|
@ -638,7 +687,7 @@ int bUnit_ReplaceString(char *str, int len_max, const char *str_prev, double sca
|
||||||
for (unit = usys_iter->units; unit->name; unit++) {
|
for (unit = usys_iter->units; unit->name; unit++) {
|
||||||
int ofs = 0;
|
int ofs = 0;
|
||||||
/* in case there are multiple instances */
|
/* 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_base, unit)))
|
||||||
changed = true;
|
changed = true;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -647,35 +696,9 @@ int bUnit_ReplaceString(char *str, int len_max, const char *str_prev, double sca
|
||||||
}
|
}
|
||||||
unit = NULL;
|
unit = NULL;
|
||||||
|
|
||||||
if (changed == 0) {
|
/* replace # with add sign when there is no operator between it and the next number
|
||||||
/* 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
|
|
||||||
*
|
*
|
||||||
* "1*1# 3*100# * 3" -> "1 *1, 3 *100 * 3"
|
* "1*1# 3*100# * 3" -> "1*1+ 3*100 * 3"
|
||||||
*
|
*
|
||||||
* */
|
* */
|
||||||
{
|
{
|
||||||
|
@ -683,25 +706,19 @@ int bUnit_ReplaceString(char *str, int len_max, const char *str_prev, double sca
|
||||||
const char *ch = str;
|
const char *ch = str;
|
||||||
|
|
||||||
while ((str_found = strchr(str_found, SEP_CHR))) {
|
while ((str_found = strchr(str_found, SEP_CHR))) {
|
||||||
|
bool op_found = false;
|
||||||
|
|
||||||
int op_found = 0;
|
/* any operators after this? */
|
||||||
/* any operators after this?*/
|
|
||||||
for (ch = str_found + 1; *ch != '\0'; ch++) {
|
for (ch = str_found + 1; *ch != '\0'; ch++) {
|
||||||
|
|
||||||
if (*ch == ' ' || *ch == '\t') {
|
if (*ch == ' ' || *ch == '\t') {
|
||||||
/* do nothing */
|
continue;
|
||||||
}
|
|
||||||
else if (ch_is_op(*ch) || *ch == ',') { /* found an op, no need to insert a ',' */
|
|
||||||
op_found = 1;
|
|
||||||
break;
|
|
||||||
}
|
|
||||||
else { /* found a non-op character */
|
|
||||||
op_found = 0;
|
|
||||||
break;
|
|
||||||
}
|
}
|
||||||
|
op_found = (ch_is_op(*ch) || ELEM(*ch, ',', ')'));
|
||||||
|
break;
|
||||||
}
|
}
|
||||||
|
|
||||||
*str_found++ = op_found ? ' ' : ',';
|
/* If found an op, comma or closing parenthesis, no need to insert a '+', else we need it. */
|
||||||
|
*str_found++ = op_found ? ' ' : '+';
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -23,7 +23,10 @@ class UnitsTesting(unittest.TestCase):
|
||||||
('METRIC', 'LENGTH', "33.3dm", "1", 0.1),
|
('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.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', "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
|
# From 'internal' Blender value to user-friendly printing
|
||||||
|
|
Loading…
Reference in New Issue