Noise() function is very slowwwww

On 16/05/2013 at 16:53, xxxxxxxx wrote:

User Information:
Cinema 4D Version:   13 
Platform:   Windows  ;   
Language(s) :     C++  ;

---------
Hi,

When I use the Noise() function in my shader plugin. It's crazy slow. Not only does it render very slow. But it's very, very slow everywhere else too.
For example: When I open the material. It's extremely slow to open. And the preview image takes a very long time to render that little preview image in the corner.

This extreme slowness does not happen when I create a material using the built-in shader options:
-Create a new material
-In the color channel's Texture option. Add a noise shader
-In the noises's Noise option. Select Cranal

Here's how I'm writing the code in my plugin:

Vector MyShader::Output(BaseShader *chn, ChannelData *cd)  
{  
    
  Vector p = cd->p;  
  p *= 10;  
  LONG seed = 555;  
  C4DNoise *mynoise = C4DNoise::Alloc(seed);  
  Real n = mynoise->Noise(NOISE_CRANAL, FALSE, p, 0.0, 1.0, FALSE, 0.25, 0.25, 0);  
  C4DNoise::Free(mynoise);  
  
  return Vector(n);  
}

What's making my Noise() function so super slow?
How do I make my own implementation of the Noise() function work as fast as the ones that are already built into the materials options?

-ScottA

On 16/05/2013 at 18:01, xxxxxxxx wrote:

you should allocate the c4dnoise in initrender() and free it in freerender(). normally you do
also call c4dnoise.InitFBM() before sampling any points, but i think noise() won't return 
anything / throw a console error when the cranal noise would rely on the FBM noise.

On 16/05/2013 at 18:49, xxxxxxxx wrote:

I wanted to allocate the Noise() function in the InitRender() method. But that function requires the vector position of the shader as one of it's params. And I don't see how to handle that kind of scenario in the InitRender() method.

The turbulence() functions are allocated in the Output() method. And they work fine there without being slow. So I put my Noise() in the same place.
I need to see an example how to write the Noise() function properly.

-ScottA

On 16/05/2013 at 22:39, xxxxxxxx wrote:

The C4DNoise allocator does not require a position vector as input, only the seed value. As littledevil already pointed
out, you can (actually need to, because this is what makes it slow) allocate the C4DNoise once only instead of each
and every time a color is calculated.

What do you mean with "the turbulence() functions are allocated in the Output() method"?

-Nik

On 16/05/2013 at 23:31, xxxxxxxx wrote:

I think I've got it figured out.
I was trying to use the Noise() function inside of the InitRender() method. And that was tripping me up because the Noise() function requires a position parameter that I could not figure out how to satisfy inside of the the InitRender() method.

I don't know why I was thinking that I HAD to use the Noise() function inside the InitRender() method along with the allocation.
For some reason I thought I had to keep them together in the same scope. I guess I do that kind of thing so often that I've developed a habit. And forgot that I don't have to keep them together like that.

If I allocate a C4DNoise instance in InitRender(). And then keep my Noise() function inside the Output() method. It all seems to work fine. And it's not slow anymore.

-ScottA

On 17/05/2013 at 00:17, xxxxxxxx wrote:

If you allocate it in InitRender() (which is the right way to do it), do NOT forget to free it in FreeRender(), otherwise you will end up with a nice pile of memory leaks, that eat up more and more memory with every render.

Remember to use the -debug command line parameter (which used to be c4d_debug.txt) to have the console window come up and do some memory tracing.

On 17/05/2013 at 07:56, xxxxxxxx wrote:

I've always used the c4d_debug.txt file to do my debugging. But I would like to try using the command line approach to see if I like it better.
But the problem is I've never done this before. And I don't know how to do it.

After searching around for a bit. I found some examples for plain C++

`int main(int argc, char** argv)

But C4D plugins don't have an int main() function. So I don't know how to set it up.
And then I'm not sure how to use it.
In the C++ examples I'm finding. They are peppered with commands throughout the code.
Is there a way to have the entire plugin be checked for memory leaks with one -debug instance like the c4d_debug.txt works?

-ScottA
`

On 17/05/2013 at 08:14, xxxxxxxx wrote:

Call Cinema 4D from the command-line just like you would call any other program from the command-line.

On 17/05/2013 at 08:35, xxxxxxxx wrote:

Ah. Ok.
I was confusing "command args" with "command line".

Thanks,
-ScottA

On 17/05/2013 at 09:49, xxxxxxxx wrote:

Since there are no examples in the SDK for how to use the Noise() function.
I'm posting a simple example. Just in case anyone needs it:

  
//This is an example of using the Noise() function in a shader plugin  
  
#include "c4d.h"  
#include "c4d_symbols.h"  
#include "myshader.h"  
#include "lib_noise.h"  
  
#define PLUGIN_ID 1000000  //temp ID only!!!  
  
  
class MyShader : public ShaderData  
{  
  private:   
  C4DNoise *mynoise;  
  
  public:  
      virtual Bool               Init(GeListNode *node);  
      virtual    Vector             Output(BaseShader *chn, ChannelData *cd);  
      virtual    INITRENDERRESULT   InitRender(BaseShader *sh, const InitRenderStruct &irs);  
      virtual    void               FreeRender(BaseShader *sh);  
  
      static NodeData *Alloc(void) { return gNew MyShader; }  
};  
  
Bool MyShader::Init(GeListNode *node)  
{  
  BaseContainer *data = ((BaseShader* )node)->GetDataInstance();  
  
  mynoise = NULL;  
  
  return TRUE;  
}  
  
INITRENDERRESULT MyShader::InitRender(BaseShader *sh, const InitRenderStruct &irs)  
{  
  BaseContainer *data = sh->GetDataInstance();  
  
  LONG seed = 555;  
  mynoise = C4DNoise::Alloc(seed);  
    
  return INITRENDERRESULT_OK;  
}  
  
Vector MyShader::Output(BaseShader *chn, ChannelData *cd)  
{      
  Vector p = cd->p;  
  p *= 10;            //The number of repeats  
  
  Real n = mynoise->Noise(NOISE_CRANAL, FALSE, p, 0.0, 1.0, FALSE, 0.25, 0.25, 0);  
  
  return Vector(n);  
}  
  
void MyShader::FreeRender(BaseShader *sh)  
{  
  mynoise->Free(mynoise);  
}  
  
Bool RegisterShaderPlugin(void)  
{  
  return RegisterShaderPlugin(PLUGIN_ID,GeLoadString(IDS_MY_SHADER),0, MyShader::Alloc,"myshader",0);  
}

-ScottA

On 17/05/2013 at 10:07, xxxxxxxx wrote:

Scott, I'm so confused when you say "function" everytime, because this word relates to a global
function in the usual terms. The Noise() "function" is a method in this case, a method of the
C4DThread class. :slightly_smiling_face:

On 17/05/2013 at 10:16, xxxxxxxx wrote:

Dangerous, Scott :)

You should definitely check 'mynoise' for NULL after allocating it in InitRender(), and then return INITRENDERRESULT_OUTOFMEMORY if it's NULL. If I think about it, you should always check all pointers for NULL:

  
INITRENDERRESULT MyShader::InitRender(BaseShader *sh, const InitRenderStruct &irs;)   
{   
    if (!sh)   
        return INITRENDERRESULT_OUTOFMEMORY;   
    BaseContainer *data = sh->GetDataInstance();   
  
    LONG seed = 555;   
    mynoise = C4DNoise::Alloc(seed);   
    if (!mynoise)   
        return INITRENDERRESULT_OUTOFMEMORY;   
       
    return INITRENDERRESULT_OK;   
}   

The line in FreeRender() will probably not work. Since Free() is a static function of the BaseShader class, it has to be as follows (also add a NULL check here, just in case).

void MyShader::FreeRender(BaseShader *sh)   
{   
    if (mynoise) BaseShader::Free(mynoise);   
}

And as a last tip: I would not set the mynoise pointer to NULL in the Init() function. To have a defined state of the MyShader class after allocation, you must initialize all members in the class constructor. Also use the INSTANCEOF() macro to indicate that your shader class is an instance of BaseShader. That enables you to use SUPER to call the parent class' functions (shown further down in Init()).

  
class MyShader : public ShaderData   
{   
    INSTANCEOF(MyShader, BaseShader)   
  
    private:   
    C4DNoise *mynoise;   
  
    public:   
        virtual Bool               Init(GeListNode *node);   
        virtual    Vector             Output(BaseShader *chn, ChannelData *cd);   
        virtual    INITRENDERRESULT   InitRender(BaseShader *sh, const InitRenderStruct &irs;);   
        virtual    void               FreeRender(BaseShader *sh);   
  
        static NodeData *Alloc(void) { return gNew MyShader; }   
  
        MyShader() : mynoise(NULL)   
        { }   
};   

Use Init() only to initialize (= set default values for) the parameters in your shader's BaseContainer. And again, an additional NULL pointer check for 'data'.

  
Bool MyShader::Init(GeListNode *node)   
{   
    BaseContainer *data = ((BaseShader* )node)->GetDataInstance();   
    if (!   
  
    // Set default parameters in 'data' here   
    data->SetReal(MYSHADER_PARAMETER_BLAH, 1.0);   
    data->SetLong(MYSHADER_PARAMETER_FOOBAR, 123);   
  
    return SUPER::Init(node);   
}   

On 17/05/2013 at 10:30, xxxxxxxx wrote:

I'm just curious: Why does the C4D API use the INSTANCEOF() macro? In my humble opinion, it
is doing some "hidden magic" (which is what macros always do) and this one is making even darker
magic. From the name itself it is almost not possible to conclude it typedefs the parent class as
SUPER.

> typedef ShaderData SUPER;

So clear, and even a bit shorter. :slightly_smiling_face:

-Nik

On 17/05/2013 at 10:49, xxxxxxxx wrote:

Thanks Frank.
The code I posted works works fine for me with no memory leaks. But I do admit that it's very lean, and not using of defensive code.

I didn't now that about the INSTANCEOF() macro.
That code has always been a bit of a mystery to me about what it's really used for.

I do usually use the style:  type::Free(instance) to free allocated memory.
And I only do it that way because that's how other people use it. Not knowing why I'm using it that way. Rather than using the style: mynoise- >Free(mynoise);
Most of the time I can't use the style: mynoise->Free(mynoise); because it doesn't free the memory.
But in this case it worked. So that's what I used.
But I really don't have a good grasp of why one style works. And one style doesn't.
I'm self taught. And C++ memory management is something I've never really understood properly.

-ScottA

On 17/05/2013 at 10:58, xxxxxxxx wrote:

Originally posted by xxxxxxxx

I didn't now that about the INSTANCEOF() macro.
That code has always been a bit of a mystery to me about what it's really used for.

And here's my proof, thanks Scott. ;) The INSTANCEOF() macro is one of the most confusing things
I have seen in the C4D API. :smile:

On 17/05/2013 at 12:42, xxxxxxxx wrote:

Nah, it's not confusing. It just enables you to use SUPER::blah() instead of ShaderData::Blah(). You don't have to use it, but it saves you some typing work, especially if you override more functions than just one. No voodoo going on here.

If you ever wonder what stuff does, just look up how it's defined.

On 17/05/2013 at 12:48, xxxxxxxx wrote:

class MyShader : public ShaderData {
  
    typedef ShaderData SUPER;
    INSTANCEOF(MyShader, ShaderData);
  
};

Shorter! :) And those lines are equal. Of course if you know very well what the macro does, this
line won't interfere you while analyzing code. But for someone just getting started in C++, this
is very confusing because

1. he doesn't know what the macro does, and it's name is not really implying that it typedefs the
base class to SUPER. Yes, this is in the documentation, but having to look up what a macro
actually does is IMHO a bad design. Though, this is really just my subjective opinion.
2. he wonders why the compiler complains that SUPER is not defined when he is creating is first
subclass (he obviously forgot about the INSTANCEOF() part).

That's at least what I remember from back then. Until I actually saw the definition line in the C4D
header files, I assumed it to be some black magic.

Best,
-Niklas

On 18/05/2013 at 04:38, xxxxxxxx wrote:

Originally posted by xxxxxxxx

but having to look up what a macro actually does is IMHO a bad design

If you don't look it up, how would you ever know what it does (except if it's indicated by the macro's name)? That is not bad design, it just normal to look things up. There are functions and macros and structures and lots of other stuff... if you don't know what they are for, look it up. It's just one click.

If you only assume what stuff is good for, without verifying your assumptions, and then considering stuff black magic based on your assumption, you should rethink your strategy. Remember that old saying: "Every catastrophe's beginning is a s**tty assumption."