Create and insert material

Helper

On 02/11/2016 at 01:49, xxxxxxxx wrote:

Yes, I am confused, but everything is very good information, thanks Scott and Andreas.
I will need some time to digest all the information.

What I learned so far:
- what I did wrong initially ( "masking" a local variable by redefining it in the scope of the if)
- use autoalloc to create new material

Here is the code (based on Scott's AutoAlloc example) I am using now:

	//Create the empty material pointer
    BaseMaterial *mymat = NULL;
  
    //Check if the material already exists
    mymat = doc->SearchMaterial("my mat");
    if (!mymat)
    {
        AutoAlloc<BaseMaterial> aam(Mmaterial); //Creates an instance of the Mmaterial class
        if (!aam) return;
  
        mymat = aam.Release();   //<---Passes the instance created in aam to the pointer "mymat". Then kills aam
  
        //Now you can use mymat from here on out...
        mymat->SetName("my mat");
        doc->InsertMaterial(mymat);
        mymat->SetParameter(MATERIAL_COLOR_COLOR, Vector(1, 1, 0), DESCFLAGS_SET_0);
    }

Helper

On 02/11/2016 at 02:43, xxxxxxxx wrote:

Well, you don't have to use AutoAlloc, it's a recommendation to use it. And the way the code snippet you posted is written, it somehow makes AutoAlloc useless and kind of defeats the benefits of AutoAlloc. Basically calling AutoAlloc and directly afterwards Release() is the same as just using the normal Alloc() directly, just a bit obfuscated.

So, in order to reduce your confusion, let's write your example without AutoAlloc (after all you want to continue to use the pointer to the material later on, right?) :

    //Create the empty material pointer
    BaseMaterial* mymat = nullptr;
  
    //Check if the material already exists
    mymat = doc->SearchMaterial("my mat");
    if (!mymat)
    {
        myMat = BaseMaterial::Alloc(Mmaterial);
        if (!myMat)
            return;
  
        myMat->SetName("my mat");
        myMat->SetParameter(MATERIAL_COLOR_COLOR, Vector(1, 1, 0), DESCFLAGS_SET_0);
        doc->InsertMaterial(myMat);
    }
    // Here myMat is a valid pointer to your material
    // But remember, the owner of the material pointed to by myMat is C4D at this point
    // ...

And, please, if your confusion holds on after some time of digestion, come back here and ask. As I said, I'd really like to get this straight.

Helper

On 02/11/2016 at 07:23, xxxxxxxx wrote:

That confuses me again.
You say "Well, you don't have to use AutoAlloc, it's a recommendation to use it.",
but I read "Here at MAXON such code is not considered bad practice, but plain wrong."

So, what is the benefit of autoalloc and when to use it?

I use the mymat pointer to add the mat to an object using the texture tag.

    //#assign material to object (texture tag)
    BaseTag *tag = sphereobj->MakeTag(Ttexture);    
    tag->SetParameter(TEXTURETAG_MATERIAL, mymat, DESCFLAGS_SET_0);

In this / your example, do I have to free the material at some point?
Or is cinema the owner because it is inserted in the doc?

Do I need to use the autoalloc if I wish to change the mat after inserting it into the document?

Being on the subject.
Here my code to insert an object using alloc.
Can the code be improved using autoalloc?

//#print "Create Default Sphere."
BaseObject* sphereobj = BaseObject::Alloc(Osphere);		//Create new sphere
sphereobj->SetParameter(PRIM_SPHERE_RAD, citysphereradius, DESCFLAGS_SET_0);	// set sphere label radius			
sphereobj->SetRelPos(Vector(x,y,z));						// Set position of sphere
sphereobj->SetName(country + " - " + city);
sphereobj->SetParameter(ID_LAYER_LINK, citylayer, DESCFLAGS_SET_0);  
doc->InsertObject(sphereobj, parentobject, NULL);      //Add it to the temp document

Helper

On 02/11/2016 at 07:41, xxxxxxxx wrote:

Originally posted by xxxxxxxx

the way the code snippet you posted is written, it somehow makes AutoAlloc useless and kind of defeats the benefits of AutoAlloc.

But you're using the exact same code Andreas. That's where my code came from. By copying yours.
This is why I called it "overkill" earlier. For simple creating and insertion tasks.
Now you're starting to confuse me too. !Wacko[URL-REMOVED]

Originally posted by xxxxxxxx

And, please, if your confusion holds on after some time of digestion, come back here and ask. As I said, I'd really like to get this straight.

It would help me out if you posted an example using AutoAlloc that looks for an existing material. And creates a new one if it does not exist.
Only this time. Don't use this kind of code. Because you're saying it's useless. Even though it's copied straight from your own code examples:

    AutoAlloc<BaseMaterial> mat(Mmaterial);  
  if(!mat) return false;  
  BaseMaterial *bm = mat.Release();  
  doc->InsertMaterial(bm);

Can you please post a version of this task that uses a better implementation of AutoAlloc where you don't consider it to be useless?

Thanks,
-ScottA

*Edit- This is another thing that is not being addressed clearly here and helping to add to the confusion.

BaseMaterial *mat = BaseMaterial::Alloc(Mmaterial);  
if(!mat) return false;   
  
//Is not the same thing as this  
  
BaseMaterial *mat = nullptr;  
if(condition) mat = BaseMaterial::Alloc(Mmaterial);

When you set a pointer to nullptr like Pim is doing. You'll need to keep on checking it's existence every time you use it later on in the code. Because it's been allocated inside of the if() statement.
But if you create the pointer and allocate it all in the same line like in the docs. Then you only need to check it's existence once directly after it.
Most of the time support guys here have recommended to use: BaseMaterial *mat = nullptr;
Yet the docs mostly use the all-in-one version.
I can see this as also being a point of confusion for people not yet good with using the rules of scope.

[URL-REMOVED] @maxon: This section contained a non-resolving link which has been removed.

Helper

On 02/11/2016 at 09:44, xxxxxxxx wrote:

I'm terribly sorry for adding to the confusion.

Please, also keep in mind, that this discussion started with Pim's code, but you, Scott, made a very general statement about AutoAlloc. Pim's code was never an ideal candidate for AutoAlloc. But I had to correct your general statement about AutoAlloc, as it was simply wrong.

Originally posted by xxxxxxxx

You say "Well, you don't have to use AutoAlloc, it's a recommendation to use it.",
but I read "Here at MAXON such code is not considered bad practice, but plain wrong."

The "code is considered plain wrong" is all about not checking allocated memory. Regardless of using normal Alloc() or AutoAlloc, you need to check the result for nullptr.

AutoAlloc is just a tool to make your code less error prone and more readable in regards to memory allocations. And while we recommend it, you of course need to decide on a case by case basis, if it makes sense to use or if you gain any benefits from its use.

As explained before, AutoAlloc shifts the focus where you need to pay attention to. But if you have no error exits in between AutoAlloc and transferring the ownership via Release(), what would be the point?

But ok, a few more examples (some already posted above) :

Bool idealForAutoAlloc()
{
    AutoAlloc<BaseFile> file;
    if (!file)
        return false;
    // ... doing stuff...
    if (some error condition)
        return false;
    // ... doing stuff...    
    return true;
}

So this case is ideal, because you need to allocate the BaseFile in order to access a file, but regardless of success or error, you are not interested in keeping the BaseFile instance. So here, after AutoAlloc, your mind is completely free to do important work, you need not worry about the allocated memory at all.

Next one:

Bool goodUseOfAutoAlloc(BaseDocument* doc)
{
    AutoAlloc<BaseMaterial> mat;
    AutoAlloc<BaseMaterial> mat2;
    if (!mat || !mat2)
        return false;
    // ... doing stuff...
    if (some other error)
        return false;
    doc->InsertMaterial(mat.Release());
    return true;
}

I leave it up to you, to write the above snippet without AutoAlloc. But I think you can already sense, it will be quite a few more lines of code. And a few points where you need to pay attention to not create a memory leak.

Another one:

void probablyBetterWithStandardAlloc(BaseDocument* doc)
{
    BaseMaterial* mat = BaseMaterial::Alloc(Mmaterial);
    if (!mat)
        return;
    // doing stuff here, that can't fail...
    doc->InsertMaterial(mat);
    // ... doing more stuff, that for some reason needs to still work with the pointed material (although ownership got transferred already)
    return true;
}

This example is probably not well suited for AutoAlloc. There are no error cases, where you'd need to pay attention to not create a memory leak. The pointed material is to be used after the ownership got transferred, which would add an extra line of code in AutoAlloc case (using Release() to get hold of the actual BaseMaterial pointer). So there's little to no benefit in using AutoAlloc.

To sum this up:
- AutoAlloc can be a very nice tool...
- ... to make memory allocations less error prone (memory leaks)
- ... to have nicer and more readable code
- You still need to decide if this tool suits your needs in a given case
- Sometimes a small restructuring of your code might make it very well suited for AutoAlloc and you end with a nicer code structure

Perhaps another way to see it:
AutoAlloc lets you use memory allocations more or less like local variables.

Originally posted by xxxxxxxx
    //#assign material to object (texture tag)
    BaseTag *tag = sphereobj->MakeTag(Ttexture);    
    tag->SetParameter(TEXTURETAG_MATERIAL, mymat, DESCFLAGS_SET_0);
In this / your example, do I have to free the material at some point?
Or is cinema the owner because it is inserted in the doc?

In these very few lines? No, Cinema 4D does not get owner, just because you set a link pointing to a material in a tag. You'd need to explicitly insert the material.
Look at the docs for InsertMaterial() for example, there it is explicitly stated that the document will take the ownership of the material pointed to by the mat parameter. Basically whenever there are pointers involved, the docs should state, who's the owner or if ownership gets transferred in any way. If such comments are missing, please make us aware.

Originally posted by xxxxxxxx

Do I need to use the autoalloc if I wish to change the mat after inserting it into the document?

Should be clear by now, but you never have to use AutoAlloc. It's recommended to evaluate if it's suitable and use it where ever you can leverage its benefits.

Originally posted by xxxxxxxx

Here my code to insert an object using alloc.
Can the code be improved using autoalloc?

//#print "Create Default Sphere."
BaseObject* sphereobj = BaseObject::Alloc(Osphere);		//Create new sphere
sphereobj->SetParameter(PRIM_SPHERE_RAD, citysphereradius, DESCFLAGS_SET_0);	// set sphere label radius			
sphereobj->SetRelPos(Vector(x,y,z));						// Set position of sphere
sphereobj->SetName(country + " - " + city);
sphereobj->SetParameter(ID_LAYER_LINK, citylayer, DESCFLAGS_SET_0);  
doc->InsertObject(sphereobj, parentobject, NULL);      //Add it to the temp document

Except for the missing nullptr check after the Alloc it's fine. But that check should really be added.

@Scott: I hope, my above examples help.

Helper

On 02/11/2016 at 10:07, xxxxxxxx wrote:

Thanks Andreas. I think I've got it. Thanks for your input.

I've probably only made things more confusing in this thread by asking questions on top of Pim's questions. So I'm going to exit this thread from here on out.
The only reason I even replied to his initial question was because he asked it during the weekend. Normally would not have replied during the week.

So I'm going to shut up and go away now. Tongue

-ScottA

Helper

On 02/11/2016 at 10:11, xxxxxxxx wrote:

Well, Scott, I think your post brought up a very crucial point. It's good we discussed it. And if I was able to shed some light onto this topic, than I'm glad.
That's not supposed to mean, open questions shouldn't be asked anymore. Quite the opposite.

Helper

On 02/11/2016 at 13:06, xxxxxxxx wrote:

My main confusion is with freeing memory.
I thought the main advantage if AutoAlloc is, that you need not to worry about freeing memory.
I know now - reading all the posts above - that you only need to worry about freeing memory if you are the owner.
If cinema is the owner, for example after an insert, you need not to worry about freeing memory.
So, if you are going to insert, no need for autoalloc.
But always check whether the allocation was successful!

@Andreas: can you give some example when to free memory?

Another thing is indeed the example Scott mentioned.

    
    
    BaseMaterial *mat = BaseMaterial::Alloc(Mmaterial);  
    if(!mat) return false;   
      
    //Is not the same thing as this  
      
    BaseMaterial *mat = nullptr;  
    if(condition) 
    
    
    { mat = BaseMaterial::Alloc(Mmaterial);
    
    
     if(!mat) return false; 
    
    
    }

What is the difference and what is the best way?

Again, I need to think in detail about all the information I got.
In the mean time, I have some good code snippets I can use now.

@Scott, thanks for joining in the discussion and giving all the information and asking the good questions.
@Andreas, thanks for all the information and patience.

Helper

On 04/11/2016 at 10:15, xxxxxxxx wrote:

Hi Pim

Originally posted by xxxxxxxx

I thought the main advantage if AutoAlloc is, that you need not to worry about freeing memory.
I know now - reading all the posts above - that you only need to worry about freeing memory if you are the owner.
If cinema is the owner, for example after an insert, you need not to worry about freeing memory.
So, if you are going to insert, no need for autoalloc.

Right, I'd just not put the last sentence so black and white. Check the goodUseOfAutoAlloc() example in my last point. Although the material gets inserted later on, it still makes sense to use AutoAlloc. It's more about how many places you have (e.g. error exits), where you'd need to pay attention to free the material before inserting it into the document.

Originally posted by xxxxxxxx

But always check whether the allocation was successful!

Yes, please

Originally posted by xxxxxxxx

@Andreas: can you give some example when to free memory?

Hm? Actually I think by now we have several such examples in this thread.
If you allocate memory, you are responsible to free it, except when transferring the ownership to somebody else (like C4D).

So, let's ignore AutoAlloc for a moment, maybe it adds too much to your confusion.

Bool AddMyGreatMaterial(BaseDocument* doc)
{
    BaseMaterial *mat = BaseMaterial::Alloc(Mmaterial);
    if(!mat)
        return false; 
  
    // ...adding shaders to the material...
    // in the process an error occurs
    if (error when adding shader)
    {
         // Here the material needs to be freed,
         // otherwise the material is lost (you are owner,
         // but you'll loose the pointer after returning from the function),
         // in consequence you'd have a memory leak 
        BaseMaterial::Free(mat);
        return false;
    }
    // ... imagine the function to me more complex. More allocations, more error exits...
  
    // ... and finally the success case:
    doc->InsertMaterial(mat);
    return true;

Of course there are a plethora of other cases, where you might need to free memory. For example on functions, where the docs say, that the caller owns the returned object. Unless you pass the ownership on to somebody else, you'd be responsible.
It all comes down to ownership.
If you are the owner and don't need the memory anymore, you have to free it.
If you are the owner and are about to loose the reference to that memory (pointer), you either need to free it or should think of another place to store the pointer.

Maybe these links on Memory Leaks help to shed some more light. Just my first finds, I'm sure by a little more searching you can find way better ones: stackoverflow: "What is a memory leak", Memory Leak in Wikipedia and Microsoft's contribution to the topic.

Originally posted by xxxxxxxx

// Version A
BaseMaterial *mat = BaseMaterial::Alloc(Mmaterial);
if(!mat) return false; 
 
//Is not the same thing as this
 
// Version B
BaseMaterial *mat = nullptr;
if(condition) 
{ mat = BaseMaterial::Alloc(Mmaterial);
 if(!mat) return false; 
}
// HERE: mat needs to be checked for nullptr again!

What is the difference and what is the best way?

I'm not sure I can answer this question. After all this example is so stripped down, that I don't see a best way. It depends what you want to achieve.
The difference is in version A, you have a allocated Material pointed to by mat after those two lines of code.
In version B, it depends on the condition, if you have a valid mat pointer after those lines.

But I'm really not sure, that's what you were asking for.

In the end code is always also a matter of taste, just like spoken language. So the best way might depend on the eye of the coder... with one difference, regardless of what the best way looks like, it has to be correct. And that's also true for version B as long as you add another nullptr check after the scope of the if.

I know, memory allocations and memory leaks can be a tough topic. So I suggest, you take some time to digest this thread and maybe also take the chance to read some more on these topics. Afterwards the entire AutoAlloc topic will be a breeze and you'll easily see the benefits.

Helper

On 04/11/2016 at 10:32, xxxxxxxx wrote:

Thanks Andreas.

I will indeed take some time to digest this thread and read some more on these topics.
Thanks for all the support.

Regards, Pim