Add SimplexNoise and NoiseTexture as new resources #21569

JFonS · 2018-08-29T15:20:38Z

SimplexNoise can be used to generate parameterized fractal noise based on Open Simplex, intended to be used in GDsrcipt.

NoiseTexture uses SimplexNoise to generate noise textures for using in
shaders/visual effects.

Documentation is missing, I will make a separate PR for that later on.

Edit: added option to use noise as a normalmap,

Zylann · 2018-08-31T13:04:24Z

Should the class be named SimplexNoise even though the algorithm is actually OpenSimplex? (the name is still technically correct though)

Megalomaniak · 2018-09-02T11:07:54Z

Yes, I think SimplexNoise is better. A new user might not care for nor need to understand the difference so long as they are just familiar enough with noise types to know that it is a Perlin type noise function.

Xrayez · 2018-09-10T08:32:36Z

If the concern is the patent issue with simplex noise then based on my estimation it should expire on 2021 so we can be strategical about naming convention.. Besides, OpenSimplex implementation could be replaced with Simplex in the future.

akien-mga · 2018-09-13T10:55:27Z

thirdparty/open-simplex-noise/open-simplex-noise.h

+	extern "C" {
+#endif
+
+// -- GODOT start --


Why are those changes needed? It should be documented (or ideally avoided if we can do without).

Ah I saw the comment in the README.md. It would be nice to have a patch file like ./thirdparty/minizip/godot-zlib-1.2.4-minizip-seek.patch so that it can be easily reapplied when syncing with upstream.

akien-mga · 2018-09-13T10:57:14Z

thirdparty/README.md

+##open-simplex-noise
+
+- Upstream: https://github.com/smcameron/open-simplex-noise-in-c
+- Version: git (0d555e7, 2018)


That's 2015 :)

akien-mga · 2018-09-13T10:58:40Z

thirdparty/README.md

@@ -393,6 +393,23 @@ Files extracted from the upstream source:
 - All .h files in `src/`
 - LICENSE.txt

+##open-simplex-noise


There should be a space after ##, and two lines between sections.

That being said, since it's only two files, I think it would be better to just put them in thirdparty/misc (and document them in the relevant section). You can likely drop the README and maybe name the license file open-simplex-noise-LICENSE.

akien-mga · 2018-09-13T10:59:16Z

thirdparty/README.md

+
+- Upstream: https://github.com/smcameron/open-simplex-noise-in-c
+- Version: git (0d555e7, 2018)
+- License: The Unlicense


The short identifier is just "Unlicense": https://spdx.org/licenses/Unlicense.html

modules/opensimplex/config.py

akien-mga · 2018-09-13T11:05:25Z

modules/opensimplex/noise.cpp

@@ -0,0 +1,226 @@
+#include "noise.h"


Please add Godot's copyright header.

I'd also suggest to rename noise.{cpp,h} to simplex_noise.{cpp,h} to match the class name.

akien-mga · 2018-09-13T11:06:10Z

modules/opensimplex/noise.cpp

@@ -0,0 +1,226 @@
+#include "noise.h"
+#include "core_string_names.h"


This should be core/core_string_names.h, I removed core/ from the include path. Please also add a line between noise.h (or simplex_noise.h) and other includes.

akien-mga · 2018-09-13T11:07:20Z

modules/opensimplex/noise.h

@@ -0,0 +1,62 @@
+#ifndef OPENSIMPLEX_NOISE_H


Copyright header needed here too. The guard should likely be renamed to match the class and filename (so SIMPLEX_NOISE_H once renamed to simplex_noise.h).

akien-mga · 2018-09-13T11:08:34Z

modules/opensimplex/noise.h

+#include "image.h"
+#include "open-simplex-noise.h"
+#include "scene/resources/texture.h"
+#include <reference.h>


Godot headers are included with "", <> is for system/thirdparty headers. And core/ includes should be absolute. So this should be:

#include "core/image.h" #include "core/reference.h" #include "scene/resources/texture.h" #include <open-simplex-noise.h>

akien-mga · 2018-09-13T11:09:01Z

modules/opensimplex/noise_texture.cpp

@@ -0,0 +1,213 @@
+#include "noise_texture.h"
+#include "core_string_names.h"


Same here, add core/ and a newline.

akien-mga · 2018-09-13T11:09:18Z

modules/opensimplex/noise_texture.h

+#include "editor/property_editor.h"
+#include "image.h"
+#include "noise.h"
+#include <reference.h>


Same comment as in noise.h.

akien-mga · 2018-09-14T12:30:48Z

modules/opensimplex/doc_classes/SimplexNoise.xml

+			<argument index="1" name="height" type="int">
+			</argument>
+			<description>
+				Generate a noise image with the requested [code]width[\code] and [code]height[\code], based on the current noise parameters.


You got those slashes backwards :)

akien-mga · 2018-09-14T12:31:02Z

modules/opensimplex/doc_classes/SimplexNoise.xml

+			</argument>
+			<description>
+				Generate a tileable noise image, based on the current noise parameters. 
+				Generated seamless images are always square ([code]size[\code]x[code]size[\code]).


akien-mga · 2018-09-14T12:33:51Z

modules/opensimplex/SCsub

+
+thirdparty_dir = '#thirdparty/misc/'
+thirdparty_source = thirdparty_dir + 'open-simplex-noise.c'
+env.Append(CPPPATH=thirdparty_dir)


Since it's only one file, and not likely to be packaged by Linux distros and thus unbundled for the time being, I'd suggest not to include thirdparty/misc to the include path, and instead include thirdparty/misc/open-simplex-noise.h directly in the relevant files.

This saves us an extra 18 characters on every single compilation command line, which is nice to avoid logs growing to crazy sizes and Windows aborting the build due to too long instructions.

modules/opensimplex/register_types.cpp

modules/opensimplex/register_types.h

akien-mga · 2018-09-14T12:35:55Z

modules/opensimplex/simplex_noise.h

+#include "core/reference.h"
+#include "scene/resources/texture.h"
+
+#include <open-simplex-noise.h>


If you do change it to thirdparty/misc/open-simplex-noise.h as I now suggested, it should go back to using double quotes as it's no longer a "system" include.

SimplexNoise can be used to generate parameterized fractal noise based on Open Simplex. NoiseTexture uses SimplexNoise to generate noise textures for using in shaders/visual effects.

akien-mga · 2018-09-14T14:57:38Z

Thanks a ton! :)

ghost · 2018-09-14T19:50:14Z

Fantastic! XD Going to try it out soon.

PetePete1984 · 2018-09-19T14:11:52Z

Before it's finalized in a release build: the correct spelling would be "persistence", not "persistance".
https://github.com/godotengine/godot/search?q=persistance&type=Code
Only concerns the new module as far as I can see.

smcameron · 2018-09-19T22:31:11Z

Hello.

If the concern is the patent issue with simplex noise then based on my estimation it should expire on 2021 so we can be strategical about naming convention.. Besides, OpenSimplex implementation could be replaced with Simplex in the future.

An argument against changing the underlying noise function in the future would be that this would change the behavior of any programs relying on it for e.g. deterministic procedural generation, which for some programs (e.g. something along the lines of No Man's Sky) this would amount to a breaking change.

santouits · 2018-09-20T15:51:48Z

I also think that it should be named Open Simplex Noise because:

That is it's correct name.
It's a different algorithm.
There is also a wikipedia page.
It doesn't hide that there is an open alternative to the simplex noise, so it is important to the opensource culture.
If in the future simplex noise patent expires, it can be added too without breaking old projects.

JFonS · 2018-09-20T16:00:18Z

I have no preference on the name other than SimplexNoise being shorter. Feel free to open a PR with the change or an appropriate issue to discuss this.

Wircoark · 2018-09-22T16:51:58Z

Are noise textures supposed to work within the Visual Shader Editor? I added a noise texture, fed it UV´s and put it in the Color output of a CanvasItem shader but it just shows up black. I tried right clicking the noise texture and tried to "edit" it so I could choose "SimplexNoise" like you would if you just add this texture to a Sprite but no settings are exposed. Figured I´d mention it here if it´s a bug, or maybe this is a bug of the Visual Shader editor, rather than the noise texture?

JFonS · 2018-09-22T18:59:51Z

@Wircoark It's a quirk of the visual shader editor. If you use the constant texture node and you create a new texture from the visual node itself then you have no way to modify it's parameters (gradient or curve textures have the same issue).

I might try to fix it, but for the moment you can workaround it by creating a new NoiseTexture resource in memory (green plus icon in the inspector), saving it to a resource file and then loading it in the visual shader node. This way you will be able to open the resource's inspector through the file system dock and edit all of it's parameters.

SethRzeszutek · 2018-11-19T17:51:50Z

I am on the Alpha 3.1, when using the reference code from the docs I come up with OpenSimplexNoise not being declared in the current scope errors. Running the simple code in the Docs fail.
More information is on this is here: https://godotengine.org/qa/35936/issue-using-opensimplex-godot-alpha-doesnt-register-module.

Zylann · 2018-11-19T18:32:16Z

@SethRzeszutek It was added after 3.1 alpha1. Try 3.1 alpha2, it has simplex noise in it: https://downloads.tuxfamily.org/godotengine/3.1/alpha2/

SethRzeszutek · 2018-11-19T18:34:39Z

I am on the alpha, I downloaded it yesterday. Even the about section says GODOT 3.1.alpha, is there an alpha2?

Zylann · 2018-11-19T18:46:25Z

@SethRzeszutek it seems the editor doesn't display which alpha it is, but you should try the official alpha2 link I provided above, this one definitely has OpenSimplexNoise.

SethRzeszutek · 2018-11-19T21:17:43Z

That link allowed for me to use openSimplex, I guess I somehow downloaded the wrong alpha? Thank you!

JFonS requested a review from akien-mga as a code owner August 29, 2018 15:20

JFonS force-pushed the add_noise_textures branch from c00ac00 to 3c01755 Compare August 29, 2018 16:58

akien-mga added feature proposal topic:core topic:thirdparty labels Aug 29, 2018

akien-mga added this to the 3.1 milestone Aug 29, 2018

This was referenced Aug 30, 2018

Simplex Noise functions (2D, 3D, and 4D) #1268

Closed

Inegrate opensimplex into godot core Zylann/godot_opensimplex#6

Closed

Zylann mentioned this pull request Sep 10, 2018

Incommpatible with Godot 3.1 Zylann/godot_opensimplex#7

Closed

akien-mga reviewed Sep 13, 2018

View reviewed changes

modules/opensimplex/config.py Show resolved Hide resolved

akien-mga reviewed Sep 13, 2018

View reviewed changes

JFonS force-pushed the add_noise_textures branch 3 times, most recently from f90d77a to 8ed68f5 Compare September 14, 2018 10:24

akien-mga reviewed Sep 14, 2018

View reviewed changes

modules/opensimplex/register_types.cpp Show resolved Hide resolved

akien-mga reviewed Sep 14, 2018

View reviewed changes

modules/opensimplex/register_types.h Show resolved Hide resolved

akien-mga reviewed Sep 14, 2018

View reviewed changes

akien-mga approved these changes Sep 14, 2018

View reviewed changes

JFonS force-pushed the add_noise_textures branch from 8ed68f5 to b5b1745 Compare September 14, 2018 13:17

Add SimplexNoise and NoiseTexture as new resources

f12a1b8

SimplexNoise can be used to generate parameterized fractal noise based on Open Simplex. NoiseTexture uses SimplexNoise to generate noise textures for using in shaders/visual effects.

JFonS force-pushed the add_noise_textures branch from b5b1745 to f12a1b8 Compare September 14, 2018 13:24

akien-mga merged commit 2aad7f1 into godotengine:master Sep 14, 2018

clayjohn mentioned this pull request Sep 22, 2018

FastNoise Implementation #22337

Closed

santouits mentioned this pull request Sep 23, 2018

Renaming SimplexNoise refs to OpenSimplexNoise #22375

Merged

aaronfranke mentioned this pull request Mar 24, 2020

Add Vector4 to the engine godotengine/godot-proposals#629

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add SimplexNoise and NoiseTexture as new resources #21569

Add SimplexNoise and NoiseTexture as new resources #21569

JFonS commented Aug 29, 2018 •

edited

Loading

Zylann commented Aug 31, 2018 •

edited

Loading

Megalomaniak commented Sep 2, 2018

Xrayez commented Sep 10, 2018

akien-mga Sep 13, 2018

akien-mga Sep 13, 2018

akien-mga Sep 13, 2018

akien-mga Sep 13, 2018

akien-mga Sep 13, 2018

akien-mga Sep 13, 2018

akien-mga Sep 13, 2018

akien-mga Sep 13, 2018

akien-mga Sep 13, 2018

akien-mga Sep 13, 2018

akien-mga Sep 13, 2018

akien-mga Sep 14, 2018

akien-mga Sep 14, 2018

akien-mga Sep 14, 2018

akien-mga Sep 14, 2018

akien-mga commented Sep 14, 2018

ghost commented Sep 14, 2018

PetePete1984 commented Sep 19, 2018 •

edited

Loading

smcameron commented Sep 19, 2018

santouits commented Sep 20, 2018

JFonS commented Sep 20, 2018

Wircoark commented Sep 22, 2018 •

edited

Loading

JFonS commented Sep 22, 2018

SethRzeszutek commented Nov 19, 2018 •

edited

Loading

Zylann commented Nov 19, 2018 •

edited

Loading

SethRzeszutek commented Nov 19, 2018

Zylann commented Nov 19, 2018 •

edited

Loading

SethRzeszutek commented Nov 19, 2018

		@@ -0,0 +1,226 @@
		#include "noise.h"
		#include "core_string_names.h"

		@@ -0,0 +1,213 @@
		#include "noise_texture.h"
		#include "core_string_names.h"

Add SimplexNoise and NoiseTexture as new resources #21569

Add SimplexNoise and NoiseTexture as new resources #21569

Conversation

JFonS commented Aug 29, 2018 • edited Loading

Zylann commented Aug 31, 2018 • edited Loading

Megalomaniak commented Sep 2, 2018

Xrayez commented Sep 10, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akien-mga commented Sep 14, 2018

ghost commented Sep 14, 2018

PetePete1984 commented Sep 19, 2018 • edited Loading

smcameron commented Sep 19, 2018

santouits commented Sep 20, 2018

JFonS commented Sep 20, 2018

Wircoark commented Sep 22, 2018 • edited Loading

JFonS commented Sep 22, 2018

SethRzeszutek commented Nov 19, 2018 • edited Loading

Zylann commented Nov 19, 2018 • edited Loading

SethRzeszutek commented Nov 19, 2018

Zylann commented Nov 19, 2018 • edited Loading

SethRzeszutek commented Nov 19, 2018

JFonS commented Aug 29, 2018 •

edited

Loading

Zylann commented Aug 31, 2018 •

edited

Loading

PetePete1984 commented Sep 19, 2018 •

edited

Loading

Wircoark commented Sep 22, 2018 •

edited

Loading

SethRzeszutek commented Nov 19, 2018 •

edited

Loading

Zylann commented Nov 19, 2018 •

edited

Loading

Zylann commented Nov 19, 2018 •

edited

Loading