Page MenuHome

Added units to some properties
ClosedPublic

Authored by Scott Petrovic (scottpetrovic) on Dec 13 2013, 12:09 AM.

Details

Summary

I am sure there are still some properties out there that to be fixed, but here is my first run through. I am not sure exactly who needs to review changes like this, so let me know if someone else needs to look at them.

Diff Detail

Event Timeline

Brecht Van Lommel (brecht) requested changes to this revision.Dec 13 2013, 12:19 AM

Looks good to me except the angle. For reviewers, I've added the user interface project as well. Generally if you're not sure you can assign to Campbell, Sergey or me and we will reassign to the right developers.

source/blender/makesrna/intern/rna_sculpt_paint.c
537 ↗(On Diff #298)

I don't think this works, angles are assumed to be in radians. Bastien did a bunch of conversion here recently, I guess this one was missed still. {46eef60d93fd0d52ed4b94750f7a3248db5594ee}.

To fix this properly you'd need to convert old files to radians in do_versions, readfile.c.

Scott Petrovic (scottpetrovic) updated this revision to Unknown Object (????).Dec 13 2013, 1:34 AM

I removed the angles property for now. If I do the conversion fix for the angles, I would rather do it in another diff. I like to keep the commits more focused on one aspect.

Looks good to me.

Looks good.

I have a feeling that there are many more percentage fields though. Currently many percentages are represented in values from 0-1. Would be nice to convert these to 0%-100%

@William Reynish (billrey) - I was also noticing many fields that are clamped to 0-1 as well. Changing to percentage would be a good fit. I was going to bring that up separately because I didn't know if there was some data conversion issues that might happen if the format changed. I haven't spent that much time looking at it, so it might not be an issue. I think there are also some other areas that are in pixels as well, but I can't tell based off of the tooltip. This commit was just a quick run through the code using the "find all in solution" method.

I'm not so sure about changing 0..1 to 0..100. It may be more familiar to users, but it's problematic for node setups and drivers, for those 0..1 ranges are more natural.

Perhaps some places need 0..1 and others 0..100, but I don't know what the rule would be to decide when to use one or the other.

@Brecht Van Lommel (brecht): Perhaps 0-100% is just a display thing then?

Those props could internally continue to be represented as values from 0-1?

We can do the discussion elsewhere, but I think it will be confusing to have it display percentages but use a different range internally, having to do a mental conversion when you think about the setup.