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

Wrong\missing chinese font in windows since v1.11 #14144

Open
sum2012 opened this issue Feb 15, 2021 · 35 comments
Open

Wrong\missing chinese font in windows since v1.11 #14144

sum2012 opened this issue Feb 15, 2021 · 35 comments
Labels
GE emulation Backend-independent GPU issues
Milestone

Comments

@sum2012
Copy link
Collaborator

sum2012 commented Feb 15, 2021

Only happened on my chinese freind.
Last work v1.10
First bad v1.11
I will get more information
1
2
3

@sum2012 sum2012 added this to the v1.12.0 milestone Feb 15, 2021
@unknownbrackets
Copy link
Collaborator

All backends? I recall this area in Gran Turismo having problems before and requiring vertex cache off or some flushing bug fix...

-[Unknown]

@sum2012
Copy link
Collaborator Author

sum2012 commented Feb 15, 2021

Reproduce with this setting
1
2
3
4

@sum2012
Copy link
Collaborator Author

sum2012 commented Feb 15, 2021

That is Vertex Cache and Lazy texture caching
Can we close issue ?

@hrydgard
Copy link
Owner

Lazy texture caching does break the text in this game, I don't think it's unique to Chinese? It's the same in English, right?

@hrydgard hrydgard added the GE emulation Backend-independent GPU issues label Feb 15, 2021
@sum2012
Copy link
Collaborator Author

sum2012 commented Feb 15, 2021

It is interesting that don't effect english with same setting with chinese.
1
2

@sum2012
Copy link
Collaborator Author

sum2012 commented Feb 15, 2021

It is effect all render engine,opengl,dx9,dx11 ,vulkan

@sum2012
Copy link
Collaborator Author

sum2012 commented Feb 15, 2021

On hand build test
last good v1.10.3-712-g7ed1ade56
first bad v1.10.3-769-g350cb11f9

@nassau-tk
Copy link
Contributor

Of course, This issue happen on Japanese.(It use jpn0.pgf same.)

And, Korean (kr0.pgf) too.
#13190 (comment)

@sum2012
Copy link
Collaborator Author

sum2012 commented Feb 15, 2021

Thanks @nassau-tk information
Last good v1.10.3-741-g7ed1ade56
First bad v1.10.3-747-g350cb11f9

@hrydgard
Copy link
Owner

It's a known issue with "lazy texture caching" setting. Should be fine if you turn it off, no?

@sum2012
Copy link
Collaborator Author

sum2012 commented Feb 15, 2021

Yes, it is fine if I turn it off.
Git bisect result:
3e3a40d in #13466

@sum2012
Copy link
Collaborator Author

sum2012 commented Feb 18, 2021

many chinese people are starting complain text adventures 's game font
Screenshot_2021-02-19-07-58-21-300_com baidu tieba

@unknownbrackets
Copy link
Collaborator

Maybe Chinese game patches use kernel memory, and we don't rehash those textures?

-[Unknown]

@hrydgard
Copy link
Owner

Hm! Yeah, in that case we could make the check more specific, to ask ppge where it put the font texture and only ignore hashing that one...

@hrydgard
Copy link
Owner

That should hopefully fix it.

@sum2012
Copy link
Collaborator Author

sum2012 commented Feb 19, 2021

v1.11.2-127-g2ac66446a still don't fix

@hrydgard
Copy link
Owner

Hm, thanks for reporting. Really thought that would do it.... I don't see how else 3e3a40d could have broken it, unless the Chinese patches uses exactly that same texture address for some reason...

@sum2012
Copy link
Collaborator Author

sum2012 commented Feb 19, 2021

Gran Turismo is offical chinese (asia version,have chinese and english)

@hrydgard
Copy link
Owner

Oh, okay. Well, even stranger then.

@sum2012
Copy link
Collaborator Author

sum2012 commented Feb 19, 2021

Not sure frame dump would useful
recording.zip

@hrydgard
Copy link
Owner

Hm, didn't we fix this, or at least it works if Lazy texture cache is disabled?

@hrydgard hrydgard modified the milestones: v1.12.0, Future-Prio Aug 29, 2021
@unknownbrackets
Copy link
Collaborator

I think maybe yes - there may have been multiple issues here. It seems like there were more issues (which we may have fixed) after v1.11, but I'm not sure if Gran Turismo specifically ever worked with lazy texture caching before. It makes sense for it to break text rendering like that...

Maybe we could make lazy texture caching smarter, if it's doing letter by letter. I think these textures are small, maybe lazy or not it should still hash small textures, likely to be cheap and more likely to change...

-[Unknown]

@sum2012
Copy link
Collaborator Author

sum2012 commented Sep 5, 2022

v1.13.1-745-ga42807ea6 still issue with "lazy texture caching" setting

@hrydgard
Copy link
Owner

hrydgard commented Sep 5, 2022

yeah, well, that's a speed hack so...

I'm gonna reorganize the settings soon so it's obvious which ones are "fast but dangerous" and which ones are more about quality and so on.

@unknownbrackets
Copy link
Collaborator

Hm, normally textureCache_->Invalidate(dstBasePtr + (dstY * dstStride + dstX) * bpp, height * dstStride * bpp, GPU_INVALIDATE_HINT); should be invalidating these for lazy texture hashing. Does it help to move that outside the if, above const uint32_t numBytes = width * height * bpp;?

-[Unknown]

@sum2012
Copy link
Collaborator Author

sum2012 commented Sep 5, 2022

@unknownbrackets no help

@sum2012
Copy link
Collaborator Author

sum2012 commented Sep 5, 2022

Using original code
textureCache_->Invalidate(dstBasePtr + (dstY * dstStride + dstX) * bpp, height * dstStride * bpp, GPU_INVALIDATE_HINT);
change to
textureCache_->Invalidate(dstBasePtr + (dstY * dstStride + dstX) * bpp, height * dstStride * bpp, GPU_INVALIDATE_ALL);

Fix the issue

@unknownbrackets
Copy link
Collaborator

Is that true even if you comment out these lines?

// This is an active signal from the game that something in the texture cache may have changed.
gstate_c.Dirty(DIRTY_TEXTURE_IMAGE);

Or is this what's causing that (try commenting it out)?

entry->invalidHint++;

GPU_INVALIDATE_ALL is meant to be used with a full invalidate call, not a targeted one. Normally it's gpu->InvalidateCache(0, -1, GPU_INVALIDATE_ALL);. If it's entry->invalidHint++;, maybe we can just add that above inside if (type != GPU_INVALIDATE_ALL) {... since framesUntilNextFullHash is only checked on a new frame. I guess that's likely the issue, and English was hitting gstate_c.Dirty(DIRTY_TEXTURE_IMAGE);.

-[Unknown]

@sum2012
Copy link
Collaborator Author

sum2012 commented Sep 6, 2022

This one make font worse

// This is an active signal from the game that something in the texture cache may have changed.
gstate_c.Dirty(DIRTY_TEXTURE_IMAGE);

1

second change (comment out)

entry->invalidHint++;

font still wrong, but have font flicking in a screen,

@unknownbrackets
Copy link
Collaborator

Thanks, interesting. So then it's the current texture, but it's not texture 0. What if you change this:

	if (type == GPU_INVALIDATE_ALL) {
		// This is an active signal from the game that something in the texture cache may have changed.
		gstate_c.Dirty(DIRTY_TEXTURE_IMAGE);
	} else {
		// Do a quick check to see if the current texture could potentially be in range.
		const u32 currentAddr = gstate.getTextureAddress(0);
		// TODO: This can be made tighter.
		if (addr_end >= currentAddr && addr < currentAddr + LARGEST_TEXTURE_SIZE) {
			gstate_c.Dirty(DIRTY_TEXTURE_IMAGE);
		}
	}

To:

	/*if (type == GPU_INVALIDATE_ALL) {
		// This is an active signal from the game that something in the texture cache may have changed.
		gstate_c.Dirty(DIRTY_TEXTURE_IMAGE);
	} else {*/
		// Do a quick check to see if the current texture could potentially be in range.
		const u32 currentAddr = gstate.getTextureAddress(0);
		// TODO: This can be made tighter.
		if (addr_end >= currentAddr && addr < currentAddr + LARGEST_TEXTURE_SIZE) {
			gstate_c.Dirty(DIRTY_TEXTURE_IMAGE);
		} else if (type == GPU_INVALIDATE_ALL) {
			WARN_LOG(HLE, "Invalidate for %08x size=%08x was outside texture at %08x (level1=%08x)", addr, size, currentAddr, gstate.getTextureAddress(1));
		}
	//}

What is it logging there? Just trying to see why it wouldn't dirty DIRTY_TEXTURE_IMAGE. We used to always dirty DIRTY_TEXTURE_IMAGE on block transfer iirc, but some games do block transfers a lot so we wanted to remove it.

-[Unknown]

@sum2012
Copy link
Collaborator Author

sum2012 commented Sep 6, 2022

PPSSPP do not hit the line of WARN_LOG(HLE, "Invalidate for %08x size=%08x was outside texture at %08x (level1=%08x)", addr, size, currentAddr, gstate.getTextureAddress(1));
edit : It is keeping my change #14144 (comment)
edit 2 : The font still good

@sum2012
Copy link
Collaborator Author

sum2012 commented Sep 6, 2022

textureCache_->Invalidate(dstBasePtr + (dstY * dstStride + dstX) * bpp, height * dstStride * bpp, GPU_INVALIDATE_ALL);
Change to GPU_INVALIDATE_FORCE also work

@unknownbrackets
Copy link
Collaborator

unknownbrackets commented Sep 6, 2022

I might be a bit confused.

	if (type == GPU_INVALIDATE_ALL) {
		// This is an active signal from the game that something in the texture cache may have changed.
		gstate_c.Dirty(DIRTY_TEXTURE_IMAGE);
	} else {
		// Do a quick check to see if the current texture could potentially be in range.
		const u32 currentAddr = gstate.getTextureAddress(0);
		// TODO: This can be made tighter.
		if (addr_end >= currentAddr && addr < currentAddr + LARGEST_TEXTURE_SIZE) {
			gstate_c.Dirty(DIRTY_TEXTURE_IMAGE);
		}
	}

If I understood correctly, commenting out the first part - gstate_c.Dirty(DIRTY_TEXTURE_IMAGE); in the ALL case - breaks it. So that is what is making it work, right?

I basically proposed changing it to:

		// For all types, including GPU_INVALIDATE_ALL
		const u32 currentAddr = gstate.getTextureAddress(0);
		// TODO: This can be made tighter.
		if (addr_end >= currentAddr && addr < currentAddr + LARGEST_TEXTURE_SIZE) {
			gstate_c.Dirty(DIRTY_TEXTURE_IMAGE);
		} else {
log goes here
		}

And it doesn't log? That's weird because if it's not going to the log part, it must be running gstate_c.Dirty(DIRTY_TEXTURE_IMAGE); which should be the part that makes it work.

If you delete that whole section and put gstate_c.Dirty(DIRTY_TEXTURE_IMAGE); instead, and then put return; after that, does it work?

-[Unknown]

@sum2012
Copy link
Collaborator Author

sum2012 commented Sep 7, 2022

I think that if (addr_end >= currentAddr && addr < currentAddr + LARGEST_TEXTURE_SIZE) is true
so that program do not go to else if part

	/*if (type == GPU_INVALIDATE_ALL) {
		// This is an active signal from the game that something in the texture cache may have changed.
		gstate_c.Dirty(DIRTY_TEXTURE_IMAGE);
	} else {*/
		// Do a quick check to see if the current texture could potentially be in range.
		const u32 currentAddr = gstate.getTextureAddress(0);
		// TODO: This can be made tighter.
		if (addr_end >= currentAddr && addr < currentAddr + LARGEST_TEXTURE_SIZE) {
			gstate_c.Dirty(DIRTY_TEXTURE_IMAGE);
		} else if (type == GPU_INVALIDATE_ALL) {
			WARN_LOG(HLE, "Invalidate for %08x size=%08x was outside texture at %08x (level1=%08x)", addr, size, currentAddr, gstate.getTextureAddress(1));
		}
	//}

Yes breaks it.
``c++
if (type == GPU_INVALIDATE_ALL) {
// This is an active signal from the game that something in the texture cache may have changed.
gstate_c.Dirty(DIRTY_TEXTURE_IMAGE);
} else {
// Do a quick check to see if the current texture could potentially be in range.
const u32 currentAddr = gstate.getTextureAddress(0);
// TODO: This can be made tighter.
if (addr_end >= currentAddr && addr < currentAddr + LARGEST_TEXTURE_SIZE) {
gstate_c.Dirty(DIRTY_TEXTURE_IMAGE);
}
}


If I understood correctly, commenting out the first part - `gstate_c.Dirty(DIRTY_TEXTURE_IMAGE);` in the ALL case - breaks it.  So that is what is making it work, right?

@sum2012
Copy link
Collaborator Author

sum2012 commented Sep 7, 2022

3:
Did you mean ?

void TextureCacheCommon::Invalidate(u32 addr, int size, GPUInvalidationType type) {
gstate_c.Dirty(DIRTY_TEXTURE_IMAGE);
return;
)

text bad , in a screen text flicking

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

4 participants