Page MenuHome
Differential D2174

Cycles: Make UI for subdivision more clear
ClosedPublic
Actions

Authored by Mai Lavelle (maiself) on Aug 24 2016, 6:40 PM.

Details

Reviewers
Brecht Van Lommel (brecht)
Group Reviewers
Cycles
Commits
rBS5849651a4650: Cycles: Make UI for subdivision more clear
rC6a205b90252f: Make UI for subdivision more clear
rB5849651a4650: Cycles: Make UI for subdivision more clear
Summary

Cycles: Make UI for subdivision more clear

Users have been getting a bit confused by the way things are worded/arranged in the UI. This patch makes a few changes to the UI to make it more clear how to use subdivision:

  • make Subdivide UVs option inactive when adaptive subdivision is enabled as UV subdivision is currently unsupported
  • add "px" to dicing rates in the Geometry Panel
  • display the final dicing rate in the modifier
  • reworded "Dicing Rate" in the modifier to "Dicing Scale" to make more clear that this is a multiplier for the scene dicing rate and added a note the the tooltip pointing the user to that setting in the Geometry Panel

Diff Detail

Event Timeline

Mai Lavelle (maiself) updated this revision to Diff 7300.Aug 24 2016, 6:40 PM
Mai Lavelle (maiself) retitled this revision from to Cycles: Make UI for subdivision more clear.
Mai Lavelle (maiself) updated this object.
Mai Lavelle (maiself) added a reviewer: Cycles.
Ronny G (nutel) added a subscriber: Ronny G (nutel).EditedAug 24 2016, 6:49 PM

small modification by me

its a little longer but more clear i think - brecht :)?

mais patch

Ray Molenkamp (LazyDodo) added a subscriber: Ray Molenkamp (LazyDodo).Aug 24 2016, 6:57 PM

Should we clear the checkmark for subdivide UV? Cause judging by that screenshot it gives the impression that Subdivide UV is on, and you can't turn it off

Mai Lavelle (maiself) updated this revision to Diff 7301.Aug 24 2016, 7:06 PM

Made a small correction so that the UI displays the actual final dicing rate.

Mai Lavelle (maiself) added a comment.Aug 24 2016, 7:08 PM

@Ray Molenkamp (LazyDodo), the option can still be turned off, it just wont have any effect in the render. Not sure clearing it is a good idea.

Brecht Van Lommel (brecht) accepted this revision.Aug 24 2016, 8:06 PM
Brecht Van Lommel (brecht) added a reviewer: Brecht Van Lommel (brecht).
Brecht Van Lommel (brecht) added a subscriber: Brecht Van Lommel (brecht).

Good improvement I think, but I would tweak the final dicing rate text a bit.

release/scripts/startup/bl_ui/properties_data_modifier.py
934–939

Maybe put this all one a single line and add some spacing.

if show_adaptive_options and ob.cycles.use_adaptive_subdivision:
    col = layout.column()
    col.label("Dicing Rate: Render %.2f px, Preview %.2f px" %
        (scene.cycles.dicing_rate * ob.cycles.dicing_rate,
         scene.cycles.preview_dicing_rate * ob.cycles.dicing_rate))
This revision is now accepted and ready to land.Aug 24 2016, 8:06 PM
Brecht Van Lommel (brecht) added a comment.Aug 24 2016, 8:10 PM

The Subdivide UV setting still has an effect in the viewport, so perhaps we shouldn't grey it out. Either way this should be temporary until both support smooth UV subdivision.

Greg Zaal (gregzaal) awarded a token.Aug 25 2016, 7:22 AM
Mai Lavelle (maiself) added a comment.Aug 25 2016, 5:05 PM

Can remove the greying out of the Subdivide UV option if you'd prefer. There may be a problem later when this is supported: old files might have this checked and will then have subdivided UVs in renders when they didn't before. How should we handle that? I think I'd rather have renders prioritized over viewport, but maybe you have a better idea?

release/scripts/startup/bl_ui/properties_data_modifier.py
934–939

Having the text on one line uses up too much horizontal space I think, goes off the end of the panel for me unless I resize things.

Brecht Van Lommel (brecht) added a comment.Aug 26 2016, 12:34 AM
In D2174#50782, @Mai Lavelle (maiself) wrote:

Can remove the greying out of the Subdivide UV option if you'd prefer. There may be a problem later when this is supported: old files might have this checked and will then have subdivided UVs in renders when they didn't before. How should we handle that? I think I'd rather have renders prioritized over viewport, but maybe you have a better idea?

Not really, neither is great, you can leave it greyed out. Hopefully by the time this is released as a non-experimental feature smooth UV subdivision will be supported.

release/scripts/startup/bl_ui/properties_data_modifier.py
934–939

Ok, it depends on the properties editor width, can use two lines to be safe.

I would still add an extra separator above this text since it looks a bit cramped against the Dicing Scale button.

Vuk Gardašević (lijenstina) added a subscriber: Vuk Gardašević (lijenstina).Aug 26 2016, 2:54 AM

The py script is PEP 8 compliant so:

Line 905 has a PEP 8 E502 warning - backlash is redundant between brackets.
It's not needed.

show_adaptive_options = (engine == "CYCLES" and md == ob.modifiers[-1] and
                         scene.cycles.feature_set == "EXPERIMENTAL")
Closed by commit rB5849651a4650: Cycles: Make UI for subdivision more clear (authored by Mai Lavelle (maiself)). · Explain WhyAug 26 2016, 6:16 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
intern/
cycles/
blender/
addon/
6 lines
release/
scripts/
startup/
bl_ui/
26 lines

Diff 7301

View Options

intern/cycles/blender/addon/properties.py

Loading...
View Options

release/scripts/startup/bl_ui/properties_data_modifier.py

Loading...