[OLD-1] AGS engine Linux port

Started by BigMc, Sun 03/06/2012 19:04:20

Previous topic - Next topic

BigMc

Quote from: Crimson Wizard on Fri 22/02/2013 18:25:07
I was thinking merely about adding a human(end-user)-readable changelog to the repository.

I'm thinking that is a very good idea.

s_d

Quote from: scottchiefbaker on Fri 22/02/2013 18:12:37
Do we want to clean up the build process a bit before we look at an "official" release? There are still quite a few warnings on compile.

Nah.  Those aren't customer facing, and very likely have little impact on the stability of the engine.  I would consider that a low-level background task.

I would consider crash bugs number 1 priority, and then graphical/audio artifacts or script interpreter bugs (any of those left?) as number 2 priority.  And after those, a proper setup utility, especially for non-PulseAudio sound device setup, since lack of audio init can crash games.  We can probe for PulseAudio, and if it's not there, check if ALSA is presenting a MIDI device, and index DIGIMID for it.  After covering PulseAudio and bare ALSA, that's nearly everyone (EsounD & aRtsd are almost dead, and JACK is mostly pro-audio, but has an Allegro plugin, so it probably wouldn't be too hard to support, just not high on my list).

By the way, regarding that setup tool, I did contact Electroshokker, and he replied that he's willing to give us that, pending CJ's permission.  Electroshokker is of the opinion that the sources he has are bound to the old (closed) license.  So, I PM'd CJ here, about two weeks ago, but no reply.  Does he come around still?  His last post was in January.

s_d

Quote from: BigMc on Tue 19/02/2013 09:11:14
s_d, most of the iffy stuff goes on in graphics_mode.cpp.

Along these lines, is it just me, or is init_gfx_mode() actually recursive?  Surely a loop would be more straightforward?  Especially since it appears this is where I'd be filtering (somehow? or probing?) for exotic resolution modes, and potentially fixing them up before whacking them into X.

Here's my take on the issue;  basically, it looks like we (or at the very least, Allegro) takes heavy advantage of an undocumented feature.  Or rather, undocumented, but compatible, resolution modes.  You can, of course, query your X display to print a list of compatible modes (for example, "xrandr --query"), but the list will be incomplete.  It will have none of the nice resolutions near 320x200 that we want.

So what do we do?  We calculate a scalar, apply it to the current resolution to calculate a target display mode, and just give it a shot.  More often than not, the display mode happens to be valid (in that the graphics driver supports it somehow).  I'm not sure why that is.  Anyway, we're not the only ones... apparently that's done elsewhere in the software world, as well.  Where we fall down here is that when X returns back the "unsupported mode" return code, we choke rather than doing... well, anything.

In fact, it would be better (in this case) to continue at the current resolution and skip the mode switch altogether!  But, I think, loads of things are already calculated by this point, and thusly it would be quite difficult to go fix them all up after the fact.   Which comes back to needing to somehow properly detect a bogus display resolution.  From a little research (now that I can access Wadjet Eye's beta bug reports) I see that 1680x1050 is not actually that uncommon of a display resolution for Linux users.  With further research, I've learned that it was actually a very popular display resolution for high-end gaming laptops (15.4"-17" panels) between 2003-2006.  Which is why so many appear to be running Linux now.

What I also learned is that others running that resolution are *not* crashing!  That certainly complicates matters.

Anyway, hopefully I'm not annoying you folks too much with my rambling... just thinking out loud (or in paragraphs, actually).

Crimson Wizard

#383
Quote from: s_d on Sun 24/02/2013 09:47:30
Along these lines, is it just me, or is init_gfx_mode() actually recursive?
Yes..... but I guess you haven't yet seen the parts with "goto" jumps up from "while" loop?

Quote from: s_d on Sun 24/02/2013 09:47:30I see that 1680x1050 is not actually that uncommon of a display resolution for Linux users.  With further research, I've learned that it was actually a very popular display resolution for high-end gaming laptops (15.4"-17" panels) between 2003-2006.
It is also the resolution I am using right now on my desktop!

Quote from: s_d on Sun 24/02/2013 09:47:30
Anyway, hopefully I'm not annoying you folks too much with my rambling... just thinking out loud (or in paragraphs, actually).
The only thing that annoys me - and sorry for being so frank - is when people talk but not do. I hope you are not just telling us what's wrong, but planning to find an actual solution meanwhile ;).

s_d

Quote from: Crimson Wizard on Sun 24/02/2013 14:02:44
The only thing that annoys me - and sorry for being so frank - is when people talk but not do. I hope you are not just telling us what's wrong, but planning to find an actual solution meanwhile ;).

Yes, I am, of course.  I don't mean to criticize code quality, I'm just puzzled by CJ's intent, in some places.  I was hoping, rather than being pointed to even more mind-gobbling abuse of C, I might occasionally benefit from your insight into some of the weirdness.  But, of course, someone had to suffer first, and had no help, so I could do so as well.  Also, there isn't any while-loop escapism occurring in graphics_mode.cpp, where I'm looking now.  :)

I've only spent hardly a month in the code (plus a few weeks last summer building bero's port for a fan game project).  I'm trying to devise an adequate solution for these mode switching crashes on Linux.  I'm a little stuck right now when I learned that it's not the display mode specifically;  it appears that other Linux users, and indeed yourself, are able to play games in the full-screen scaled resolution mode for 1680x1050 (the one calculated in engine_init_gfx_filters() ).  So, the failure may be per-driver, per-adapter or something else.

I'm starting to understand why BigMC wants to eliminate the mode switching, though (in addition to not reliably switching back if the game crashes).  If we can't know ahead of time whether or not the mode switch is going to crash, then that makes this a sticky problem, indeed.  So, right now, my focus is to find a good method of discovering supported modes.  Also, I don't know Allegro very well either, so I'm studying that, too.

Perhaps just keep to myself until I have patches, then?

Crimson Wizard

Quote from: s_d on Sun 24/02/2013 18:07:55I don't mean to criticize code quality, I'm just puzzled by CJ's intent, in some places.
I think the reason is very simple: these pieces were written by less experienced coder (and maybe hacked later to save time). If you compare different parts of engine, and, especially, new editor, you will notice a significant change in style and code quality between them. Engine bears legacy code from 1998.

Quote from: s_d on Sun 24/02/2013 18:07:55
Also, there isn't any while-loop escapism occurring in graphics_mode.cpp, where I'm looking now.  :)
https://github.com/adventuregamestudio/ags/blob/master/Engine/ac/dialog.cpp#L549
https://github.com/adventuregamestudio/ags/blob/master/Engine/ac/invwindow.cpp#L181
:)

Quote from: s_d on Sun 24/02/2013 18:07:55
Perhaps just keep to myself until I have patches, then?
Well, the thing is, there are very few people actualy working on the engine, and our work has been pretty separated so far, everyone keeping to its own task. It is a bit difficult to refer to everything... I mean, it would be nice if someone could help you, but I don't think it will be me, because I am pretty much focused on different things now.

BigMc

Quote from: s_d on Sun 24/02/2013 18:07:55
So, right now, my focus is to find a good method of discovering supported modes.

Well there's Allegros get_gfx_mode_list(). But after another look at ali3dsw.cpp, I found that every mode is expected to work when get_gfx_mode_list() fails to supply the supported modes. Maybe the crashes are caused by the last 'return true' in the following code?

Code: AGS
bool ALSoftwareGraphicsDriver::IsModeSupported(int driver, int width, int height, int colDepth)
{
#if defined(ANDROID_VERSION) || defined(PSP_VERSION) || defined(IOS_VERSION)
  // Everything is drawn to a virtual screen, so all resolutions are supported.
  return true;
#endif

  if (_windowed)
  {
    return true;
  }
  if (_gfxModeList == NULL)
  {
    _gfxModeList = get_gfx_mode_list(driver);
  }
  
  if (_gfxModeList != NULL)
  {
    // if a list is available, check if the mode exists. This prevents the screen flicking
    // between loads of unsupported resolutions
    for (int i = 0; i < _gfxModeList->num_modes; i++)
    {
      if ((_gfxModeList->mode[i].width == width) &&
        (_gfxModeList->mode[i].height == height) &&
        (_gfxModeList->mode[i].bpp == colDepth))
      {
        return true;
      }
    }
    strcpy(allegro_error, "This graphics mode is not supported");
    return false;
  }
  return true;
}

s_d

Quote from: Crimson Wizard on Sun 24/02/2013 18:20:19
It is a bit difficult to refer to everything... I mean, it would be nice if someone could help you, but I don't think it will be me, because I am pretty much focused on different things now.

All very reasonable.  That's why I was "wondering out loud".  If someone sees something that rings a bell, then they could mention it, and otherwise ignore it.  Nobody should ever feel compelled to spend time helping me out, just if it's convenient the knowledge happens to be loaded in their local cache  ;)

s_d

Quote from: BigMc on Sun 24/02/2013 19:03:26
Well there's Allegros get_gfx_mode_list(). But after another look at ali3dsw.cpp, I found that every mode is expected to work when get_gfx_mode_list() fails to supply the supported modes. Maybe the crashes are caused by the last 'return true' in the following code?

Yes, I found the same thing, but it's worse than that.  I just learned that get_gfx_mode_list() always returns a null list for any GFX_SAFE type, GFX_AUTODETECT type, or any other "magic" Allegro driver.

Code: CPP

int ALSoftwareGraphicsDriver::GetAllegroGfxDriverID(bool windowed)
{
#ifdef _WIN32
  if (windowed)
    return GFX_DIRECTX_WIN;
  return GFX_DIRECTX;
#else
  if (windowed)
    return GFX_AUTODETECT_WINDOWED;
  return GFX_AUTODETECT_FULLSCREEN;
#endif
}


I'm not positive that GFX_AUTODETECT_* count as magic drivers, but I think it would be safe to assume so, since I just tested this, and sure enough, _gfxModeList is null.  We need another method (preferably standard to X) of discovering the modes.  I'm researching this now  :)

s_d

BigMC, it appears that the crash here:

Code: LOG

AGS: Attempt to switch gfx mode to 384 x 240 (32-bit)
X Error of failed request:  BadValue (integer parameter out of range for operation)
  Major opcode of failed request:  150 (XFree86-VidModeExtension)
  Minor opcode of failed request:  10 (XF86VidModeSwitchToMode)


...is fundamentally caused because the Allegro auto-detect driver has no clue how to respond to X errors.  I read over the list of available Allegro graphics drivers, and saw that there is a native X windows driver (GFX_XWINDOWS).

So, I thought to try it out, and it Works For MeTM, including a properly returned list of modes.  I authored an awful patch on my github with some additional debugging (I co-opted the editor debugging options to add a verbosity control).  Now, the recursive mode-switching behaviour occurs, where the X mode-switch calls occur, and when the return failed, it iterates over the candidate resolutions and seems to finally settle on one:

Code: LOG
AGS: Switching to graphics mode
AGS: Widescreen side borders: game resolution: 320 x 200; desktop resolution: 1366 x 768
AGS: Widescreen side borders: disabled (not necessary, game and desktop aspect ratios match)
AGS: Attempt to switch gfx mode to 320 x 200 (16-bit)
AGS: Failed, resolution not supported
AGS: Widescreen side borders: game resolution: 320 x 240; desktop resolution: 1366 x 768
AGS: Widescreen side borders: gfx card does not support suitable resolution. will attempt 426 x 240 anyway
AGS: Attempt to switch gfx mode to 426 x 240 (16-bit)
AGS: Failed, resolution not supported
AGS: Attempt to switch gfx mode to 320 x 240 (16-bit)
AGS: Failed, resolution not supported
AGS: Widescreen side borders: game resolution: 320 x 200; desktop resolution: 1366 x 768
AGS: Widescreen side borders: disabled (not necessary, game and desktop aspect ratios match)
AGS: Attempt to switch gfx mode to 320 x 200 (15-bit)
AGS: Failed, resolution not supported
AGS: Widescreen side borders: game resolution: 320 x 240; desktop resolution: 1366 x 768
AGS: Widescreen side borders: gfx card does not support suitable resolution. will attempt 426 x 240 anyway
AGS: Attempt to switch gfx mode to 426 x 240 (15-bit)
AGS: Failed, resolution not supported
AGS: Attempt to switch gfx mode to 320 x 240 (15-bit)
AGS: Failed, resolution not supported
AGS: 320x200 not supported, trying with 2x filter
AGS: Widescreen side borders: game resolution: 320 x 200; desktop resolution: 1366 x 768
AGS: Widescreen side borders: disabled (not necessary, game and desktop aspect ratios match)
AGS: Attempt to switch gfx mode to 320 x 200 (16-bit)
AGS: Failed, resolution not supported
AGS: Widescreen side borders: game resolution: 320 x 240; desktop resolution: 1366 x 768
AGS: Widescreen side borders: enabled, attempting resolution 320 x 240
AGS: Attempt to switch gfx mode to 320 x 240 (16-bit)
AGS: Succeeded. Using gfx mode 320 x 240 (16-bit)


I think this more like how it was intended to probe.

In addition to mangling the revision history of my github, this probably probably isn't "pull-request" quality code, but I'd appreciate a second set of eyes to peek at it before I try to send a build out to a tester.  Most of all, I'd like to see how a machine exhibiting this crash works with the X driver, and with "-verbose" so I can see it iterate over the available modes.

BigMc

Allegros documentation says that get_gfx_mode_list() does not guarantee a result, so switching to another driver is still a hack. A better solution would be not to switch resolutions if there is no list of supported resolutions.

s_d

#391
Quote from: BigMc on Mon 25/02/2013 22:21:57
Allegros documentation says that get_gfx_mode_list() does not guarantee a result, so switching to another driver is still a hack. A better solution would be not to switch resolutions if there is no list of supported resolutions.

Yeah.  You're right  :(

Should we allow it to probe the available resolutions (using the X windows driver), since it can at least properly catch the failure return codes, but upon reaching the end of the list unsuccessfully (or if the list is empty) stop it from switching?

EDIT:  On second thought, after looking through the mode fetching code, it looks like XF86VidModeGetAllModeLines() is used to query X for the modelines, and further, the only failures states for Allegro returning that list are if it cannot alloc a buffer for the list.  I think XF86VidMode is no longer an extension, but is now built into Xorg from 2010 or so, and further, if it were missing, then XF86VidModeSwitchToMode wouldn't be supported anyway.  I agree it is a hack, but the more I look, the more it seems like a hack which also is an incremental improvement.  The mode list returned should always contain at least one mode, the current mode.

BigMc

I'm not objecting to any changes, I'm just advising. :)

s_d

#393
Quote from: BigMc on Tue 26/02/2013 22:54:33
I'm not objecting to any changes, I'm just advising. :)

Ah, OK.  Thanks, it's super helpful!

The thing is, I only have one report of "crash on launch" and it's obvious that only the unsupported resolution was attempted.  It is probable that other modes were available, maybe compatible ones, but the first one AGS guessed was wrong.  My understanding is that major differences between the two drivers are XWINDOWS will make X calls to request mode lines, and has a handler for XF86VidModeSwitchToMode() failure return codes (also some caveat regarding hardware cursors), because the "magic" driver is going to use the underlying XWINDOWS driver, anyway.  But it chokes on a bad mode switch and bang, engine quits.  Anyway it's hard to contact this tester (he hasn't responded to me) and I have no other way to reproduce it because AGS is working well everywhere else.  Of course, that's the usual thing with random internet testers :)

So, what I will do is try to prepare a "pretty" patch using the X windows driver.  I will to make it refuse to switch modes if none are returned by get_gfx_mode_list().  A couple of opinions;  do you think it would be good to return false from IsModeSupported() only for Linux?  Right now the mobile ports return early.  Does Windows (or old OS X) need that return true?  Do you think it's worth plumbing up an undocumented "try anyway, shoot yourself in the foot" option to permit blind mode-switching?  I don't know for sure if this behavior is needed somewhere.

BigMc

Quote from: s_d on Wed 27/02/2013 01:15:10
So, what I will do is try to prepare a "pretty" patch using the X windows driver.  I will to make it refuse to switch modes if none are returned by get_gfx_mode_list().  A couple of opinions;  do you think it would be good to return false from IsModeSupported() only for Linux?  Right now the mobile ports return early.  Does Windows (or old OS X) need that return true?  Do you think it's worth plumbing up an undocumented "try anyway, shoot yourself in the foot" option to permit blind mode-switching?  I don't know for sure if this behavior is needed somewhere.

So you will implement the possibility to keep the desktop resolution? Because right now if IsModeSupported() always reports false somewhere (because there is no list of supported modes), then the engine won't start at all, right?

s_d

Quote from: BigMc on Wed 27/02/2013 13:10:55
So you will implement the possibility to keep the desktop resolution? Because right now if IsModeSupported() always reports false somewhere (because there is no list of supported modes), then the engine won't start at all, right?

Well, what choice do I have? :)

Of course Init() will fail (causing init_gfx_mode() to return, and eventually initialize_engine() returns, causing an AGS early exit), but I must somehow satisfy the feature of not requiring a mode switch if none is compatible.

We can either quit (gracefully?) with an AGS error informing the user that no compatible display mode ("Failed, resolution not supported"), which is not the same thing as an improperly handled internal X error, or I can figure out some way to display the game in the middle of the screen in it's native resolution (which is almost certainly smaller than the desktop display resolution).  I'm not really sure how to accomplish that, yet, but I at least have to try.

BigMc

Quote from: s_d on Wed 27/02/2013 21:26:33
We can either quit (gracefully?) with an AGS error informing the user that no compatible display mode ("Failed, resolution not supported"), which is not the same thing as an improperly handled internal X error,
It is effectly the same: AGS won't work in fullscreen mode for people who are affected.

Ok, that can maybe improved by changing the driver, but we are talking about changing the return value of IsModeSupported() now.
Quote
or I can figure out some way to display the game in the middle of the screen in it's native resolution (which is almost certainly smaller than the desktop display resolution).  I'm not really sure how to accomplish that, yet, but I at least have to try.
We still have StdScale filters and the possibility to select a matching one.

s_d

Quote from: BigMc on Wed 27/02/2013 21:47:41
Ok, that can maybe improved by changing the driver, but we are talking about changing the return value of IsModeSupported() now.

Most definitely.  There is no specific reason why the first attempted resolution would happen to be supported, but without changing the driver, it will not try any others.  The next one in the list could have worked!

Quote from: BigMc on Wed 27/02/2013 21:47:41
We still have StdScale filters and the possibility to select a matching one.

In the tester's log output, I don't see the "Automatic scaling: disabled" message printed, so a candidate scaling factor was calculated.  I'll work on this for a while and see what I can do to apply it while in desktop resolution (for starters I'll try skipping engine_init_graphics_mode() and see what happens).

BigMc

Quote from: s_d on Wed 27/02/2013 22:23:06
There is no specific reason why the first attempted resolution would happen to be supported, but without changing the driver, it will not try any others.  The next one in the list could have worked!

But when we talk about changing the return value of IsModeSupported(), we talk about the case where no list of available modes is available and in that case it does not matter how many resolutions are tried, all will fail.

s_d

Before I split the IsModeSupported() test away from set_gfx_mode(), I need to do some testing to be sure I haven't broken other things.  Or, another way I could start would be to hardcode the modelist to NULL on my machine and proceed from there until I can make it properly stay in my desktop resolution, with the correct scaling factor.

SMF spam blocked by CleanTalk