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

Fix a heap overflow in block transfers, fix most save picture downloads, optimizations #6141

Merged
merged 22 commits into from
May 26, 2014

Conversation

unknownbrackets
Copy link
Collaborator

The heap overflow was responsible the crashes I reported before, afaict.

This orders the copy properly such that save pictures generally work. I've also added some optimizations so we can skip checking framebuffers for most memcpy operations (e.g. textures.)

This also flips the y when blitting for downloading framebuffers, which should not be done generally when blitting framebuffers (e.g. between each other.)

-[Unknown]

It seems like this is where it was being done...
Otherwise, it only works if it does it every frame, and then it's a frame
behind.  Also fixes Tales of Phantasia X's fades and etc.
This should improve performance when there are lots of little transfers.
@daniel229
Copy link
Collaborator

Fixes the Grand Knights History slow down,.
Cause flickering in the sea in boku no natsuyasumi 4

@unknownbrackets
Copy link
Collaborator Author

Was the sea working before? What framebuffers and transfers is it doing?

-[Unknown]

@daniel229
Copy link
Collaborator

Yes it does,with simulate block transfers.

W[G3D]: GLES\Framebuffer.cpp:1906 Block transfer download 04000000 -> 04044000

01

@daniel229
Copy link
Collaborator

also slightly flickering in Grand Knights History.

@unknownbrackets
Copy link
Collaborator Author

How's that? I made a stupid mistake in the fb range check. Is Grand Knights History still faster?

-[Unknown]

@daniel229
Copy link
Collaborator

No flickering,but Grand Knights History slow agains,

In that case, it's better to read it once, than multiple times.
Improves performance and graphics in Grand Knights History.
@unknownbrackets
Copy link
Collaborator Author

Okay, that should've improved Grand Knights History. It does a lot of block transfers. How does it look now?

-[Unknown]

@daniel229
Copy link
Collaborator

It look fine now,no slow down

@solarmystic
Copy link
Contributor

Nicely done @unknownbrackets. This resolves all the remaining newly discovered bugs in #6137 (which I reopened) using Read Framebuffers to Memory (CPU) with Danganronpa, Ys Seven, and Trails in the Sky.

@daniel229
Copy link
Collaborator

seem not good for god of war
02

@unknownbrackets
Copy link
Collaborator Author

God of War's framebuffers are misdetected in size, and so all kinds of things go wrong (it thinks things are offsets that aren't, and when it downloads full buffers which it still always does, it overwrites important data.) It can probably be improved (although there are potential slowdown problems.)

-[Unknown]

Performance improvement in Tactics Ogre.
@daniel229
Copy link
Collaborator

color bar problem in Tactics Ogre has been fixed.

08

09

This way it can do it itself as necessary in the right order.
Wasn't working properly, rendered to the wrong target.

No longer need the UpdateFromMemory() thing, now.
Fixes FF4 battle transition effect, Trails in the Sky menu, etc.
Makes Vempire look right (has never worked before in any rendering mode.)
This was it's properly ordered with draws, and it runs on the right GL
context.
@hrydgard
Copy link
Owner

Hm, are those block transfers consecutive in a display list? If so maybe we can check for the stripes and merge them.

@unknownbrackets
Copy link
Collaborator Author

I don't know, they came from here:
#4025 (comment)

But, that is a good point. From Grand Knights History's log, it seems to do them in a row.

-[Unknown]

@daniel229
Copy link
Collaborator

here is the debug log for super robot taisen oe with sceKernel* disable.
ppsspplogsrwoe

@unknownbrackets
Copy link
Collaborator Author

Block transfer: 0b52f2a0/100 -> 04044000/100, 256x272x4 (0,0)->(0,0)
Block transfer: 0b5732b0/100 -> 04000000/100, 256x272x4 (0,0)->(0,0)
Block transfer: 0b52f2a0/100 -> 04044000/100, 256x272x4 (0,0)->(0,0)
Block transfer: 0b5732b0/100 -> 04000000/100, 256x272x4 (0,0)->(0,0)

Hmm, I don't know why that wouldn't work, although clearly the bpp is double the framebuffer bpp...

And to be sure, a29d2e4 doesn't help (in case it's misdetecting the size)?

-[Unknown]

@daniel229
Copy link
Collaborator

It displays in half screen,another part is white
01

@unknownbrackets
Copy link
Collaborator Author

And it's supposed to be full screen, right? Does replacing this:

DrawPixels(dstBuffer, dstX, dstY, srcBase, dstBuffer->format, srcStride, width, height);

With:

DrawPixels(dstBuffer, dstX, dstY, srcBase, dstBuffer->format, dstBuffer->fb_stride, width, height);

Help? Well, the width will still be half, but does it at least show the correct left half?

-[Unknown]

@daniel229
Copy link
Collaborator

It should be like this
02

withthis branch,
The left side is mixing the right side,and display on left side.

@unknownbrackets
Copy link
Collaborator Author

Completely untested, but may work: 6243d0e

-[Unknown]

@daniel229
Copy link
Collaborator

Yeah,It does.

@daniel229
Copy link
Collaborator

Is there a fix for vidoe To Aru Kagaku no Railgun

@unknownbrackets
Copy link
Collaborator Author

That and God of War are the only issues left, right?

I'm not sure. Log? I guess it's writing the video somewhere in a way we're not detecting, and then invalidating. If only Vempire invalidates BEFORE blitting (which erases the framebuffer if we upload the content like that), then I guess we can just shaft Vempire, it's kinda a forgettable game.

Maybe I could check if something has been rendered to recently. Is it rendering to the video framebuffers?

-[Unknown]

@daniel229
Copy link
Collaborator

debug log for To Aru Kagaku no Railgun
ppsspplog

@daniel229
Copy link
Collaborator

@unknownbrackets
Copy link
Collaborator Author

Oh, so there's a small square in the bottom left. I forgot about 2x render resolution etc.

-[Unknown]

@daniel229
Copy link
Collaborator

Great,all fixed now,let's try another game that get affect on Simulate Block Transfer.

@daniel229
Copy link
Collaborator

Gachitora Abarenbou Kyoushi in High School
the ground is black without Simulate Block Transfer,with it,it is somehow corrcet,somehow flickering.

debuglog
ppsspplog

Video
https://drive.google.com/file/d/0BzGZGDfFE68zLUsyQkg4T2JUV2s/edit?usp=sharing

@unknownbrackets
Copy link
Collaborator Author

I'll look at it, but I'm probably going to hold off adding any new features/fixes to this until after it's merged to avoid making rebasing, reviewing, and testing harder.

-[Unknown]

@daniel229
Copy link
Collaborator

All right.

@unknownbrackets
Copy link
Collaborator Author

Hmm, I only see one upload logged there, there are a number of downloads but they don't look that crazy... hmm. There's no demo, right?

-[Unknown]

@hrydgard
Copy link
Owner

I'm happy to merge this as is, we can fix remaining issues later. @unknownbrackets , ready?

@daniel229
Copy link
Collaborator

Oh,there is a demo http://www.pspdemocenter.com/page.php?id=3733

@unknownbrackets
Copy link
Collaborator Author

Yep, I'm ready.

Well, my main concern is not breaking anything, but I think the heap corruption etc. is all gonna be for the better.

-[Unknown]

hrydgard added a commit that referenced this pull request May 26, 2014
Fix a heap overflow in block transfers, fix most save picture downloads, optimizations
@hrydgard hrydgard merged commit eead104 into hrydgard:master May 26, 2014
@unknownbrackets unknownbrackets deleted the gpu-blocktransfer branch May 26, 2014 18:18
@unknownbrackets
Copy link
Collaborator Author

For Gachitora Abarenbou Kyoushi in High School, it copies from each framebuffer, alternating. When it copies from one buffer, it works fine. When it copies from the other, it gets incorrect data. Everything looks otherwise the same...

Is there any GL state that applies to reading pixels, like viewport or something?

-[Unknown]

@solarmystic
Copy link
Contributor

This also fixes the photo taking capability of Soul Calibur during custom character creation when using the Simulated Block Transfer option. It was upside down before this.

799sc

Fixed now:-

822sc

This previously required the Read Framebuffers to Memory (CPU) option to be active in older pre-Block Transfer builds. Otherwise, only a black screen would be captured:-

750sc

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.

4 participants