Page MenuHome

Fix T74552: Distribute negatives in number input
Needs ReviewPublic

Authored by Hans Goudey (HooglyBoogly) on Thu, May 21, 10:51 PM.

Details

Summary

This applies a relatively simple solution for fixing some unintuitive cases in the unit handling code.

Currently entering -1m50cm evaluates to -0.5m, and similarly 1'6" evaulates to just half a foot.
So effectively there's an implied + just between the numbers, which is quite confusing.

This works by adding parentheses so the negative distributes to the block of values before the next operator.

For example:

BeforeAfter
-1m50cm + 1m -2m50cm-(1m50cm) + 1m -(2m50cm)
-4m + 0.5 / -1.1-(4m) + 0.5 / -(1.1)
-4m + 0.5 / -1.1 + -7m50cm * 3-(4m) + 0.5 / -(1.1) + -(7m50cm) * 3
-1'6"-(1'6")
-5m50cm + 1m * 4 - sin(6)m + 6m50cm - 8m70cm-(5m50cm) + 1m * 4 - (sin(6)m) + 6m50cm - (8m70cm)
-1m50cm*2-(1m50cm)*2
-1m50cm-1m50cm-1m50cm-(1m50cm)-(1m50cm)-(1m50cm)
1e-2m1e-2m
-1.87E-3km-(1.87E-3km)
-1e-2cm-(1e-2cm)
-18 m * -17cm-(18 m) * -(17cm)

I don't see much of a downside to this, especially because it's a contained step and doesn't touch any of the other unit handling code.

Diff Detail

Repository
rB Blender
Branch
numinput-negative-distribution (branched from master)
Build Status
Buildable 8201
Build 8201: arc lint + arc unit

Event Timeline

Hans Goudey (HooglyBoogly) requested review of this revision.Thu, May 21, 10:51 PM
Hans Goudey (HooglyBoogly) created this revision.
Hans Goudey (HooglyBoogly) planned changes to this revision.Thu, May 21, 11:03 PM

Need to handle this case:

BeforeAfter
-1m50cm-1m50cm-1m50cm-(1m50cm)-1m50cm-(1m50cm)
  • Handle consecutive negative blocks properly
  • Remove print statements
Campbell Barton (campbellbarton) requested changes to this revision.EditedFri, May 22, 5:46 AM

Looks good,

Checking if there are any issues, and noticed scientific notation doesn't work when mixed with units, eg: -1e-2cm.
If that can be solved it would be be good, although I'm not sure we can reliably do this when some units can mix numbers with e too, the solution might end up being messy.

As long as this doesn't impact Python-only expressions, I don't think it's an issue. Double checked this doesn't give different results to Python, see: P1411.

Tests in tests/python/bl_pyapi_bpy_utils_units.py should be updated to include cases fixed by this patch.

source/blender/blenkernel/intern/unit.c
743

Try avoid functions casting const to non-const, in this case this function could return an int offset.

This revision now requires changes to proceed.Fri, May 22, 5:46 AM
  • Fix negative distribution in scientific notation
  • Also fix potential problem looking backward past the beginning of a string
Hans Goudey (HooglyBoogly) edited the summary of this revision. (Show Details)
Hans Goudey (HooglyBoogly) marked an inline comment as done.