Per-Asset Opaque Normal Scaling - #114
Conversation
- Implemented the normalScale to the surface material slangh - Added normal scale multiplier to decode and add opacity call - added getNormalScale and normal_scale linkage to model and material info - Added normalScale to gpuStruct - Updated GPU padding and gpu data assembly with new point
MarkEHenderson
left a comment
There was a problem hiding this comment.
If you could manipulate the global normal scaling within the toolkit, would you still want this MR? We're generally resistant to putting data into OpaqueSurfaceMaterial if it can be avoided.
Increasing the size of OpaqueSurfaceMaterial has a pretty heavy perf impact, and it only has 14 spare bytes before we'd have to increase it again.
This MR would permanently bring that down 12. If this is truly only a feature to make asset development faster, then permanently occupying that space may prevent more important uses of that space in the future.
| uint16_t samplerFeedbackStamp; | ||
|
|
||
| // 25 | ||
| float16_t normalScale; |
There was a problem hiding this comment.
There's no need to use an arbitrary 0-10 range if you're also using float16_t.
If you want the better precision than that provides, or want to fit into 1 byte instead of 2, then pack it as a unorm (unsigned normal):
static const float kMaxNormalInstensity = 10.f;
...
uint16_t packed = packUnorm1x16(intensity / kMaxNormalInstensity);
...
float value = unpackUnorm1x16(packed) * kMaxNormalInstensity;
There was a problem hiding this comment.
Oh I thought this was already committed into my branch, sorry. My intent is to drop this to 1 byte so we can get more space out of the struct
Global intensity factor really doesn't help that much in a collaborative environment if people have unstandardized contributions, but ignoring that, it becomes less accessible to see every single normal suddenly jolt up or down when you're trying to see the changes on just this one asset. Now if we go the opposite direction of focusing in on a single texture, I think there's a severe underestimation in the amount of time and effort someone has to spend just to see the change. Load up a normal, tweak a number, re-ingest it, refresh mod. Repeat. Not to mention I now have to get the right editing software for it. The way the displacement slider works is by far the most logically satisfying way of doing it. I can actually tune directly in the toolkit and see live. Displacement and normals feel like something that should be directly editable and instantly viewable because of how they compound with textures overlaying them. I don't think rough, metal, or albedo ever have this issue. I do understand the concern of adding more data to the GPU struct, so if these options were available ONLY in the toolkit (IE: the sliders exist and then I can actually apply them to the normal map) I'd be happy, but I guess I'd like to understand why the line is being drawn on a normal map. I find it weird that the struct has metalness and roughness constants being put through despite their data structures being by far the SIMPLEST to modify (literally black and white!). Height maps have the same thing but it seems like the classification is understood: it can compound and contribute to other textures via the terrain baker. Wouldn't the normal map stand to the same? |
- Removed whitespace
The objective of this PR is to add a variable to USD asset definitions that lets you scale the intensity at a per-asset level.
The variable, normal_scale, has an arbitrary range ( 0 to 10.0f ) modelled after the base normal intensity debug scaler.
Sample:
It behaves by taking the asset normal_scale and multiplying it by the global intensity factor within remix. Really the hope is that this speeds prototyping up, highlights include:
NOTE: Implementation was achieved by taking up 1 byte in the GPU data struct originally padded. That may be a sacred thing, and no benchmarking was done after this change.