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

Sun lens flare in Wipeout Pure flickers since #13327 #13344

Closed
hrydgard opened this issue Aug 28, 2020 · 10 comments
Closed

Sun lens flare in Wipeout Pure flickers since #13327 #13344

hrydgard opened this issue Aug 28, 2020 · 10 comments
Labels
GE emulation Backend-independent GPU issues
Milestone

Comments

@hrydgard
Copy link
Owner

This is happening since #13327, more specifically 5ade93a.

The previous behavior of the sun always being visible is almost certainly wrong, but so is this flicker. We might have to get to the bottom of this. I don't know if it's doing depth texturing, or trying to read the depth map with the CPU, or what.

Quick video clip:
wipeoutflicker.zip

@hrydgard hrydgard added the GE emulation Backend-independent GPU issues label Aug 28, 2020
@hrydgard hrydgard added this to the v1.11.0 milestone Aug 28, 2020
@hrydgard
Copy link
Owner Author

Confirmed that on the real PSP, the lens flare effect is turned off when the sun is behind something. In PPSSPP now though, it seems entirely random.

@hrydgard
Copy link
Owner Author

The minimal change that restores the previous, also wrong but more tolerable, behavior, is:

Line 473 in SoftwareTransformCommon.cpp:

-               bool writingColor = gstate.getColorMask() != 0xFFFFFFFF;
+               bool writingColor = gstate.getColorMask() != 0;

Given how the PSP's masks work, the original code (before the "culprit" change) is quite obviously wrong indeed. Question is what to do about this - will depend on how the game is actually accomplishing the lens flare effect, which I haven't looked into much yet.

@unknownbrackets
Copy link
Collaborator

I still haven't looked at this properly, but is the flicker ONLY when the sun shouldn't show at all, or does it now flicker at all times?

Perhaps it just indicates that we're only detecting one clear out of two frames, causing the flicker.

-[Unknown]

@hrydgard
Copy link
Owner Author

hrydgard commented Aug 30, 2020

It feels like it's trying to somehow read the depth buffer or similar at the sun position to find out when the lens flare should be drawn, but it's reading from a texture full of garbage. It's not every second frame or anything like that - if the camera is still, it doesn't flicker.

@hrydgard
Copy link
Owner Author

hrydgard commented Dec 20, 2020

Experimenting a bit with the change, it's clear that it's actually reading the color buffer using the CPU to figure out whether to draw the lens flare or not. Probably checking alpha/stencil. So if we read back the color buffer every frame into PSP VRAM, the lens flare would be correct. If we simply turn off the first-frame readback of color buffers (which that safesize thing is about), we get back the old behavior which is that it reads from a buffer of zeroes and apparently always shows the lensflare, which look much better than the flickering.

So I'm thinking about just doing a compat.ini setting, it's not worth inflicting a full frame color readback every frame just for the lens flare, even if it would be accurate, until I can implement the Vulkan-only async readbacks I've been planning for some time...

@hrydgard hrydgard modified the milestones: v1.11.0, v1.12.0 Dec 20, 2020
hrydgard added a commit that referenced this issue Dec 20, 2020
See #13344.

Will try to figure out something better after the next release...
@unknownbrackets
Copy link
Collaborator

Did #13941 improve this issue? Might have to remove from compat.ini to confirm.

-[Unknown]

@Panderner
Copy link
Contributor

Did #13941 improve this issue? Might have to remove from compat.ini to confirm.

-[Unknown]

No. #13941 won't help for this issue when first frame readback is enabled for this game the sun still flickers.

@hrydgard hrydgard modified the milestones: v1.12.0, Future Aug 29, 2021
@ghost
Copy link

ghost commented Oct 9, 2021

Sun lens flare still flickering, DisableFirstFrameReadback doesn't help.

@hrydgard hrydgard modified the milestones: Future, v1.14.0 Aug 22, 2022
@hrydgard
Copy link
Owner Author

hrydgard commented Aug 29, 2022

This will be worked on as part of #15923 .

Here's the disassembly of the function that reads a 4x4 pattern from the depth buffer:

z_un_0888c0a8:

	addiu	sp,sp,-0x20
	li	a1,0x200
	sw	s0,0x10(sp)
	sw	ra,0x14(sp)
	bne	a0,a1,pos_0888C1D0
	lui	s0,0x8AA
	lw	a3,0x5504(s0)
	beq	a3,zero,pos_0888C1D0
	nop	
	jal	z_un_0881e098
	move	a0,a3
	lw	a3,0x08AA550C
	lui	a0,0x8B2
	li	a1,0x3E8
	beq	a3,a1,pos_0888C108
	lw	a0,-0x182C(a0)
	addiu	a2,a3,-0x3               /// Guessing these two lines are about the 4x4 loop counters
	addiu	a3,a3,0x4
	slt	t0,a2,a3
	bne	t0,zero,pos_0888C120
	li	a1,0x0
	b	pos_0888C1A4
	lui	a2,0x8AA

pos_0888C108:

	mtc1	zero,f12
	swc1	f12,0x74(a0)
	lw	s0,0x10(sp)
	lw	ra,0x14(sp)
	jr	ra
	addiu	sp,sp,0x20

pos_0888C120:

	lw	s0,0x5504(s0)
	lw	t1,0x08AA5508
	sll	t0,a2,0x9
	addiu	t2,t1,0x4
	addiu	t1,t1,-0x3

pos_0888C138:

	bltz	a2,pos_0888C190
	slti	t3,a2,0x110
	beq	t3,zero,pos_0888C190
	move	t3,t1
	slt	v0,t3,t2
	beq	v0,zero,pos_0888C190
	addu	v0,t1,t0
	addu	v0,v0,v0
	addu	v0,s0,v0

pos_0888C15C:

	bltz	t3,pos_0888C180
	slti	v1,t3,0x1E0
	beql	v1,zero,pos_0888C184
	addiu	t3,t3,0x2
	lhu	v1,0x0(v0)                      <<<---- The depth buffer read
	sltiu	v1,v1,0x3B3
	beql	v1,zero,pos_0888C184
	addiu	t3,t3,0x2
	addiu	a1,a1,0x1

pos_0888C180:

	addiu	t3,t3,0x2

pos_0888C184:

	slt	v1,t3,t2
	bne	v1,zero,pos_0888C15C
	addiu	v0,v0,0x4

pos_0888C190:

	addiu	a2,a2,0x2
	slt	t3,a2,a3
	bne	t3,zero,pos_0888C138
	addiu	t0,t0,0x400
	lui	a2,0x8AA

pos_0888C1A4:

	mtc1	a1,f12
	sh	a1,0x5510(a2)
	bgez	a1,pos_0888C1C0
	cvt.s.w	f12,f12
	lui	a1,0x4F80
	mtc1	a1,f13
	add.s	f12,f12,f13

pos_0888C1C0:

	lui	a1,0x3D80
	mtc1	a1,f13
	mul.s	f12,f12,f13              // Multiply by 0.0625 . That's 1/16th, so taking the average of the 16 samples
	swc1	f12,0x74(a0)

pos_0888C1D0:

	lw	s0,0x10(sp)
	lw	ra,0x14(sp)
	jr	ra
	addiu	sp,sp,0x20

@hrydgard hrydgard modified the milestones: v1.14.0, Future-Prio Sep 18, 2022
@hrydgard
Copy link
Owner Author

This is solved now since #19759 with depth buffer software rasterization, although at a performance cost. This cost will be minimized before the next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GE emulation Backend-independent GPU issues
Projects
None yet
Development

No branches or pull requests

3 participants