[Shared] fix UI transition/fade effects being framerate dependent #1223
Conversation
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
Razish
commented
Mar 3, 2024
| @@ -7812,17 +7813,17 @@ static qboolean Item_Paint(itemDef_t *item, qboolean bDraw) | |||
| if (DC->realTime > item->window.nextTime) | |||
| { | |||
| float rx, ry, a, c, s, w, h; | |||
| item->window.nextTime = DC->realTime + item->window.offsetTime; | |||
| item->window.nextTime = DC->realTime + (item->window.offsetTime * tAdjust); | |||
There was a problem hiding this comment.
I'm not sure if also adjusting the debouncer is correct since we're adjusting the translation based on the current frametime / 60fps frametime.
bartrpe
approved these changes
Mar 5, 2024
There was a problem hiding this comment.
I'm not the one to give a solid opinion on the good coding practice and whether solution is proper for the environment etc. buuut
I've tested the entire UI, checked the references and whether the adjustment is provided in every required variable & function and I can say it is. Checked the references to the timer, ran through call stack and reference tree, everything looks fine, every place that would need the adjustment, has it.
ensiform
approved these changes
Apr 29, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Currently the
transition/fadekeywords in menus will behave differently according to your FPS.This PR makes it consistent with the original behaviour as if you had 60 FPS - which is likely what they were designed for.
You can see these in effect in SP from the main menu:
New(transition)Controls(fade in)Demonstration of before/after this fix from JA++ with a custom menu:
https://github.com/JACoders/OpenJK/assets/844370/e84b79f1-5da6-4f6a-99fc-f67782263605
Demonstration of fixed behaviour in existing SP menu:
https://github.com/JACoders/OpenJK/assets/844370/16814f7c-94f3-4df3-9631-275bccd461ba