-
Notifications
You must be signed in to change notification settings - Fork 112
Software Based Strobing Implementation (Black Frame Insertion) #343
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.