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

ShaderGen: Remove virtual methods and string from ShaderGeneratorInterface. #3431

Merged
merged 1 commit into from Jan 5, 2016
Merged

Conversation

stenzek
Copy link
Contributor

@stenzek stenzek commented Jan 2, 2016

Addresses issue 9219. This is because LinearDiskCache reads/writes to the storage of ShaderUid, therefore ShaderUid must be a POD struct. So I've removed the constructor of the uid classes (moving this to the shader gen function themselves), and the virtual methods/string from the base class.

Since all the callers of these methods take a template parameter of the derived class anyway, there was no benefit to them being virtual. ShaderGeneratorInterface is now acting as a fallback for the methods unimplemented by the derived classes.

Additionally, adds a static assert to LinearDiskCache to ensure no non-POD types are used in the future.

gcc and clang both correctly optimize out all the Write() calls in the shader generation method. MSVC does it for some of them, but not all (some of the varargs cases and where a function is called, eg GetInterpolationQualifier, which is not being inlined for some reason). Aside from duplicating the shader generation code to include only the uid-writing parts, I can't think of a more straightforward solution to this problem.

@@ -52,6 +52,7 @@ class LinearDiskCache
u32 OpenAndRead(const std::string& filename, LinearDiskCacheReader<K, V> &reader)
{
using std::ios_base;
static_assert(std::is_pod<K>::value, "K is a POD type");

This comment was marked as off-topic.

… string buffer to ShaderCode

This fixes the crashes occuring at startup with a non-empty shader cache.
Because LinearDiskCache reads/writes to the storage of ShaderUid, ShaderUid must be trivially copyable.
Additionally, adds a static assert to LinearDiskCache to ensure this doesn't happen in the future.

The initialization of ShaderUid data has been moved to the code generation functions, so the above condition holds true.
@degasus
Copy link
Member

degasus commented Jan 4, 2016

LGTM

lioncash added a commit that referenced this pull request Jan 5, 2016
ShaderGen: Remove virtual methods and string from ShaderGeneratorInterface.
@lioncash lioncash merged commit 0509292 into dolphin-emu:master Jan 5, 2016
@stenzek stenzek deleted the shadercache branch January 9, 2016 04:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants