open-ead / sead

Decompilation of sead: the standard C++ library for first-party Nintendo games
190 stars 26 forks source link

gfx: Add Viewport header #111

Closed MonsterDruide1 closed 2 years ago

MonsterDruide1 commented 2 years ago

This change is Reviewable

aboood40091 commented 2 years ago

The data type of the member is Graphics::DevicePosture, which is currently already defined in gfx/seadProjection.h, but it would be better if it could be separated into its proper header gfx/seadGraphics.h. I have mistakenly put Graphics as a namespace even though it's a class. I fixed this in my own repo.

MonsterDruide1 commented 2 years ago

Okay, fixed that - thanks for the suggestion!

MonsterDruide1 commented 2 years ago

The use of the destructor within Super Mario Odyssey suggests that it has been declared =default in the header, as the existing destructors never get called by any code. However, they do still exist... how is something like that achievable? How can the destructor be inline and explicit at the same time?

Another thing I noticed is that the constructor is always used by other classes and never called inline, except for a single place: al::VewRenderer::drawView seems to construct a Viewport-instance in stack for some calculation, but does not call a constructor, apparently inlined it. How is this possible?

leoetlino commented 2 years ago

The use of the destructor within Super Mario Odyssey suggests that it has been declared =default in the header, as the existing destructors never get called by any code. However, they do still exist... how is something like that achievable? How can the destructor be inline and explicit at the same time?

inline and explicit are two completely orthogonal things.

Not sure what you mean by an explicit destructor though, as destructors cannot be marked explicit. Do you mean an explicitly defaulted destructor?

You can do = default; in the .cpp file, which will cause the destructor to be emitted as a standalone function, That doesn't prevent inlining, though -- functions that are part of the same translation unit as the destructor will still be able to inline it (if LLVM decides it's worth inlining).

MonsterDruide1 commented 2 years ago

Sorry, I'm not fluent with all of the terminology. By explicit I meant that the function does exist in the list of functions and does have a symbol, not the explicit keyword... - is there a better name for it?

What I meant here is that the destructor functions with their symbol are never called, but the destructor is always inlined instead. This leads me to think that the destructor was declared = default; in the header, so its definition is in the same TU as every usage afterwards. However, this causes the function not to be emitted as a standalone function with its symbol, as it should get optimized away due to no usages afterwards. How can a setup like this exist?

The second thing I mentioned should be mostly straight-forward: The constructor got inlined in one single place which definitely is not in the same Translation Unit.

leoetlino commented 2 years ago

Sorry, I'm not fluent with all of the terminology. By explicit I meant that the function does exist in the list of functions and does have a symbol, not the explicit keyword... - is there a better name for it?

Not really. I'd just say "emitted".

What I meant here is that the destructor functions with their symbol are never called, but the destructor is always inlined instead. This leads me to think that the destructor was declared = default; in the header, so its definition is in the same TU as every usage afterwards. However, this causes the function not to be emitted as a standalone function with its symbol, as it should get optimized away due to no usages afterwards. How can a setup like this exist?

Viewport's destructor is virtual. The emission of virtual functions follows these rules:

In your case, Viewport has no key function so the vtable and inline virtual functions are only emitted if the class is referenced. You can get Clang to emit the Viewport destructor if you define Viewport::Viewport() (in seadViewport.cpp).

MonsterDruide1 commented 2 years ago

Okay, so header-defined can still emit a function, if it is defined virtual - that was the key part missing in my knowledge here. This means that it should be okay like this, I think.

MonsterDruide1 commented 2 years ago

^ bump