Merge lp:~widelands-dev/widelands/bug-1817686-SDL-pixel-format into lp:widelands

Proposed by GunChleoc
Status: Merged
Merged at revision: 8999
Proposed branch: lp:~widelands-dev/widelands/bug-1817686-SDL-pixel-format
Merge into: lp:widelands
Diff against target: 21 lines (+10/-1)
1 file modified
src/graphic/graphic.cc (+10/-1)
To merge this branch: bzr merge lp:~widelands-dev/widelands/bug-1817686-SDL-pixel-format
Reviewer Review Type Date Requested Status
Benedikt Straub Approve
Review via email: mp+363729@code.launchpad.net

Commit message

Fail with SDL messagebox if SDL_BYTESPERPIXEL != 4

To post a comment you must log in.
Revision history for this message
Benedikt Straub (nordfriese) wrote :

Tested it.
The log says »ERROR: Wrong SDL_BYTESPERPIXEL, expected 4 but got 1« as expected.
But no message box appears, the program exits immediately.

By the way, the assert could be removed now, right? Since the triggering failure is now caught beforehand.

8996. By GunChleoc

Tweaked error message and removed assert.

Revision history for this message
GunChleoc (gunchleoc) wrote :

I guess the driver is too messed up to even display the message box then. I have now made the log message identical to the dialog message and removed the assert.

Revision history for this message
Benedikt Straub (nordfriese) wrote :

It´s a perfectly normal built-in Intel graphics card (VGA compatible controller, Atom Processor Z36xxx/Z37xxx Series Graphics & Display) with default drivers – no reason why it should behave like that, especially in such a random way. It always worked before…

The code looks good to me; and if the SDL is messed up, that´s a problem with the graphics driver or possibly the SDL, but not in widelands.

review: Approve
Revision history for this message
GunChleoc (gunchleoc) wrote :

Thanks for the review and testing

@bunnybot merge

Revision history for this message
kaputtnik (franku) wrote :

> built-in Intel graphics card

I should had bet for this :-D There were several problems with this graphics card in the past.

Glad it's working for you now.

Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 4535. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/499382080.
Appveyor build 4322. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1817686_SDL_pixel_format-4322.

Revision history for this message
bunnybot (widelandsofficial) wrote :

Refusing to merge, since Travis is not green. Use @bunnybot merge force for merging anyways.

Travis build 4535. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/499382080.

Revision history for this message
hessenfarmer (stephan-lutz) wrote :

travis failure is transient. (apt-get failed)

@bunnybot merge force

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/graphic/graphic.cc'
2--- src/graphic/graphic.cc 2019-02-23 11:00:49 +0000
3+++ src/graphic/graphic.cc 2019-02-27 16:20:48 +0000
4@@ -114,7 +114,16 @@
5 " size %d %d\n"
6 "**** END GRAPHICS REPORT ****\n",
7 SDL_GetCurrentVideoDriver(), disp_mode.format, disp_mode.w, disp_mode.h);
8- assert(SDL_BYTESPERPIXEL(disp_mode.format) == 4);
9+ const int bytes_per_pixel = SDL_BYTESPERPIXEL(disp_mode.format);
10+ if (bytes_per_pixel != 4) {
11+ const std::string error_message = (boost::format("SDL should report 4 bytes per pixel, but %d were reported instead.\nPlease check that everything's OK with your graphics driver.") % bytes_per_pixel).str();
12+ log("ERROR: %s\n", error_message.c_str());
13+ SDL_ShowSimpleMessageBox(SDL_MESSAGEBOX_ERROR, "Video Error",
14+ error_message.c_str(),
15+ nullptr);
16+ exit(1);
17+
18+ }
19 }
20
21 std::map<std::string, std::unique_ptr<Texture>> textures_in_atlas;

Subscribers

People subscribed via source and target branches

to status/vote changes: