Rework the aspect ratio calculation (Fixes 240p mode) #3472

phire · 2016-01-08T03:00:51Z

Calculations are now based on signal timings rather than pixels, as it didn't make a lot of sense to do things with pixels.

Now handles all 240i/240p/480i/480p modes without any special casing. Will even handle modes like 960i or 243@59.74hz

Despite the changes in equations, the resulting aspect ratios are identical.

Video Interface simply isn't aware about widescreen. Instead, the render class can multiply by 1.3333333 to get the 16:9 aspect ratio.

phire · 2016-01-08T09:15:20Z

@mirrorbender could you take a look at this? It should have the exact same aspect ratios as your old code.

phire · 2016-01-08T09:20:05Z

This is ready for review/merging now. Replaces #3346

JMC47 · 2016-01-08T09:21:23Z

lgtm in basic testing of various games and aspect ratios.

Source/Core/Core/HW/VideoInterface.cpp

+
+	// 5. Calculate the final ratio and scale to 4:3
+	float ratio = horizontal_active_ratio / vertical_active_ratio;
+	if(std::isnormal(ratio))	// Check we have a sane ratio and haven't propagated any infs/nans/zeros


delroth · 2016-01-08T10:29:11Z

@phire: This comment grants you the permission to merge this pull request whenever you think it is ready. After addressing the remaining comments, click this link to merge.

Other than these two nits, code LGTM. @dolphin-emu-bot allowmerge

mirrorbender · 2016-01-08T19:29:12Z

@phire I looked over it quickly. The numbers are correct (as much as that is possible here). I can go over the code more closely if you want me to, but I trust that you and the others who have reviewed and tested it have already done a fine job of that, and at a glance it looks fine to me. So I'd say go ahead and merge it.

phire · 2016-01-09T00:29:19Z

@mirrorbender Now that I've separated the vertical and horizontal calculations and moved away from pixels, I can see that the vertical timings are wrong by half a line per field. The NTSC timings should be 262.5 / 242.5 which works out to 525 / 485 (instead of the current 525 / 486)

You can confirm this by looking Figure 7 on page 11 of SMPTE 170M-2004.
Actually it goes a step further and defines the vertical blanking per field as 20 lines + 1.5us in Detail Y-Y and Table 3 on page 13.

Half a line makes no sense in pixels, but it makes perfect sense for a CRT where the horizontal and vertical timings aren't directly connected and the image is rotated slightly anyway.

I'll submit a second PR in the future to address this.

mirrorbender · 2016-01-09T00:56:23Z

I'm having trouble following the diagrams in terms of where the half lines go in the picture (half line at the end of field 1 and beginning of field 2?), but my reasoning for 486 was that two fields of 242.5 resulted in 484 full lines plus 2 separate half lines, which meant 486 was a better number if those half-lines were scanned to it's own vertical space within the active frame. However, assuming a slight tilt, I can see how 485 lines would be a better approximation of the appropriate height of the screen in a CRT TV.

phire · 2016-01-09T01:26:30Z

The half lines are at the start and end of the frame. The standard doesn't say it, but Field 1 is the even/lower field and Field 2 is the upper/odd field.
With the tilt, the start of line 1 is the same height on the screen as the start of line 2, except line 1 starts half-way along the screen. It's the scan pattern which is tilted, I suspect TVs were build with a slight rotation of the coils in the opposite direction, so the tilt would be canceled out.

For a digital video format which can't do half-lines, padding it to 486 lines (or discarding both half-lines and dropping down to 484 lines) make sense, but 485 lines is completely nonsensical.

They are now based on signal timings rather than pixels, as it didn't make a lot of sense to do things with pixels. Now handles all 240i/240p/480i/480p modes without any special casing. Despite the diffrent equaions, this should result in the exact same aspect ratio as the previous code.

phire · 2016-01-09T01:36:17Z

Comments have been updated to fix typos and add some details from the BT-470-6 standard.

Rework the aspect ratio calculation (Fixes 240p mode)

dolphin-emu-bot · 2016-01-09T23:51:55Z

FifoCI detected that this change impacts graphical rendering. Here are the behavior differences detected by the system:

aeon-charge-attack on ogl-lin-nouveau: diff
chibi-robo-zfighting on ogl-lin-nouveau: diff
fortune-street-white-box on ogl-lin-nouveau: diff
mtennis-zfreeze on ogl-lin-nouveau: failed to render
thps4-shadow on ogl-lin-nouveau: failed to render
chibi-robo-zfighting on ogl-lin-radeon: diff
monkeyball-fuse on ogl-lin-radeon: diff
rs2-zfreeze on ogl-lin-radeon: diff
sms-bubbles on ogl-lin-radeon: diff
sw3-dt on ogl-lin-radeon: diff

_{^{automated-fifoci-reporter}}

fallaha56 · 2016-01-10T10:03:45Z

hi, aspect ratios still broken on N64 VC games -> going 16:9 when 4:3, bug report filed

phire · 2016-01-10T10:11:22Z

Technically that's correct emulation, as on a real Wii/TV you are required
to manually switch between 16:9 and 4:3 modes for virtual console games.

But we really should add .ini files for all virtual console games to
override that.

Scott Mansell

On 10 January 2016 at 23:03, fallaha56 notifications@github.com wrote:

hi, aspect ratios still broken on N64 VC games -> going 16:9 when 4:3, bug
report filed

—
Reply to this email directly or view it on GitHub
#3472 (comment).

JMC47 · 2016-01-10T10:12:38Z

Yes, any issue made is invalid.

On Sun, Jan 10, 2016, 5:11 AM Scott Mansell notifications@github.com
wrote:

Technically that's correct emulation, as on a real Wii/TV you are required
to manually switch between 16:9 and 4:3 modes for virtual console games.

But we really should add .ini files for all virtual console games to
override that.

Scott Mansell

On 10 January 2016 at 23:03, fallaha56 notifications@github.com wrote:

hi, aspect ratios still broken on N64 VC games -> going 16:9 when 4:3,
bug
report filed

—
Reply to this email directly or view it on GitHub
<#3472 (comment)
.

—
Reply to this email directly or view it on GitHub
#3472 (comment).

fallaha56 · 2016-01-10T10:13:49Z

thanks @phire i did wonder if you guys were simply being too accurate ;-)

but yeah some form of override would be cool thanks

@JMC47 i wouldn't describe it as invalid! it makes using VC a massive pain...

JMC47 · 2016-01-10T10:18:07Z

Hehehe! It is accurate to the Wii, though. That's why it's not valid.

On Sun, Jan 10, 2016, 5:14 AM fallaha56 notifications@github.com wrote:

thanks @phire https://github.com/phire i did wonder if you guys were
simply being too accurate ;-)

but yeah some form of override would be cool

@JMC47 https://github.com/JMC47 wouldn't describe it as invalid! it
makes using VC a massive pain...

—
Reply to this email directly or view it on GitHub
#3472 (comment).

JMC47 · 2016-01-10T10:19:47Z

I am okay with ini overrides, though.

On Sun, Jan 10, 2016, 5:17 AM Justin C jmc4789@gmail.com wrote:

Hehehe! It is accurate to the Wii, though. That's why it's not valid.

On Sun, Jan 10, 2016, 5:14 AM fallaha56 notifications@github.com wrote:

thanks @phire https://github.com/phire i did wonder if you guys were
simply being too accurate ;-)

but yeah some form of override would be cool

@JMC47 https://github.com/JMC47 wouldn't describe it as invalid! it
makes using VC a massive pain...

—
Reply to this email directly or view it on GitHub
#3472 (comment)
.

fallaha56 · 2016-01-10T10:26:26Z

thanks @JMC47...fingers hurting from the added mouseclicks resetting ratios ;) can refile issue as 'finger injury' if that's better lol ;))

Normalize aspect ratio calculations to 4:3

9c0dae4

Video Interface simply isn't aware about widescreen. Instead, the render class can multiply by 1.3333333 to get the 16:9 aspect ratio.

phire force-pushed the TVs_dont_have_pixels branch 2 times, most recently from cf372b1 to b658331 Compare January 8, 2016 09:12

phire changed the title ~~[WIP] Rework the aspect ratio calculation (Fixes 240p mode)~~ Rework the aspect ratio calculation (Fixes 240p mode) Jan 8, 2016

booto mentioned this pull request Jan 8, 2016

VI: use double height to calc AR in DS mode #3346

Closed

delroth added this to the Dolphin Release 5.0 milestone Jan 8, 2016

delroth reviewed Jan 8, 2016
View reviewed changes

phire force-pushed the TVs_dont_have_pixels branch from b658331 to c1a1ac1 Compare January 9, 2016 00:43

phire force-pushed the TVs_dont_have_pixels branch from c1a1ac1 to 25b91f1 Compare January 9, 2016 01:29

phire force-pushed the TVs_dont_have_pixels branch from 25b91f1 to c49a82b Compare January 9, 2016 01:31

delroth added a commit that referenced this pull request Jan 9, 2016

Merge pull request #3472 from phire/TVs_dont_have_pixels

39971ec

Rework the aspect ratio calculation (Fixes 240p mode)

delroth merged commit 39971ec into dolphin-emu:master Jan 9, 2016

phire deleted the TVs_dont_have_pixels branch February 2, 2016 11:46

Rework the aspect ratio calculation (Fixes 240p mode) #3472

Rework the aspect ratio calculation (Fixes 240p mode) #3472

phire commented Jan 8, 2016

phire commented Jan 8, 2016

phire commented Jan 8, 2016

JMC47 commented Jan 8, 2016

This comment was marked as off-topic.

delroth commented Jan 8, 2016

mirrorbender commented Jan 8, 2016

phire commented Jan 9, 2016

mirrorbender commented Jan 9, 2016

phire commented Jan 9, 2016

phire commented Jan 9, 2016

dolphin-emu-bot commented Jan 9, 2016

fallaha56 commented Jan 10, 2016

phire commented Jan 10, 2016

JMC47 commented Jan 10, 2016

fallaha56 commented Jan 10, 2016

JMC47 commented Jan 10, 2016

JMC47 commented Jan 10, 2016

fallaha56 commented Jan 10, 2016

Rework the aspect ratio calculation (Fixes 240p mode) #3472

Rework the aspect ratio calculation (Fixes 240p mode) #3472

Conversation

phire commented Jan 8, 2016

phire commented Jan 8, 2016

phire commented Jan 8, 2016

JMC47 commented Jan 8, 2016

This comment was marked as off-topic.

delroth commented Jan 8, 2016

mirrorbender commented Jan 8, 2016

phire commented Jan 9, 2016

mirrorbender commented Jan 9, 2016

phire commented Jan 9, 2016

phire commented Jan 9, 2016

dolphin-emu-bot commented Jan 9, 2016

fallaha56 commented Jan 10, 2016

phire commented Jan 10, 2016

JMC47 commented Jan 10, 2016

fallaha56 commented Jan 10, 2016

JMC47 commented Jan 10, 2016

JMC47 commented Jan 10, 2016

fallaha56 commented Jan 10, 2016