Resident Evil 2 - Dual Shock Ver. (USA) | Cosmetic Issue (Box)

[d3adsy]

This is a cosmetic issue only, gameplay is unaffected.

When traversing the hallway beyond the first licker encounter, there is a nearly translucent but visible rectangle present in the center of the screen. The rectangle appears on pre-rendered backgrounds where the player is below the fluorescent lighting.

Example(s):

The same issue occurred in prior versions of Duckstation (unsure if fixed, taken from a YouTube playthrough):

Expected Result: Captured from original PS1 hardware

Save state of the location where the cosmetic issue is present. Resident Evil 2 - Dual Shock Ver. (USA) (Disc 1) (Leon)_1.zip

[RobertPeip]

Note on how to possibly fit it:

• capture all mdec commands executed for decoding this area in a emulator
• write a testrom that will send these to the mdec and execute it on a real psx and emulator
• compare pixel by pixel from the mdec response
• probably first pixel of each mdec block is enough as the difference covers a full block area anyway

[Kuba-J]

for comparison Resident Evil 1

[vanfanel]

Could this be related to the different known MDEC decoding algorithms? Apparently, Swanstation supports two decoding routines, which make the games look different: https://github.com/libretro/swanstation/commit/834a6f12413dfa7c58675ad54cad522cd0438663

[RobertPeip]

possible fix in the changes of this commit from Duckstation: https://github.com/stenzek/duckstation/commit/6d6659c85ea83b33b21d573083f6872fa61b0896

However, it's most likely not the IDCT precision as mentioned in the commit text, as the core already works in this stage with even higher precision, but maybe in the stages after that. They are also modified but not mentioned in the commit text what is modified, so needs to be investigated.

[vanfanel]

@RobertPeip Thanks for looking into this again, Robert! I hope this is the key to finally have these backgrounds corrected :)

[paulb-nl]

It would help to have a digital capture of those screens from a PS1Digital. Pixel perfect so integer scaling without interpolation or gamma correction.

The inside of the shop as shown in #295 would also be good because that one is easy to get to and I have been working with that one already.

[paulb-nl]

Adding rounding up to the 15 bit output from MDEC makes a big difference. The backgrounds look more accurate and the rectangles are gone. I tried it with below changes. I don't know if it can be done in a cleaner way in VHDL.

diff --git a/rtl/mdec.vhd b/rtl/mdec.vhd
index d2dc274..7921a75 100644
--- a/rtl/mdec.vhd
+++ b/rtl/mdec.vhd
@@ -1196,9 +1196,21 @@ begin
                when COLORMAP_16_1 =>
                   if (color_readValid = '1') then
                      colormapState <= COLORMAP_16_2;
-                     FifoOut_Din( 4 downto  0) <= color_readData(7 downto 3);
-                     FifoOut_Din( 9 downto  5) <= color_readData(15 downto 11);
-                     FifoOut_Din(14 downto 10) <= color_readData(23 downto 19);
+                     if (color_readData(7 downto 3) /= "11111" and color_readData(2) = '1') then
+                        FifoOut_Din( 4 downto  0) <= std_logic_vector(unsigned(color_readData(7 downto 3)) + 1);
+                     else
+                        FifoOut_Din( 4 downto  0) <= color_readData(7 downto 3);
+                     end if;
+                     if (color_readData(15 downto 11) /= "11111" and color_readData(10) = '1') then
+                        FifoOut_Din( 9 downto  5) <= std_logic_vector(unsigned(color_readData(15 downto 11)) + 1);
+                     else
+                        FifoOut_Din( 9 downto  5) <= color_readData(15 downto 11);
+                     end if;
+                     if (color_readData(23 downto 19) /= "11111" and color_readData(18) = '1') then
+                        FifoOut_Din(14 downto 10) <= std_logic_vector(unsigned(color_readData(23 downto 19)) + 1);
+                     else
+                        FifoOut_Din(14 downto 10) <= color_readData(23 downto 19);
+                     end if;
                      FifoOut_Din(15) <= rec_bit15;
                   end if;

@@ -1206,9 +1218,21 @@ begin
                   if (color_readValid = '1') then
                      colormapState <= COLORMAP_16_1;
                      FifoOut_Wr    <= '1';
-                     FifoOut_Din(20 downto 16) <= color_readData(7 downto 3);
-                     FifoOut_Din(25 downto 21) <= color_readData(15 downto 11);
-                     FifoOut_Din(30 downto 26) <= color_readData(23 downto 19);
+                     if (color_readData(7 downto 3) /= "11111" and color_readData(2) = '1') then
+                        FifoOut_Din( 20 downto  16) <= std_logic_vector(unsigned(color_readData(7 downto 3)) + 1);
+                     else
+                        FifoOut_Din( 20 downto  16) <= color_readData(7 downto 3);
+                     end if;
+                     if (color_readData(15 downto 11) /= "11111" and color_readData(10) = '1') then
+                        FifoOut_Din( 25 downto  21) <= std_logic_vector(unsigned(color_readData(15 downto 11)) + 1);
+                     else
+                        FifoOut_Din( 25 downto  21) <= color_readData(15 downto 11);
+                     end if;
+                     if (color_readData(23 downto 19) /= "11111" and color_readData(18) = '1') then
+                        FifoOut_Din(30 downto 26) <= std_logic_vector(unsigned(color_readData(23 downto 19)) + 1);
+                     else
+                        FifoOut_Din(30 downto 26) <= color_readData(23 downto 19);
+                     end if;
                      FifoOut_Din(31) <= rec_bit15;
                   end if;

[Kuba-J]

Great work!

[RobertPeip]

Thank you so much for finding this.

The calculation could be combined in a intermediate value, so the if/else are not duplicated. Either as R/G/B signals outside the process or as variables inside. I can do that also if you want.

But overall it's not that important, way more important is that you found something :)

[birdybro]

Could do a function for the if else /= 11111 part and just call it inline to the assignments.

[Kuba-J]

huge difference

old core new core

[paulb-nl]

The calculation could be combined in a intermediate value, so the if/else are not duplicated. Either as R/G/B signals outside the process or as variables inside. I can do that also if you want.

Please do. I am always fighting with VHDL. :smile:

But overall it's not that important, way more important is that you found something :)

I tried other things like implementing RLE and IDCT from the hardware RE described here https://www.psxdev.net/forum/viewtopic.php?f=70&t=551

12 bit RLE output with 17 bit IDCT sum and 13 bit temp result, adjusted the YUV to RGB coefficients but these only resulted in small changes.

I started to use a capture from PS1 as reference. These pixels you can say look like an animal with 2 eyes. With Duckstation the 2 eyes are missing. It is from the top left of the image here: https://github.com/MiSTer-devel/PSX_MiSTer/issues/295#issuecomment-2316063207

[RobertPeip]

This should behave the same:

diff --git "a/rtl/mdec.vhd" "b/rtl/mdec.vhd"
index d2dc274..b07debe 100644
--- "a/rtl/mdec.vhd"
+++ "b/rtl/mdec.vhd"
@@ -1128,6 +1128,9 @@ begin

    -- Output
    process (clk2x)
+      variable color_round16_r : std_logic_vector(4 downto 0);
+      variable color_round16_g : std_logic_vector(4 downto 0);
+      variable color_round16_b : std_logic_vector(4 downto 0);
    begin
       if rising_edge(clk2x) then

@@ -1193,23 +1196,39 @@ begin
                when COLORMAP_8_4 => if (color_readValid = '1') then colormapState <= COLORMAP_8_1; FifoOut_Din(31 downto 24) <= color_readData(7 downto 0); FifoOut_Wr <= '1'; end if;

                -- 16 bit
-               when COLORMAP_16_1 =>
+               when COLORMAP_16_1 | COLORMAP_16_2 =>
                   if (color_readValid = '1') then
-                     colormapState <= COLORMAP_16_2;
-                     FifoOut_Din( 4 downto  0) <= color_readData(7 downto 3);
-                     FifoOut_Din( 9 downto  5) <= color_readData(15 downto 11);
-                     FifoOut_Din(14 downto 10) <= color_readData(23 downto 19);
-                     FifoOut_Din(15) <= rec_bit15;
-                  end if;
-                  
-               when COLORMAP_16_2 =>
-                  if (color_readValid = '1') then
-                     colormapState <= COLORMAP_16_1;
-                     FifoOut_Wr    <= '1';
-                     FifoOut_Din(20 downto 16) <= color_readData(7 downto 3);
-                     FifoOut_Din(25 downto 21) <= color_readData(15 downto 11);
-                     FifoOut_Din(30 downto 26) <= color_readData(23 downto 19);
-                     FifoOut_Din(31) <= rec_bit15;
+                     
+                     if (color_readData(7 downto 3) /= "11111" and color_readData(2) = '1') then
+                        color_round16_r := std_logic_vector(unsigned(color_readData(7 downto 3)) + 1);
+                     else
+                        color_round16_r := color_readData(7 downto 3);
+                     end if;
+                     if (color_readData(15 downto 11) /= "11111" and color_readData(10) = '1') then
+                        color_round16_g := std_logic_vector(unsigned(color_readData(15 downto 11)) + 1);
+                     else
+                        color_round16_g := color_readData(15 downto 11);
+                     end if;
+                     if (color_readData(23 downto 19) /= "11111" and color_readData(18) = '1') then
+                        color_round16_b := std_logic_vector(unsigned(color_readData(23 downto 19)) + 1);
+                     else
+                        color_round16_b := color_readData(23 downto 19);
+                     end if;
+                     
+                     if (colormapState = COLORMAP_16_1) then
+                        colormapState <= COLORMAP_16_2;
+                        FifoOut_Din( 4 downto  0) <= color_round16_r;
+                        FifoOut_Din( 9 downto  5) <= color_round16_g;
+                        FifoOut_Din(14 downto 10) <= color_round16_b;
+                        FifoOut_Din(15) <= rec_bit15;
+                     else
+                        colormapState <= COLORMAP_16_1;
+                        FifoOut_Wr    <= '1';
+                        FifoOut_Din(20 downto 16) <= color_round16_r;
+                        FifoOut_Din(25 downto 21) <= color_round16_g;
+                        FifoOut_Din(30 downto 26) <= color_round16_b;
+                        FifoOut_Din(31) <= rec_bit15;
+                     end if;
                   end if;

                -- 24 bit

[birdybro]

Would something like this work as well?

function calculate_rounded_color(data : std_logic_vector(4 downto 0); valid_bit : std_logic) return std_logic_vector is
begin
    if (data /= "11111" and valid_bit = '1') then
        return std_logic_vector(unsigned(data) + 1);
    else
        return data;
    end if;
end function;

...

when COLORMAP_16_1 | COLORMAP_16_2 =>
    if (color_readValid = '1') then
        -- Calculate rounded color values using the function
        color_round16_r := calculate_rounded_color(color_readData( 7 downto  3), color_readData( 2));
        color_round16_g := calculate_rounded_color(color_readData(15 downto 11), color_readData(10));
        color_round16_b := calculate_rounded_color(color_readData(23 downto 19), color_readData(18));

        if (colormapState = COLORMAP_16_1) then
            colormapState <= COLORMAP_16_2;
            FifoOut_Din( 4 downto  0) <= color_round16_r;
            FifoOut_Din( 9 downto  5) <= color_round16_g;
            FifoOut_Din(14 downto 10) <= color_round16_b;
            FifoOut_Din(15)           <= rec_bit15;
        else
            colormapState <= COLORMAP_16_1;
            FifoOut_Wr <= '1';
            FifoOut_Din(20 downto 16) <= color_round16_r;
            FifoOut_Din(25 downto 21) <= color_round16_g;
            FifoOut_Din(30 downto 26) <= color_round16_b;
            FifoOut_Din(31)           <= rec_bit15;
        end if;
    end if;

I'm not as familiar with functions in vhdl so maybe the syntax is wrong, just curious.

[RobertPeip]

Yes, this should also work and in general it makes sense to use functions. It's just sometimes harder to understand the signal flow when they are used.

But in this case here, it's totally fine. Paul can choose whichever solutions is prefered for a pull request :)