PixarAnimationStudios / OpenSubdiv

An Open-Source subdivision surface library.
graphics.pixar.com/opensubdiv
Other
2.89k stars 561 forks source link

Forward Declaration and Usage in Hbr Source #206

Closed ghost closed 9 years ago

ghost commented 11 years ago

To whom it may concern,

I know that a good C++ programer tends to use forward declaration instead of including the header file of a class.

From my perspective, I think by forward declaring a class, we can then declare and use the pointer or reference of this class without including the header file of the class. However, in such case, we can not use the pointer to call the method of the class, because the detailed implementation of the class is not known.

I am currently reading the Hbr source and I am confused about the usage of forward declaration in opensubdiv / hbr / halfedge.h where the calss HbrHalfedge is defined. The following is one of HbrHalfedge's methods:

HbrHalfedge<T>* GetPrev() const { (some code); GetFace()->GetNumVertices() (some code) }

However GetFace()'s return type is HbrFace* .

By reading the source, I find that halfedge.h does not include face.h which define the HbrFace class. Instead, forward declaration is used in halfedge.h: template <class T> class HbrFace

So I wonder why the GetNumVertices() which is the method of HbrFace(defined in face.h) can be called in halfedge.h without including the face.h?

Thanks for your time.

Best regards, Tyler

ghost commented 11 years ago

I have been thinking about this problem these days. I check the following issue which was closed one year ago.(https://github.com/PixarAnimationStudios/OpenSubdiv/issues/3) I also read through this commit mentioned in the issue.(https://github.com/PixarAnimationStudios/OpenSubdiv/commit/7a6fd95f6baf03ac563f9f058a3c5849acaa5eac)


Now I think I partially understand how this tricky implementation works. But I still have a question about the future usage of these Hbr headers. I would be very happy, If someone can correct or confirm my current understanding.

The main idea is to break the circular include and still allow Hbr classes' methods to call each other. (I know classes defined in Hbr headers are template classes, but for convenience I call it class here )

The #include statements are placed in the middle of the class definition and implementation of methods, which allows the code in the headers included knows enough about the class during the compile process.

halfedge.h --(included in middle)--> mesh.h --(included in middle)--> face.h

Therefore foreward declaration of HbrFace ( defined in face.h ) in halfedge.h is enough, because halfedge.h is included in the middle of mesh.h which is included in the middle of face.h. This means we can presume that halfedge.h knows enough about HbrFace's methods when compiling the program that include these headers, so we can call HbrFace's methods in halfedge.h without include face.h.


But I still have a question, does the include order of these Hbr headers matters in latter client-side usage? I mean should face.h be included before mesh.h and mesh.h included before halfedge.h?

manuelk commented 11 years ago

Sorry for the slow reply: i was out of the office and some some catching up to do.

You are correct: Hbr is fighting some circular inclusion problems and the solution was the not-so-elegant kludge of forward-declaring, then including in the middle of the implementation.

I have not run recently into a case where the order of inclusion of the Hbr headers was causing any problems, but i make no promises that any random inclusion order is going to work... This is somewhat unfortunate, but i am hoping that this complication is offset by the benefits of having the entire API protected behind versioned name-spaces.

ghost commented 11 years ago

That's OK. I am on summer vacation now~ Thank you for answering my question and I agree that the API is well protected by the versioned name-space.

Yesterday a friend of mine told me an interesting feature of Template Class in C++. I think it may help solve the middle inclusion dilemma.

A Template Class's methods will not be instantiated until it is first called. I searched online and find the following reference: http://msdn.microsoft.com/en-us/library/7k8twfx7.aspx http://msdn.microsoft.com/en-us/library/7y5ca42y.aspx

This feature may be used to solve circular inclusion of class templates. For example, the methods of template classA and template classB call each other, but I don't need to use the middle inclusion trick. Because when classA and classB are instantiated in test.cpp, all required headers have already been included (test.cpp includes ClassA.h which includes ClassB.h)

File: test.cpp Include ClassA.h Instantiate ClassA and ClassB Call each other and only at this time will the compiler try to find the definition of "callMemberFunctionOfClassX" And at that time all headers have already been included

#include "ClassA.h"
int main(int argc, char** argv){

ClassA<int> a;
ClassB<int> b;

a.callMemberFunctionOfClassB(&b);
b.callMemberFunctionOfClassA(&a);

return 0;
}

File: ClassB.h Call ClassA's methods without including ClassA.h

#ifndef CallingEachOther_ClassB_h
#define CallingEachOther_ClassB_h

template <class T> class ClassA;

template <class T>
class ClassB {
public:
    void someMemberFunctionB();
    void callMemberFunctionOfClassA(ClassA<T> *a);

};

template <class T>
void ClassB<T>::callMemberFunctionOfClassA(ClassA<T> *a){
    a->someMemberFunction();   
}

template<class T>
void ClassB<T>::someMemberFunctionB() {

    // do some thing related to ClassB    
}

#endif

FIle: ClassA.h ClassB.h is included in the beginning not in the middle

#ifndef CallingEachOther_ClassA_h
#define CallingEachOther_ClassA_h

#include "ClassB.h"

template <class T>
class ClassA {
public:
    void someMemberFunction();
    void callMemberFunctionOfClassB(ClassB<T> *b);
};

template <class T>
void ClassA<T>::someMemberFunction(){

    //do something related to ClassA
}

template <class T>
void ClassA<T>::callMemberFunctionOfClassB(ClassB<T> *b){
    b->someMemberFunctionB();
}

#endif

As for Hbr headers (class templates), they will only be instantiated when the client uses them to represent edge, face or mesh. e.g., Face. At that time, I think all required headers are included.

Therefore, in my local code base, I tried to move the middle inclusion in face.h, fvarData.h, hierarchicalEdit.h and mesh.h to the beginning. After that I also have to forward declare HbrfvarData in face.h which is missed in face.h. Then glViewer, limitEval and simpleCPU all work well. ( I didn't test other examples. )

So I think this feature may be used to tackle the middle inclusion problem and make split namespace in these headers get together.

manuelk commented 11 years ago

Interesting... we are using this compiler trick in several places already. I'll be looking into this, but i would need to make sure this change does not break PRman's build...

manuelk commented 9 years ago

Obsoleting this thread : Hbr is mostly deprecated in 3.0 and for regression purposes, it is unlikely that we will introduce changes in that code. Thanks for bringing these issues to our attention: i believe the new code does not have this problem any more.