mateosss / matter

Customizable GRUB theme inspired by Material Design
Boost Software License 1.0
873 stars 51 forks source link

Added resolution check #34

Closed ahellqui closed 5 years ago

ahellqui commented 5 years ago

The script will now check for 1920x1080 resolution and set it as the resolution if it is available, if it isn't available the script will exit.

This change introduces hwinfo as a dependency, but in case the user doesn't want to install it or just doesn't want to check for the correct resolution I added the -n switch to disable any resolution checking.

At least I have specified a number of resolutions in my grub config that I wouldn't want the script to overwrite, so instead I made it comment out the already present line and add a new one meaning that uninstalling immediately starts using the old resolutions again. I can remove this if it isn't a wanted feature.

mateosss commented 5 years ago

This looks pretty well, however neither in my computer nor in my virtual machine, which both support 1920x1080 resolutions hwinfo --framebuffer throw anything, so executing sudo ./set-matter.sh always returns 1920x1080 is not available on your system, run the script with the -n option to set the theme anyways

ahellqui commented 5 years ago

It works fine on my computer, I tried looking into it and apparently hwinfo doesn't work on some Intel integrated gpu:s.

If you don't have one of those I really don't know where the problem lies. Do you get no output or just not 1920x1080 if you run sudo hwinfo --framebuffer?

mateosss commented 5 years ago

I have an integrated Intel HD Graphics 630. After the command finishes running, nothing is shown in the console (while it is running however it shows three or four lines that get deleted).

ahellqui commented 5 years ago

Ok that's even more strange, provided that the command is run as root it should give at least a few options, perhaps if you redirect the output to a file you can see something.

But according to what I could find hwinfo should work if grubs videoinfo works so I have no idea what's going on.

I don't know how to test this without access to other hardware . Since the documentation of hwinfo is quite scarce and it works on my computer we might have to just close this one unless there is some other tool that does the job that I don't know about.

mateosss commented 5 years ago

Did you try xrandr?

ahellqui commented 5 years ago

I don't really know what I'm talking about so take this with a pinch of salt, but the thing is that grub only works with resolutions supported by the graphics card via vesa bios extensions which is also the reason that the videoinfo command in grub probably won't display all the available resolutions of your monitor (at least not in my case). When the graphics cards drivers kick in later in the boot process you get access to more redolutions through something like kms or X.

Hwinfo is the only program I can find that lists the available vbe resolutions, xrandr and multiple others I've looked into displays all the resolutions supported by the graphics card after boot. It might be possible to do something with /dev/fb0 but that is outside of my knowledge.

mateosss commented 5 years ago

I see, I don't know the correct solution neither, but I would like this to be merged.

I think it may be safe to assume that if xrandr lists 1920x1080, grub will support that resolution. (and from what I remember if it doesn't it will fallback to a supported one, so it is not that big of a deal).

I would prefer xrandr over hwinfo because I think it is important to support intel hd graphics, which are the most widely used notebook gpus.

would you mind to update the PR?

ahellqui commented 5 years ago

I'm actually abroad right now, but I can update it when I get back home. I suppose that the script woking sometimes is better than never.

ahellqui commented 5 years ago

I edited the script to use xrandr instead of hwinfo as requested and the script is now ready for review again.

mateosss commented 5 years ago

This is perfect thanks, and sorry for the delay!