Skip to content

Give MS priority over LOD in Image opcodes#4207

Merged
georgemoralis merged 1 commit intoshadps4-emu:mainfrom
mikusp:lod_msaa
Apr 7, 2026
Merged

Give MS priority over LOD in Image opcodes#4207
georgemoralis merged 1 commit intoshadps4-emu:mainfrom
mikusp:lod_msaa

Conversation

@mikusp
Copy link
Copy Markdown
Contributor

@mikusp mikusp commented Apr 1, 2026

A draft as I'm not sure it is sound, but I'd like to get CTR ingame. One shader there hits ASSERT(!inst_info.has_lod || !has_ms);, and looking at the offending shader, it does image_load_mip with T# describing a Color2DMsaa image. That looks incorrect, but I'm not sure if the cause is an issue with shad or the original hardware ignores the _mip part and goes on.

With this change, the game progresses further and can get into a race with graphical glitches

@squidbus
Copy link
Copy Markdown
Collaborator

squidbus commented Apr 2, 2026

Do you have any info on where the mip level for the instruction comes from in the shader? If it's always 0 maybe we can detect it and it should work as if it's a regular image load?

@StevenMiller123
Copy link
Copy Markdown
Collaborator

Where would that be, given this image_load_mip?

/*00000000006c*/ image_load_mip  v[0:3], v[0:3], s[0:7] dmask:15

@StevenMiller123
Copy link
Copy Markdown
Collaborator

StevenMiller123 commented Apr 2, 2026

For what it's worth, v2 is a hardcoded 0 (v_mov_b32 v2, 0), everything else has some calculation involved.

@squidbus
Copy link
Copy Markdown
Collaborator

squidbus commented Apr 2, 2026

I believe it would be v2 in that case, yes. So I believe a solution would be to walk back through the arg to its source and detect the constant 0, and then use that to ignore has_lod (if both has_lod and has_ms are true).

Although personally I'm not sure if we need to bother checking or we should just ignore has_lod if has_ms is true, and just assume that its the 0 case. Basically I think what this PR is doing should be a fine place to start if you don't want to bother with that method, although I think the warning message could be simplified a bit, just need to indicate that we hit a MIP instruction with a multisampled texture and that we're assuming the mip level is 0. As far as I know you can't have multiple levels on a multisampled texture anyway, although I don't know how hardware would behave if someone actually tried.

@mikusp
Copy link
Copy Markdown
Contributor Author

mikusp commented Apr 2, 2026

I see that indeed, arg.IsImmediate() there returns true and the value is 0.

You mention that the check is not essential there, however, if we ignore has_lod, the argument passed for mipid would be interpreted as fragid according to the current code and Table 8.8 in AMD Sea Islands Instruction Set Architecture document. If that is fine I can simplify the warning message and mark the PR as ready for review

@squidbus
Copy link
Copy Markdown
Collaborator

squidbus commented Apr 2, 2026

That's true, it may be worth checking out the v3 value here as well and seeing if somehow we have x, y, mipid, fragid or something.

@mikusp
Copy link
Copy Markdown
Contributor Author

mikusp commented Apr 2, 2026

Yes, v3 is also set but the value is not an immediate there

/*000000000014: 4a06006a         */ v_add_i32       v3, vcc, vcc_lo, v0

@squidbus
Copy link
Copy Markdown
Collaborator

squidbus commented Apr 2, 2026

Doesn't need to be an immediate. I think the logic would be, if has_lod && has_mip and the mipid is an immediate 0, ignore the mipid and read fragid from the next one. If mipid is not immediate 0, we have a problem.

@mikusp mikusp force-pushed the lod_msaa branch 2 times, most recently from f00f216 to 1abb11f Compare April 2, 2026 18:33
@mikusp mikusp marked this pull request as ready for review April 6, 2026 21:29
@georgemoralis georgemoralis merged commit e64f038 into shadps4-emu:main Apr 7, 2026
11 checks passed
marecl pushed a commit to marecl/shadPS4 that referenced this pull request Apr 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants