Skip to content

Commit

Permalink
Fix 3dfx passing frame time instead of FPS to RENDER_SetSize
Browse files Browse the repository at this point in the history
  • Loading branch information
@realnc
realnc committed May 11, 2020
1 parent ea666ee commit 9d97bf2
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/hardware/voodoo_interface.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ static void Voodoo_UpdateScreen(void) {
if (v->ogl) {
v->ogl_dimchange = false;
} else {
RENDER_SetSize(v->fbi.width, v->fbi.height, 16, vdraw.vfreq, 4.0/3.0, false, false);
RENDER_SetSize(v->fbi.width, v->fbi.height, 16, 1000.0f / vdraw.vfreq, 4.0/3.0, false, false);
}

Voodoo_VerticalTimer(0);
Expand Down

7 comments on commit 9d97bf2

@i30817
Copy link

@i30817 i30817 commented on 9d97bf2 May 14, 2020 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this fix is just for libretro? I 'sideported' your delete[] vs delete commit to my copy of the patch, that will just run in dosbox itself, but i shouldn't do that for this one right?

edit: or maybe not now that i think about it, if it was in the original, it can't be about the libretro api.

edit2: oh, it's supposed to be 1s/60frames, so the ideal timeslice for a single frame. I'll side port this too i guess.

@realnc
Copy link
Owner Author

@realnc realnc commented on 9d97bf2 May 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this fix is just for libretro? I 'sideported' your delete[] vs delete commit to my copy of the patch, that will just run in dosbox itself, but i shouldn't do that for this one right?

edit: or maybe not now that i think about it, if it was in the original, it can't be about the libretro api.

No, it's not just for libretro. It's a genuine mistake by kekko. RENDER_SetSize() takes FPS as parameter, but the code was passing the frame time instead (in this case, 16.7 instead of 60.) When running as a libretro core, reporting the correct FPS to the frontend is vital, otherwise retro_run() will get called 16.7 times per second rather than 60. I'm not sure what the effects of this bug are in stand-alone dosbox.

@i30817
Copy link

@i30817 i30817 commented on 9d97bf2 May 14, 2020 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uh, but your code makes it that value:

vdraw.vfreq = 1000.0f/60.0f;
1000/vdraw.vfreq = 1000/1000.0f/60.0f = 1.0f / 60.0f = 0.01666666666

Am i missing something? If it's supposed to be 60 that is.

@realnc
Copy link
Owner Author

@realnc realnc commented on 9d97bf2 May 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

vdraw.vfreq is not 60. It's 16.7. It's initialized in Voodoo_Initialize() with:

vdraw.vfreq = 1000.0f/60.0f;

That means it contains the frame time, not the frame rate.

@i30817
Copy link

@i30817 i30817 commented on 9d97bf2 May 14, 2020 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But that's what i meant; 1000/16.7 is 0.016666.... not 60. If the argument is supposed to be frames per second, what is 0.01666 frames per second?

edit: I understand you have to use vfreq, in case some future code changes the target fps, but isn't it supposed to be 1000*vdraw.vfreq?

@realnc
Copy link
Owner Author

@realnc realnc commented on 9d97bf2 May 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But that's what i meant; 1000/16.7 is 0.016666

Uh, 1000 / 16.7 is 60 :-P

@i30817
Copy link

@i30817 i30817 commented on 9d97bf2 May 14, 2020 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fuuuuck. Never mind me. I just put in '1000/1000/60' in google and the order of operations precedence fucked me.

Please sign in to comment.