Page MenuHome

Ocean modifier : allow spray maps to be baked
ClosedPublic

Authored by Phil Stopford (philstopford) on Aug 5 2020, 2:46 AM.
Tags
None
Subscribers
None
Tokens
"Love" token, awarded by Robonnet."Love" token, awarded by mistajuliax."Love" token, awarded by astrand130.

Details

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:

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

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:

  1. 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.

  1. 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.

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

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

@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.