gcc/Linux fixes #21

z33ky · 2020-06-14T09:24:49Z

Hi there,

I thought I'd try compiling this for Linux and hit some errors. This is my attempt at resolving them.
Some of the fixes should also benefit the Windows build (i.e. errors in format specifiers).
I've based this off of fixes I did for Entropy : Zero, so there's separate commits for gcc 8 and gcc 9/10. Between those the fixes are just kinda bunched together though. I can pull them apart (to separate commits for each individual fix or type of fix) if you like.

This crashes on Linux as pKeyValuesData is NULL when deleteThis() is invoked. This likely is caused by another issue, but fixing this here seems good regardless.

This is important for case-sensitive filesystems/operating systems (i.e. Linux).

It used new char[] before, but it seems when the buffer is allocated using filesystem->ReadFileEx() we should free() the buffer, not delete[] it. CUtlBuffer also free()s the buffer, so malloc() should be the saner choice here.

z33ky

I've left some comments for stuff that may raise an eyebrow. If you have any further questions or requests I'll be happy to oblige.
I have not tried compiling with MAPBASE_RPC or MAPBASE_VSCRIPT. If you're interested in picking up these changes I'll take a look if they require any work for gcc/Linux.

z33ky · 2020-06-14T09:27:13Z

sp/src/game/client/hl2/c_script_intro.cpp

-	if (pPlayer)
-		return GetAbsAngles() + (pPlayer->LocalEyeAngles() - pPlayer->GetAbsAngles());
+	if (pPlayer) {
+		static QAngle angAngles;


This copies what EyeAngles() does. Ideally we wouldn't return a reference from this method. Using a static here can cause unexpected behavior (i.e. player0.LocalEyeAngles() == player1.LocalEyeAngles() will always be true since the second call with "overwrite" the value from the first call).

It may be worth turning these statics into member variables for when Mapbase begins running on MP and a player-by-player distinction may be necessary, but this is fine for now.

z33ky · 2020-06-14T09:28:13Z

sp/src/game/server/hl2/script_intro.cpp

-		return GetAbsAngles() + (m_hPlayer->LocalEyeAngles() - m_hPlayer->GetAbsAngles());
-	else
+	if (m_hPlayer.Get()) {
+		static QAngle angAngles = GetAbsAngles() + (m_hPlayer->LocalEyeAngles() - m_hPlayer->GetAbsAngles());


Same thing as for the client...

z33ky · 2020-06-14T09:33:01Z

sp/src/game/server/logicentities.cpp

 	switch (m_iPositionType)
 	{
 	case POSITION_BBOX:
 		case POSITION_EARS:
-		case POSITION_ORIGIN:			angAngles = &pEntity->GetAbsAngles(); break;


This creates a pointer to a temporary this method eventually returns.
I could have gone with a static like POSITION_ATTACHMENT, but it seems cleaner to just not return a reference.
I changed GetPosition() to do the same.

sp/src/game/server/mapbase/ai_grenade.h

sp/src/game/shared/mapbase/matchers.cpp

z33ky · 2020-06-14T09:53:12Z

sp/src/public/tier1/utlsoacontainer.h

@@ -109,6 +109,95 @@ template<class T> class CStridedConstPtr
 	}
 };

+class CFltx4StridedPtr


The manual code duplication is unfortunate, but according to a gcc warning the __vector-specifier would get lost in the templated-code, meaning that (slow) unaligned access to the floats was used. By manually expanding CStridedPtr<fltx4> the compiler can emit the proper vectorized access instructions.
I don't know if MSVC actually just does carry over the specifiers or like the old gcc silently throws it away.

z33ky · 2020-06-14T10:13:03Z

sp/src/materialsystem/stdshaders/SDK_eyes_vs20.fxc

@@ -0,0 +1 @@
+SDK_Eyes_vs20.fxc


With this link in place it will create two shaders named SDK_Eyes_vs20 and SDK_eyes_vs20 respectively. I don't know which of these is the canonical/proper name so building both should work in either case.
I'm not sure if this works out on Windows though. On Linux this file is "missing" because of case sensitivity. I don't know if shader names are case sensitive. Renaming and fixing up the references will also work, but I'll need to know which one is the correct name.

The "official" (non-SDK_) shader is upper-case "Eyes". I'm guessing that's the right one and will just rename the file.

I checked HL2's stdshader_dx9.so, where it's listed as eyes_vs20. I think that's a pretty good indicator it should be lower-case, if it makes any difference at all.

Blixibon

Thank you for contributing these fixes. I have been planning on trying to make Mapbase more suitable for other operating systems (e.g. Linux and OSX) for a while now.

I've left a few comments and replies to ask some questions and clarify why some of my code is the way it is. I think most of the changes in this PR are acceptable for Mapbase, although they may require some rigorous testing before they're merged.

sp/src/materialsystem/stdshaders/shatteredglass.cpp

Blixibon · 2020-06-15T01:23:01Z

sp/src/game/client/clientshadowmgr.cpp

@@ -1468,7 +1468,8 @@ void CClientShadowMgr::InitDepthTextureShadows()
 		m_DummyColorTexture.InitRenderTargetSurface( r_flashlightdepthres.GetInt(), r_flashlightdepthres.GetInt(), IMAGE_FORMAT_BGR565, true );
 #else
 #if defined(MAPBASE) //&& !defined(ASW_PROJECTED_TEXTURES)
-		m_DummyColorTexture.InitRenderTarget( m_nDepthTextureResolution, m_nDepthTextureResolution, RT_SIZE_OFFSCREEN, nullFormat, MATERIAL_RT_DEPTH_NONE, false, "_rt_ShadowDummy" );
+		// SAUL: we want to create a render target of specific size, so use RT_SIZE_NO_CHANGE
+		m_DummyColorTexture.InitRenderTarget( m_nDepthTextureResolution, m_nDepthTextureResolution, RT_SIZE_NO_CHANGE, nullFormat, MATERIAL_RT_DEPTH_NONE, false, "_rt_ShadowDummy" );


I recall something about this particular line change from the VDC fixes causing issues with Mapbase's projected textures. I'm not sure if I'm misremembering or thinking of a different line of code, but did you re-add Saul's fix here because you thought it was a mistake or because it's actually needed?

Maybe you're thinking of depthTex, which is initialized a few lines down (line 1497 on this branch)? Because it has the same fix applied as here. Like the commit states, this change is taken from Entropy : Zero.
When I tested my binaries (on Linux) the flashlight was cut off - only one corner was showing. Eventually I figured out that if r_flashlightdepthres was larger than my vertical screen resolution it would bug out - smaller or equal and it'd look fine.
I knew Entropy : Zero also had some projected texture enhancements and the bug was not present there, so I diffed the two files. After I changed this line it worked fine with larger resolutions.

z33ky · 2020-06-15T09:47:00Z

Thank you for contributing these fixes. I have been planning on trying to make Mapbase more suitable for other operating systems (e.g. Linux and OSX) for a while now.

I've left a few comments and replies to ask some questions and clarify why some of my code is the way it is. I think most of the changes in this PR are acceptable for Mapbase, although they may require some rigorous testing before they're merged.

Yeah, there are changes all over the place, so I get that.
Let me know if I can assist you with testing. I just loaded up a map, walked around and shot stuff a little.

Blixibon

I compiled the server and client changes on my machine (which is Windows 10 64-bit) and I tested some of the code affected by this PR. All of it seems to works fine in-game, although there were a couple of compilation errors I pointed out in this review.

I haven't tested the shader changes yet due to some unrelated ongoing shader work. I'll get back to you on that later.

I'm starting to think this may not need rigorous testing as much as it just needs proper preparation from users of Mapbase's code because of how much it does to it. I can handle that part, so this may not need much more before it's clear to be merged.

sp/src/game/server/mapbase/ai_grenade.h

Blixibon · 2020-06-16T04:00:56Z

sp/src/game/shared/mapbase/matchers.cpp

@@ -5,18 +5,20 @@
 // $NoKeywords: $
 //=============================================================================

+// Must be included before cbase.h, otherwise some macros break the Linux build.
+#include <regex>


This causes an error on my end:

7>e:\mapbase\mapbase-hl2-master\sp\src\game\shared\mapbase\matchers.cpp(9): error C2220: warning treated as error - no 'object' file generated 7>e:\mapbase\mapbase-hl2-master\sp\src\game\shared\mapbase\matchers.cpp(9): warning C4627: '#include <regex>': skipped when looking for precompiled header use 7> Add directive to 'cbase.h' or rebuild precompiled header

Perhaps there should be an #ifdef here for GCC/Linux to include it before cbase.h while it's included after cbase.h on Windows?

I changed it, I think it should work now.
I'll rebase my changes to squash the fixup!s if it now compiles without changes on your end.

This is still failing to compile on my end, actually. I'm not sure how that's even possible since I don't have __GNUC__ defined and that line is #ifdef'd out. I'm using the regular VS2013 compiler in Visual Studio 2013 Community.

Perhaps #ifdef __GNUC__ could instead use #define/#unfdef to disable the macros which are causing issues?

Perhaps #ifdef __GNUC__ could instead use #define/#unfdef to disable the macros which are causing issues?

I got heaps of errors, but it was actually just min and max that cause problems. I'm #undefining them now and including minmax.h after, just in case one expects the macros to be there. Should work now for both platforms!

This compiles for me now. I personally would prefer this code to be within an #ifdef __GNUC__ block since it isn't necessary on Win32 and it would make it clear that this is for a specific platform, but just doing it like this doesn't seem to be causing any issues.

sp/src/game/server/mapbase/ai_grenade.h

Blixibon · 2020-06-19T15:09:57Z

I haven't found any other issues since your last update. If you're satisfied with the state this is in, I will merge it as a part of the next Mapbase update, which is currently slated to release within the next few days or the upcoming week.

This is important for case-sensitive filesystems/operating systems (i.e. Linux).

Caused cut off shadows on Linux. Taken from Entropy : Zero.

z33ky · 2020-06-19T16:11:06Z

Yeah cool, I squashed the fixes.

Let me know if you want to add the #ifdef __GNUC__ around the cbase.h/regex fix. I think it's best to keep divergence for specific platforms to a minimum. Usually this keeps compatibility issues for less-used configurations in check, though in this case I don't think it makes a difference since the compatibility issue stems from an external header.

Also let me know if you want me to send you Linux-binaries for this release.
I understand if you're concerned about putting an "official" tag on binaries you didn't build yourself; I'll make a Github release on my fork instead if that is the case.

Blixibon · 2020-06-21T16:56:12Z

@z33ky This PR has now been merged as a part of Mapbase v4.1.

I had to remove a bunch of E:Z1 tags after the merge, which I think might've been partly Sourcetree's fault, but other than that it looks like it's gone smoothly.

I've put you in Mapbase's credits as a contributor. If you're in Mapbase's Discord server, I can give you the "Contributor" role if you tell me what your handle is.

gcc/Linux fixes

z33ky added 5 commits June 14, 2020 11:14

Fix crash on startup

48fb4ab

This crashes on Linux as pKeyValuesData is NULL when deleteThis() is invoked. This likely is caused by another issue, but fixing this here seems good regardless.

Fix #includes casing

d72d817

This is important for case-sensitive filesystems/operating systems (i.e. Linux).

Get rid of some unnecessary temporary buffers

2c60bd3

Fix gcc 8.2 build errors

77fada1

Switch to malloc() for C_SceneEntity::LoadScene()

4d45c32

It used new char[] before, but it seems when the buffer is allocated using filesystem->ReadFileEx() we should free() the buffer, not delete[] it. CUtlBuffer also free()s the buffer, so malloc() should be the saner choice here.

z33ky force-pushed the mapbase/gcc-linux branch 2 times, most recently from 0c57aae to 12c3509 Compare June 14, 2020 10:06

z33ky commented Jun 14, 2020

View reviewed changes

Blixibon reviewed Jun 15, 2020

View reviewed changes

Blixibon reviewed Jun 16, 2020

View reviewed changes

sp/src/game/server/mapbase/ai_grenade.h Outdated Show resolved Hide resolved

z33ky force-pushed the mapbase/gcc-linux branch 2 times, most recently from a815769 to 1cc2c22 Compare June 17, 2020 05:36

z33ky added 4 commits June 19, 2020 18:02

Fix gcc 9/10 & mapbase build errors

9d0f519

Fix stdshaders #includes casing

5ba311b

This is important for case-sensitive filesystems/operating systems (i.e. Linux).

Fix shadow render target

eca6fd8

Caused cut off shadows on Linux. Taken from Entropy : Zero.

Fix compilation without MAPBASE_RPC

ae1162d

z33ky force-pushed the mapbase/gcc-linux branch from 1cc2c22 to ae1162d Compare June 19, 2020 16:03

Blixibon merged commit 1f69727 into mapbase-source:master Jun 21, 2020

Bitl mentioned this pull request Nov 21, 2020

Add pull requests, fixes, and features from other SDK 2013 repos and other sources. FIREFIGHT-RELOADED/FIREFIGHT-RELOADED-src-sdk-2013#12

Closed

VDeltaGabriel pushed a commit to Project-P2009/p2009_sdk that referenced this pull request Apr 27, 2023

Merge pull request mapbase-source#21 from z33ky/mapbase/gcc-linux

c6494f1

gcc/Linux fixes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gcc/Linux fixes #21

gcc/Linux fixes #21

z33ky commented Jun 14, 2020

z33ky left a comment

z33ky Jun 14, 2020

Blixibon Jun 15, 2020

z33ky Jun 14, 2020

z33ky Jun 14, 2020

z33ky Jun 14, 2020

z33ky Jun 14, 2020

z33ky Jun 15, 2020 •

edited

Loading

z33ky Jun 15, 2020

Blixibon left a comment •

edited

Loading

Blixibon Jun 15, 2020

z33ky Jun 15, 2020

z33ky commented Jun 15, 2020

Blixibon left a comment •

edited

Loading

Blixibon Jun 16, 2020

z33ky Jun 16, 2020

Blixibon Jun 17, 2020

z33ky Jun 17, 2020 •

edited

Loading

Blixibon Jun 17, 2020 •

edited

Loading

Blixibon commented Jun 19, 2020

z33ky commented Jun 19, 2020

Blixibon commented Jun 21, 2020

		@@ -0,0 +1 @@
		SDK_Eyes_vs20.fxc

gcc/Linux fixes #21

gcc/Linux fixes #21

Conversation

z33ky commented Jun 14, 2020

z33ky left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

z33ky Jun 15, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Blixibon left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

z33ky commented Jun 15, 2020

Blixibon left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

z33ky Jun 17, 2020 • edited Loading

Choose a reason for hiding this comment

Blixibon Jun 17, 2020 • edited Loading

Choose a reason for hiding this comment

Blixibon commented Jun 19, 2020

z33ky commented Jun 19, 2020

Blixibon commented Jun 21, 2020

z33ky Jun 15, 2020 •

edited

Loading

Blixibon left a comment •

edited

Loading

Blixibon left a comment •

edited

Loading

z33ky Jun 17, 2020 •

edited

Loading

Blixibon Jun 17, 2020 •

edited

Loading