BaseObject::Free crashes my C4D [SOLVED]

On 25/06/2015 at 02:32, xxxxxxxx wrote:

User Information:
Cinema 4D Version:   R16 
Platform:      Mac OSX  ; 
Language(s) :     C++  ;

---------
Hey guys,

I have an objectData plugin where I make a cube editable.
I have a problem when I implement the line BaseObject::Free(myChild).
I use MCOMMAND_MAKEEDITABLE and I know you have to free your object when you use MCOMMAND_CURRENTSTATEOBJECT.
Do you need to free your object when you make it editable?
Or has it something to do with a static_cast? (With or without?)

When I did, it crashes C4D.
When I make the line a comment, it works again.
So I think the problem has to be there or connected with that line.

Thanks for the help.
Best regards,
Bamboudha

On 25/06/2015 at 02:39, xxxxxxxx wrote:

post code

On 25/06/2015 at 03:43, xxxxxxxx wrote:

Hi,

In my opinion, you don't need his code to check the problem.
But it's probably something like this:

  
BaseDocument *doc = hh->GetDocument;  
BaseObject *cube = BaseObject::Alloc(Ocube);  
if (!cube) return nullptr;  
BaseObject* cubeRes;  
PolygonObject* polyCube;  
ModelingCommandData cd;  
cd.doc = doc;  
cd.op = cube;  
if (SendModelingCommand(MCOMMAND_MAKEEDITABLE, cd))  
{  
  //BaseObject::Free(cube);  
  cubeRes = static_cast<BaseObject*>(cd.result->GetIndex(0));  
  if (!cubeRes) return nullptr;  
  polyCube = (PolygonObject* )cubeRes;  
  if (!polyCube) return nullptr;  
  doc->InsertObject(polyCube, nullptr, nullptr);  
}  

I know that for SendModelingCommand(MCOMMAND_CURRENTSTATETOOBJECT) you indeed have to free it, to save memory, but the docs are a bit vague about that, imo.

The docs state they both return an object, but that's it.

If I test the above code, I get the same problem as bamboudha.
I guess you don't have to free your object if you use MCOMMAND_MAKEEDITABLE.
But, I'd love to get an explanation why it behaves like this.

With kind regards,
Casimir Smets

On 25/06/2015 at 05:11, xxxxxxxx wrote:

May be use something like this ?

Not tested.

  
PolygonObject *MakeEditableCube()  
{  
  AutoAlloc<BaseObject> cube(Ocube);  
  if (!cube) return nullptr;  
  
  ModelingCommandData cd;  
  cd.doc = doc;  
  cd.op = cube;  
  if (SendModelingCommand(MCOMMAND_MAKEEDITABLE, cd))  
  {  
      cube.Free(); //crash...  
  
      PolygonObject* polyCube = static_cast<PolygonObject*>(cd.result->GetIndex(0));  
      if (!polyCube) return nullptr;    
      return polyCube;  
  }  
  return nullptr;    
}  

On 25/06/2015 at 05:19, xxxxxxxx wrote:

Hi,

I've tested that, but it is not working...
And what do you mean by memory leak? Freeing the cube before the static cast?
If you mean that, it shouldn't be a problem, imo, because the modelingcommanddata stores the cubedata.

Also, a moderator told me to do that before the static_cast, BUT that was with MCOMMAND_CURRENTSTATETOOBJECT. But the Modelingcommanddata should be the same in both ways.

Greetings,
Casimir Smets

On 25/06/2015 at 07:31, xxxxxxxx wrote:

The code that's being shown here uses a HierarchyHelp and given that the OP mentioned an ObjectData plugin, I assume that the code is meant to be part of an ObjectData::GetVirtualObjects() call.

You cannot insert anything into a document in GVO, simply return the generated BaseObject pointer. That alone might well be causing the crash.

Steve

On 25/06/2015 at 07:34, xxxxxxxx wrote:

I think you need to remove

  
cd.doc = doc  

> You cannot insert anything into a document in GVO, simply return the generated BaseObject pointer. That alone might well be causing the crash.

Yes this is a problem too.

On 25/06/2015 at 08:00, xxxxxxxx wrote:

Hi,

I have to note, the code I posted is not his code.
I might have mixed up some code from GVO and Message.
I've tested it now in GVO with a proper return (I was looking at some own code where I apply currentstatetoobject, but inside MSG_COMMAND (where it is allowed to insert objects), sorry for the confusion!) and it still crashes.

Also:

cd.doc = doc

This needs to be there, or the modelingcommand fails.
Either this: cd.doc = doc OR cd.doc = hh->GetDocument().

This is my code inside GVO (again, not from bamboudha, it may differ) (the concept should be the same) :

  
BaseObject* MyObjectData::GetVirtualObjects(BaseObject* op, HierarchyHelp* hh)  
{  
  BaseDocument* doc = hh->GetDocument();  
  BaseObject* cube = BaseObject::Alloc(Ocube);  
  if (!cube) return nullptr;  
  BaseObject* cubeRes;  
  PolygonObject* polyCube;  
  ModelingCommandData cd;  
  cd.doc = doc;  
  cd.op = cube;  
  if (!SendModelingCommand(MCOMMAND_MAKEEDITABLE, cd))  
  {  
      return nullptr;  
  }  
  else  
  {  
      cubeRes = static_cast<BaseObject*>(cd.result->GetIndex(0));  
      if (!cubeRes) return nullptr;  
      polyCube = (PolygonObject* )cubeRes;  
      if (!polyCube) return nullptr;  
  }  
return polyCube  
}  

If he has something like this, or if he maybe inserts this in a parent, and returns the parent, it's the same as my approach.
And I have the crash as well if I try to free my cube.
Feel free to test it yourself...

I was wondering that maybe the ModelingCommand would free it itself, but that makes no sense, I guess, because with MCOMMAND_CURRENTSTATETOOBJECT you have to free it.

Maybe some input from the moderators would be nice?

Greetings,
Casimir Smets

On 25/06/2015 at 08:16, xxxxxxxx wrote:

Typically, for GVO(), it is best to use a temporary 'fake' document.  Simply use:

AutoAlloc<BaseDocument> tdoc;
if (!tdoc)
   return nullptr;

Use that in cd.doc.  It might also be required that you insert the cube into this document for the command to work.  Some don't require it, others do.  When the method goes out of scope (you exit GVO()), both the BaseDocument and anything attached to it are automatically freed.

On 25/06/2015 at 08:44, xxxxxxxx wrote:

Hi Robert,

In my experience, you don't need to use a temporary fake document for MCOMMAND_MAKEEDITABLE or MCOMMAND_CURRENTSTATETOOBJECT. It works perfect with the active document.
And about inserting the cube, I've also tested this inside MSG_COMMAND, so my cube was allready created, and the same problem occured there.

It's just some weird problem if you try to free the object.

Greetings,
Casimir Smets

On 25/06/2015 at 15:17, xxxxxxxx wrote:

Here's some code which works correctly (obviously NOT called from GVO!) :

  
{   
     BaseObject *cube = BaseObject::Alloc(Ocube);   
     AutoAlloc<BaseDocument> doc2;   
     doc2->InsertObject(cube, nullptr, nullptr);   
  
     if(cube != nullptr)   
     {   
          ModelingCommandData cd;   
          cd.doc = doc2;   
          cd.op = cube;   
          if (SendModelingCommand(MCOMMAND_MAKEEDITABLE, cd))   
          {   
               BaseObject* polyCube = static_cast<BaseObject*>(cd.result->GetIndex(0));   
               if(polyCube != nullptr)   
               {   
                    doc->InsertObject(polyCube, nullptr, nullptr);   
                    doc->Message(MSG_UPDATE);   
               }   
          }   
     }   
}   

It doesn't crash and there are no memory leaks because the temporary document goes out of scope and it and its objects are destroyed.

Steve

On 26/06/2015 at 00:45, xxxxxxxx wrote:

Hi guys,

Thanks for all the replies.
But I have to do this in GVO, otherwise the user can't change the points of the cube.

Best regards,
Bamboudha

On 30/06/2015 at 04:58, xxxxxxxx wrote:

Hi Bamboudha,

as Mohamed already asked in his first answer, please post us some code, so we can understand, what you are doing in GVO. I think, then we should be able to find a solution pretty fast.

On 30/06/2015 at 05:15, xxxxxxxx wrote:

Hi guys,

My code is practically the same as Casimir's.
Only difference is that I put my cube as a child of a baseobject and return that baseobject.

Originally posted by xxxxxxxx

 
BaseObject* MyObjectData::GetVirtualObjects(BaseObject* op, HierarchyHelp* hh)  
{  
  BaseDocument* doc = hh->GetDocument();  
  BaseObject* cube = BaseObject::Alloc(Ocube);  
  if (!cube) return nullptr;  
  BaseObject* cubeRes;  
  PolygonObject* polyCube;  
  ModelingCommandData cd;  
  cd.doc = doc;  
  cd.op = cube;  
  if (!SendModelingCommand(MCOMMAND_MAKEEDITABLE, cd))  
  {  
      return nullptr;  
  }  
  else  
  {  
      cubeRes = static_cast<BaseObject*>(cd.result->GetIndex(0));  
      if (!cubeRes) return nullptr;  
      polyCube = (PolygonObject* )cubeRes;  
      if (!polyCube) return nullptr;  
  }  
return polyCube  
}  

thanks for the help!
with kind regards,
Bamboudha

On 01/07/2015 at 05:50, xxxxxxxx wrote:

Hi,

I finally had the time to check. MCOMMAND_MAKEEDITABLE frees the original object internally. That's why you get the crashes. So you only need to free the cube in case of an error.
I admit, the docs are a bit vague in this point. I will add comments to the docs, but it may take some time, as I have to check all the other commands as well...

Furthermore I think, due to the temporary document Steve's code is a little bit cleaner. You simply don't have to worry about the command applying changes to the active document, which would not be allowed in GVO.

@Casimir: In your code in the end you double cast the result from the command.

        cubeRes = static_cast<BaseObject*>(cd.result->GetIndex(0));
        if (!cubeRes) return nullptr;
        polyCube = (PolygonObject* )cubeRes;
        if (!polyCube) return nullptr;

There's no need for this. A cast is not a function executing any code. Here it's merely telling the compiler to interpret the memory the pointer is pointing to in a different way. So you tell the compiler: "Use the pointer as a pointer to BaseObject. Oh, wait a minute, forget it, use it as pointer to a PolygonObject."
And you also don't have to check for nullptr after each cast. The pointer doesn't change by getting casted. It's, as I already said, only being reinterpreted.
So this is enough:

        polyCube = static_cast<PolygonObject*>(cd.result->GetIndex(0));
        if (!polyCube) return nullptr;

I recommend some read on casting, for example on cplusplus.com.

On 01/07/2015 at 07:42, xxxxxxxx wrote:

Hi Andreas,

Thanks for your tips!
I will change them in my code and I will definately read some more on casting.

Greetings,
Casimir Smets

On 01/07/2015 at 07:43, xxxxxxxx wrote:

Hi,

Thank you very much for your explanation. very helpfull.

kind regards,
Bamboudha