Remove hardcoded resolution list

[Hoikas]

Thanks to a snippet devised by @cwalther in #12, we no longer require a hardcoded map of aspect ratios to resolutions. This will better facilitate using all resolutions supported by the user's hardware.

[branan]

woot.

[cwalther]

This doesn’t work for 16:9.375 (1024x600) – should be " [%g:%g]".

[Hoikas]

In my tests, 1024x600 appeared as 16:9, which is what I desired. That resolution seems to have gained the 16:9 designation even if it's technically 16:9.375. I'd rather not confuse the users.

[cwalther]

Ah, OK. I personally would find it confusing to be presented with such a mathematically wrong result (as my comment below shows), but if what you say about the popular designation is true (I wouldn’t know, I’m unfamiliar with netbook territory), then I agree. Disregard the following comment in that case.

[cwalther]

I just noticed that with these error bounds, 1440x852 (which for some inexplicable reason is a supported resolution on my 1680x1050 MacBook Pro, while the perfect match 1440x900 isn’t !?!) is wrongly recognized as 16:9.375 (it’s actually 16:9.4667… 120:71… not a nice number any way you dice it), because its matchCheck is 1.00978. I suggest narrowing the error bounds to 0.995…1.005. That way it won’t be recognized at all, which IMO is the right way to treat such nonstandard resolutions. (The reason the bounds are needed at all is so that 1280x854 is recognized as 3:2 – matchCheck is 1.00078 there.)

Hmm, or maybe a better criterion would be to check whether (w+1)/(h-1) >= r[0]/r[1] >= (w-1)/(h+1), i.e. whether the resolution is within 1 pixel wiggle room in any direction from the exact aspect ratio. Like this:

    def _AspectRatio(self, w, h):
        """Returns the appropriate aspect ratio for the given resolution"""
        ratios = ((5, 4), (4, 3), (3, 2), (16, 10), (5, 3), (16, 9), (16, 9.375),)
        w = float(w) # comes in as string, want float (not int) for division
        h = float(h)
        for r in ratios:
            # resolution is within 1 pixel wiggle room in any direction from the exact aspect ratio (needed to recognize 1280x854 as 3:2)
            if (w+1)/(h-1) >= float(r[0])/float(r[1]) >= (w-1)/(h+1):
                return " [%g:%g]" % (r[0], r[1])
        return ""

[cwalther]

[This comment is supposed to appear on line 1297, which is outside of the diff. Let’s see whether that works.]

if vidRes.getString() == vidResList[res]:

This is never true (unless the resolution is one with an unrecognized aspect ratio). I suggest if self.GetVidResField() == vidResList[res]:.

[cwalther]

[Alright, since commenting on lines outside of the visible diff only works halfways, this one as a standard comment. I guess I could make it in the diff of 934adb6, but I’m not sure if anyone would still notice it there.]

Line 1298:

videoField.setValue( float(res) / float(numRes))

This looked more correct before, when it said videoField.setValue( float(res) / (numRes - 1)). When the last resolution in the list is selected, i.e. res == numRes - 1, we want the slider value to be 1.0.

[Hoikas]

That sounds like a much more elegant solution than offering a +/-0.1 range on the ratio. I'm still not convinced that I should be using %g for the string formatting though--I think most users are accustomed to seeing integers there since several manufacturers push those nonstandard resolutions labelled as "16:9" as opposed to "16:9.375". Of course, it's been a long time since I've been "most users" so I may not be right.

Re Line 1297 you're definitely right there. That's definitely my mistake. The same goes for Line 1298.

[branan]

@Hoikas, please ping me when you and @cwalther finalize the finalize this patchset so I can test it. I'll stay hands-off for now.

[Hoikas]

Okay, I've pushed fixes for the things you've mentioned so far, @cwalther. Let me know if you see anything else!

[cwalther]

Almost matches what I tested this morning now, so this should be good to go. I’m going to give it a test on CWE-ou too.

[Hoikas]

I'll hold off on uploading another patch to crucible until you've done that test.

[cwalther]

Found one more bug: Clicking the “Defaults” button will break the dialog with a Python exception. Fix:

diff -r 1b896b45b4bf Python/xOptionsMenu.py
--- a/Python/xOptionsMenu.py    Wed Apr 04 22:15:34 2012 -0400
+++ b/Python/xOptionsMenu.py    Thu Apr 05 14:19:28 2012 +0200
@@ -1545,7 +1545,7 @@
                     videoField.setValue( float(res) / (numRes - 1))
                 else:
                     videoField.setValue( 0 )
-        self.SetVidResField(res)
+        self.SetVidResField(vidRes)

     def InitVideoControlsGUI(self):
         xIniDisplay.ReadIni()

[Hoikas]

D'oh -- fixed it

[branan]

if @cwalther doesn't see any other issues, I'll merge this.

[cwalther]

Just tested on CWE-ou again, looks good now.