search

MiSTer-devel / SMS_MiSTer

Sega Master System for MiSTer
45 stars 43 forks source link

Revert blanking changes to fix pal + border #134

Closed birdybro closed 2 years ago

birdybro commented 2 years ago

The blanking changes in this commit --> https://github.com/MiSTer-devel/SMS_MiSTer/commit/cbef92c6e7c7afb2997d807a584f38db2ae46c8a - negatively impacted PAL + border going to the scaler. They must have been done incorrectly. This was also leading to direct video output over PAL with border on looking incorrect.

Fixes --> https://github.com/MiSTer-devel/SMS_MiSTer/issues/120

If the blanking needs to be fixed further, care must be taken to test scaled and analog output next time.

birdybro commented 2 years ago

Dang it... sorry for another waste of time with reading this. Closing, disregard. Still have to adjust the blanking better for PAL before I submit this, needs a little tweaking... Found an edge case.

birdybro commented 2 years ago

Nevermind again, this actually does resolve the behavior we were seeing, I was confusing an option that I had toggled on in reverse when testing. This is good to merge if you think it's okay.

paulb-nl commented 2 years ago

Not sure what you are trying to do but the issue you linked to is caused by forced 216p cropping when the output resolution is 1080p and this revert does not change that.

If you want to fix that then you should send a PR that makes cropping optional.

birdybro commented 2 years ago

Hrm, I'll do that. I was having problems with it in testing but now I forget what was wrong. I need to focus on one thing at a time. Thank you.

birdybro commented 2 years ago

@paulb-nl this is what I was seeing, but I guess this is correct because PAL regions had the unfortunate disadvantage of squished aspect ratio SMS games?

https://user-images.githubusercontent.com/16388068/178158238-12e8509f-ee23-40ba-9b58-365355649d7e.mp4

paulb-nl commented 2 years ago

Well yes originally PAL always had a squashed picture so you could say this is correct but regardless I think the PAL option with border should match the aspect ratio without border.

The aspect ratio should be changed here for PAL: https://github.com/MiSTer-devel/SMS_MiSTer/blob/29aa9f2ccfa61a467dbbf56ebe09d10c195aedc8/SMS.sv#L201-L202 The choice is then if you want to have the squished PAL aspect ratio or not.

birdybro commented 2 years ago

The Display Aspect Ratio is not observed anyways at the moment, I plan to fix that after the vcrop maybe gets merged and after smode_M1 is fixed to stop forcing 224p high with border on. With border on smode_M1 (224p mode) should go to 240 resolution vertically, so that's incorrect as well. That's for another PR though.

With the border on and if the game is an SMS game, it should be 282x240 at all times, currently it's not doing that. If it were, then border on would be a 47:35 aspect ratio for NTSC and a 376:277 aspect ratio for PAL. 47:35 is incorrect for codemasters + border currently because the output is 282x224 for some reason. Game gear aspect ratio I put there was to fix it being way too wide, but the correct display aspect ratio is actually different, however I was waiting on the extended mode to be added and other things to get the resolutions all fixed in all possible modes.

But having a conditional for PAL's DAR is probably unnecessary since it's merely academic.