VideoCommon: Fix direct normal+tangent+binormal with index3 set

[Pokechu22]

Fixes https://bugs.dolphin-emu.org/issues/12952. I haven't tested the ARM change at all other than verifying that it compiles (so it's probably completely broken), but I have tested the x64 change.

[Pokechu22]

JMC's testing indicates that this works correctly on android, too (quake GX is fixed, while rs2.dff and RogueSquadron3Bumpmapping.dff both render correctly).

[JMC47]

I don't think there's any other games that uses this, unfortunately, not even the Rogue Squadron games.

[JMC47]

Tested and verified to work on both Android and Desktop

[JosJuice]

AArch64 LGTM other than above nits.

[dolphin-emu-bot]

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

• quake-gx on ogl-lin-mesa: diff
• quake-gx on sw-lin-mesa: diff
• quake-gx on ogl-lin-radeon: diff
• quake-gx on uberogl-lin-radeon: diff

_{^{automated-fifoci-reporter}}

[AdmiralCurtiss]

#11058 (comment) looks more likely related to this PR.

[Pokechu22]

That does look like the assertion is being actually violated, so there probably was a bug beforehand that's only being exposed now, but without a stacktrace I can't tell where that number comes from.

[dvessel]

but without a stacktrace I can't tell where that number comes from.

Will this do?

31:53:683 Common/Arm64Emitter.cpp:537 E[JIT]: Warning: An error occurred.

64-bit load/store must use aligned offset: 1892

  Condition: (imm & 0x7) == 0
  File: /Users/foo/Projects/mac/dolphin/Source/Core/Common/Arm64Emitter.cpp
  Line: 537
  Function: EncodeLoadStoreIndexedInst

Ignore and continue?
Process 31192 stopped
* thread #21, name = 'CPU-GPU thread', stop reason = EXC_BREAKPOINT (code=1, subcode=0x101ab59d8)
    frame #0: 0x0000000101ab59d8 Dolphin`Arm64Gen::ARM64XEmitter::EncodeLoadStoreIndexedInst(this=0x0000000128cc8000, op=996, Rt=X25, Rn=X29, imm=1892, size='@') at Arm64Emitter.cpp:537:5
   534
   535 	  if (size == 64)
   536 	  {
-> 537 	    ASSERT_MSG(DYNA_REC, (imm & 0x7) == 0, "64-bit load/store must use aligned offset: {}", imm);
   538 	    imm >>= 3;
   539 	  }
   540 	  else if (size == 32)
Target 0: (Dolphin) stopped.
(lldb) bt
* thread #21, name = 'CPU-GPU thread', stop reason = EXC_BREAKPOINT (code=1, subcode=0x101ab59d8)
  * frame #0: 0x0000000101ab59d8 Dolphin`Arm64Gen::ARM64XEmitter::EncodeLoadStoreIndexedInst(this=0x0000000128cc8000, op=996, Rt=X25, Rn=X29, imm=1892, size='@') at Arm64Emitter.cpp:537:5
    frame #1: 0x0000000101abb6e8 Dolphin`Arm64Gen::ARM64XEmitter::STR(this=0x0000000128cc8000, type=Unsigned, Rt=X25, Rn=X29, imm=1892) at Arm64Emitter.cpp:1681:5
    frame #2: 0x0000000100ba5ab8 Dolphin`JitArm64::mfspr(this=0x0000000128cc8000, inst=UGeckoInstruction @ 0x000000015f7ff8ec) at JitArm64_SystemRegisters.cpp:338:5
    frame #3: 0x0000000100ba61f4 Dolphin`JitArm64::mftb(this=0x0000000128cc8000, inst=UGeckoInstruction @ 0x000000015f7ff92c) at JitArm64_SystemRegisters.cpp:408:3
    frame #4: 0x0000000100baabb8 Dolphin`JitArm64::DynaRunTable31(this=0x0000000128cc8000, inst=UGeckoInstruction @ 0x000000015f7ff97c) at JitArm64_Tables.cpp:478:3
    frame #5: 0x0000000100baadd0 Dolphin`JitArm64::CompileInstruction(this=0x0000000128cc8000, op=0x0000000128d500c0) at JitArm64_Tables.cpp:493:3
    frame #6: 0x0000000100b668c8 Dolphin`JitArm64::DoJit(this=0x0000000128cc8000, em_address=2148005684, b=0x0000600003945af8, nextPC=2148005648) at Jit.cpp:961:7
    frame #7: 0x0000000100b6531c Dolphin`JitArm64::Jit(this=0x0000000128cc8000, em_address=2148005684, clear_cache_and_retry_on_failure=true) at Jit.cpp:684:9
    frame #8: 0x0000000100b64f5c Dolphin`JitArm64::Jit(this=0x0000000128cc8000, em_address=2148005684) at Jit.cpp:616:3
    frame #9: 0x0000000100bb5e88 Dolphin`JitTrampoline(jit=0x0000000128cc8000, em_address=2148005684) at JitBase.cpp:22:7
    frame #10: 0x0000000172a090b4

[JosJuice]

I'll take on this issue.

[shuffle2]

fwiw this PR seems to have broken nightly-generic buildbot https://dolphin.ci/#/builders/23

[delroth]

More accurately, AFAICT, this PR is missing a generic implementation of the fix it implemented (and added tests for). The newly broken tests just reflect the fact that generic is missing the bug fix.

[Pokechu22]

That's weird, as I locally tested it against the generic/non-JIT/software vertex loader, as it was initially not broken.

It looks like what happened is that I tested via defining #define COMPARE_VERTEXLOADERS in VertexLoaderBase, but if the two vertex loaders generate different data, it uses the JIT vertex loader and the error messages don't end up in the test output.