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

Update GPU Whitelist System #2178

Merged
merged 8 commits into from
Jun 28, 2024

Conversation

blusewill
Copy link
Contributor

@blusewill blusewill commented Jun 26, 2024

Because too many people says their Laptop goes Brrrrrrr when running winutil

I decided to ban every single Laptop GPU boots into Matrix Mode

Also added some low Powered NVIDIA GPU Such as GT Series.

Sort of*
fixes: #2170
fixes: #2023

@og-mrk
Copy link
Contributor

og-mrk commented Jun 27, 2024

@blusewill
I've made a PR to your Fork which improves the function overall readability and shortens the length of it, if you're interested in reviewing it, then here's a link to it.

…gpu-2024-06-27

Re-Formate 'Invoke-WinUtilGPU.ps1' Private Function to be Shorter
@blusewill
Copy link
Contributor Author

Alright! Thanks for the Formatting

@Marterich
Copy link
Contributor

Marterich commented Jun 27, 2024

First of all, great work both of you :)
Regarding better readability, you could also take a look at replacing the if constructs with a switch case statement using Wildcards
This would reduce the number of times $gpuName needs to be referenced in the code

https://learn.microsoft.com/en-us/previous-versions/windows/it-pro/windows-powershell-1.0/ff730937(v=technet.10)?redirectedfrom=MSDN

Edit: it's also possible to refactor the code to use only a single for each loop which improves performance and readability alike.
I'll see if I create a PR to your branch later today

@og-mrk
Copy link
Contributor

og-mrk commented Jun 27, 2024

First of all, great work both of you :) Regarding better readability, you could also take a look at replacing the if constructs with a switch case statement using Wildcards This would reduce the number of times $gpuName needs to be referenced in the code

https://learn.microsoft.com/en-us/previous-versions/windows/it-pro/windows-powershell-1.0/ff730937(v=technet.10)?redirectedfrom=MSDN

Thanks for the info @Marterich , checked it out and do see that it may improve the readability of the code.. but I can't figure out a way to do C 's Switch style of Multiple cases (Pattern Matching), one code to execute, like the follow:

switch (x){
  case 1:
  case 2:
  case 3:
  case 4:
  case 5:
  case 6:
      printf ("The number you entered is >= 1 and <= 6\n");
      break;
}

The closest I got is a StackOverFlow Question on this topic.

Edit: it's also possible to refactor the code to use only a single for each loop which improves performance and readability alike. I'll see if I create a PR to your branch later today

... I was about to write how it might be difficult to achieve this (because of *NVIDIA* pattern matching in second ForEach loop).. but after some thinking about it.. it might be easier than I initially thought, I'll leave it to you, but if you want I'll have the code in a section below, based on your idea, which will hopefully work without a problem. (gonna hide it in a Collapsible Section.. to see if you can guess the answer 😉)

Note

The code in next section doesn't use Switch Statement(s), only If Statement(s).. you've to implement the Switch one on your own @Marterich if you want it 👍

The Updated Code
function Invoke-WinUtilGPU {
    $gpuInfo = Get-CimInstance Win32_VideoController
foreach ($gpu in $gpuInfo) {
    $gpuName = $gpu.Name

    # GPUs to blacklist from using Demanding Theming
    if ($gpuName -like "*NVIDIA GeForce*M*" -OR
        $gpuName -like "*NVIDIA GeForce*Laptop*" -OR
        $gpuName -like "*NVIDIA GeForce*GT*" -OR
        $gpuName -like "*AMD Radeon(TM)*" -OR
        $gpuName -like "*UHD*") {
        return $false
    }

    # GPUs to whitelist on using Demanding Theming
    if ($gpuName -like "*NVIDIA*" -OR
        $gpuName -like "*AMD Radeon RX*") {
        return $true
    }
}

}

@Marterich
Copy link
Contributor

Marterich commented Jun 27, 2024

@blusewill
I created a PR to your branch containing a refactored version that cleans up the script quite a bit more (IMHO)
If you want to modify something even further, feel free to do so :)
@og-mrk I took inspiration from your code as well while refactoring this

My changes in https://github.com/blusewill/pull/3
  • Removed Whitelist part and default to $true if the LowPowerGPUs are not found
  • factor out the GPUs into a List to separate the logic from the content. This allows adding new GPUs to the List without modifying the Logic part.
  • remove unnecessary variable assignment for $gpuName as this is not used elsewhere

Blueswill refactor for performance and readability
@blusewill
Copy link
Contributor Author

There is so many long time contributor lol

@Marterich
Copy link
Contributor

@blusewill fyi in case you didn't know
If you edit the PR Description to something like
fixes: #2170
fixes: #2023
it will link the Issues and close them automatically as soon as your PR gets merged :)

@blusewill
Copy link
Contributor Author

Oh really? That's a thing in Github Action now?

Let me edit it

@Marterich
Copy link
Contributor

Oh really? That's a thing in Github Action now?

Let me edit it

It's actually a GitHub native function
https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue

@blusewill
Copy link
Contributor Author

Oh really? That's a thing in Github Action now?
Let me edit it

It's actually a GitHub native function https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue

Oh really! I didn't know that. Thanks for telling me that.

@ChrisTitusTech
Copy link
Owner

Great job all

@ChrisTitusTech ChrisTitusTech merged commit 5b36925 into ChrisTitusTech:main Jun 28, 2024
@blusewill blusewill deleted the gpu-whitelist branch July 9, 2024 15:43
@ChrisTitusTech ChrisTitusTech added the skip-changelog Skip Change Logs label Sep 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip-changelog Skip Change Logs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tool is now stuttering while scrolling on install panel. Huge GPU utilization
4 participants