Ocean modifier : allow spray maps to be baked
ClosedPublic
Actions

Authored by Phil Stopford (philstopford) on Aug 5 2020, 2:46 AM.

Details

Reviewers

Hans Goudey (HooglyBoogly)
Brecht Van Lommel (brecht)

Commits

rB3d78e98d52d3: Ocean Modifier: allow spray maps to be baked

Summary

In comments made by a tester on https://developer.blender.org/rB17b89f6dacba007bf3a2189b36ea335bc389b661, it seems that baking of the spray maps would be useful.

This differential adds that capability. Both the spray map and its inverse are baked out in this change, for maximum convenience and to avoid assuming what the user wants.

Diff Detail

Repository: rB Blender

Event Timeline

Phil Stopford (philstopford) requested review of this revision.Aug 5 2020, 2:46 AM

Phil Stopford (philstopford) created this revision.

@Hans Goudey (HooglyBoogly) : Just curious whether you think this is fully baked (ha!) or not as a patch.

I can't really speak to the utility of the feature, but considering it's basically the same as the foam maps I'd say it's close to there. Two things:

Even though the existing code uses short for the do_spray variables, I think this should probably use bool since it's new code.
Also, why not only bake the inverse spray map if the inverse button is selected in the UI?

In D8470#208975, @Hans Goudey (HooglyBoogly) wrote:

I can't really speak to the utility of the feature, but considering it's basically the same as the foam maps I'd say it's close to there. Two things:

Even though the existing code uses short for the do_spray variables, I think this should probably use bool since it's new code.

Wouldn't that raise questions of consistency? It could be done in a separate patch, I think.

Also, why not only bake the inverse spray map if the inverse button is selected in the UI?

I can't predict how users want to use the spray, so it seemed less restrictive just to output the full set (and also makes the loading pipline easier if the user has switched the inverse option in the meantime). Given the distinct filename, the user could then discard what is not interesting to them. Baking both allows me to populate both the Eplus and Eminus from the cache to support toggling the option.

In D8470#208977, @Phil Stopford (philstopford) wrote:

Wouldn't that raise questions of consistency? It could be done in a separate patch, I think.

Yes, that would be fine as a separate patch.

I can't predict how users want to use the spray, so it seemed less restrictive just to output the full set (and also makes the loading pipline easier if the user has switched the inverse option in the meantime). Given the distinct filename, the user could then discard what is not interesting to them. Baking both allows me to populate both the Eplus and Eminus from the cache to support toggling the option.

Thanks for the explanation, this makes enough sense.

This revision is now accepted and ready to land.Aug 14 2020, 9:06 PM

Kenzie (kenziemac130) awarded a token.Aug 16 2020, 10:57 PM

@Hans Goudey (HooglyBoogly) : Not sure about timelines. When might this land?

Julien Leménicier (mistajuliax) awarded a token.Aug 21 2020, 10:14 AM

@Hans Goudey (HooglyBoogly) : Just nudging this in case it can fit for 2.91. If not, no worries.

Hey, sorry it took so long to look at this. Since it's basically just the same as foam I think this is a fine change for Bcon2.
But looking at it again, it seems these buffers are allocated even if "do_spray" is off. I know that's the case for the other maps, but I think this will be less commonly used, it may be good to do the allocations conditionally.

@Hans Goudey (HooglyBoogly) : I'm not sure how to do that without polluting the cache and/or result in order to track the spray usage (and even foam usage). It looked messy, so I opted to simply go-with-the-flow. This also makes it less awkward for reloading the baked data - I just get what's there and map it in identically to the foam. It's quite a naive approach, but seemed less problematic.

Brecht Van Lommel (brecht) accepted this revision.Oct 20 2020, 1:07 PM

Closed by commit rB3d78e98d52d3: Ocean Modifier: allow spray maps to be baked (authored by Hans Goudey (HooglyBoogly)). · Explain WhyOct 20 2020, 7:48 PM

This revision was automatically updated to reflect the committed changes.

Hans Goudey (HooglyBoogly) added a commit: rB3d78e98d52d3: Ocean Modifier: allow spray maps to be baked.

Robert Windefjord (Robonnet) awarded a token.Oct 21 2020, 11:12 AM