Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Another attempt at framebuffer size estimate changes #6211

Merged
merged 11 commits into from
Jun 3, 2014

Conversation

unknownbrackets
Copy link
Collaborator

This needs testing, and probably there are things that don't work. I tried to expand the logic that guesses at the size of a framebuffer to cover the cases I could see.

This fixes the broken games in #6093 for me. It does make the Valkyrie Profile video correct as well, but I'm not sure if there's still an issue there since videos maybe ought to "prompt" the resizing code... this can mean framebuffers remain a bit larger, though, hmm.

@solarmystic, if you could test some games with this it'd be a real help.

-[Unknown]

To avoid cases where they remain too small, and to make sure size always
matches up sanely.
This avoids a blink when sizing up.
@hrydgard
Copy link
Owner

hrydgard commented Jun 1, 2014

Looks good, and blitting on fb resize is a good idea/fix.

I'll merge when test results look good.

Even if it's wider but short, it should not reduce the height of the
buffer.
@solarmystic
Copy link
Contributor

@hrydgard @unknownbrackets Did the usual git merge remotes of your branch.
Test results for your perusal:-

capture

General observations:-

a. Valkyrie Profile Lenneth is improved, but contrary to what is mentioned in the pull request, the ingame video playback is still broken for me in the test build.

Notable mentions include the Lenneth Valkyrie Transformation scene and the opening gates scene in the beginning. Only a black screen is seen where the videos were supposed to be played, but gameplay resumes as normal after a short while. One can still hear the video's audio being played in the background though.

vp1

vp2

b. All other games in the list are unaffected by this change, those that were working are still working just fine after applying this pull request to the current master.

Instead, draw in the top corner.  This way, even if framebuffers get
smaller things stay consistent.
@unknownbrackets
Copy link
Collaborator Author

Oh, it seems like it now requires "simulate block transfer", but that's previous to these changes.

Anyway, sorry. I've made more major changes which seem to resolve things for me in a way that makes a lot more sense to me logically. Now we just take the width/height of the framebuffer from the corner of the FBO and have fun.

I double checked 3x etc. and read framebuffers to memory and blitting, but it's very possible I broke something. So many places that were treating vfb->width as the FBO width...

Dragon Ball problem still exists though, hmm. Actually, it's the Frogger issue again.

-[Unknown]

@dbz400
Copy link
Contributor

dbz400 commented Jun 2, 2014

Not too sure if it is my own issue ? i got wrong color elsewhere (R/B swapped)
uljs00311_00000

@unknownbrackets
Copy link
Collaborator Author

And that only happens with these changes merged? This shouldn't be able to affect that...

-[Unknown]

@dbz400
Copy link
Contributor

dbz400 commented Jun 2, 2014

Yep , i just tried latest master build , it is okay .Let'see if other can reproduce it or may be just my own issue.

@unknownbrackets
Copy link
Collaborator Author

That's... weird. Can you bisect it? Which commit did it start with?

-[Unknown]

@dbz400
Copy link
Contributor

dbz400 commented Jun 2, 2014

Sure let me check .(However , i think it is fine on your side?)

@daniel229
Copy link
Collaborator

dragon ball looks fine
01

@unknownbrackets
Copy link
Collaborator Author

You don't have the "inverse colors" post-processing shader selected, do you?

-[Unknown]

@dbz400
Copy link
Contributor

dbz400 commented Jun 2, 2014

I tried to rebuild the source again , it is fine now :) strange.

@solarmystic
Copy link
Contributor

@unknownbrackets Yeah, I just retested Valkyrie Profile again using Simulate Block Transfer and now the video playback is perfect. I'll have to make a note of that.

@unknownbrackets
Copy link
Collaborator Author

One thing I'm not sure about is if I need to adjust the scissor coordinates too. I'm a little confused about what things are relative to...

-[Unknown]

@unknownbrackets
Copy link
Collaborator Author

@daniel229 how are the Natsuyasumi with the latest changes?

-[Unknown]

@daniel229
Copy link
Collaborator

No changing.

@daniel229
Copy link
Collaborator

Natsuyasumi 1
W[G3D]: GLES\Framebuffer.cpp:2056 Block transfer download 04000000 -> 04110000
I[G3D]: GLES\ShaderManager.cpp:146 Linked shader: vs 14 fs 21
I[G3D]: GLES\ShaderManager.cpp:146 Linked shader: vs 17 fs 28
I[G3D]: GLES\ShaderManager.cpp:146 Linked shader: vs 17 fs 30
I[G3D]: GLES\ShaderManager.cpp:146 Linked shader: vs 14 fs 32
I[G3D]: GLES\ShaderManager.cpp:146 Linked shader: vs 34 fs 25
I[G3D]: GLES\ShaderManager.cpp:146 Linked shader: vs 34 fs 36
I[G3D]: GLES\ShaderManager.cpp:146 Linked shader: vs 38 fs 36
I[G3D]: GLES\ShaderManager.cpp:146 Linked shader: vs 20 fs 40
I[G3D]: GLES\ShaderManager.cpp:146 Linked shader: vs 17 fs 42
I[G3D]: GLES\ShaderManager.cpp:146 Linked shader: vs 14 fs 40
I[SCEGE]: GLES\Framebuffer.cpp:890 Resizing FBO for 00088000 : 481 x 273 x 3
W[G3D]: GLES\Framebuffer.cpp:2042 Intra-buffer block transfer (not supported) 04088000 -> 04110000
W[G3D]: GLES\Framebuffer.cpp:2044 Inter-buffer block transfer 04000000 -> 04110000
I[SCEGE]: GLES\Framebuffer.cpp:1824 Decimating FBO for 00088000 (480 x 272 x 3), age 6
I[SCEGE]: GLES\Framebuffer.cpp:1824 Decimating FBO for 00000000 (480 x 272 x 3), age 6

@daniel229
Copy link
Collaborator

natsuyasumi 4
W[G3D]: GLES\Framebuffer.cpp:2042 Intra-buffer block transfer (not supported) 04088000 -> 040cc000
I[SCEGE]: GLES\Framebuffer.cpp:890 Resizing FBO for 00000000 : 481 x 273 x 0

@unknownbrackets
Copy link
Collaborator Author

Log from both games with this line at the end of EstimateDrawingSize() would help the most:

NOTICE_LOG(SCEGE, "%08x V: %ix%i, R: %ix%i, S: %ix%i, STR: %i, THR:%i, Z:%08x = %ix%i", gstate.getFrameBufAddress(), viewport_width,viewport_height, region_width, region_height, scissor_width, scissor_height, fb_stride, gstate.isModeThrough(), gstate.isDepthWriteEnabled() ? gstate.getDepthBufAddress() : 0, drawing_width, drawing_height);

It looks like they are both using 481x273 framebuffers though.

-[Unknown]

@daniel229
Copy link
Collaborator

natsuyasumi 1
N[SCEGE]: GLES\Framebuffer.cpp:698 44000000 V: 481x273, R: 480x272, S: 480x272, STR: 512, THR:1, Z:44110000 = 480x272
N[SCEGE]: GLES\Framebuffer.cpp:698 44000000 V: 480x272, R: 480x272, S: 480x272, STR: 512, THR:1, Z:44110000 = 480x272
N[SCEGE]: GLES\Framebuffer.cpp:698 44000000 V: 481x273, R: 480x272, S: 480x272, STR: 512, THR:0, Z:44110000 = 480x272
N[SCEGE]: GLES\Framebuffer.cpp:698 44088000 V: 481x273, R: 480x272, S: 480x272, STR: 512, THR:1, Z:44110000 = 481x273
N[SCEGE]: GLES\Framebuffer.cpp:698 44088000 V: 480x272, R: 480x272, S: 480x272, STR: 512, THR:1, Z:44110000 = 480x272
N[SCEGE]: GLES\Framebuffer.cpp:698 44088000 V: 481x273, R: 480x272, S: 480x272, STR: 512, THR:0, Z:44110000 = 481x273

ntsuyasumi 4
N[SCEGE]: GLES\Framebuffer.cpp:698 44000000 V: 481x273, R: 480x272, S: 480x272, STR: 512, THR:1, Z:00000000 = 481x273
N[SCEGE]: GLES\Framebuffer.cpp:698 44000000 V: 481x273, R: 480x272, S: 480x272, STR: 512, THR:0, Z:44110000 = 481x273
N[SCEGE]: GLES\Framebuffer.cpp:698 44088000 V: 481x273, R: 480x272, S: 480x272, STR: 512, THR:1, Z:00000000 = 481x273
N[SCEGE]: GLES\Framebuffer.cpp:698 44088000 V: 481x273, R: 480x272, S: 480x272, STR: 512, THR:0, Z:44110000 = 481x273
N[SCEGE]: GLES\Framebuffer.cpp:698 44000000 V: 481x273, R: 480x272, S: 480x272, STR: 512, THR:1, Z:00000000 = 481x273
N[SCEGE]: GLES\Framebuffer.cpp:698 44000000 V: 481x273, R: 480x272, S: 480x272, STR: 512, THR:0, Z:44110000 = 481x273

@unknownbrackets
Copy link
Collaborator Author

Okay, I'm just going to force 481 -> 480, since I've seen that behavior also in Jewel Summoner, and it's clear there aren't 273 lines of VRAM.

-[Unknown]

@daniel229
Copy link
Collaborator

natsuyasumi games looks good now,however I found previous unknownbrackets@c2ccfd5 breaks Castlevania Rondo of Blood original,black screen.

add that NOTICE_LOG
https://gist.github.com/daniel229/191516050f124a6610e7

@unknownbrackets
Copy link
Collaborator Author

Oh okay, I'm back. So, if you force the width to let's say 480, does it work? Reverting that part just prevents it from shrinking the active portion of the framebuffer... it theoretically should not be able to make the screen black...

Maybe it's scissor, somehow...

Does this or some demo do it? Or is it not the same game?
http://www.pspdemocenter.com/page.php?id=909

-[Unknown]

@daniel229
Copy link
Collaborator

480 works

@daniel229
Copy link
Collaborator

Yes,that's the same game,and seems no demo for it.Rondo of Blood is something unlocked from the game.

@unknownbrackets
Copy link
Collaborator Author

I see. Grr. Is it doing any block transfers? In Core/Reporting.cpp, replace return logOnceUsed.insert(identifier).second; with return true;, this'll make it log every transfer. I know that in some cases transfers might pick a different buffer based on height but it shouldn't happen for width...

Oh. What if you change if (MaskedEqual(v->fb_address, addr) && v->format == displayFormat_ && v->width >= 480) { to if (MaskedEqual(v->fb_address, addr)? Since we don't have multiple vfbs per address, that may be safer anyway.

But even if that does work, theoretically it will not be full screen... is it "pillarboxed"?

-[Unknown]

@daniel229
Copy link
Collaborator

if (MaskedEqual(v->fb_address, addr)) works

@daniel229
Copy link
Collaborator

the game is never full screen like you said
01

Castlevania actually uses a display buffer of 420, it seems.  Since we no
longer allow multiple FBOs at the same address, this should be safe.
@unknownbrackets
Copy link
Collaborator Author

Cool. So those games are all working? This stuff is so complex and inexact...

-[Unknown]

@daniel229
Copy link
Collaborator

Yeah.

@hrydgard
Copy link
Owner

hrydgard commented Jun 3, 2014

Anything known remaining before a merge?

@hrydgard
Copy link
Owner

hrydgard commented Jun 3, 2014

Meh, I'll merge. As always, we'll see if there's any fallout...

hrydgard added a commit that referenced this pull request Jun 3, 2014
Another attempt at framebuffer size estimate changes
@hrydgard hrydgard merged commit e6d3a9c into hrydgard:master Jun 3, 2014
@unknownbrackets unknownbrackets deleted the framebuf-estimate branch June 3, 2014 08:55
@thedax
Copy link
Collaborator

thedax commented Jun 3, 2014

For the record, this neither helped nor hurt Breath of Fire 3, so that's something.

@DonelBueno
Copy link

This pull request destroyed God of War Ghost of Sparta's performance.

From 120 fps to 60 fps on the starting area.

Otherwise, it is great. Good work.

@solarmystic
Copy link
Contributor

@DonelBueno I can verify this finding. Performance loss is almost half, as you experienced.

v0.9.8-995-g84ff3d6 (before the commit) 71 VPS
995

v0.9.8-1018-g318f641 (after the commit) 47 VPS
1018

@DonelBueno
Copy link

My GPU usage is way bellow 100% in both cases. I bet you could get a good improvement on the first screenshot with a lower rendering resolution.

Can you check the drawcalls in both scenarios?

@solarmystic
Copy link
Contributor

@DonelBueno I intentionally created a GPU limited situation for performance testing (using 3x rendering resolution) to minimize the CPU's influence on the emulation. And it makes sense in this case, since it's a framebuffer size commit which would impact GPU performance that is responsible for the reduction. All settings are the same between both tests.

@DonelBueno
Copy link

As I said, in my case I was limited by the CPU in both tests (My GPU usage was about 20-35% in both tests, closer to 20% in the first one), so it obviously affects CPU performance.

That's why I asked you to check drawcalls, which are very CPU dependent.

@solarmystic
Copy link
Contributor

@DonelBueno I had a look at the drawcalls and it was above 19000 for both cases.

EDIT:- No, that's incorrect, during the actual fight scenes (when you get to control Kratos) it drops down to nearly half that (~11k), but it fluctuates rapidly.

@unknownbrackets
Copy link
Collaborator Author

Hmm. As long as I have simulate block transfer off in God of War, performance doesn't seem to be any worse than before on master with this merged...

-[Unknown]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants