Software Based Strobing Implementation (Black Frame Insertion) #343
Conversation
engine/client/cl_scrn.c
Outdated
| @@ -101,6 +101,18 @@ void SCR_DrawFPS( void ) | |||
| /*if( !avgrate ) avgrate = ( maxfps - minfps ) / 2.0f; | |||
| else */avgrate += ( calc - avgrate ) / host.framecount; | |||
|
|
|||
| int strobeInterval = r_strobe->integer; | |||
| int eFPS; //Effective FPS (strobing effect) | |||
There was a problem hiding this comment.
Move variable defintions to the start of the block for C89(because MSVC) compliance.
engine/client/gl_rmain.c
Outdated
| */ | ||
| void R_Strobe( void ) | ||
| { | ||
| int getInterval = r_strobe->integer; // Check through modified tag first? |
There was a problem hiding this comment.
You might want move sCounter to R_Strobe function. Just declare it static, and it will be saved between function calls.
engine/client/gl_rmain.c
Outdated
| if ( (getInterval == 0) || ((swapInterval == 0) && (getInterval != 0)) ) | ||
| { | ||
| if (getInterval != 0) //If v-sync is off, turn off strobing | ||
| Cvar_Set("r_strobe", "0"); |
There was a problem hiding this comment.
There should be a console message like "Warning: r_strobe requires gl_swapInterval." or something like that.
| } | ||
| } | ||
| } | ||
|
|
||
| /* | ||
| =============== | ||
| R_EndFrame | ||
| =============== | ||
| */ | ||
| void R_EndFrame( void ) |
There was a problem hiding this comment.
Before implementing frame skip, it should be tested is frame skip compatible with mod's custom renderers, like Paranoia, HLFX, Trinity or others.
There was a problem hiding this comment.
I don't know a convenient and efficient way to check if this is compatible with custom renderers. I first thought testing it and checking gl errors but I don't think this is a good way. Maybe it is better to disable this completely for custom renderers (except VBO? ) ?
There was a problem hiding this comment.
Frame skipping itself may break something even in the engine, so engine side must be completely ready for frame skipping. World VBO is now a part of engine renderer. :)
Yeah, maybe it can be disabled for custom renderers or just filled black at the end of the frame, i.e. no skip.
There was a problem hiding this comment.
For the current version, I do not think that frame skipping would gain considerable amount of performance increase. So I will not implement it in this pull request and leave that part as is. However I did not check this in any platform other than PC.
My Android build platform is not set. Can you check this in Android and see if it turns out to be bad? Therefore this can be disabled before too late.
There was a problem hiding this comment.
@fuzun Android version is built automatically by Travis CI.
There is a link from the end of build log. Script is "travis-upload".
https://transfer.sh/rv8Vy/1024-03-45-xash3d-generic-master-b8bb3c5.apk
There was a problem hiding this comment.
I am the author of TestUFO and Founder of Blur Busters.
I can indeed confirm that this is a useful modification, especially for 100Hz+ monitors. It reduces motion blur in the same way as www.testufo.com/blackframes ....
If frameskipping is an essential feature to accept this pull request - then simply do "frame-ignoring"/"frame-blackout" instead of "frame-skipping". Basically intentionally black-out the rendered frames instead of forcing a frame skip. That mathematically works the same way, even though it wastes more GPU resources. (i.e. rendering frames only to be thrown away). But it could be an optional option as a compatibility mode.
Frameskipping for BFI is more efficient, but frame-blackout for BFI (blacking-out rendered frames) could possibly allow more plugins to work, and in theory might actually end up having better physics/mousefeel (depending on how the game engine works). Could be an option / toggle. A future change, perhaps.
Also, with www.testufo.com/blackframes -- the 120fps Razer Phone works great with 60fps black frame insertion (basically 60fps with half the motion blur of normal 60Hz). I bet this will look great on 120Hz Android phones -- for times where 120fps is impossible but you'd like to keep the low motion blur.
There was a problem hiding this comment.
@mdrejhon thanks for professional review! :)
I don't have 60+ Hz screens now, but after reading your review, it looks like a really cool feature for high frequency screens. So I think it can be merged. :)
Sadly, engine is not ready for frame skipping now and implementing this is a bit complicated because compability with Half-Life, which doesn't have this feature and hardly depends on frametime(which was mostly fixed by Valve in 2013). As it will be done on engine side, maybe someone will use it for BFI.
engine/client/gl_rmain.c
Outdated
| Cvar_Set("r_strobe", "0"); | ||
| MsgDev(D_WARN, "Strobing (Black Frame Replacement) requires V-SYNC not being turned off! (gl_swapInterval != 0) \n"); | ||
| Msg("Strobing (Black Frame Insertion) requires Vertical Sync to be enabled!\n"); |
There was a problem hiding this comment.
One message is should be enough here. :)
engine/client/gl_rmain.c
Outdated
| @@ -1327,12 +1326,12 @@ void R_RenderFrame( const ref_params_t *fd, qboolean drawWorld ) | |||
| GL_BackendEndFrame(); | |||
| } | |||
|
|
|||
| inline static void gl_InsertBlackFrame( void ) | |||
| inline static void gl_sBlackFrame( void ) | |||
There was a problem hiding this comment.
Please, follow the common naming of rendering functions. It should be like GL_InsertBlackFrame.
|
This was supposed to get updated to support phase swaps and bunch of other things along actually. Discussion: https://forums.blurbusters.com/viewtopic.php?f=7&t=3815&p=30401 New prototype: void R_Strobe(void)
{
...
if ((SwapPhaseInfo.fCounter % 2) == 0) //Starts with +
{
++SwapPhaseInfo.pCounter;
SwapPhaseInfo.isPositive = true;
}
else
{
++SwapPhaseInfo.nCounter;
SwapPhaseInfo.isPositive = false;
}
if (swapInterval < 0)
swapInterval = abs(swapInterval);
if ((swapInterval != 0) && (getInterval % 2 != 0)) //Swapping is not enabled for even intervals. Because it makes it like r_strobe -interval . It is not needed anyway since even intervals do not cause burn in problem.
{
currentTime = Sys_DoubleTime();
delta = currentTime - recentTime;
if ((delta >= (double)(swapInterval)) && (delta < (double)(2 * swapInterval)))
{
SwapPhaseInfo.isInverted = true;
}
else if (delta < (double)(swapInterval))
{
SwapPhaseInfo.isInverted = false;
}
else
{
recentTime = currentTime;
}
}
if (SwapPhaseInfo.isPositive == true && SwapPhaseInfo.isInverted == false)
{
if (abs(getInterval) % 2 == 0)
SwapPhaseInfo.isNormal = (((SwapPhaseInfo.pCounter - 1) % (abs(getInterval) + 1)) == 0) ? true : false; //even
else
SwapPhaseInfo.isNormal = (((SwapPhaseInfo.pCounter -1) % ((abs(getInterval) + 1) / 2)) == 0) ? true : false; //odd
if (abs(getInterval) == 1)
SwapPhaseInfo.isNormal = true;
}
else if (SwapPhaseInfo.isPositive == true && SwapPhaseInfo.isInverted == true)
{
if ((abs(getInterval) % 2) == 0)
SwapPhaseInfo.isNormal = (((SwapPhaseInfo.pCounter - 1) % (abs(getInterval) + 1)) == (abs(getInterval) / 2)) ? true : false; //even
else
SwapPhaseInfo.isNormal = false;
if (abs(getInterval) == 1)
SwapPhaseInfo.isNormal = false;
}
else if (SwapPhaseInfo.isPositive == false && SwapPhaseInfo.isInverted == false)
{
if ((abs(getInterval) % 2) == 0)
SwapPhaseInfo.isNormal = (((SwapPhaseInfo.nCounter - 1) % (abs(getInterval) + 1)) == (abs(getInterval) / 2)) ? true : false; //even
else
SwapPhaseInfo.isNormal = false;
if (abs(getInterval) == 1)
SwapPhaseInfo.isNormal = false;
}
else if (SwapPhaseInfo.isPositive == false && SwapPhaseInfo.isInverted == true)
{
if (abs(getInterval) % 2 == 0)
SwapPhaseInfo.isNormal = (((SwapPhaseInfo.nCounter - 1) % (abs(getInterval) + 1)) == 0) ? true : false; //even
else
SwapPhaseInfo.isNormal = (((SwapPhaseInfo.nCounter - 1) % ((abs(getInterval) + 1) / 2)) == 0) ? true : false; //odd
if (abs(getInterval) == 1)
SwapPhaseInfo.isNormal = true;
}
if (getInterval < 0)
SwapPhaseInfo.isNormal = !SwapPhaseInfo.isNormal;
if (SwapPhaseInfo.isNormal) //Show normal
{
R_Set2DMode(false);
if (SwapPhaseInfo.isPositive)
++SwapPhaseInfo.pNCounter;
else
++SwapPhaseInfo.nNCounter;
}
else //Show black frame
{
gl_GenerateBlackFrame();
if (SwapPhaseInfo.isPositive)
++SwapPhaseInfo.pBCounter;
else
++SwapPhaseInfo.nBCounter;
}
++SwapPhaseInfo.fCounter;
}Maybe revert and re-open the pull request? Or leave this as is and I will open a new pull request soon? |
|
Just open new PR based on formatting fixes from master. I did it as an example for you. Please, follow them. :) |
|
Everything else is good, and I can't wait for further development of such cool feature for engine. |
|
Yeah the idea is pretty cool actually. As far as I know it will be the first 3D engine that has built-in software based strobing. |
|
@fuzun yes. You can try to add them in menu. Just send PR to mainui_cpp repository. |
|
@fuzun I will completely review and test it, with next PR. There is also some problems with strobing on Mesa drivers. They are pretty bad with VSync and tear-free, but I think it's not your trouble. Meanwhile it works on my Android device perfectly, although my RN4X have 60Hz screen. |
New Version: #353
Simple strobe support with black frame insertion (replacement) technique.
Purpose
Provides motion clarity by eliminating motion blur on high frequency monitors.
How
Replaces certain amount of consequent frames with black frames. This makes a scanning effect like CRT monitors.
Usage
Set r_strobe cvar to the black frame insertion interval (black frame count) you want (ideal values for 144hz monitors are probably -2, -1 = 1, 2, 3) . Frame display interval can be changed with gl_swapInterval cvar. Use cl_showfps 1 to see the actual / effective fps (displayed non-black frame count in 1 second).
Note that V-Sync must be active to enable this! (gl_swapInterval != 0)
If r_strobe is positive, some rendered frames will be replaced with black frame
For example result of r_strobe = 3 will be: black-black-black-normal-black-black-black-normal-black-black-black-normal
If r_strobe is negative, the procedure will be the opposite.
For example result of r_strobe = -4 will be: normal-normal-normal-normal-black-normal-normal-normal-normal-black
Issues
Actual brightness gets reduced to half (and perceived brightness will be lowered but not as much as actual brightness) if r_strobe is set to 1 more if >1 less if <-1. This can be corrected somehow by altering gamma and brightness I believe. Because this is just like pulse width modulation synchronized to v-sync frames. And there are several methods to overcome PWM brightness issue. Implementing adaptive version of this is on todo list.
FPS will always be lowered. Actual or effective FPS (frames that are not black) gets reduced to half if r_strobe is set to 1 or -1 more if >1 less if <-1 .
This will trigger epileptic attacks on epileptic patients.
Even on 144hz monitors, flickering may happen.
Perceived picture quality will be worse. Much worse on IPS, VA or slow TN panels.
Frames that will be black are rendered for no rational reason. TODO: Consider vsync timings and do not render the supposed black frame at all.
Ghosting effect may increase.
Web Demo: https://www.testufo.com/blackframes#count=2&bonusufo=0&equalizer=1&background=000000
Not tested on Android yet!