-
Notifications
You must be signed in to change notification settings - Fork 551
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Linux/GTK3: add gamepad hotplug support #855
Linux/GTK3: add gamepad hotplug support #855
Conversation
@rofl0r If it's ok, I can port this to GTK2 too, so you can test. |
@@ -4118,6 +4128,7 @@ common_gtk_main(GApplication *app, gpointer user_data) | |||
video->SetFilterParameteri(VF_PARAM_SCANLINE_D, _scanline_filter_d); | |||
|
|||
RedrawScreen(); | |||
g_timeout_add(200, OutOfLoopJoyDeviceCheckTimerFunc, 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'd guess that between plugging in a joystick and moving the mouse to the controls menu, at least a second passes, so polling less often would make sense
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
on second thought, why poll at all ? sdl sends an event when a device is plugged in, so we could simply notify the frontend about that, or am i missing something ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We must read the event from the queue as far as I understand, but when emulation is not started, events are not processed in the loop (EmuLoop function). When EmuLoop starts, this timer fucntion exits.
@@ -39,7 +41,8 @@ u16 nbr_joy; | |||
mouse_status mouse; | |||
static int fullscreen; | |||
|
|||
static SDL_Joystick **open_joysticks = NULL; | |||
//List of currently connected joysticks (key - instance id, value - SDL_Joystick structure, joystick number) | |||
static std::unordered_map<SDL_JoystickID, std::pair<SDL_Joystick *, SDL_JoystickID> > open_joysticks; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems like overkill, currently we support a max of 16 joysticks, which means the joystick id we use fits into one byte... and the average gamer certainly has max 4 joysticks connected so everything would fit into one cache line using a dumb loop over an array. i'm pretty certain the overhead involved in the use of a map for such tiny amount of data is way slower
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
which means the joystick id we use fits into one byte
actually, this could be derived from the array index, so it's for free. we only need to make sure to fix the ordering of the array on insertion/removal of a device
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then I probably just use static array of 16 elements?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, sounds like the best option. in the unlikely case that more than 16 joys are connected, we just have to discard those > 15.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Redone with static array. Note that there's nbr_joy variable declared as extern in ctrlssdl.h that is now only keeps number of joysticks detected at the start, not the actual number of currently connected joysticks, because joysticks can be removed from any element of the array upon disconnect, so frontends should not rely on that variable (though as far as I can tell, they don't).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added int get_number_of_joysticks()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, please proceed and use it in the interface frontend and remove nbr_joys. (just grep posix subdir for nbr_joy to find the file).
with that out of the way i can test it as soon as the gtk2 conversion is done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I also make corresponding changes to gtk2 frontend in my branch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, please
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
} | ||
|
||
free( open_joysticks); | ||
for (auto & it: open_joysticks) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'd really cherish if we could stick to C++03 features for the time being, following the overall style of the existing code and not gratuitously breaking the build for people with older compilers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
open_joysticks is now static array and is iterated with int index.
thanks, i currently don't have time to test this exhaustively so i'll just go ahead and merge as-is. |
Support for reconnecting gamepads and connecting new ones while desmume is already running. For that I added corresponding event processing to ctrlssdl, also modifying how connected joystick info is stored (and fixing joystick uninit in the process: previously open_joysticks array was allocated but never populated, but used in uninit function nevertheless), and how pressed button id is calculated (previously joystick instance id was used but it changes every time gamepad is reconnected, so now joystick index is used instead).
This can be backported to GTK2 but GTK2 is now ahead of my branch after recent commits, so please review this PR as it is for now.