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

PCRTC: Proper handling for 720P/1080P video modes. #1137

Merged
merged 4 commits into from Feb 1, 2016
Merged

PCRTC: Proper handling for 720P/1080P video modes. #1137

merged 4 commits into from Feb 1, 2016

Conversation

ssakash
Copy link
Member

@ssakash ssakash commented Jan 21, 2016

Summary of changes:

  • Properly Initialize the height , width value. we previously only initialize the DH, DW register values which aren't the exact height/width. I'm actually surprised that the older check for height (pool paradise) worked ! It's usually not a problem since MAGV value is usually 0 at most instances. The correct formula for width and height are (DW + 1) / (MAGH +1) and (DH + 1) / (MAGV + 1).
  • Add a nice print debug message to easily debug games by uncommenting the code. ( I can understand if you don't want this commit though )
  • Allow 640+ height values for all Vmodes except PAL/NTSC as they need the higher height value for proper display size.

Additional Information:
Here's a nice picture to explain the role of the Display register values :

pic

Example of observed changes of the commit:

1080P (before commit)
wrong

1080P (after commit)
correct

Build download link: here

@ssakash
Copy link
Member Author

ssakash commented Jan 21, 2016

@refractionpcsx2 tested Pool paradise and it seems to be fine.

Note: This Pull request only removes a part of the problems with the 720P/1080P video modes , the other source of issue - wrong MAGH/MAGV values still needs to be dealt with for accurate display on 720P/1080P video modes.

@DonelBueno
Copy link

Any changes on Gran Turismo 4? It zooms the image in 1080i mode.

@gregory38
Copy link
Contributor

I need to check it but I think there is also a bad >>1 division on the renderer. Anyway

Allow 640+ height values for all Vmodes except PAL/NTSC as they need the higher height value for proper display size. 

It means a single cmode value but you code don't do that but !=

@ssakash
Copy link
Member Author

ssakash commented Jan 23, 2016

Any changes on Gran Turismo 4? It zooms the image in 1080i mode.

Note: This Pull request only removes a part of the problems with the 720P/1080P video modes , the other source of issue - wrong MAGH/MAGV values still needs to be dealt with for accurate display on 720P/1080P video modes.

@gregory38

It means a single cmode value but you code don't do that but !=

I'm not sure i understand you.. only NTSC/PAL use different CMOD values 2 and 3. all other Vmode's have CMOD value of 0.

@DonelBueno
Copy link

Well, GT 4 could be in the part that is solved by this pull request.

@gregory38
Copy link
Contributor

Code is difficult to read for me :P . Could you replace hard coded value with a nice constant name, this way comment would be useless.

@refractionpcsx2
Copy link
Member

So do we know what CMOD 1 is, while we're at it?

@ssakash
Copy link
Member Author

ssakash commented Jan 25, 2016

So do we know what CMOD 1 is, while we're at it?

CMOD seems to be only set on three values. 0 , 2 and 3.

video mode register docs : http://hastebin.com/pezecuguri.vhdl

Well, GT 4 could be in the part that is solved by this pull request.

I'd say that is highly unlikely till the magnification factors have been dealt with it as most games rely on them pretty heavily ;)

@DerekTurtleRoe
Copy link
Contributor

@ssakash Thanks for that doc, I have been wanting something like that for a while so I could write some of my own stuff. 😄

So before this PR, did games really just zoom the image and cut off most of the actual game?

@ssakash
Copy link
Member Author

ssakash commented Jan 26, 2016

So before this PR, did games really just zoom the image and cut off most of the actual game?

Yes, on games which use 720P/1080I/1080P modes the screen gets cropped up since we previously limited higher height values ( which should have been only intended for PAL/NTSC Vmode).

Also the above comparison picture I had posted was from an ELF called "Video mode test" which allows us to test different video modes. the magnification is still wrong though... I still need to fix that at the future. ( I wouldn't be surprised if most commercial games still exhibit broken behavior as they rely heavily on the magnification registers for display size )

Code is difficult to read for me :P . Could you replace hard coded value with a nice constant name, this way comment would be useless.

I have added some macros for the video modes.

@ssakash
Copy link
Member Author

ssakash commented Jan 26, 2016

Hmm, thinking again it's likely that the magnification registers values are manipulated to control the display ratio (or) something like that by game developers. I personally had an opinion that the registers values are forced for every single video modes but maybe I'm wrong... If that's the case then the 1080P mode at GT4 should work.

@rz5

can you test the 1080P video mode on GranTurismo 4 ? comparison screenshots would be nice if it fixes it.

I think there is also a bad >>1 division on the renderer.

I think I found one on my fourth commit , did you mean that one ? but it's not a right shift though.,..

@Blackbird88
Copy link
Contributor

Is this some region thing? I only see 1080i
image

@refractionpcsx2
Copy link
Member

There is no 1080p, the PS2 could only do 1080i maximum.

@Blackbird88
Copy link
Contributor

I thought as much :D
1080p on PS2 would be impressive feat.

@refractionpcsx2
Copy link
Member

it would lol. It can only do 1080i because it is interlacing 2 540p frames, which it can easily handle (individually anyway lol) :P

@ssakash
Copy link
Member Author

ssakash commented Jan 28, 2016

I see, GT4 doesn't have a 1080P mode ? 1080i also had some problems previously so does it work fine now ?

the VmodeTest.ELF had a 1080P mode present though.

@Blackbird88
Copy link
Contributor

Did AppVeyor generate build? I cba screwing with git tbh

@ssakash
Copy link
Member Author

ssakash commented Jan 28, 2016

comparison screenshots before the build and after the build would be nice :)

@Blackbird88
Copy link
Contributor

Old
image
New
image

gj I say!

@ssakash
Copy link
Member Author

ssakash commented Jan 28, 2016

This is with the GSDX plugin on the link provided (or) the latest development builds ?

@Blackbird88
Copy link
Contributor

I edited the post. Sorry for confusion (wrong pic!). Old is pre-fix and New is the AppVeyor GSDX.

@ssakash
Copy link
Member Author

ssakash commented Jan 28, 2016

Thanks, can you provide some comparison pictures of old build and new build taken with F8 (Hotkey) , It captures the whole frame. would be nice :)

@Blackbird88
Copy link
Contributor

Oddly the new GSDX only generates .bmp screenshot. Old one generates the .gs dump fine :O

@ssakash
Copy link
Member Author

ssakash commented Jan 28, 2016

Oddly the new GSDX only generates .bmp screenshot. Old one generates the .gs dump fine :O

for GS dump, you need Shift + F8

On the other hand, I guess I was right about the dynamic manipulation of the magnification registers, I had reached his opinion after some debugging on Bomber man Kart.

The original DW register : 2559
Real width = (2559 + 1) / (MAGH + 1)

In first case , MAGH = 3 -> It outputted 640x448 (during race)
In second case , MAGH = 7 -> It outputted 320x448 (during trophy screen)

While the display was fine on both of those scenarios so I guess it'd be safe to assume that the real display size/position gets manipulated dynamically according to magnification factors instead of having one as constant for each video mode.

@Blackbird88
Copy link
Contributor

@ssakash
Copy link
Member Author

ssakash commented Jan 28, 2016

@Blackbird88

I actually wanted the screenshots generated by F8 :P

@Blackbird88
Copy link
Contributor

I packed both .bmp and .gs ;)

@ssakash
Copy link
Member Author

ssakash commented Jan 28, 2016

Cool, Thanks ! I'm not sure why but the media fire links are loading a bit too slow for me...

can you link the images directly on Git ?

@Blackbird88
Copy link
Contributor

Old
gsdx_20160128114914
New
gsdx_20160128115510

Sadly Github doesn't support bmp lol

@DonelBueno
Copy link

Instamerge, please. GT 4 on Software mode is perfect in 1080i.

GetDeviceSize() already does a half division based on the INT and FFMD
registers.
@ssakash
Copy link
Member Author

ssakash commented Jan 30, 2016

@refractionpcsx2 has confirmed that this also fixes the bad screen issue on Gitaroo Man [PAL] version. http://forums.pcsx2.net/Thread-GSDX-PCRTC-Gitaroo-Man-screen-cut-off?pid=504985#pid504985

gregory38 added a commit that referenced this pull request Feb 1, 2016
PCRTC: Proper handling for 720P/1080P video modes.
@gregory38 gregory38 merged commit 85f64b8 into PCSX2:master Feb 1, 2016
@ssakash ssakash deleted the CRTC_Scaling branch February 1, 2016 11:08
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.

None yet

6 participants