Ocean, extending the ocean modifier to support additional spectra

Archive Blender Development
1

I’ve been trying to extend the existing ocean modifier to support alternative spectra, principally based on the 3 models in aaOcean. Amaan implemented a stable approach, whereby changing resolution would not change the surface, but rather add/remove detail. Further, aside from the typical Phillips spectrum, two others exist (Pierson-Moskowitz and Texel MARSEN ARSLOE (TMA))

I’ve tried to insert them alongside the existing model in ocean.c, but admit to some uncertainty about how to add the spectrum choice itself into the blender side of things. I don’t know where this UI/database glue resides.

I’ve put the preliminary .c file here:

It’s not yet build tested, or runtime tested, as I’d really like to get the glue in place.

10 Likes
2

At the risk of appearing impatient, I wanted to nudge this. The developer documentation left me rather confused and crawling over undocumented source code doesn’t seem like ‘the right way’ to do something.

3

I can’t help with that, but good luck - the results from aaOcean look really great, keep it up!

4

The way this is done is by reading developer documentation (which admittedly isn’t great) and existing source code. If there’s anything unclear, then please ask more specific questions.

If you search for OceanModifier or OCEAN in the source code, it will show you all the places where the ocean modifier integrates into the UI.

5

So this looks pretty involved so far. Here’s what I’ve been poking at :

I have renamed the variables I’ve added to ocean.c to align with the standards.
I’ve tweaked the constructors in ocean.c to take the new values for spectrum, surface tension, and TMA values. BKE_ocean.h is also updated.
I’ve added the new variables into the OceanModifier struct DNA_modifier_types.h file, but admit to uncertainty about the padding requirements.

I’m currently bumping into confusion for the auto-generated RNA C/H files.
I haven’t figured out how to set things up so that the automatically generated stuff in rna_prototypes_gen.h and rna_modifier_gen.c get populated with the new changes. I’ve spent quite some time trying to find the associated entries for the existing ocean modifier, in order to add my supplementary bits, but I can’t find them under any of the makesrna related files (so far). For example, searching for ‘foam_coverage’, in the hope it may get me some way further, I find no instances across the code where I don’t have my new settings also defined. So, I’m confused.

6

I’ve pushed my changes here :

I still can’t get the new fields in the UI to show up. I also fully expect that padding needs to be adjusted.

7

I have not looked at your code just some quick ideas…

Does it build? Because makesdna and makesrna won’t if you have set it up wrongly.

If it builds, you probably still need to update the UI python code. Guess somewhere around here:
https://developer.blender.org/diffusion/B/browse/master/release/scripts/startup/bl_ui/properties_data_modifier.py$699

8

It builds without issue; I’ll take a look at the python bits - hadn’t even occurred to me so thanks for the nudge.

9

So, I’m getting closer - I don’t seem to be getting values fed in properly, though, so had to hard-code values for testing.

I admit to confusion. If I run the debugger, the values are fine at this call in ocean.c:
BKE_ocean_init(ocean,
omd->resolution * omd->resolution,
omd->resolution * omd->resolution,
omd->spatial_size,
omd->spatial_size,
omd->wind_velocity,
omd->smallest_wave,
1.0,
omd->wave_direction,
omd->damp,
omd->wave_alignment,
omd->depth,
omd->time,
omd->spectrum,
omd->surface_tension,
omd->peak_sharpen_tma,
omd->prefetch_tma,
do_heightfield,
do_chop,
do_normals,
do_jacobian,
omd->seed);

but in that called function, when I do this, ‘spectrum’ is a nonsense value:

o->_spectrum = spectrum;

For example, if omd->spectrum is an int of 3, the value of ‘spectrum’ at o->_spectrum = spectrum ought to be 3. It isn’t - it’s some extreme value, as though an overflow has occurred.

image
image2560×1400 482 KB

1 Like
10

OK. The spectrum is now working. I have to dig into more of the internals now to see what needs to be done to avoid the surface changing with resolution. More soon

4 Likes
11

OK, so now the surface doesn’t change radically with resolution changes, which is a key advantage of Amaan’s work. The code in github has my latest changes. I’d like to get the spectrum choice as a menu, and to ghost fields that aren’t relevant for the chosen spectrum.

2 Likes
12

Some tease images :

Phillips:

image
image2560×1400 1.3 MB

image
image2560×1400 2.21 MB

image
image2560×1400 2.24 MB

Pierson-Moskowitz:

image
image2560×1400 2.3 MB

TMA (this is being reviewed; I’m still not entirely happy with it):

image
image2560×1400 2.38 MB

8 Likes
13

One more question. I wanted to supply a menu instead of the spectrum numeric input. Looking at the Python code, I see this:

prop = RNA_def_property(srna, "geometry_mode", PROP_ENUM, PROP_NONE);
RNA_def_property_enum_sdna(prop, NULL, "geometry_mode");
RNA_def_property_enum_items(prop, geometry_items);
RNA_def_property_ui_text(prop, "Geometry", "Method of modifying geometry");
RNA_def_property_update(prop, 0, "rna_Modifier_update");

where the enum comes from :

  static const EnumPropertyItem geometry_items[] = {
{MOD_OCEAN_GEOM_GENERATE,
 "GENERATE",
 0,
 "Generate",
 "Generate ocean surface geometry at the specified resolution"},
{MOD_OCEAN_GEOM_DISPLACE,
 "DISPLACE",
 0,
 "Displace",
 "Displace existing geometry according to simulation"},
#  if 0
{MOD_OCEAN_GEOM_SIM_ONLY,
 "SIM_ONLY",
 0,
 "Sim Only",
 "Leaves geometry unchanged, but still runs simulation (to be used from texture)"},
#  endif
{0, NULL, 0, NULL, NULL},
};

I tried something similar for the spectra :

static const EnumPropertyItem spectrum_items[] = {
{MOD_OCEAN_SPECTRUM_PHILIPS,
 "PHILIPS",
 0,
Philips",
 "Philips Spectrum"},
{MOD_OCEAN_SPECTRUM_PIERSONMOSKOWITZ,
 "PIERSONMOSKOWITZ",
 0,
 "Pierson-Moskowitz",
 "Pierson-Moskowitz spectrum"},
{MOD_OCEAN_SPECTRUM_TMA,
  "TMA",
  0,
  "TMA",
  "TMA spectrum"},
{MOD_OCEAN_SPECTRUM_PHILIPSNEW,
  "PIERSONMOSKOWITZ",
  0,
  "Philips (experimental)", "Philips Spectrum (experimental)"},
  {0, NULL, 0, NULL, NULL},
};

and :

prop = RNA_def_property(srna, "spectrum", PROP_ENUM, PROP_NONE);
RNA_def_property_enum_sdna(prop, NULL, "spectrum");
RNA_def_property_enum_items(prop, spectrum_items);
RNA_def_property_ui_text(prop, "Spectrum", "Spectrum");
RNA_def_property_update(prop, 0, "rna_OceanModifier_init_update");

This results in the compiler whining about redefinitions:

'BL::OceanModifier::spectrum_PIERSONMOSKOWITZ': redefinition; previous definition was 'enumerator' (compiling source file D:\development\blender-git\blender\intern\cycles\blender\blender_camera.cpp)	bf_intern_cycles	D:\development\blender-git\build_windows_Release_x64_vc16_Release\source\blender\makesrna\intern\RNA_blender_cpp.h	18025

I don’t understand this. Guidance would be extremely welcome.

EDIT: resolved; needed to dig deeper in to the header files to get this working.

15

Brute force seems to have made things work, so now there’s a menu to choose from.

image
image2327×1479 2.19 MB

3 Likes
16

Not sure how to proceed from here, in case there is interest in these changes going to blender proper.

3 Likes
17

Did you already submit your code to https://developer.blender.org/ ?

18

I’m fighting with the ‘create diff’ tool. I have a local diff that I created, isolated to the change pinning the spectra to the surface (allowing for the resolution to be changed without the surface fundamentally changing). The ‘create diff’ tool whines that :

Unhandled Exception ("AphrontQueryException")
#1048: Column 'filename' cannot be null

for reasons that I don’t understand. Diff is

diff --git "a/e:\\ocean.c" "b/blender\\source\\blender\\blenkernel\\intern\\ocean.c"
index 9faa61f..5c96258 100644
--- "a/e:\\ocean.c"
+++ "b/blender\\source\\blender\\blenkernel\\intern\\ocean.c"
@@ -812,6 +812,38 @@ void BKE_ocean_simulate(struct Ocean *o, float t, float scale, float chop_amount
   BLI_task_pool_free(pool);
 }
 
+unsigned int generateUID(float xCoord, float zCoord)
+{
+  // a very simple hash function. should probably do a better one at some point
+  register float angle;
+  register float length;
+  register float coordSq;
+  register float id_out;
+  unsigned int returnVal = 1;
+
+  if (zCoord != 0.0f && xCoord != 0.0f) {
+    coordSq = zCoord * zCoord + xCoord * xCoord;
+    length = sqrt(coordSq);
+    angle = xCoord / length;
+    angle = acos(angle);
+    angle *= 57.295779513082320876798154814105f;  // RadsToDegs(angle);
+
+    if (angle > 180.0f)
+      angle = 360.0f - angle;
+
+    id_out = coordSq + (length * angle) + 0.5f;
+
+   if (angle == 0.0f)
+      returnVal = (unsigned int)coordSq;
+    else if (zCoord <= 0.0f)
+      returnVal = (unsigned int)floor(id_out);
+    else
+      returnVal = INT_MAX - (unsigned int)floor(id_out);
+  }
+  return returnVal;
+}
+
+
 static void set_height_normalize_factor(struct Ocean *oc)
 {
   float res = 1.0;
@@ -986,10 +1018,13 @@ void BKE_ocean_init(struct Ocean *o,
   }
 
   /*srand(seed);*/
-  rng = BLI_rng_new(seed);
-
   for (i = 0; i < o->_M; i++) {
     for (j = 0; j < o->_N; j++) {
+      // This ensures we get a value tied to the surface location, avoiding dramatic surface
+      // change with changing resolution.
+      int new_seed = seed + generateUID(o->_kx[i], o->_kz[j]);
+
+      rng = BLI_rng_new(new_seed);
       float r1 = gaussRand(rng);
       float r2 = gaussRand(rng);
19

Unless it’s something really simple I usually try to do all the feature work on a branch. Commit everything to that branch and switch back to master, make update, then checkout your branch again and run git merge master, then git diff -U100 master > my_change.diff

That works on Linux anyway. Hopefully that helps?

20

Now at https://developer.blender.org/D6871 but I could not find reviewers in the system to assign. The ocean modifier looks to be orphaned, in this regard.

21

I’ve moved the multi-spectrum work into a different repository and am exploring a different approach, based on the work in :

under

This looks to be cleaner and aligns better with literature.