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

Change the names for shader compilation settings #8416

Merged
merged 1 commit into from Jul 5, 2021

Conversation

JosJuice
Copy link
Member

QT_TR_NOOP("Synchronous (Ubershaders)"),
QT_TR_NOOP("Asynchronous (Ubershaders)"),
QT_TR_NOOP("Asynchronous (Skip Drawing)"),
QT_TR_NOOP("Traditional"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the other names. But I think 'Traditional' is worse than even 'Synchronous'. 'Synchronous' at least tries to explain it, but 'Traditional' only is meaningful if you know what the Tradition is. If it would be changed back, I would fully approve.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess reverting just that one to "Synchronous" would make sense in a way... It is the only option that compiles shaders synchronously during normal gameplay, after all. What do others think about this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what we discussed in IRC, basically everything is synchronous. Calling one option Synchronous is not accurate, so I'm against that. Traditional works because it is a valid name: it is the traditional method of shader compilation handling in Dolphin and elsewhere.

Copy link
Member Author

@JosJuice JosJuice Oct 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what we discussed in IRC, basically everything is synchronous.

That was when we were talking about slowdown caused by the GPU performing work. But the options other than this one don't do synchronous shader compilation (disregarding what happens if you change certain settings while the game is running), and shader compilation is exactly what this setting is about, which is why I think synchronous makes sense for this option.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My first thought is that "Standard" would probably be nice - while it's similar to "Traditional", it is more obvious for the user that they are "normal" or "the most regular".

In an emulator like this, I can imagine people thinking "Traditional" is related to classic/retro modes, as in "looking like it used to on a real console and TV back in the day".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Normal" was the original suggestion, but phire pointed out that it's actually pretty abnormal from a graphics standpooint, it's just the normal for us. Hence, Traditional.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can make sense to an emulator programmer. But to reference assumed knowledge is not a substitute for an attempt at actual description.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Idea: 'Wait For Compile'?

@leoetlino leoetlino added the RFC Request for comments label Nov 8, 2019
@jordan-woyak
Copy link
Member

This is miles better than the current "Synchronous" naming.
I don't hate "Traditional" but I might suggest "Specialized".

@stenzek
Copy link
Contributor

stenzek commented Feb 5, 2020

I'm still not a fan of this because it doesn't communicate which shaders are being used, nor how they are being compiled. How does "traditional" mean "specialized shaders and stall emulation until compilation is finished?"

Extending what @jordan-woyak suggested, something like:

  • Specialized
  • Specialized+Ubershaders
  • Ubershaders
  • Specialized+Skip

Would better represent the behavior. At least it specifies which shaders are actually being used, even if the matter in which it affects emulation isn't. But I still prefer the current names.

@JosJuice
Copy link
Member Author

JosJuice commented Feb 5, 2020

@stenzek Something that probably will be a problem with your suggestion is that people are likely to see that there's an option that just says Ubershaders and pick it simply because they've heard that ubershaders are good (even though this option is actually worse than Specialized + Ubershaders). I do think the naming makes sense on a technical level, but that specific thing has the potential to lead to a lot of trouble.

@nbohr1more
Copy link

I would call it "pre-baked" rather than "specialized" and call Ubershaders "realtime emulated"

@JosJuice
Copy link
Member Author

JosJuice commented Feb 5, 2020

But the specialized shaders aren't pre-baked. They're generated at runtime (barring a shader cache).

@nbohr1more
Copy link

They are pre-baked in the sense that their code variants are all known a-priori. I guess you could call them "predicted"?

@JosJuice
Copy link
Member Author

JosJuice commented Feb 5, 2020

No, that is exactly what they are not.

Copy link
Contributor

@blaahaj blaahaj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(edit: deleted comment that was duplicate of a previous comment)

@jordan-woyak
Copy link
Member

jordan-woyak commented Feb 5, 2020

@stenzek The problem with the current phrasing is the discrepancy in what's being described.

  • A. Synchronous
    • "synchronous" describes waiting on specialized shader compilation.
  • B. Synchronous (Ubershaders)
    • "synchronous" describes waiting on Ubershader compilation.
  • C. Asynchronous (Ubershaders)
    • "asynchronous" describes not waiting on specialized shader compilation but we still have to wait for Ubershaders which are just as "synchronous" as in mode "B" yet we use the completely opposite word. This is the problem!
  • D. Asynchronous (Skip Drawing)

The difference from mode "B" to mode "C" is use of specialized shaders after they compile in the background but the current phrasing implies there is less waiting in mode "C" which is not true.
If anything there is more waiting because of poor driver behavior.

"B" describes waiting on the thing in the parentheses, Ubershaders, while "C" describes not waiting on the thing NOT in the parentheses, specialized shaders, which aren't even mentioned. They describe different properties of the implementation and it's confusing.

When I want to test the mode that uses just Ubershaders I have to stop and think every time about which of "B" or "C" is the one without specialized shaders.

Every mode is "synchronous" except "D". "C" has some "asynchronous" compilation going on but it is still very much "synchronous".

I like the words Traditional or Specialized for "A", Exclusive or Ubershaders for "B", Hybrid or Specialized+Ubershaders for "C", and "D" I don't care. Async, Skip Drawing, Specialized + Skip.. whatever.. anything ever suggested.

Just please let us stop calling "B" synchronous and "C" Asynchronous. :P

@KatherineFtw
Copy link

Ok, so. Let me tackle this from a separate angle. As a user, what do I see for all of these? I see "Ubershaders" and three variants. By it's self, Ubershaders, Traditional, Sync and ASync, Specialized, Skip. Their all meaningless in informing me on what they all do, or very well what they are, mean, or if default is accurate/ok.

At the end of the day, realize that most people do not know, and probably don't care.

I suggest it's handled like RPCS3's Options section of the wiki. It's helped me more than anything else suggested combined.

https://wiki.rpcs3.net/index.php?title=Help:Default_Settings

TL;DR: No amount of arguing over which jargon to use will actually resolve anything to non-jargon informed users.

@stenzek
Copy link
Contributor

stenzek commented Feb 8, 2020

@jordan-woyak B/C shouldn't involve any waiting for ubershaders as they are all compiled at startup.

Maybe another way of expressing the options would be better. For example, a slider with specialised shaders on the left, specialised+ubershaders in the middle, ubershaders on the right. So defining performance from fastest to slowest, but stutter from most to least. Skip drawing could be an additional checkbox as it's unrelated to ubershaders, and checking it would disable/grey out the slider.

@KatherineFtw having such documentation is great, but I fear that users won't read it. We're lucky if people read the in-dialog.text, let alone opening their browser and searching :(

Copy link
Member

@Ebola16 Ebola16 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As long as the changes are technically correct, I'm ok with the current version of this PR. I had a problem with the previous naming scheme because the names were too similar. People could report a problem with "synchronous" shaders and really mean "Synchronous (Ubershaders)" but they only remembered the first word of the setting.

@MayImilae
Copy link
Contributor

So pondering this some more, one option for this that we could do is to go with a more accurate name for specialized shaders and just stick (Default) beside it. Like this:

  • Specialized (Default)
  • Exclusive Ubershaders
  • Hybrid Ubershaders
  • Skip Drawing

@JosJuice
Copy link
Member Author

I would be fine with that. Does anyone have anything against it?

@mbc07
Copy link
Contributor

mbc07 commented Apr 10, 2021

The new names LGTM.

On a side note, since we're changing the naming, I would probably throw a note on the Exclusive Ubershaders description text to inform the users they should prefer Hybrid Ubershaders instead of Exclusive unless they really know what they're doing. I've seen quite a bunch of people straight up selecting "Exclusive" instead of "Hybrid" thinking it's better then complaining on the forums and on other communities that everything became slow...

@JosJuice
Copy link
Member Author

Hm, I wonder what the change should be in more concrete terms... What do you think about replacing "Only recommended for high-end systems" with "Don't use this unless you have a very powerful GPU"?

@JMC47
Copy link
Contributor

JMC47 commented Apr 10, 2021

I'm good with those names.

@JosJuice
Copy link
Member Author

I've made the changes.

@mbc07
Copy link
Contributor

mbc07 commented Apr 10, 2021

What do you think about replacing "Only recommended for high-end systems" with "Don't use this unless you have a very powerful GPU"?

People with high end GPUs might still feel "encouraged" to select this, and that group is also the most likely to be running 6x+ IR with AA and other expensive settings, those coupled with Exclusive Ubershaders can bring even the mightiest of the cards to its knees.

What about "Requires a very powerful GPU, use only if you still experience shader compilation stuttering with Hybrid Ubershaders"?

@MayImilae
Copy link
Contributor

MayImilae commented Apr 10, 2021

Hm, I wonder what the change should be in more concrete terms... What do you think about replacing "Only recommended for high-end systems" with "Don't use this unless you have a very powerful GPU"?

I'm ok with this, but I suggest putting in a comment dating it to 2021. What defines a powerful GPU will change and it will be convenient to have a date assigned to it for when it comes time to alter that.

EDIT: Also yes to what mbc07 said, but I have some specific wording that might be better.

"Ubershaders will always be used. Provides a near stutter-free experience at the cost of high GPU performance requirements. Don't use this unless you encountered stuttering with Hybrid Ubershaders and have a powerful GPU."

Saying powerful GPU last is probably better, imo.

@JosJuice
Copy link
Member Author

JosJuice commented Apr 10, 2021

@mbc07 Would you be fine with "Don't use this unless you have a very powerful GPU and experience shader compilation stuttering with Hybrid Ubershaders"? I feel like "Requires a very powerful GPU" feels a bit redundant with the sentence that comes before.

@MayImilae What do you think would be a good way to include the 2021 wording?

EDIT: May's edit to mbc07's suggestion is fairly similar to mine. I'll go with May's (perhaps with the word "very" added).

@JosJuice
Copy link
Member Author

Ohh, wait, do you mean a source code comment for the 2021 thing? In that case I'll just throw it in there right away.

@MayImilae
Copy link
Contributor

Mmmhm, I meant source code comment for the 2021 thing just to give future devs an idea of what "powerful" meant there.

@mbc07
Copy link
Contributor

mbc07 commented Apr 10, 2021

May's description LGTM

@JosJuice
Copy link
Member Author

The description is now:

Ubershaders will always be used. Provides a near stutter-free experience at the cost of very high GPU performance requirements.

Don't use this unless you encountered stuttering with Hybrid Ubershaders and have a very powerful GPU.

@blaahaj
Copy link
Contributor

blaahaj commented Apr 10, 2021

3/4 options involve "specialized" shaders. However, only 1/4 options says "specialized"?

@JMC47
Copy link
Contributor

JMC47 commented Apr 17, 2021

This is an improvement over the current situation. Things we can do in the future is try to better describe what the modes do in the options since they do use mixes of the shaders. There's no such thing as a Hybrid Ubershader - it's a simplification of a mode that combines ubershaders and specialized shaders.

@leoetlino leoetlino merged commit ccc2b7b into dolphin-emu:master Jul 5, 2021
10 checks passed
@JosJuice JosJuice deleted the shader-compilation-names branch July 6, 2021 06:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFC Request for comments