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

D3D: remove load texture on creation optimization #1501

Merged
merged 3 commits into from
Jan 11, 2015

Conversation

degasus
Copy link
Member

@degasus degasus commented Nov 5, 2014

No description provided.

@degasus
Copy link
Member Author

degasus commented Nov 6, 2014

on irc: I would expect that very similar work is required (copy resource to system visible memory, then copy to video private).

@kayru
Copy link
Contributor

kayru commented Nov 6, 2014

On the second thought, it might be the case that upload to video private is done synchronously. This would make it faster than UpdateSubresource.

@degasus
Copy link
Member Author

degasus commented Nov 6, 2014

@kayru So do you think this optimization matters? I want to implement a texture pool like your rendertarget one. So this optimization is in my way :/

@kayru
Copy link
Contributor

kayru commented Nov 6, 2014

I think the best thing is always to measure real impact.

@JMC47
Copy link
Contributor

JMC47 commented Nov 7, 2014

NSMBWii EFB2RAM 3x IR Master: 89 FPS
NSMBWii EFB2RAM 3x IR PR: 71 FPS

1x IR is 119 fps on both builds.

@pauldacheez
Copy link
Contributor

Texture pooling would likely be more beneficial than this optimization anyway. If you can reimplement this later for a win-win, though, that'd be cool.

@kayru
Copy link
Contributor

kayru commented Nov 8, 2014

@degasus what is the problem with passing in initial data on texture creation? You should still be able to use UpdateSubresource to change content later.

@degasus
Copy link
Member Author

degasus commented Nov 8, 2014

@kayru With pooling, we'll hit this path less often. It's also in the way of decoding directly into a mapped staging buffer.

@PatrickFerry
Copy link
Contributor

So why remove the optimisation if it badly affects higher IR values?

@JMC47
Copy link
Contributor

JMC47 commented Dec 1, 2014

To make things faster in the long term.

@PatrickFerry
Copy link
Contributor

So maybe this PR can wait until a replacement gets made?

Edit: Please :( puppy eyes

@degasus
Copy link
Member Author

degasus commented Dec 1, 2014

@ZephyrSurfer That's the reason why this PR wasn't touched in the last three weeks ;'-)

@degasus degasus force-pushed the texture_creation branch 2 times, most recently from 3a43f94 to 128d497 Compare January 10, 2015 10:55
This reverts an optimization which isn't worth imo. Every texture uploads have to alloc vram and a staging buffer, so there is no need to do both in the same call.
This variable isn't use any more.
@degasus
Copy link
Member Author

degasus commented Jan 10, 2015

degasus> so is this PR as good as master?
JMC47> for me, at leats
JMC47> it seems fine in NSMBWii
JMC47> I don't see a big slowdown
mimimi> in D3D with 3xIR, pr1501 and master 5010 are at the same speed

So ready to merge?

The old slowdown wasn't because of this optimization. I did also remove the hint to mark 1-level textures as default instead of dynamic. But it seems to be faster to create map-able textures.

@CrossVR
Copy link
Contributor

CrossVR commented Jan 10, 2015

LGTM, the changes in the PR look to be purely cosmetic.

degasus added a commit that referenced this pull request Jan 11, 2015
D3D: remove load texture on creation optimization
@degasus degasus merged commit 6c46f27 into dolphin-emu:master Jan 11, 2015
@degasus degasus deleted the texture_creation branch August 9, 2015 08:59
@waddlesplash
Copy link
Contributor

This PR is the cause of the D3D backend crashing with Intel cards. With previous revision, tried multiple startups of both games & the emulator and it never crashes, with this PR it crashes 100% of the time in SMG2 just past the Wiimote screen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
7 participants