Implement scaled partial texture updates #2679

Tinob · 2015-06-30T01:21:37Z

No description provided.

JMC47 · 2015-06-30T02:21:04Z

Works as expected with no noticeable regressions.

You'll definitely need to squash the commits after things have been reviewed.

mimimi085181 · 2015-06-30T05:31:18Z

This is maybe a dumb question, but could you make it so, that it uses the texture pool for the new texture? I'm not sure if this is possible, and for new super mario bros it would only be a tiny boost for 1 frame when switching worlds, but still...

Also, what if this upscaling was imlemented for all normal textures? Would that properly upscale the clothes in the Last Story?

degasus · 2015-06-30T07:48:33Z

I think this function should be splitted from DoPartialTextureUpdate(), eg

void TextureCache::TCacheEntry::ReScale(const TCacheEntryBase& entry);

// Makes a scaled copy of old_entry. 
new_entry->ReScale(old_entry);

So we can reuse the allocator and move the rescale logic into videocommon.

Linktothepast · 2015-06-30T11:41:09Z

Tino could you remove the recent gameini change where scaled efb copies were disabled for nsmb with this pr? #2666
Testing wise seems fine with both backends.

Tinob · 2015-06-30T13:03:11Z

@mimimi085181: we can use the texture pool but it will complicate the code a little, but will not bring any noticeable speed improvements. I don't have the last history but are the clothes generated by efb copy?
@degasus: the scale math can be moved to videocommon easily, what is not easy to put in common code is the scaled copy code maybe we can separate the process, one function to make a new entry with the new size and then a new function to make the copy?
@Linktothepast: will rebase and modify the files when i get home tonight.

degasus · 2015-06-30T13:07:39Z

There is already a function to create a new texture:
https://github.com/dolphin-emu/dolphin/blob/master/Source/Core/VideoCommon/TextureCacheBase.h#L147
So yes, I've also thought about a new virtual function for the copy.

Tinob · 2015-06-30T13:18:15Z

:) yes i was thinking in that function to create the new texture, the only missing part then is the copy function, you think in something specific like a simple copy or something more generic with parameter so we can re use it?

degasus · 2015-06-30T13:20:42Z

Maybe also with both a source_rect and a dest_rect. So it can also be used for the partial texture update itself.

mimimi085181 · 2015-06-30T15:08:22Z

The clothes in the Last Story are a story of its own. Yes, they are generated via efb copies, and those efb copies are not used as textures in Dolphin. And trying the same as for new super mario bros, but with efb copies are the target/big texture didn't work either. At least i couldn't get it to work.

So if normal textures could just be upscaled as well, i think that might work for the clothes in the Last Story. And it might upscale the coins in nsmb when using efb2ram.

Source/Core/VideoBackends/OGL/TextureCache.cpp

+		s_ColorCopyProgram.Bind();
+		glUniform4f(s_ColorCopyPositionUniform, 0.0f, 0.0f,
+			(float)config.width, (float)config.height);
+		glDrawArrays(GL_TRIANGLE_STRIP, 0, 4);


Tinob · 2015-06-30T15:54:27Z

mimimi085181: that is an interesting case, so the efb copies are used but the destination address does not belong to an active texture?

degasus · 2015-06-30T15:56:57Z

@mimimi085181 About your tiny boost with the pool: Pooling is much faster than allocating and freeing. But on NSMBW, this is only done once. So you shouldn't see any speedup ingame.

mimimi085181 · 2015-06-30T15:59:31Z

@Tinob: I have an heuristic for the efb copies that need to be in ram. If i do efb2ram for these efb copies, and remove all texture cache entries that overlap with those, there are still clothes that are displayed wrong. On top of doing efb2ram for the right efb copies, i also need to enable hashing for efb copies, so a change in some other random efb copy is detected. My skills are too limited to see what is going on there. Maybe there's an easy explanation.

@degasus: I know there won't be a speedup by pooling for nsmb, because the speedup would only affect 1 frame when loading a new world. But for consistency it might be nice. Or if it was done for all normal textures.

JMC47 · 2015-07-01T02:38:52Z

I'd like to try and get this merged before the progress report goes up. A super high res image with coins would be cool for the progress report.

degasus · 2015-07-01T07:29:36Z

@JMC47 unlikely

neobrain · 2015-07-01T13:27:07Z

There is a small problem with the approach taken here: It doesn't take the active texture filtering mode into account when copying the new data into a scaled EFB copy texture. When nearest-neighbor filtering is used, the image will appear blurred compared to the original version (since we currently use a linear filter for updating the texture). This could be dealt with by updating the texture with the proper filter (i.e. by replicating the source texels into a "scale_factor x scale_factor" block of pixels using the nearest-neighbor filter where necessary).

However, one issue with my proposed approach is that there is not "one" filter, but there may be different ones for magnification/minification. Maybe it's useful regardless. I figured I'd point out the issue in any case ;p

Tinob · 2015-07-02T02:48:59Z

@neobrain: you are right, it will be really difficult to emulate native filtering right in upscaled copies, I prefer to stay with a basic approach unless issues are detected because it could be really complicated and gpu intensive if more scaling filters need to be implemented.

JMC47 · 2015-07-02T03:35:52Z

Are there any blocking issues remaining on this?

@Tinob Maybe should remove the line disabling scaled EFB Copies in the INI file?

Source/Core/VideoBackends/D3D/TextureCache.h

@@ -19,14 +19,17 @@ class TextureCache : public ::TextureCache
 private:
 	struct TCacheEntry : TCacheEntryBase
 	{
-		D3DTexture2D *const texture;
+		D3DTexture2D * texture;


Source/Core/VideoCommon/TextureCacheBase.cpp

+					iter++;
+					continue;
+				}
+				if (entry_to_update->config.width != w || entry_to_update->config.height != h)


degasus · 2015-07-02T08:57:56Z

@Tinob Nice rewrite of this feature, I really like it. And sorry for so many comments ;)

@neobrain Through I agree on your comment, I fear there is no way to deal with it. Upscaling is an enhancement which isn't safe by default, especially when dealing with accurate texture sampler settings. And I don't want to spam our texture cache with one rescaled texture per possible sampler state.

neobrain · 2015-07-02T10:16:23Z

@degasus @Tinob: A fix for the specific case where both the mag and the min filter are set to nearest-neighbor sampling is entirely possible and within reasonable scope. Just needs the linear filter in the current code conditionally replaced with point filtering, and the texture region be reloaded upon filter changes. While a generic solution probably will not work indeed, at least this path should be pursued.

Source/Core/VideoCommon/TextureCacheBase.cpp

+					srcrect.bottom = entry_to_update->config.height;
+					dstrect.right = w;
+					dstrect.bottom = h;
+					newentry->CopyRectangleFromTexture(entry_to_update, srcrect, dstrect);


degasus · 2015-07-02T12:20:54Z

LGTM

Tinob · 2015-07-05T13:36:22Z

Sorry for get missing but i really needed a little rest, a week away from home can really drain your energy. I was looking at @neobrain proposal and i find a posible performance issue, if a game uses the same texture with small updated rectangles from efb with diferent filters in the same frames, the implementation can end redrawing the texture for every filter change. If that is not a problem the implemnation is not that hard, It adds a new field to the cache item to store the filter uses and forces regeneration on filter changes.

degasus · 2015-07-22T17:04:02Z

Filtering would still be nice, but it's more of a feature than a bug in my opinion.
@Tinob If you're still working on it, please create a new PR with filtered images.

Implement scaled partial texture updates

Tinob · 2015-07-22T17:06:13Z

No problem, Thanks

degasus reviewed Jun 30, 2015
View reviewed changes

Source/Core/VideoBackends/OGL/TextureCache.cpp

s_ColorCopyProgram.Bind();

glUniform4f(s_ColorCopyPositionUniform, 0.0f, 0.0f,

(float)config.width, (float)config.height);

glDrawArrays(GL_TRIANGLE_STRIP, 0, 4);

This comment was marked as off-topic.

Sign in to view

Tinob force-pushed the master branch 3 times, most recently from 5947a35 to dcdbbe6 Compare July 1, 2015 21:27

degasus reviewed Jul 2, 2015
View reviewed changes

Source/Core/VideoBackends/D3D/TextureCache.h

@@ -19,14 +19,17 @@ class TextureCache : public ::TextureCache

private:

struct TCacheEntry : TCacheEntryBase

{

D3DTexture2D *const texture;

D3DTexture2D * texture;

This comment was marked as off-topic.

Sign in to view

degasus reviewed Jul 2, 2015
View reviewed changes

Source/Core/VideoCommon/TextureCacheBase.cpp

iter++;

continue;

}

if (entry_to_update->config.width != w || entry_to_update->config.height != h)

This comment was marked as off-topic.

Sign in to view

This comment was marked as off-topic.

Sign in to view

Implement scaled partial texture updates

d8cd2c3

Tinob force-pushed the master branch from dcdbbe6 to d8cd2c3 Compare July 2, 2015 11:54

degasus reviewed Jul 2, 2015
View reviewed changes

degasus added a commit that referenced this pull request Jul 22, 2015

Merge pull request #2679 from Tinob/master

6bcdae6

Implement scaled partial texture updates

degasus merged commit 6bcdae6 into dolphin-emu:master Jul 22, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement scaled partial texture updates #2679

Implement scaled partial texture updates #2679

Tinob commented Jun 30, 2015

JMC47 commented Jun 30, 2015

mimimi085181 commented Jun 30, 2015

degasus commented Jun 30, 2015

Linktothepast commented Jun 30, 2015

Tinob commented Jun 30, 2015

degasus commented Jun 30, 2015

Tinob commented Jun 30, 2015

degasus commented Jun 30, 2015

mimimi085181 commented Jun 30, 2015

This comment was marked as off-topic.

Tinob commented Jun 30, 2015

degasus commented Jun 30, 2015

mimimi085181 commented Jun 30, 2015

JMC47 commented Jul 1, 2015

degasus commented Jul 1, 2015

neobrain commented Jul 1, 2015

Tinob commented Jul 2, 2015

JMC47 commented Jul 2, 2015

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

degasus commented Jul 2, 2015

neobrain commented Jul 2, 2015

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

degasus commented Jul 2, 2015

Tinob commented Jul 5, 2015

degasus commented Jul 22, 2015

Tinob commented Jul 22, 2015

Implement scaled partial texture updates #2679

Implement scaled partial texture updates #2679

Conversation

Tinob commented Jun 30, 2015

JMC47 commented Jun 30, 2015

mimimi085181 commented Jun 30, 2015

degasus commented Jun 30, 2015

Linktothepast commented Jun 30, 2015

Tinob commented Jun 30, 2015

degasus commented Jun 30, 2015

Tinob commented Jun 30, 2015

degasus commented Jun 30, 2015

mimimi085181 commented Jun 30, 2015

This comment was marked as off-topic.

Tinob commented Jun 30, 2015

degasus commented Jun 30, 2015

mimimi085181 commented Jun 30, 2015

JMC47 commented Jul 1, 2015

degasus commented Jul 1, 2015

neobrain commented Jul 1, 2015

Tinob commented Jul 2, 2015

JMC47 commented Jul 2, 2015

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

degasus commented Jul 2, 2015

neobrain commented Jul 2, 2015

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

degasus commented Jul 2, 2015

Tinob commented Jul 5, 2015

degasus commented Jul 22, 2015

Tinob commented Jul 22, 2015