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

VideoInterface: start counting half-lines at 0 instead of 1 #8334

Merged
merged 1 commit into from Aug 28, 2019

Conversation

@hosaka-corp
Copy link
Contributor

commented Aug 26, 2019

Found it easier to reason about VideoInterface::Update() if we start counting half-lines at 0 instead of 1.

  • Re-organize VideoInterface::Update() to count half-lines starting at 0 instead of 1
  • Use horizontal position when checking if we should assert some display interrupt. This should make them slightly more accurate to the hardware (granularity changes to half-lines, instead of only checking on vertical line boundaries)
  • Add some more descriptive comments

@hosaka-corp hosaka-corp force-pushed the hosaka-corp:vi-update-reorg branch from bca1a43 to 4bd7d00 Aug 26, 2019

@hosaka-corp

This comment has been minimized.

Copy link
Contributor Author

commented Aug 26, 2019

Fixed to account for impending #8230

@BhaaLseN
Copy link
Member

left a comment

Since #8230 is merged, you'll have to rebase this.

{
s_ticks_last_line_start = CoreTiming::GetTicks();
u32 h_thresh = (reg.HCT > m_HTiming0.HLW) ? 1 : 0;

This comment has been minimized.

Copy link
@BhaaLseN

BhaaLseN Aug 26, 2019

Member

No need to save on letters here; especially when your PR is meant to make it easier to reason about code ;)

Suggested change
u32 h_thresh = (reg.HCT > m_HTiming0.HLW) ? 1 : 0;
u32 horizontal_threshold = (reg.HCT > m_HTiming0.HLW) ? 1 : 0;

Not sure if thats the best name though (or whether my guesses of what h and thresh were meant to stand for were correct).

@hosaka-corp hosaka-corp force-pushed the hosaka-corp:vi-update-reorg branch from 4bd7d00 to 4c2a7ef Aug 26, 2019

@JMC47

This comment has been minimized.

Copy link
Contributor

commented Aug 26, 2019

This breaks the FMV loading in NTSC (PAL works) Mario Kart: Double Dash after the Lakitu/Fish thing. The game isn't frozen, the FMV just doesn't play. You can skip it like normal and play the game.

VideoInterface: start counting half-lines at 0 instead of 1
- Re-organize VideoInterface::Update() to count half-lines starting at 0 instead of 1
- Use horizontal position when checking if we should assert some display interrupt
- Add some more descriptive comments

@hosaka-corp hosaka-corp force-pushed the hosaka-corp:vi-update-reorg branch from 4c2a7ef to 9e3b867 Aug 27, 2019

@hosaka-corp

This comment has been minimized.

Copy link
Contributor Author

commented Aug 27, 2019

Forgot to adjust the half-line count when servicing MMIO reads on the vertical beam position register.
This appears to solve the issue with Double Dash.

@JMC47

This comment has been minimized.

Copy link
Contributor

commented Aug 27, 2019

Everything I tested appears to work now.

@booto
booto approved these changes Aug 28, 2019
@booto

This comment has been minimized.

Copy link
Contributor

commented Aug 28, 2019

LGTM

@stenzek stenzek merged commit 71ff97c into dolphin-emu:master Aug 28, 2019

10 checks passed

default Very basic checks passed, handed off to Buildbot.
Details
lint Build succeeded on builder lint
Details
pr-android Build succeeded on builder pr-android
Details
pr-deb-dbg-x64 Build succeeded on builder pr-deb-dbg-x64
Details
pr-deb-x64 Build succeeded on builder pr-deb-x64
Details
pr-freebsd-x64 Build succeeded on builder pr-freebsd-x64
Details
pr-osx-x64 Build succeeded on builder pr-osx-x64
Details
pr-ubu-x64 Build succeeded on builder pr-ubu-x64
Details
pr-win-dbg-x64 Build succeeded on builder pr-win-dbg-x64
Details
pr-win-x64 Build succeeded on builder pr-win-x64
Details
@@ -317,7 +317,7 @@ void RegisterMMIO(MMIO::Mapping* mmio, u32 base)
// MMIOs with unimplemented writes that trigger warnings.
mmio->Register(
base | VI_VERTICAL_BEAM_POSITION,
MMIO::ComplexRead<u16>([](u32) { return 1 + (s_half_line_count - 1) / 2; }),
MMIO::ComplexRead<u16>([](u32) { return 1 + (s_half_line_count) / 2; }),

This comment has been minimized.

Copy link
@adroitwhiz

adroitwhiz Aug 30, 2019

are the parentheses around s_half_line_count necessary anymore?

@rlnilsen

This comment has been minimized.

Copy link

commented Sep 12, 2019

If you enable progressive scan and disable PAL60 mode on 10888, Visual Controller Test is broken with a black screen.

https://bugs.dolphin-emu.org/issues/11851

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
7 participants
You can’t perform that action at this time.