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

VideoCommon: Clear blend configuration if color/alpha update disabled #11406

Merged
merged 1 commit into from Feb 14, 2023

Conversation

Pokechu22
Copy link
Contributor

This works around an Intel driver bug where, on D3D12 only, dual-source blending behaves incorrectly if the second source is unused on. This bug is visible in skyboxes in Super Mario Sunshine, which first draw clouds and sun flare in greyscale and then draw the sky afterwards with a source factor of 1 and a dest factor of 1-src_color (this results in the clouds being tinted blue). This process is done on an RGB888 framebuffer, so alpha update is disabled. (Color update is enabled; note that if you look at this in Dolphin's fifo analyzer, it won't be enabled because they use the BP mask functionality to only change the blending functions and not alpha/color update.)

Here are some examples of how this renders (SMSSkyFifologs.zip):

SMSSky.dff

00000000_2023-01-04_19-42-02
Clouds (objects 0-30)

00000000_2023-01-04_19-42-08
Sky only (object 31)

00000000_2023-01-04_19-42-10
Sky with clouds (objects 0-31)

00000000_2023-01-04_19-42-14
Full scene

00000000_2023-01-04_19-46-26
Broken version of scene

SMSTitle.dff

00000000_2023-01-04_19-42-47
Clouds (objects 0-30)

00000000_2023-01-04_19-42-51
Sky only (object 31)

00000000_2023-01-04_19-42-55
Sky with clouds (objects 0-31)

00000000_2023-01-04_19-43-09
Full scene

00000000_2023-01-04_19-45-35
Broken version of scene

MarioSunshineWater.dff (sms-water on fifoci)

00000000_2023-01-04_19-41-00
Clouds (objects 0-28)

00000000_2023-01-04_19-41-05
Sky only (object 29)

00000000_2023-01-04_19-41-08
Sky with clouds (objects 0-29)

00000000_2023-01-04_19-41-15
Full scene

00000000_2023-01-04_19-45-50
Broken version of scene

@Pokechu22
Copy link
Contributor Author

Not sure what's happening with smg-mmg on osx. I do see that they're using an RGBA6666 framebuffer and drawing the shadows in the alpha channel, and then they do something complicated to actually draw the shadows.

I did a quick investigation into how the shadows work. First, with depth testing enabled, they draw the shadow texture across all possible floor triangles, setting alpha to 0xff where the shadow can exist and to 0 where it would be blocked by the depth test (I think). The step that actually draws the shadows involves setting the dest factor to 1-dst_alpha and the source factor to dst_alpha. The TEV is configured to output the shadow texture's intensity to the alpha channel and 0 to the rgb channels; the shadow texture has a higher value for parts in shadow and is black on non-shadowed parts. The alpha test is set so that if the TEV outputs a pixel with an alpha of 0, it is discarded. Depth testing is disabled at this point. Then for each triangle that might have a shadow cast on it:

  1. They disable color write (but leave alpha write enabled)
  2. They disable BPMEM_CONSTANTALPHA
  3. They draw the triangle. For sections where the shadow was present, and the first step allowed it (presumably a simulation of the depth test), this replaces the EFB's alpha channel with the shadow (inverted, so the parts in shadow are black (I assume the shadow never is fully white originally, so this will never write an alpha of 0)).
  4. They enable color write (and leave alpha write enabled).
  5. They enable BPMEM_CONSTANTALPHA with a value of 0. This means that 0 is always written to the EFB's alpha channel (and Dolphin changes the alpha blend mode in this case to source factor 1, dest factor 0).
  6. They draw an 8-vertex triangle strip where all elements are 0. I think this is just writing some data to force a pipeline flush, but I'm not 100% sure.
  7. They draw the same triangle. This ends up multiplying the RGB value of the EFB by alpha value in the EFB, which is the inverted shadow value, so it darkens the ground where the shadow exists. It also sets the alpha value in the EFB to 0. (The source factor in the blend is irrelevant since the TEV outputs all black.)

That said, the difference seems to be a change of 1 on the red and blue channels on one pixel and on the green channel on another. This probably isn't that important, but it might be a good idea to do some further testing of the shadows in Mario Galaxy on an apple machine (which I don't have access to).

@Pokechu22
Copy link
Contributor Author

Pokechu22 commented Jan 7, 2023

Based on #11412 it doesn't seem like the differences on M1 are it switching between the fbfetch blend path and the regular blend path (fbfetch blend is used on one of the crystals on M1, but not on the shadow). Theoretically that could happen because RequiresDualSrc would behave differently if srcfactoralpha got cleared because alpha update is disabled so the fallback would be used in different cases.

So, I'm not sure where the M1 difference is coming from, and if it's just another driver quirk or something else.

That said... a difference of 1 is a problem, as the whole reason why we use dual source blend is (to my understanding) to emulate the quantization that happens when writing to an RGBA6666 framebuffer (and more specifically the blend factor not being quantized). If a difference of 1 can exist, something's wrong with that quantization. (I'm pretty sure that fifoci isn't using force 24 bit color). But I suspect that this is a more general issue and not a reason to hold this PR back.


EDIT: I read a bit more, and according to the old post about D3D9 removal dual source blending is mainly needed for BPMEM_CONSTANTALPHA instead. Which makes sense, as that feature is about using a potentially radically different alpha value for blending and writing into the framebuffer. The quantization behavior is a separate thing.

@TellowKrinkle
Copy link
Contributor

I went to look at what the difference might be from, but I couldn't reproduce it on my M1 MacBook Pro (before and after were identical on both Metal and MVK)

Though my computer is outputting the frames at 1280x720 instead of the CI's 808x456, so I can't compare to the CI's results to see which matches closer

@Pokechu22
Copy link
Contributor Author

Try enabling graphics -> advanced -> frame dumping -> dump at internal resolution (which also affects screenshots). You'll also want to use native/1x internal resolution. Though I'd expect the difference to be visible without those settings too.


Intel confirmed the bug over at IGCIT/Intel-GPU-Community-Issue-Tracker-IGCIT#214 but it'll be a few months before it'll actually be fixed (and included in a release).

@TellowKrinkle
Copy link
Contributor

Okay, ran with internal resolution dumping and still no difference between before and after, though there's a pretty big difference between my local test and the CI. Maybe from different OS versions or something?

Comparison of locally run test to CI image

Local:
mtl-after-1

Difference vs CI:
diff

@Pokechu22
Copy link
Contributor Author

Probably it's fine to not worry about the difference, then.

This works around an Intel driver bug where, on D3D12 only, dual-source blending behaves incorrectly if the second source is unused on. This bug is visible in skyboxes in Super Mario Sunshine, which first draw clouds and sun flare in greyscale and then draw the sky afterwards with a source factor of 1 and a dest factor of 1-src_color (this results in the clouds being tinted blue). This process is done on an RGB888 framebuffer, so alpha update is disabled. (Color update is enabled; note that if you look at this in Dolphin's fifo analyzer, it won't be enabled because they use the BP mask functionality to only change the blending functions and not alpha/color update, for whatever reason.)
@phire phire merged commit ba3c38a into dolphin-emu:master Feb 14, 2023
14 checks passed
@dolphin-emu-bot
Copy link
Contributor

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

  • smg-mmg on mvk-osx-m1: diff
  • smg-mmg on mtl-osx-m1: diff

automated-fifoci-reporter

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
5 participants