Closed jjeziorski closed 4 years ago
Awesome, appreciated!
We're still finishing preparing the next release of appleseed (2.1.0-beta) so PR reviews are on hold for a few more days, but we will get back to you.
Thanks again!
@dictoon Yes, I've figured. No pressure.
I tried your changes and here are some results. I didn't investigate any of the problems.
Builds fine but fails to copy files:
...
3>Moving output files...
3>The system cannot find the file specified.
3>The system cannot find the file specified.
...
2>appleseed-max2020-impl.vcxproj -> N:\appleseed\appleseed-max\src\..\build\appleseed-max2020-impl\x64\Ship\appleseed-max2020-impl.dll
2>Moving output files...
2>The system cannot find the file specified.
2>The system cannot find the file specified.
...
...
2> Creating library N:\appleseed\appleseed-max\src\..\build\appleseed-max2019-impl\x64\Ship\appleseed-max2019-impl.lib and object N:\appleseed\appleseed-max\src\..\build\appleseed-max2019-impl\x64\Ship\appleseed-max2019-impl.exp
2>utilities.obj : error LNK2001: unresolved external symbol "__declspec(dllimport) public: virtual void __cdecl renderer::Source::evaluate_uniform(class renderer::DynamicSpectrum<float,31> &,class foundation::Color<float,1> &)const " (__imp_?evaluate_uniform@Source@renderer@@UEBAXAEAV?$DynamicSpectrum@M$0BP@@2@AEAV?$Color@M$00@foundation@@@Z)
2>utilities.obj : error LNK2001: unresolved external symbol "__declspec(dllimport) public: virtual void __cdecl renderer::Source::evaluate_uniform(class renderer::DynamicSpectrum<float,31> &)const " (__imp_?evaluate_uniform@Source@renderer@@UEBAXAEAV?$DynamicSpectrum@M$0BP@@2@@Z)
2>N:\appleseed\appleseed-max\src\..\build\appleseed-max2019-impl\x64\Ship\appleseed-max2019-impl.dll : fatal error LNK1120: 2 unresolved externals
A combination of the problems above:
...
3>appleseed-max2018.vcxproj -> N:\appleseed\appleseed-max\src\..\build\appleseed-max2018\x64\Ship\appleseed-max2018.dlr
3>appleseed-max2018.vcxproj -> N:\appleseed\appleseed-max\src\..\build\appleseed-max2018\x64\Ship\appleseed-max2018.pdb (Partial PDB)
3>The system cannot find the file specified.
3>The system cannot find the file specified.
...
2> Creating library N:\appleseed\appleseed-max\src\..\build\appleseed-max2018-impl\x64\Ship\appleseed-max2018-impl.lib and object N:\appleseed\appleseed-max\src\..\build\appleseed-max2018-impl\x64\Ship\appleseed-max2018-impl.exp
2>utilities.obj : error LNK2001: unresolved external symbol "__declspec(dllimport) public: virtual void __cdecl renderer::Source::evaluate_uniform(class renderer::DynamicSpectrum<float,31> &,class foundation::Color<float,1> &)const " (__imp_?evaluate_uniform@Source@renderer@@UEBAXAEAV?$DynamicSpectrum@M$0BP@@2@AEAV?$Color@M$00@foundation@@@Z)
2>utilities.obj : error LNK2001: unresolved external symbol "__declspec(dllimport) public: virtual void __cdecl renderer::Source::evaluate_uniform(class renderer::DynamicSpectrum<float,31> &)const " (__imp_?evaluate_uniform@Source@renderer@@UEBAXAEAV?$DynamicSpectrum@M$0BP@@2@@Z)
2>N:\appleseed\appleseed-max\src\..\build\appleseed-max2018-impl\x64\Ship\appleseed-max2018-impl.dll : fatal error LNK1120: 2 unresolved externals
Same thing:
...
3>appleseed-max2017.vcxproj -> N:\appleseed\appleseed-max\src\..\build\appleseed-max2017\x64\Ship\appleseed-max2017.dlr
3>appleseed-max2017.vcxproj -> N:\appleseed\appleseed-max\src\..\build\appleseed-max2017\x64\Ship\appleseed-max2017.pdb (Partial PDB)
3>The system cannot find the file specified.
3>The system cannot find the file specified.
...
2> Creating library N:\appleseed\appleseed-max\src\..\build\appleseed-max2017-impl\x64\Ship\appleseed-max2017-impl.lib and object N:\appleseed\appleseed-max\src\..\build\appleseed-max2017-impl\x64\Ship\appleseed-max2017-impl.exp
2>utilities.obj : error LNK2001: unresolved external symbol "__declspec(dllimport) public: virtual void __cdecl renderer::Source::evaluate_uniform(class renderer::DynamicSpectrum<float,31> &,class foundation::Color<float,1> &)const " (__imp_?evaluate_uniform@Source@renderer@@UEBAXAEAV?$DynamicSpectrum@M$0BP@@2@AEAV?$Color@M$00@foundation@@@Z)
2>utilities.obj : error LNK2001: unresolved external symbol "__declspec(dllimport) public: virtual void __cdecl renderer::Source::evaluate_uniform(class renderer::DynamicSpectrum<float,31> &)const " (__imp_?evaluate_uniform@Source@renderer@@UEBAXAEAV?$DynamicSpectrum@M$0BP@@2@@Z)
2>N:\appleseed\appleseed-max\src\..\build\appleseed-max2017-impl\x64\Ship\appleseed-max2017-impl.dll : fatal error LNK1120: 2 unresolved externals
Regarding
Remove explicit windows platform specification (if needed it should be added to .user files)
This was done to match the requirements of the various 3ds Max SDKs. I don't know how important it is, but that's the reason behind the hardcoded SDK versions.
@dictoon Thanks for testing. It's rather strange esp. missing symbols. I'll investigate this.
I just had another look at this. The missing symbols are my fault: I'm linking against an appleseed library built without spectral support (which, btw, is the correct thing to do with the Max plugin because it doesn't support nor need spectral support), so please ignore those.
I think the third argument passed to post-build.cmd
(%targetdir%
) is wrong: in the case of 3ds Max 2017, it's set here to
"N:\appleseed\appleseed-max\src\..\build\appleseed-max2017\x64\Release\appleseed-max2017.dlr"
instead of
"N:\appleseed\appleseed-max\src\..\build\appleseed-max2017\x64\Release\"
(which I presume is what you intended).
Hi @jjeziorski,
I've spent quite a bit of time on this PR with the goal to merge it as soon as possible.
I've done three things:
post-build.cmd
invocations.All versions of the plugin now build properly in all build configurations, and they all work properly inside their respective 3ds Max version (I've only tested the Ship configuration for each version of the plugin).
Unfortunately, as far as I can tell the factoring you did led to some lost subtleties, such as no whole program optimization / link-time code generation in Release (it should only be enabled in Ship), and possibly others.
I don't think I'll have enough time to check all three configurations (Debug, Release and Ship) for all four Max versions to make sure no other settings have been lost.
Would you be able to help? If not, I'm afraid it is too risky to merge this PR.
Too big and too risky. @jjeziorski I would love to merge some of your refactorings, they seem very useful, but they would need to be split into smaller PRs or at least smaller commits that can be fully checked for regression. This isn't the case of this PR. Closing for now.
Here's a suggested change to simplify maintenance of multiple VS projects.
Main changes:
.user
files)Please note that while I've tried to maintain same behavior in all cases, I was unable to test all configurations (I don't have all SDKs). As such please take extra care to verify if it works as expected.