MiSTer-devel / C64_MiSTer

112 stars 56 forks source link

feat(): Allow for 1440p vertical crop #123

Closed asturur closed 2 years ago

asturur commented 2 years ago

This change allow for the vertical crop option to be enabled also on 1440p. It give us a 1920x1440 nearly full screen C64 with just a couple of pix of border all around. Not enabling vertical crop won't change much.

I have a 1536 screen with which i can make some tests later if this change is welcome.

asturur commented 2 years ago

Hi @sorgelig, any input on this PR?

sorgelig commented 2 years ago

need to check.. probably vcrop should be 240, not 204.

asturur commented 2 years ago

this is specific for 1920x1440 to get a 4:3 full screen. you get rid of the empty large borders ( optionally, because crop is still optional ) and the scaler can get up to 7x vertical scaling. If you want to keep the empty borders ( 240 or more lines ) you simply don't enable crop.

i picked 204 over 200 as for other resolutions in the code because there is space at 1440 and also in this way you can still see a tiny border around the 320x200 central area that sometimes will flash with colors. I can post some picture with the phone

asturur commented 2 years ago

ok wait, i measure the AR, and there is a change is 1.21 instead of 1.34. Fixing it

sorgelig commented 2 years ago

I prefer not to introduce implied features. vcrop in this context should be a visible area from top to bottom. Cropping here is a compromise to fit the video with max size sacrificing the border. If there is space on top/bottom then it should be filled by border without additional black frame.

asturur commented 2 years ago

What do you mean with implied feature? This crop seems as any other crop we enable on the other resolutions, the only difference is that the actual scaler is throwing away pixels from also left and right because there is no space in the screen.

But that seem to me a feature that is embedded in the scaler and that the user opt in enabling the crop.

the good part is that most of c64 softwares have nothing in that borders.

i ll post pictures when i m back home

sorgelig commented 2 years ago

I mean that 1440/204 is not integer scale. Integer scale is 1440/240. So there is no point to cut 36 lines as border is taller than that.

asturur commented 2 years ago

i can take pictures with both modes, maybe clarifies my intention.

asturur commented 2 years ago

Let's start with a consideration, If i want preserve aspect ratio, HV integer is often a no go, and v-integer + ar original is the way i usually set it up.

Current situation at V integer, no crop. The dark area is very small. i think is a 5x IMG_2925

This is the 240 crop as you suggest. WIDE not set, is an improvement overall. this is 6x IMG_2926

those are NTSC and PAL, 204 crop, 7x WIDE set ( obviously as you noted, is just 1428 px so 12 are left dark on the screen, we could 205 but does not change much ) IMG_2928 IMG_2929

Now the aspect ratio.... Pal is correct at around 1.34, while NTSC get shrunk at 1.2. I m not sure if this is something that happens also with the other crops. i did no test yet but i'll do, to understand if this is a regression that this choice introduce.

Said so i understand you may have a strong opinion about not letting the user extremely reduce the border, but to me as a user, that border is always empty. Maybe there are software that uses it? demos?

As another note: 240 crop v-integer is way similar to no crop, full screen scaling, the difference is very little. You gain of course integer scaling but you obtain more or less the same size. Picture for reference IMG_2927

I have also additional questions. VCROP is not available as a choice if in the general ini setting of mister i enable the scale to integer only. That would be fine, if it wasn't that when then i go to enable it, the only way to actually use the crop are the integer scale mode. IF i select scale normal ( so basically use all the screen lines regardless of an integer multiplier ) the crop is still applied, but the cropped space is maintained inside the screen. but in black. Do you know why it behave so?

Let me know what do you think. I think 7x is nicer offer a larger view for those screens, and that more choices are better than less choices. and that 6x ( 240 ) is still better than nothing if we can't have 7x.

asturur commented 2 years ago

oh sorry, as an added note, some line above my code, we already have 2 situations of cropping at 200. So if is not right, maybe we could take the occasion and remove those too.

asturur commented 2 years ago

@sorgelig if you really prefer i can put the crop to 240. We don't gain much compared to normal scaling, but at least we activate integer scaling for this resolution at a normal size.

sorgelig commented 2 years ago

C64 originally has border, so more or less it should be available. Some games use it. Almost all demos use it. Many intros also use it. Sometime important info may be output on border too. Crop feature here is not to make C64 borderless.

asturur commented 2 years ago

Just tell me clearly, crop at 240 or no crop? I understand you don't like crop at 204.

sorgelig commented 2 years ago

I will add crop myself. I have display supporting both 1440p and 1536p.

asturur commented 2 years ago

i have too. i would appreciate if you explain what is wrong on the pr and let me improve and contribute.

On Wed, 5 Jan 2022 at 14:06, Alexey Melnikov @.***> wrote:

I will add crop myself. I have display supporting both 1440p and 1536p.

— Reply to this email directly, view it on GitHub https://github.com/MiSTer-devel/C64_MiSTer/pull/123#issuecomment-1005669499, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJDQQDWQGZ3TQ3RLFUGS5LUUQ65RANCNFSM5K6FMSIA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you authored the thread.Message ID: @.***>

sorgelig commented 2 years ago

But i've already explained above. C64 should have border as original HW had it. So 240p crop for 1440p display should be more suitable than 204p.

asturur commented 2 years ago

That is fine, i can make the PR with 240. The point is i needed a straight answer, we talked about options and should/shouldn't till now.