Subdivision Modifier: link levels option and changed defaults to 1
AbandonedPublic
Actions

Authored by Juanfran Matheu (jfmatheu) on Jan 23 2021, 9:05 PM.

Details

Reviewers

Summary

Hi, this patch is based on this RCS and devtalk proposal by @Metin Seven (MetinSeven)

This changes the defaults of both subdivision levels to 1 and adds new lock feature to 'match' the viewport and render levels, internally it will really just use the viewport levels for both and hide the render levels while this option is active.

UI really needs some attention and feedback from UI team.

Diff 2

Diff 1

Download this Build for quick test: https://drive.google.com/file/d/1lbvvF0JQapSXig7cHab9MEw-HFySUH0b/view?usp=sharing

Diff Detail

Event Timeline

Juanfran Matheu (jfmatheu) requested review of this revision.Jan 23 2021, 9:05 PM

Juanfran Matheu (jfmatheu) created this revision.

Juanfran Matheu (jfmatheu) edited the summary of this revision. (Show Details)Jan 23 2021, 9:09 PM

Assuming the "lock" concept is eventually accepted by the team, the multi-res modifier's levels/render_levels/sculpt_levels fields and the screw modifier's steps/render_steps fields are also candidates for locking. It would be good to at least think about applying to those as well for consistency, or having a good reason not to. I'd hate to see that one RCS closed but then 1 or 2 more pop up asking for those other modifiers etc.

Metin Seven (MetinSeven) awarded a token.Jan 24 2021, 9:34 AM

Büke Beyond (beyond) awarded a token.Jan 24 2021, 9:36 AM

Many thanks, @Juanfran Matheu (jfmatheu) ! Maybe @Campbell Barton (campbellbarton) and/or @Julian Eisel (Severin) would like to review this?

Some elucidation:

9 out of 10 times I'd like both values to be the same. That means a lot of superfluous clicks while working with many subdivided objects.
I always like to start with a subdivision of 1 and increase the iterations only if necessary. But ever so often, 1 subdivision suffices.
Very often I discover that in a hurry I forgot to decrease the default rendering value of 2 on several objects, resulting in an overly complex scene during rendering.
A different iteration for rendering has a hidden risk of subsequent modifiers yielding a different result in the rendering than in the viewport.
The lock eases increasing both values at once, which you'll want most of the time.
If you'd like the rendering value to always be one iteration higher, then unlock, increase the rendering value, lock, and the values will remain synchronized.

Thanks!

Piotr Ostrowski (postrowski) awarded a token.Jan 24 2021, 10:18 AM

Johannes Kollmer (Lumpengnom) awarded a token.Jan 24 2021, 10:28 AM

Isaac Gicheha (Gicheha) added a project: User Interface.Jan 24 2021, 10:40 AM

Ilja Razinkov (IPv6) awarded a token.Jan 24 2021, 10:45 AM

Another option could be to add a Preferences setting to customize your own default Subdivision modifier iterations, in case you do use differing iterations most of the time.

I also agree with @Roger B (rboxman) that more modifiers would benefit from a lock button, such as the viewport / rendering values of the Screw modifier.

Mark B Tomlinson (MarkBTomlinson) awarded a token.Jan 24 2021, 11:55 AM

Lopo Isaac (lopoIsaac) awarded a token.Jan 24 2021, 12:58 PM

In D10187#255058, @Metin Seven (MetinSeven) wrote:

Some elucidation:

9 out of 10 times I'd like both values to be the same. That means a lot of superfluous clicks while working with many subdivided objects.

@Metin Seven (MetinSeven)
This means, lock on by default is OK? I was about to remove that to stick with the current behaviour. I would like to have some more feedback about this point.

The lock eases increasing both values at once, which you'll want most of the time.

It actually blocks the render levels and use the viewport levels for all, no sync of both values cause I thought editing only one value would be clearer, less noise and in fact it is easier from my pov.
Change that so that both properties are visible in the UI and in sync will change all the implementation basically and I'm lost in that required area XD.

A 'problem' is the right 'dot' to animate the property, would be better to have the lock aligned to the right together with the sliders but with those dots is not possible, result is very ugly.. See:

So first should be decided if keep consistency around all the modifier UI or if break it just for this corner-case for a clean-look is just fine.
Reference from @William Reynish (billreynish) and @Pablo Vazquez (pablovazquez) of how the ideal UI would be:

I wasn't sure if breaking that consistency was OK but if UI team give me green light I will put it that way without the dots. To have something like:

Thanks @Juanfran Matheu (jfmatheu) ,

If the lock is on by default (with a subdivision value of 1) that would be the most convenient to me personally, and maybe to most modelers?

If the lock would keep both values related to each other, that would make it slightly more convenient if you want to work with a consistently higher iteration for rendering. While the lock is active, you could increase the viewport iteration with 1, and automatically increase the rendering value with 1 as well, even if the value was already higher.

But... This does raise the question: what would the rendering value become if you change the viewport iterations from 1 to 3? So maybe your current approach is best: while the lock is active, both values are the same, and in unlocked mode, each value is separate.

Regarding the UI design I would personally prefer the approach of @William Reynish (billreynish) / @Pablo Vazquez (pablovazquez):

Thanks!

If the lock is on by default (with a subdivision value of 1) that would be the most convenient to me personally, and maybe to most modelers?

If the lock would keep both values related to each other, that would make it slightly more convenient if you want to work with a consistently higher iteration for rendering. While the lock is active, you could increase the viewport iteration with 1, and automatically increase the rendering value with 1 as well, even if the value was already higher.

But... This does raise the question: what would the rendering value become if you change the viewport iterations from 1 to 3? So maybe your current approach is best: while the lock is active, both values are the same, and in unlocked mode, each value is separate.

@Metin Seven (MetinSeven) All this points are making me think what about if there is only one 'Levels' slider that modifier actively use but then you have a field for extra subdivs for the render that is disabled by default (like the Local Camera field of the View panel in the sidebar) but you can enable and set the increment of levels you want for your render against viewport. Concept behind would change but it would be easier I guess.

Regarding the UI design I would personally prefer the approach of @William Reynish (billreynish) / @Pablo Vazquez (pablovazquez):

I made this change to match that reference, better but hacky:

In D10187#255095, @Juanfran Matheu (jfmatheu) wrote:

@Metin Seven (MetinSeven) All this points are making me think what about if there is only one 'Levels' slider that modifier actively use but then you have a field for extra subdivs for the render that is disabled by default (like the Local Camera field of the View panel in the sidebar) but you can enable and set the increment of levels you want for your render against viewport. Concept behind would change but it would be easier I guess.

Sounds good. I think it's important that the option to separate the viewport and rendering subdivisions is clear to the user, but to have the separate rendering value disabled by default, with the active lock.

The values would then always be in sync until you deactivate the lock.

In D10187#255101, @Metin Seven (MetinSeven) wrote:[[ URL | name ]]

Sounds good. I think it's important that the option to separate the viewport and rendering subdivisions is clear to the user, but to have the separate rendering value disabled by default, with the active lock.

The values would then always be in sync until you deactivate the lock.

@Metin Seven (MetinSeven) Not really, that concept I exposed would be only one slider and NO lock feature, no sync. User will only be aware of one 'Levels' slider.
By other side, we would have a toggle with extra subdivs for render, this means, if that is active, the render will have at least a value of {levels} + 1 subdivision levels.

At UI level would look something like:

[Here Levels are 3 but render will use +1 subdiv more than viewport]
Which is convenient for artists this way as you don't need to change both values to maintain that 1,2,3... whatever subdiv distance between both viewport and render.
That means if you always want render to be +1 the viewport you could do it with this concept.

[Here Levels are 3 for all as no extra subdivs for render are being used]
So you have the same value for everything and it doesn't cause confusion nor add or hide redundant properties because all them will have their own purposes.
Note: the toggle could be removed as is not really neccessary but just added it so idea is more clear.

In summary:
All this would match the requirements of the proposal and even would solve some UI and RNA issues regarding this patch and possible alternative making it an easier implementation that will also add more benefits for the user as described above and with not much development effort.

I would like to mention @Julian Eisel (Severin) @William Reynish (billreynish) @Campbell Barton (campbellbarton) to discuss this new concept I have explained here.
Not sure if a TD is needed, just if that is the proper way to formally discuss a design like this. But just wanted to know your general thoughts towards this idea.

Thanks for your attention

Hi @Juanfran Matheu (jfmatheu) ,

I like your solution: making the rendering value additive to the viewport value in stead of absolute.

Looking forward to read the replies of the other devs.

Thanks!

I think relative is the way to go. Similar can be done with geometric Scale everywhere, one Uniform value, and 3 more for normalized multiplicative distortion. The UI can convert them internally. I am waiting for this comment to be marked irrelevant by Brecht. :-)

These little things add up to lots of wasted time when the artist is repeating themselves, or drawing finicky mouse gestures to change common values.

RedMser (RedMser) added a subscriber: RedMser (RedMser).Jan 24 2021, 10:28 PM

Juanfran Matheu (jfmatheu) planned changes to this revision.Jan 25 2021, 10:03 AM

michael knubben (michaelknubben) awarded a token.Jan 27 2021, 4:49 PM

SavMartin awarded a token.Jan 28 2021, 7:39 PM

Mikail (fiendish55) awarded a token.Jan 28 2021, 11:10 PM

Andrey (andruxa696) awarded a token.Jan 29 2021, 12:34 AM

Changed UI as requested (talk in Blender.chat), to avoid dynamic UI and keep both sliders. Still a little hacky though. Also renamed all from 'lock' to 'link'.

Question: should this link_levels property should be enabled or disabled by default?

Juanfran Matheu (jfmatheu) edited the summary of this revision. (Show Details)Jan 29 2021, 12:06 PM

In D10187#255058, @Metin Seven (MetinSeven) wrote:

Some elucidation:

9 out of 10 times I'd like both values to be the same. That means a lot of superfluous clicks while working with many subdivided objects.

Where can we see what you are using blender for?
What workflow do you perform?
It looks like you are using it wrong.
You dont need corresponding values 9/10 times, you need subdivided render and not subdivided viewport most of the time, because of perfomance, especially at heavy scenes.

I like your solution: making the rendering value additive to the viewport value in stead of absolute

Sounds like a nightmare to control for scene with 10k objects.

In D10187#256846, @Paul Kotelevets (1D_Inc) wrote:

In D10187#255058, @Metin Seven (MetinSeven) wrote:

Some elucidation:

9 out of 10 times I'd like both values to be the same. That means a lot of superfluous clicks while working with many subdivided objects.

Where can we see what you are using blender for?
It looks like you are using it wrong.
You dont need corresponding values 9/10 times, you need subdivided render and not subdivided viewport most of the time.

I'd like my viewport result to be as close as the rendering result as possible, for example because subsequent modifiers behave differently when the subdivision values differ.

Also, most of the time one subdivision suffices, while the default modifier behavior of two subdivisions causes unnecessary overhead when you've got many objects and forgot to lower the rendering subdivisions in a hurry.

You can find my work here.

In D10187#256847, @Metin Seven (MetinSeven) wrote:

I'd like my viewport result to be as close as the rendering result as possible, for example because subsequent modifiers behave differently when the subdivision values differ.

Aw, character modeling...
I meant mostly scenes like that, with billions of vertices, where corresponding viewport and render values will kill the ability to work with scene, so most objects have 0 viewport and 1-3 render values .
So, we will need to design a workaround for such a cases, or, at least, to take them into account.

In D10187#256854, @Paul Kotelevets (1D_Inc) wrote:

In D10187#256847, @Metin Seven (MetinSeven) wrote:

I'd like my viewport result to be as close as the rendering result as possible, for example because subsequent modifiers behave differently when the subdivision values differ.

Aw, character modeling...
I meant mostly scenes like that, with billions of vertices, where corresponding viewport and render values will kill the ability to work with scene.
So, we will need to design a workaround for such a cases, or, at least, to take them into account.

I understand. As I wrote in a previous message in this thread, the best approach would be an option in the Preferences to configure your preferred subdivision modifier settings.

In D10187#256855, @Metin Seven (MetinSeven) wrote:

an option in the Preferences to configure your preferred subdivision modifier settings.

Sounds good. Thank you.

Juanfran Matheu (jfmatheu) retitled this revision from Subdivision Modifier: set both subdiv levels to 1 and add lock feature to Subdivision Modifier: link levels option and changed defaults to 1.Feb 2 2021, 1:59 PM

Felix Kütt (fkytt) awarded a token.Feb 3 2021, 11:22 PM

Vitalii Vlasov (Fracture128) awarded a token.Feb 4 2021, 9:40 AM

Love this, really hope to see other similar inputs (screw modifier, resolution of curves, etc) updated to this style.

We talked about this in the UI meeting last week. Visually, the "Diff 2" approach works, but the consensus was that it does not make sense to support this for one specific situation, and that it should rather be a general feature of the UI.
Note that this does raise the scope of the patch quite a bit, but it makes more sense than having to implement this individually in every situation. There is a real maintenance burden to building this specific UI in ever situation and ensuring the flag is used when accessing DNA data correctly.

Here's how I would suggest implementing this:

The "Sync Values" boolean value needs to be stored in files, so create a flag in DNA for it. (done)
Create a boolean property in RNA, "Sync Levels". (done)
Add a UI template that adds the two properties and handles the updates to synchronize them. From the conclusion in D8057, this can't be handled at the RNA level, must be handled by the UI code, likely a special case in the button event handler. (Though this part could use bit more thought, it's the trickiest part).
Using the template in more places would be as simple as adding the flag and the property and switching the layout to use the template.

Some signature like this would work:

void uiTemplateLinkedProperties(uiLayout *layout,
                                PointerRNA *ptr,
                                const char *property_a,
                                const char *property_b,
                                const char *label_a,
                                const char *label_b)

Campbell thought a uiLayout flag might work, but I think that would be a bit riskier and more likely to break.

The defaults change is fine, but it may be a breaking change, so it's added to T84380: UI: Discussion Topics & Possible To-Do's for 3.0

I would suggest closing this patch and starting a new one if you're interested in implementing this more generally.

This revision now requires changes to proceed.Feb 9 2021, 7:28 PM

Sounds good.
What is the difference between RNA and DNA?

And what if just to wait until 3.0 and put it at RNA level legally?

@Hans Goudey (HooglyBoogly) Thanks!

I agree that this could be made in a more generic way that could be easily reused in other parts of Blender which can benefit from this sync feature.
All I would need is to know the consensus the UI team has reached of the areas/properties that can benefit from this feature and that have priority (like there are many target places but should be a limit to not add it everywhere).

By other side, I'm not familiar with UI templates but will take a look to that part of code and probably some patches to "get in touch" with it - if you have any recommendation of some piece of code I should definitely take a look at, it's very welcome - also will follow the proposed 'signature'.

Closing this patch to start with the new and generic implementation of the sync/link feature.

Steffen Hartmann (Harti) awarded a token.Oct 7 2021, 6:32 PM