Page MenuHome

Units: new 'Fixed Units' option
ClosedPublic

Authored by Jacques Lucke (JacquesLucke) on Sep 27 2018, 3:59 PM.

Details

Summary

Limitations:

  • does not work together with "Separate Units" yet
  • only for length, area, volume, mass and time for now

Diff Detail

Repository
rB Blender
Branch
fixed_units (branched from blender2.8)
Build Status
Buildable 2153
Build 2153: arc lint + arc unit

Event Timeline

Brecht Van Lommel (brecht) requested changes to this revision.EditedSep 28 2018, 12:24 PM

I think it would better to get rid of the "Fixed Units" setting, it feels a bit like a setting added onto a design rather than really integrated. Instead we could have an "Adaptive" item in the enums for each.

  • Unit System
  • Rotation: Degrees | Radians
  • Length: Adaptive | Meters | Centimers | ...
  • Time: Adaptive | Seconds | Minutes | ..
  • Mass: Adaptive | ...

Is getting Separate Units to work difficult? I would expect it to basically take the fixed unit as the maximum unit to display, but then it can still separate for smaller units.

This unit should also be used as the default unit when typing in values. Right now when you set Length to Centimeters and type in 10, it shows 1000cm which I think is unexpected. This should work in buttons and when typing values while transforming.

This revision now requires changes to proceed.Sep 28 2018, 12:24 PM

I see, will change the patch accordingly.

Will try to implement the other two features as well.

  • let user choose between adaptive and specific units for display

Is this better?

I did not start implementing the other two things you mentioned yet.

  • non-adaptive unit and Separate Units can be used at the same time now
  • use preferred unit if no other is specified when parsing user input

Looks great. Only thing I'm missing still is when you type G 10, then it still seems to use meters regardless of the chosen length unit.

Yes that is true, there is also another problem still, it does not take the unit scale into account.
Fill fix both issues.

  • take unit scale into account
  • use preferred unit in modal operators like grab

There is also this function which might need to be updated: https://docs.blender.org/api/current/bpy.utils.units.html?highlight=to_value#bpy.utils.units.to_value
Will have to check how to do it in a nice way...(like maybe you should be able to pass scene.unit_settings into it? (maybe you can do that already, haven't tested it)

Hi Jacques, Brecht, I have a few points I want to address from a user point of view:

Remove adaptive unit setting
What is the reasoning for keeping legacy behaviour? Adaptive nature of current unit system is why people do not use it.
It would be better for both maintainers and users with adaptive units removed.

Remove presets
No reason to keep them around anymore.

Unit system improvements look marvelous! Great job Jaques!

Thank you, glad you like it.

I'm not sure whether we should remove the adaptive behavior. But I guess you discussed it enough, so I'll remove it :)

Right, the presets should be removed, I did not even know they are there haha

We should keep adaptive, even if we change the default away from it. People have been using it and some like this behavior. It does not complicate the code much.

There is also this function which might need to be updated: https://docs.blender.org/api/current/bpy.utils.units.html?highlight=to_value#bpy.utils.units.to_value
Will have to check how to do it in a nice way...(like maybe you should be able to pass scene.unit_settings into it? (maybe you can do that already, haven't tested it)

I don't think you can, but that indeed seems like something we should support.

We should keep adaptive, even if we change the default away from it. People have been using it and some like this behavior.

Well people might not know better. It would be interesting to see how many of them would stick to adaptive after this patch goes in.

Then at least let's make "fixed units" the default for every unit type.

  • remove unit system presets
  • remove adaptive units
  • deduplicate string to number code (hopefully without breaking something...)
  • revert removal of adaptive units
  • make fixed units the default

I could not tell from the code changes, does units.to_string support "fixed units"?

In 2.7 i get this output:

>>> bpy.utils.units.to_string('METRIC', 'LENGTH', 1)
'1m'

>>> bpy.utils.units.to_string('METRIC', 'LENGTH', 0.001)
'1mm'

With this patch it would be useful to specify the unit:

>>> bpy.utils.units.to_string('METRIC', 'LENGTH', 'MILLIMETERS', 1)
'1000mm'

>>> bpy.utils.units.to_string('METRIC', 'LENGTH', 'MILLIMETERS', 0.001)
'1mm'

Add-ons like 3D Print Toolbox could use that, currently it's a bunch of ugly boilerplate code to do the same.

one thing that is a bit ugly is that I have to use +1 and -1 quite often due to the way units are organized.
A simple fix would be to use a signed data type in the dna, so that Adaptive could mean -1. Unfortunately it looks like signed char is not allowed in the dna.
I could use another type like short, but 2 bytes are not really needed for just ~10 different values.

I'm unsure about what's the best way to extend the to_string method. Just passing in an UnitSettings instance seems relatively easy. In the other side it would definitely be more helpful if an addon developer could use the function the way you describe it, but that would mean we need many kinds of different checks and conversions. (I think)

[EDIT:] Also this function call bpy.utils.units.to_string('METRIC', 'LENGTH', 'MILLIMETERS', 1) is very redundant. Everything could be derived from bpy.utils.units.to_string(1, 'MILLIMETERS'). However that might not work for all units, e.g. 'ton'...

to_string should not be affected by the scene settings (or it should be possible to override them). Or developers would be forced to change scene settings first, before using to_string if they need output in certain units (I hate so much when this happens).

...is very redundant. Everything could be derived from bpy.utils.units.to_string(1, 'MILLIMETERS')

That makes perfect sense!

However it does not make sooo much sense to have this function bpy.utils.units.to_string(1, 'MILLIMETERS') because all it does is appending mm to the number, basically. Also it is not clear what unit the 1 has.

one thing that is a bit ugly is that I have to use +1 and -1 quite often due to the way units are organized.
A simple fix would be to use a signed data type in the dna, so that Adaptive could mean -1. Unfortunately it looks like signed char is not allowed in the dna.
I could use another type like short, but 2 bytes are not really needed for just ~10 different values.

For a setting in the scene, a few bytes of memory usage is not a concern at all. Only for data that is e.g. per vertex do we really care.

Anyway, I think just set ADAPTIVE = 255 should work, or whatever the maximum value is if you increase the size.

I'm unsure about what's the best way to extend the to_string method. Just passing in an UnitSettings instance seems relatively easy. In the other side it would definitely be more helpful if an addon developer could use the function the way you describe it, but that would mean we need many kinds of different checks and conversions.

I think there are just two different use cases that require two different functions. One is to display a length following the rest of the UI, and that can take the scene unit settings. And then the other would be to display it with a specific unit where you pass all the parameters in manually.

However it does not make sooo much sense to have this function bpy.utils.units.to_string(1, 'MILLIMETERS') because all it does is appending mm to the number, basically. Also it is not clear what unit the 1 has.

In this function input is always a scene unit. In this case function will do some math with the unit scale value and output 1000mm.

Edit: Unit Scale scene setting does not affect to_string function.

I'm not completely convinced.
Can we change the API in a separate patch?
It feels like a separate change, not directly related to how units are handled in the scene.

  • remove old lying comment

In this case function will do some math with the unit scale value

Correction: currently to_string will always assume Unit Scale to be 1.0, so developers would do Unit Scale conversion manually before passing the value to function. Which makes sense, Unit Scale conversion is not complicated and is optional, so please ignore that part.

Can we change the API in a separate patch?
It feels like a separate change, not directly related to how units are handled in the scene.

My boilerplate code for handling units is not going anywhere, I can wait, so no pressure.

Can we change the API in a separate patch?
It feels like a separate change, not directly related to how units are handled in the scene.

Fine with me.

Good, then I think that this patch is complete, or did I forget something?

Brecht Van Lommel (brecht) requested changes to this revision.Oct 2 2018, 6:08 PM

Only very minor comments now.

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

Code style: use /* */ for comments.

source/blender/makesrna/intern/rna_scene.c
3014–3016

Let's extend this tooltip to explain what this is now useful for, compared to the other units.

"Scale to use when converting between blender units and dimensions. When working at microscopic or astronomical scale, a small or large unit scale respectively can be used to avoid numerical precision problems."

This revision now requires changes to proceed.Oct 2 2018, 6:08 PM
  • cody style and tooltip
This revision is now accepted and ready to land.Oct 2 2018, 6:52 PM

Also remember to document this in the release notes, if you weren't planning to already:
https://wiki.blender.org/wiki/Reference/Release_Notes/2.80/More_Features

Hi Jacques, hi guys! I just saw Pablo's notification and video and wanted to say big thanks! Been waiting (and probably not alone) for this for so long. Didn't try it yet, but judging from the video, it seems like a nice implementation. Congrats!

One thing i noticed in the video though, is the term 'length'. I could be wrong, since English is not my native, but being in the engineering field i feel the term fell a bit short in that category - as i see it, length is just one of 'dimensional' properties of a certain body, such as height and width.
Could it be that the term 'distance' is a more appropriate and generic term for that property? I feel it corresponds more correctly to a property of 'apartness' between two points - which in itself could include length, width, height of an object as well as the distance between objects.

Don't know, could be wrong, wikipedia leans both ways:

https://en.wikipedia.org/wiki/Distance
https://en.wikipedia.org/wiki/Length

Thanks for your comment. I'm not 100% sure what is correct but I think length is better than distance.
Also because the unit will not only be used for distances but also areas etc.

Here it says the quantity name is "length": https://en.wikipedia.org/wiki/International_System_of_Units

Yes, perhaps you're right.
Anyway, big thanks once again, to you and the rest of the crew!

Not directly related, but anyway .. ref [1]

Maybe we could tackle inconsistency with default values for length in python.

Expecting a single default value to generate same result whatever the unit system, for real-world objects like bolts or mountains.
As it, it is not the case, objects size are unit system dependents -> creating cubes with metric or imperial system lead to different cubes sizes.
Fortunately there is a simple solution to handle this and fix all those issues at once.

Unit conversion must not occur on default values !
We must only divide default by unit scale and always define default values as meter.
This way objects size will remains consistent and correct whatever the unit system is.

default size in meter / unit scale = blender unit.

So a cube with 1 (meter) as default will remains the same cube when created using imperial feet as unit.

[1] rightclickselect post