sead icon indicating copy to clipboard operation
sead copied to clipboard

gfx: Add Viewport header

Open MonsterDruide1 opened this issue 2 years ago • 6 comments

This change is Reviewable

MonsterDruide1 avatar Aug 09 '22 15:08 MonsterDruide1

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.

aboood40091 avatar Aug 12 '22 02:08 aboood40091

Okay, fixed that - thanks for the suggestion!

MonsterDruide1 avatar Aug 13 '22 11:08 MonsterDruide1

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?

MonsterDruide1 avatar Aug 13 '22 21:08 MonsterDruide1

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).

leoetlino avatar Aug 15 '22 20:08 leoetlino

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.

MonsterDruide1 avatar Aug 16 '22 09:08 MonsterDruide1

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:

  • if the class has a key function (defined as the first non-pure and non-inline virtual function), then the vtable and inline virtual functions are emitted in the TU that defines the key function
  • if the class has no key function, then the vtable and inline virtual functions are emitted in every single translation unit that uses the class, and then those definitions are deduplicated by the linker

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).

leoetlino avatar Aug 27 '22 23:08 leoetlino

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 avatar Aug 31 '22 15:08 MonsterDruide1

^ bump

MonsterDruide1 avatar Sep 09 '22 15:09 MonsterDruide1