Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LOD Generator Helper Presets #63

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

scoan-sd
Copy link

Related issues
No related issues AFAIK.

Describe the changes
This PR introduces the ability to define sharable/reusable LOD generation presets. A new ScriptableObject type, LODGeneratorPreset defines most LOD generation settings. LODGeneratorHelpers can reference a preset, which will override the helper's settings. Additionally, you can enable customization on an asset-by-asset basis if you need to tweak any settings beyond what a given preset provides.

Some properties (Auto-collect renderers, override save path, and specified renderers) remain helper-driven rather than preset-driven.

To ensure backwards-compatibility, LODGeneratorHelpers default to customization enabled, no preset specified. Disabling customization will "snap" that helper's settings to the specified preset. If no preset is specified, the plugin's traditional default LOD settings will be used.

How to test

  • Create a LOD preset asset (In the inspector, Create->Mesh Simplifier->LOD Preset). Configure the preset.
  • Add a LOD Generator Helper component to something that can be LOD'd. Disable Customize Settings and reference your preset in the Lod Generator Preset property.
  • You should see the helper's settings change to match the preset.
  • Re-enable Customize Settings to tweak any settings (which should start from the preset's settings). Re-disable customization to revert to the preset's settings.

Additionally, LOD Generator Helpers created before presets were introduced should function identically as before (previously set settings should not have changed, LODs should generate the same).

Screenshots
Screenshots and video!

The preset scriptable object:
LODPreset_ScriptableObject

New settings on LOD Generator Helper
LODPreset_LODGeneratorHelper

Preset selection (note that preset-driven properties are greyed out, and the component's properties have been updated to match the preset)
LODPreset_PresetSelected

Video showing flow:
https://user-images.githubusercontent.com/103901155/180583699-5708715f-bed8-443f-860c-b92cb26c8f1a.mp4

Known issues
If the properties of a LODGeneratorPreset are updated, objects referencing that preset are not automatically updated (nor are their LODs regenerated). This would require searching the asset database for objects referencing a changed preset. Unity's search extensions package (https://github.com/Unity-Technologies/com.unity.search.extensions) makes this straightforward, but I didn't want to introduce the dependency. In the meantime, manually enabling & disabling customization on any LODGeneratorHelpers ref'ing a changed preset will update that helper's properties.

Further details
🤷‍♂️

scoan-sd added 7 commits July 22, 2022 16:17
LODGeneratorHelper has two new properties,
A LODGeneratorPreset obj ref and a "customization" toggle.
If customization is enabled (default), the helper drives all properties itself.
If customization is disabled, properties are driven by a specified preset.
If no preset is specified, defaults matching previous behavior are used.
When a LOD Generator Helper has customization disabled,
Properties driven by the specified preset are visible but greyed out.
Code between calls EditorGUI.BeginDisabledGroup and EditorGUI.EndDisabledGroup scoped for clarity.
Properties not driven by a preset are listed first.
This way, there's only a single block of greyed-out properties when customization is disabled.
@scoan-sd
Copy link
Author

A bit of background: an artist on my team wanted the ability to specify generic LOD settings for things like "Large Prop", "Small Prop", "Player Character", etc.

We both got tired of copying helper components around, so I figured adding referencable presets would be useful.

@Whinarn
Copy link
Owner

Whinarn commented Jul 23, 2022

Thanks for the PR!
I'll have a look at it within a few days when I get some time over.

@Whinarn
Copy link
Owner

Whinarn commented Jul 26, 2022

I like the idea behind this, but I'm not sure that this is the behavior that I'd expect and want myself.

I think that it would be better if instead of just copying over the settings from the preset asset, the settings from the preset asset are used directly instead. This way you can update the preset, and those settings are used anywhere it's referenced automatically. It could still be done so that you can load preset settings and then customize it for each instance, but this would mean that the preset link would be broken. But that wouldn't be different from the behavior of this PR right now. I think that'd be acceptable and expected.

Then you wouldn't need to have a checkbox to customize settings. You simply use a preset, or not (which means customize on instance level). The downside is that the automatic collection of renderers is required then, unless it's possible to specify those in addition to using the presets for everything else per LOD level 🤔

What do you think?

@scoan-sd
Copy link
Author

scoan-sd commented Jul 26, 2022

I think that it would be better if instead of just copying over the settings from the preset asset, the settings from the preset asset are used directly instead.

This way you can update the preset, and those settings are used anywhere it's referenced automatically.

This would definitely be a better behavior! Customize checkbox or not, it should probably work this way and I can update the PR so LODGenerator.GenerateLODs() pulls settings from the ref'd preset directly when customization is disabled.

That said: regardless of whether LOD generation pulls from a preset's properties or properties on the helper, LODs won't be regenerated on a property change (currently you need to open the LOD'd prefab, destroy existing LODs from the helper, and regenerate them). My expectation is for a prefab's LODs to be automatically regenerated when its helper's settings or ref'd preset change; my plan was to tackle this in a subsequent PR.

It could still be done so that you can load preset settings and then customize it for each instance, but this would mean that the preset link would be broken.

So if I'm understanding this right, the idea is that a LODGeneratorHelper would contain both a preset reference and the full suite of LOD settings, but the preset reference would be nulled the moment you customize any settings? Or you'd need to null the preset reference before you could customize settings?

There's some advantage to retaining a reference to the preset you used as a "base" before customization, in case you want to revert your customizations but don't remember which preset you used. Though in practice, you'll probably be choosing from a small number of presets and could guess based on the asset, so it might not be as useful as I'm assuming 😄

Another advantage to retaining the preset ref on a customized asset is for validation. On my current project, we plan to validate that customized settings aren't "too" customized from a known preset. When a given preset is used as a base, the same # of levels must be used but a few settings (eg. quality, transition height, and shadow casting) can be tweaked within some range. The thought being, if you want to deviate far from an existing preset then you should probably create a new preset and use that instead.

This is obviously a validation requirement specific to my project (and so not a part of this PR), but it's not possible unless a ref to the preset assigned before customization is retained.

Then you wouldn't need to have a checkbox to customize settings. You simply use a preset, or not (which means customize on instance level).

Ultimately, I think the value of explicitly toggling customization is in clarifying the user's intent. My model for this is in how Unreal handles property overrides on material instances; you have to explicitly enable customization of a given property, so when a parent material's properties are changed the instance receives un-overridden changes while retaining local overrides. Notably, you can enable customization of a property without changing its value!

Toggling customization without overriding values acts to say "I like the current values of my base, but if my base changes I want to keep using the old values", which is potentially useful.

The downside is that the automatic collection of renderers is required then, unless it's possible to specify those in addition to using the presets for everything else per LOD level 🤔

This was also part of the reason I opted to copy settings over when setting the preset, yeah. The Renderer list could be pulled out of LODLevel and specified on the helper alongside a specified preset or custom settings, though the code gets a bit more complicated to ensure backwards compatibility.

I think your intuition is correct that for uncustomized helpers, the ref'd preset's settings should be used directly. I do feel that holding on to the preset ref after customization is useful for validation and reversion, and explicitly toggling customization clarifies the user's intent.

@Whinarn
Copy link
Owner

Whinarn commented Jul 27, 2022

Customize checkbox or not, it should probably work this way and I can update the PR so LODGenerator.GenerateLODs() pulls settings from the ref'd preset directly when customization is disabled.

Thinking about this again, I don't think it's necessary anymore. Because you only need the settings when you generate the LODs. So maybe it's just enough to copy it over again when generating the LODs instead of just during OnValidate like now.
In this method or something:

private void GenerateLODs()

My expectation is for a prefab's LODs to be automatically regenerated when its helper's settings or ref'd preset change; my plan was to tackle this in a subsequent PR.

Right. This would be good. But definitely a bonus. I think it could probably be a button on the preset asset since it would be a more heavy operation potentially. Like "Apply changes to all prefabs" or something. But as you said before, it'd be good not to introduce any additional dependencies just for that.

There's some advantage to retaining a reference to the preset you used as a "base" before customization, in case you want to revert your customizations but don't remember which preset you used.

Yeah, you're probably right. Keep the toggle with a base reference.

This was also part of the reason I opted to copy settings over when setting the preset, yeah. The Renderer list could be pulled out of LODLevel and specified on the helper alongside a specified preset or custom settings, though the code gets a bit more complicated to ensure backwards compatibility.

Yeah, skip that. I was only thinking about possibilities, but you're right that it complicates things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants