Use a flipped coordinate system in framebuffers to avoid manual flipping of texture coordinates #8130

hrydgard · 2015-11-01T01:20:09Z

Simplifies a lot of things and gets rid of a few shaders, although there are a few new differences between buffered and non-buffered, as we are effectively flipped again when drawing to the backbuffer directly, which we do in non-buffered.

This will help #8016 .

unknownbrackets · 2015-11-01T01:47:37Z

GPU/Directx9/TextureCacheDX9.cpp

@@ -1012,7 +1011,7 @@ void TextureCacheDX9::ApplyTextureFramebuffer(TexCacheEntry *entry, VirtualFrame
 			// And also the UVs, same order.
 			const float uvleft = u1 * invWidth;
 			const float uvright = u2 * invWidth;
-			const float uvtop = 1.0f - v1 * invHeight;
+			const float uvtop = 1.0f - v1 * invHeight;  // TODO: Seems we should ditch the "1.0f - "


Yeah, definitely, the positions need to agree too. Otherwise it won't use the correct subset.

-[Unknown]

Oh, this is DX9... yeah maybe this is a typo, oops...

-[Unknown]

unknownbrackets · 2015-11-01T15:31:59Z

GPU/GLES/Framebuffer.cpp

@@ -659,7 +665,7 @@ void FramebufferManager::ResizeFramebufFBO(VirtualFramebuffer *vfb, u16 w, u16 h

 	vfb->fbo = fbo_create(vfb->renderWidth, vfb->renderHeight, 1, true, (FBOColorDepth)vfb->colorDepth);
 	if (old.fbo) {
-		INFO_LOG(SCEGE, "Resizing FBO for %08x : %i x %i x %i", vfb->fb_address, w, h, vfb->format);
+		WARN_LOG(SCEGE, "Resizing FBO for %08x : %i x %i x %i", vfb->fb_address, w, h, vfb->format);


Hmm, I don't think this is really a "warning" per se.

-[Unknown]

Ah, that was supposed to be temporary.

LunaMoo · 2015-11-01T17:00:48Z

Cool this fixed #4665

hrydgard · 2015-11-01T17:02:50Z

Neat. Not quite sure why though :)

There are still a couple of bugs lurking so I can't merge yet.

LunaMoo · 2015-11-01T17:10:11Z

Well there are some problems with effects, where select cursor or maybe just all transparent effects, shows inverted graphics inside of them, but other than that looks nice:).
Edit: oh those transparent effects might be related to bezier, I was testing macross just to see if any of the old issues there changed, and the jet engine effects are also inverted, probably one of the things not done yet.:)

Possibly it also affected #6736 but I don't have this one.

unknownbrackets · 2015-11-01T17:57:53Z

Okay, so just to take inventory:

What this does is draw everything "right side up" until drawing to the actual backbuffer. Previously we used the OpenGL "upside down" coordinate system throughout all drawing. Example:

4 XXXXXXXXXX      AAAAAAAAAA 0
3 XXXXXXXXXX      BBBBBBBBBB 1
2 CCCCCCCCCC  ->  CCCCCCCCCC 2
1 BBBBBBBBBB      XXXXXXXXXX 3
0 AAAAAAAAAA      XXXXXXXXXX 4

Then when drawing to the final backbuffer, the image is flipped appropriately to match the OpenGL system. In this way, textures (which use the "right side up" coordinate system) match framebuffers.

Problems and what I assume the solution is (still need to audit, though I've reviewed the code):

Rendering coordinates: Flipped using the projection matrix
Viewport params: No longer need to be flipped
Texturing from framebuffers: No longer need to be flipped
Memory -> framebuffer upload
: Textured into "right side up" coordinate system
Framebuffer -> memory download: No longer need to be flipped
Overlarge framebuffers: Accounted for by flipping rendering system accounting for bufferHeight
Texturing from framebuffer offset: Accounted for when blitting
Memory -> framebuffer offset: Accounted for in flipped coord system on upload
Framebuffer -> memory offset: Accounted for by flipping y1/y2 in y/h calculation

I'm most concerned about the last ones. If #4665 was fixed by this, it probably means there was a bug in that code already. Just to make sure I'm clear, sometimes OpenGL framebuffers are reused and larger than the height we "know" they have.

Before this ended up as the ASCII art above, where X's represent non-valid pixel data.

There's two ways the coordinate system could be flipped:

  Original           Type 1            Type 2
------------      ------------      ------------
X XXXXXXXXXX      AAAAAAAAAA 0      XXXXXXXXXX X
X XXXXXXXXXX      BBBBBBBBBB 1      XXXXXXXXXX X
2 CCCCCCCCCC  ->  CCCCCCCCCC 2  vs  AAAAAAAAAA 0
1 BBBBBBBBBB      XXXXXXXXXX X      BBBBBBBBBB 1
0 AAAAAAAAAA      XXXXXXXXXX X      CCCCCCCCCC 2

I think we are using the more logical type 1 (which will avoid problems with resizing.) As such, when flipping offsets, we need to account for this.

An example of download (glReadPixels):

  01234
0 ABCDE 6
1 FGHJI 5
2 JKLMN 4
3 OPQRS 3
4 TUVWX 2
- ----- 1
- ----- 0

OpenGL's coordinates on on the right side, sane coordinates are on the left. When downloading 0,0 with width and height of 4, we want OpenGL row 6 first, then 5, then 4, then 3. More than that, we also have to ensure we account for the offset - OpenGL row 0 is garbage data for us in this example, since it is outside the valid pixels.

#4665 looks very much like it was previously texturing or something from a 512 tall framebuffer, and it read the wrong half, and did so even upside down. If it was fixed now, that's great, but it seems like now other games have the reverse problem: downloads are upside down.

-[Unknown]

unknownbrackets · 2015-11-01T19:53:04Z

GPU/GLES/Framebuffer.cpp

@@ -1348,7 +1359,7 @@ void FramebufferManager::BlitFramebuffer(VirtualFramebuffer *dst, int dstX, int
 		// Should maybe revamp that interface.
 		float srcW = src->bufferWidth;
 		float srcH = src->bufferHeight;
-		DrawActiveTexture(0, dstX1, dstY, w * dstXFactor, h, dst->bufferWidth, dst->bufferHeight, !flip, srcX1 / srcW, srcY / srcH, srcX2 / srcW, (srcY + h) / srcH, draw2dprogram_);
+		DrawActiveTexture(0, dstX1, dstY, w * dstXFactor, h, dst->bufferWidth, dst->bufferHeight, srcX1 / srcW, srcY / srcH, srcX2 / srcW, srcY2 / srcH, draw2dprogram_);


Hmm, this is a bit confusing, since the factors are fixed to 1 here. Should probably change dstY to dstY1 and srcY to srcY1 to match, and either add * dstYFactor to h or remove from w. Before y was always "unscaled" in this statement, although that's sorta confusing anyway.

-[Unknown]

Hm, yeah, should clean this up. Anyway, it's in the !useBlit path so not likely to cause the remaining issues on my hardware at least..

unknownbrackets · 2015-11-01T21:11:14Z

Viewport y offset is off in Star Ocean 1 outside towns. This game uses a 272 Y scale (meaning, 272 * 2 total Y viewport height.)

-[Unknown]

hrydgard · 2015-11-01T21:34:06Z

Yeah, I believe the viewport clipping code in StateMapping might not be working properly. It hasn't been converted to view the world the new way...

hrydgard · 2015-11-01T21:47:34Z

@unknownbrackets in "Framebuffer -> memory offset"

Where's this flipping calculation you refer to? ("Accounted for by flipping y1/y2 in y/h calculation")

unknownbrackets · 2015-11-01T21:51:21Z

Oh, btw, we need to set flipped = false in all the debug buffers at the end of Framebuffer.cpp. I can send a pull.

About the offset... when downloading:

        int byteOffset = y * vfb->fb_stride * 4;
        glReadPixels(0, y, vfb->fb_stride, h, glfmt, GL_UNSIGNED_BYTE, packed + byteOffset);

Right now, it offsets the memory and y1 position it downloads from. This code was designed for flipped framebuffers, though, and so I think somewhere things need to be accounted for.

-[Unknown]

hrydgard · 2015-11-01T22:07:02Z

I think that should be right, actually - seems it might have been wrong before?

unknownbrackets · 2015-11-01T22:18:16Z

Maybe I'm confused. This is all very annoying.

  01234
0 ABCDE 6
1 FGHJI 5
2 JKLMN 4
3 OPQRS 3
4 TUVWX 2
- ----- 1
- ----- 0

Won't glReadPixels y=0, h=4 do the following:

Start at OpenGL Y = 0 (right side 0).
Download one stride and place it in memory.
Advance y by 1 (OpenGL y = 1 on the right side.)
Download one stride again, and repeat 4 times.
Finalize at OpenGL Y = 3, the last row downloaded.

This means that memory will now look like this, right?

-----
-----
TUVWX
OPQRS

Or am I missing something? Edit: no wait, maybe it would be the flipped view of this, but either way it'd be from the bottom, right? Before, it was flipped, so it worked out:

  01234
- ----- 6
- ----- 5
4 TUVWX 4
3 OPQRS 3
2 JKLMN 2
1 FGHJI 1
0 ABCDE 0

-[Unknown]

hrydgard · 2015-11-01T22:29:16Z

Well, now that we don't flip Y/V in various ways anymore, except for the final blit to the screen, you could now consider 0,0 to be top left in image space, it's really arbitrary if we call positive Y "up" or "down". glReadPixel will read from increasing Y line numbers into increasing memory addresses. When you consider GL coordinates the normal way, that would be upwards, but in our point of view, it's downwards and glReadPixels should thus behave the way we want...

unknownbrackets · 2015-11-01T23:26:07Z

Okay, sorry, I had it backwards. Thanks.

Well, the only thing I'm seeing wrong at this point is the viewport in Star Ocean 1. Grand Knights History and 3rd Birthday work with the other fixes so far.

-[Unknown]

LunaMoo · 2015-11-02T14:02:16Z

Should this branch help with #8016 at this point? Since there doesn't seem to be any difference for that.

Great work on figuring out all those problems through, seems like it's not breaking anything anymore while taking care of another ancient issue:).
Guessing will have to rebase my small display replacement again after this:3.

hrydgard · 2015-11-02T14:20:16Z

@LunaMoo No you won't, I'm going to merge your branch first and rebase this branch myself, don't worry.

LunaMoo · 2015-11-02T14:22:43Z

Oh I'm not worried about that:3, I'm actually glad I had to use rebase a few times since I finally learned how and it's pretty easy even when there are conflicts to resolve:). Git is kind of overwhelming without some practice.

hrydgard · 2015-11-02T15:25:31Z

Oh and I forgot to answer, yeah, it will help with #8016. I will commit the fixes to that after merging this.

… fix bugs in games, too. Fixes the strange camera issues in FF:CC.

… flips where we actually need them.

Add some missing projection matrix flips (flipped viewport emulation) Fix issue with DrawPixels in non-buffered in D3D9

hrydgard · 2015-11-02T19:13:28Z

Last commit broke Wipeout in GL buffered, will need to figure out that before merge.

BTW does Star Ocean work in cities with all the stencil stuff?

Use a flipped coordinate system in framebuffers to avoid manual flipping of texture coordinates

Flip display layout editor coordinates to match #8130:)

unknownbrackets reviewed Nov 1, 2015
View reviewed changes

hrydgard force-pushed the flip-framebuffers branch from a0625f4 to fff1364 Compare November 1, 2015 14:37

unknownbrackets reviewed Nov 1, 2015
View reviewed changes

hrydgard mentioned this pull request Nov 2, 2015

Wrong resolution used for postprocessing shaders with non standard buffer size #8016

Closed

hrydgard added 6 commits November 2, 2015 20:07

Remove a whole lot of flipping special cases

f4248cb

Fix a few more flips

94b7713

Fix non-buffered rendering

e9770b3

Move the viewport to the right place.

069a7b0

Fix non-through rectangles

c1db771

Shifted projection matrices need a few more parts flipped. This could…

1ce6801

… fix bugs in games, too. Fixes the strange camera issues in FF:CC.

hrydgard and others added 8 commits November 2, 2015 20:09

Remove more obsolete v-flip support

86bfce4

Two wrongs can appently make a right. Remove an inadvertent flip, add…

0266211

… flips where we actually need them.

Minor cleanups

6b28622

Fix a Y flip typo

bd64d5b

Make some optional arguments non-optional.

14652f0

Set the flipped flag to false on debug buffers

95240d4

Fix some typos in D3D9 blits (removed parameter).

099f62b

Add some missing projection matrix flips (flipped viewport emulation) Fix issue with DrawPixels in non-buffered in D3D9

Switch viewport overage to the new world of sanity.

3a7edd8

hrydgard force-pushed the flip-framebuffers branch from 5e41343 to 3a7edd8 Compare November 2, 2015 19:12

unknownbrackets' fixes for last commit

a1ad278

hrydgard added a commit that referenced this pull request Nov 2, 2015

Merge pull request #8130 from hrydgard/flip-framebuffers

728d9e5

Use a flipped coordinate system in framebuffers to avoid manual flipping of texture coordinates

hrydgard merged commit 728d9e5 into master Nov 2, 2015

hrydgard deleted the flip-framebuffers branch November 2, 2015 19:55

LunaMoo added a commit to LunaMoo/ppsspp that referenced this pull request Nov 2, 2015

Flip display layout editor coordinates to match hrydgard#8130:)

ae93fbb

LunaMoo added a commit to LunaMoo/ppsspp that referenced this pull request Nov 2, 2015

Flip display layout editor coordinates to match hrydgard#8130:)

71a0928

hrydgard added a commit that referenced this pull request Nov 3, 2015

Merge pull request #8144 from LunaMoo/reverse_flip

b7242a6

Flip display layout editor coordinates to match #8130:)

This was referenced Nov 3, 2015

Tokimeki Memorial 4 graphic issue. #6379

Closed

Yu-Gi-Oh! GX Tag Force Card summoning (card cut-off / cropped) #7124

Closed

Dante's Inferno - Graphics issue - Bloom #4845

Closed

Danball Senki W background broken after going to the game menu #8150

Closed

sum2012 mentioned this pull request Nov 3, 2015

Motto Nuga-Cel(ULJM05654):character's position is wrong in the battle #8149

Closed

This was referenced Nov 4, 2015

Gachitora missing shadows #8164

Closed

FF4 missing text. #8166

Closed

hrydgard mentioned this pull request Nov 4, 2015

FF Type-0 shadow broken #8169

Closed

daniel229 mentioned this pull request Dec 4, 2015

Screen rotation is not correct #8255

Closed

unknownbrackets mentioned this pull request Jan 20, 2016

Update the README for 1.2.0 #8466

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use a flipped coordinate system in framebuffers to avoid manual flipping of texture coordinates #8130

Use a flipped coordinate system in framebuffers to avoid manual flipping of texture coordinates #8130

hrydgard commented Nov 1, 2015

unknownbrackets Nov 1, 2015

unknownbrackets Nov 1, 2015

unknownbrackets Nov 1, 2015

hrydgard Nov 1, 2015

LunaMoo commented Nov 1, 2015

hrydgard commented Nov 1, 2015

LunaMoo commented Nov 1, 2015

unknownbrackets commented Nov 1, 2015

unknownbrackets Nov 1, 2015

hrydgard Nov 1, 2015

unknownbrackets commented Nov 1, 2015

hrydgard commented Nov 1, 2015

hrydgard commented Nov 1, 2015

unknownbrackets commented Nov 1, 2015

hrydgard commented Nov 1, 2015

unknownbrackets commented Nov 1, 2015

hrydgard commented Nov 1, 2015

unknownbrackets commented Nov 1, 2015

LunaMoo commented Nov 2, 2015

hrydgard commented Nov 2, 2015

LunaMoo commented Nov 2, 2015

hrydgard commented Nov 2, 2015

hrydgard commented Nov 2, 2015

Use a flipped coordinate system in framebuffers to avoid manual flipping of texture coordinates #8130

Use a flipped coordinate system in framebuffers to avoid manual flipping of texture coordinates #8130

Conversation

hrydgard commented Nov 1, 2015

unknownbrackets Nov 1, 2015

Choose a reason for hiding this comment

unknownbrackets Nov 1, 2015

Choose a reason for hiding this comment

unknownbrackets Nov 1, 2015

Choose a reason for hiding this comment

hrydgard Nov 1, 2015

Choose a reason for hiding this comment

LunaMoo commented Nov 1, 2015

hrydgard commented Nov 1, 2015

LunaMoo commented Nov 1, 2015

unknownbrackets commented Nov 1, 2015

unknownbrackets Nov 1, 2015

Choose a reason for hiding this comment

hrydgard Nov 1, 2015

Choose a reason for hiding this comment

unknownbrackets commented Nov 1, 2015

hrydgard commented Nov 1, 2015

hrydgard commented Nov 1, 2015

unknownbrackets commented Nov 1, 2015

hrydgard commented Nov 1, 2015

unknownbrackets commented Nov 1, 2015

hrydgard commented Nov 1, 2015

unknownbrackets commented Nov 1, 2015

LunaMoo commented Nov 2, 2015

hrydgard commented Nov 2, 2015

LunaMoo commented Nov 2, 2015

hrydgard commented Nov 2, 2015

hrydgard commented Nov 2, 2015