We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 605

Summary: SDL_GetClosestDisplayMode does not consider all modes if aspect ratios differ in certain ways
Product: SDL Reporter: Martin <name.changed.by.editors>
Component: videoAssignee: Ryan C. Gordon <icculus>
Status: RESOLVED FIXED QA Contact: Sam Lantinga <slouken>
Severity: normal    
Priority: P2    
Version: HG 2.0   
Hardware: x86   
OS: Windows Vista   

Description Martin 2008-07-27 08:48:50 UTC
I wanted to make sure that fullscreen would use the current desktop mode, so I (roughly) did this:
  if (0==SDL_GetDesktopDisplayMode(&mode)) {
    SDL_SetFullscreenDisplayMode(&mode);
  }

My desktop is set to 1280x1024 at 85 Hz, and SDL_GetDesktopDisplayMode reported just that. But upon creation of the fullscreen window, SDL switched to 1600x1200 at 75 Hz (my monitor can't go any higher).

I found out that SDL_GetClosestDisplayMode (which was called indirectly) erroneously picked that mode:

The mode list contained the following entries, among others, in the order shown:
1600x1200
1440x900
1280x1024
1280x960
1024x768
800x600
640x480

The modes are sorted descending by width, and modes of equal width are sorted descending by height (and more criteria). This is an important detail which SDL_GetClosestDisplayMode does not get right. It somewhat expects both width and height to decrease monotonously when walking through the mode list. Because most modes are 4:3, this is mostly the case, but not always, as the 1440x900 entry shows.

See this excerpt from the function's search loop (in SDL_video.c, Rev. 3622, I think):
        if ((current->w && current->h) &&
            (current->w < mode->w || current->h < mode->h)) {
            /* Out of sorted modes large enough here */
            break;
        }

In my case, after seeing 1440x900, SDL_GetClosestDisplayMode considered itself "out of modes large enough" for the target 1280x1024, since 900<1024. 1440x900 sorted before 1280x1024 because 1440>1280. So the perfect match was missed.

I suggest to correct the code as follows:
        if (current->w && (current->w < mode->w)) {
            /* Out of sorted modes large enough here */
            break;
        }
        if (current->h && (current->h < mode->h)) {
            if (current->w && (current->w==mode->w)) {
                /* Out of sorted modes large enough here */
                break;
            }
            /* Wider, but not tall enough, due to a different
            aspect ratio. This mode must be skipped, but closer
            modes may still follow. */
            continue;
        }
Comment 1 Sam Lantinga 2008-11-25 12:06:52 UTC
Your suggestion looks great.  Thanks!

Committed revision 4137.