Stretched skyboxes and PVS-Studio bugfixes #172

Xottab-DUTY · 2017-03-08T21:32:31Z

No description provided.

CrossVR · 2017-03-09T15:35:13Z

src/xrEngine/Environment.cpp

@@ -550,6 +550,9 @@ void CEnvironment::calculate_dynamic_sun_dir()

    clamp(cosAZ, -1.0f, 1.0f);
    float AZ = acosf(cosAZ);
+    extern u32 ps_r_sun_direction;


This should be changed into a member of CEnvironment called m_sun_direction. Then you can set it by making a command class that sets g_pGamePersistent->Environment().m_sun_direction in Execute().

See https://github.com/OpenXRay/xray-16/blob/dev/src/xrEngine/xr_ioc_cmd.cpp#L463 as an example. Also you could just allow arbitrary sun directions if you make it a float in degrees and convert it to radians when setting m_sun_direction.

CrossVR · 2017-03-09T15:46:37Z

src/xrGame/space_restriction.cpp

@@ -152,7 +152,7 @@ CSpaceRestriction::CBaseRestrictionPtr CSpaceRestriction::merge(
    RESTRICTIONS::const_iterator I = temp_restrictions.begin();
    RESTRICTIONS::const_iterator E = temp_restrictions.end();
    for (; I != E; ++I)
-        temp = strconcat(sizeof(S), S, *temp, ",", *(*I)->name());
+        temp = strconcat(strlen(S), S, *temp, ",", *(*I)->name());


strlen is incorrect here, because the string S is empty, so it would always fail. Instead sizeof(S) should be replaced with acc_length, which is the variable used as the size to allocate S.

CrossVR · 2017-03-09T15:48:07Z

src/xrGame/Actor_Network.cpp

@@ -685,7 +685,7 @@ BOOL CActor::net_Spawn(CSE_Abstract* DC)
    inventory().SetPrevActiveSlot(NO_ACTIVE_SLOT);

    //-------------------------------------
-    m_States.empty();
+    m_States.clear(); // Xottab_DUTY: check if replace is correct


Yeah, this looks correct, but should probably be tested if it doesn't have unintended consequences.

CrossVR · 2017-03-10T00:18:47Z

src/xrEngine/xr_ioc_cmd.cpp

@@ -713,6 +728,7 @@ void CCC_Register()

    // Render device states
    CMD4(CCC_Integer, "r__supersample", &ps_r__Supersample, 1, 4);
+    CMD2(CCC_SunDirection, "r__sun_direction", &ps_sun_direction);


I think this should be an rs_ command, r__supersample seems to be an outlier here.

rs_sun_direction seems more outlined with such commands as rs_stats, rs_fullscreen, rs_gamma and others than r__sun_direction with r__ rendering commands. Maybe the placement isn't good, but name is pretty matches to the other commands

CrossVR · 2017-03-10T00:19:36Z

src/xrEngine/xr_ioc_cmd.cpp

+    {
+        CCC_Float::Execute(args);
+        if (g_pGamePersistent)
+            g_pGamePersistent->Environment().m_sun_azimuth = ps_sun_direction * (PI / 180.0f);


Can you test what happens when r__sun_direction 180 is in the user.ltx file? Will the sun direction still be corrected?

No, it doesn't. ~~By the way... Something went wrong... r__sun_direction 0 and sun born from West, but should born from South.~~ All right with sun, except only it should get params from user.ltx

Xottab-DUTY · 2017-03-10T04:59:49Z

src/xrCore/xrDebug.cpp

@@ -97,7 +97,7 @@ size_t xrDebug::BuildStackTrace(char* buffer, size_t capacity, size_t lineCapaci

 size_t xrDebug::BuildStackTrace(EXCEPTION_POINTERS* exPtrs, char* buffer, size_t capacity, size_t lineCapacity)
 {
-    memset(buffer, capacity * lineCapacity, 0);
+    memset(buffer, 0, capacity*lineCapacity);


May be with this fix stack trace will be more appropriate, which is not in some cases(for example, just gives nothing)

Xottab-DUTY · 2017-03-10T15:21:29Z

@andrew-boyarshin It's not only rendering, but includes other various fixes. (I should pull these in other request, but what's done is done)

andrew-boyarshin · 2017-03-10T15:25:11Z

@Xottab-DUTY I guess the term "anomalies" was used from original PVS-Studio article. It was some kind of reference to gameplay of S.T.A.L.K.E.R., so it was ok. Here it was unexpected and led to misunderstanding.

CrossVR · 2017-03-10T15:27:48Z

@Xottab-DUTY You can still put those in another PR so we can at least merge the PVS fixes now.

Simply use these commands:

git branch render-fixes
git reset --hard c328536
git push -f origin pvs-fixes
git push origin render-fixes

Then you can submit a new PR for your new render-fixes branch.

CrossVR · 2017-03-10T15:29:51Z

Actually you should also remove the PVS commit from the render-fixes branch, but we can fix that in a simple rebase after this PR is merged

From commit: avoitishin/xray-16@84c7bde

Xottab-DUTY · 2017-03-11T23:06:39Z

Detached sun direction from this PR, it's not ready. You can find it in sundirection of my fork.

Xottab-DUTY changed the title ~~Stretched skyboxes and anomalies fixes~~ Dynamic sun, stretched skyboxes and anomalies Mar 8, 2017

CrossVR suggested changes Mar 9, 2017

View reviewed changes

CrossVR reviewed Mar 10, 2017

View reviewed changes

Xottab-DUTY commented Mar 10, 2017

View reviewed changes

andrew-boyarshin added Renderer wip pending review and removed wip labels Mar 10, 2017

andrew-boyarshin added the Infrastructure label Mar 10, 2017

andrew-boyarshin changed the title ~~Dynamic sun, stretched skyboxes and anomalies~~ Dynamic sun, stretched skyboxes and PVS-Studio bugfixes Mar 10, 2017

andrew-boyarshin added wip and removed pending review labels Mar 10, 2017

Xottab-DUTY added 2 commits March 12, 2017 00:06

Fix some anomalies found with PVS-Studio (#140)

fb64ae3

Sky box stretch fix

b166e17

From commit: avoitishin/xray-16@84c7bde

Xottab-DUTY changed the title ~~Dynamic sun, stretched skyboxes and PVS-Studio bugfixes~~ Stretched skyboxes and PVS-Studio bugfixes Mar 11, 2017

CrossVR approved these changes Mar 11, 2017

View reviewed changes

andrew-boyarshin added ready to merge Code Quality and removed wip Infrastructure labels Mar 12, 2017

CrossVR merged commit 8589126 into OpenXRay:dev Mar 13, 2017

Xottab-DUTY deleted the pvs-fixes branch March 13, 2017 16:49

Xottab-DUTY removed the ready to merge label Nov 15, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Stretched skyboxes and PVS-Studio bugfixes #172

Stretched skyboxes and PVS-Studio bugfixes #172

Xottab-DUTY commented Mar 8, 2017

CrossVR Mar 9, 2017

CrossVR Mar 9, 2017

CrossVR Mar 9, 2017

CrossVR Mar 10, 2017

Xottab-DUTY Mar 10, 2017

CrossVR Mar 10, 2017

Xottab-DUTY Mar 10, 2017 •

edited

Loading

Xottab-DUTY Mar 10, 2017 •

edited

Loading

Xottab-DUTY commented Mar 10, 2017

andrew-boyarshin commented Mar 10, 2017

CrossVR commented Mar 10, 2017

CrossVR commented Mar 10, 2017

Xottab-DUTY commented Mar 11, 2017 •

edited

Loading

Stretched skyboxes and PVS-Studio bugfixes #172

Stretched skyboxes and PVS-Studio bugfixes #172

Conversation

Xottab-DUTY commented Mar 8, 2017

CrossVR Mar 9, 2017

Choose a reason for hiding this comment

CrossVR Mar 9, 2017

Choose a reason for hiding this comment

CrossVR Mar 9, 2017

Choose a reason for hiding this comment

CrossVR Mar 10, 2017

Choose a reason for hiding this comment

Xottab-DUTY Mar 10, 2017

Choose a reason for hiding this comment

CrossVR Mar 10, 2017

Choose a reason for hiding this comment

Xottab-DUTY Mar 10, 2017 • edited Loading

Choose a reason for hiding this comment

Xottab-DUTY Mar 10, 2017 • edited Loading

Choose a reason for hiding this comment

Xottab-DUTY commented Mar 10, 2017

andrew-boyarshin commented Mar 10, 2017

CrossVR commented Mar 10, 2017

CrossVR commented Mar 10, 2017

Xottab-DUTY commented Mar 11, 2017 • edited Loading

Xottab-DUTY Mar 10, 2017 •

edited

Loading

Xottab-DUTY Mar 10, 2017 •

edited

Loading

Xottab-DUTY commented Mar 11, 2017 •

edited

Loading