Many thanks for this PR. The code for this looks very reasonable, however I think we need some extra documentation to go along with this.

In particular the following comment on glfwSetMouseButtonCallback is not true for buttons beyond GLFW_MOUSE_BUTTON_LAST:

 *  When a window loses input focus, it will generate synthetic mouse button
 *  release events for all pressed mouse buttons.  You can tell these events
 *  from user-generated events by the fact that the synthetic ones are generated
 *  after the focus loss event has been processed, i.e. after the
 *  [window focus callback](@ref glfwSetWindowFocusCallback) has been called.

Additionally although the documentation suggests that events might have button presses for none named buttons it's not explicitly stated, which I think we should.

If you'd like to have a go at the documentation please do, otherwise I can handle it.

One potential concern I do have is that developers may have assumed that the largest mouse button is GLFW_MOUSE_BUTTON_LAST and implemented their own state arrays up to that value without checks. This change would break their code. If we want to avoid that then we would need an initialization hint to control permitting button events > GLFW_MOUSE_BUTTON_LAST. I am currently not sure if that is required - I think @elmindreda should make that call.

Alternatively to this solution, we could move the button array to a 32bit bitfield (without changing the external API), supporting up to 32 buttons and improving the performance of some internal button test checks which loop through all buttons (thought that's not a big hit given human input speeds). However I think that change could be done after this PR was integrated.

Thanks for taking a look at this.

In particular the following comment on glfwSetMouseButtonCallback is not true for buttons beyond GLFW_MOUSE_BUTTON_LAST: [...]

That's true, so I thought I would take a look at the keyboard input documentation for reference. After all, the synthetic key release events aren't generated for non-named keys either. Here's what I found:

When a window loses input focus, it will generate synthetic key release events for all pressed keys.

It's not correct there either. Fixing it here and not there would be weird, so I'm going to make a separate MR that'll specify this.

Additionally although the documentation suggests that events might have button presses for none named buttons it's not explicitly stated, which I think we should.

Good idea.

One potential concern I do have is that developers may have assumed that the largest mouse button is GLFW_MOUSE_BUTTON_LAST and implemented their own state arrays up to that value without checks. This change would break their code.

Yeah, I have thought that might be the case. After all technically non-breaking changes can still break code.

I opened #2433 which will fix the documentation for synthetic key releases.

Given this effectively changes the API we need an init hint similar to the GLFW_JOYSTICK_HAT_BUTTONS initialization hint.

https://www.glfw.org/docs/3.3/intro_guide.html#init_hints

If you don't have time to add this let me know and I can do the work.

I surely took my time to do this, but here it is. Documentation changes included.

Thanks for the changes - I've noticed one line I think needs to be added to the documentation.

The commit message should have been "unset" but whatever.

Many thanks for this work.

Apologies for not spotting this earlier but going over the input functionality it looks like another option for setting the GLFW_MOUSE_BUTTON_LIMIT might be via glfwSetInputMode as this is somewhat similar to how GLFW_LOCK_KEY_MODS operates. What do you think? If we want to use glfwSetInputMode I can do the work if needed.

Also, named keys were renamed to key tokens in Improve documentation relating to key tokens on 5th Dec. If we want to keep the same naming style we could move from named mouse buttons to mouse button tokens. However we can move to the required naming later but would appreciate your thoughts on the mouse button term.

In completing this PR we will also need the steps as Contributing a feature, namely:

• Change log entry in README.md, listing all new symbols. We need to add the new GLFW_MOUSE_BUTTON_LIMIT symbol and brief explanation. Update 2024-02-17: DONE
• News page entry, briefly describing the feature. TODO. Update 2024-02-17: DONE
• Guide documentation, with minimal examples, in the relevant guide. Need to edit the Mouse button input section of docs/input.dox. See the current Mouse button input docs. Update 2024-02-17: DONE
• Reference documentation, with all applicable tags. Done for the current status, but if we switch to glfwSetInputMode needs reworking Update 2024-02-17: DONE
• Cross-references and mentions in appropriate places. Done for the current status, but if we switch to glfwSetInputMode needs reworking Update 2024-02-17: DONE
• Credits entries for all authors of the feature. DONE

I think we should also probably modify events.c to set GLFW_MOUSE_BUTTON_LIMIT off so it receives all events for mouse buttons.

Once again I can do the work if you don't have the time.

Apologies for not spotting this earlier but going over the input functionality it looks like another option for setting the GLFW_MOUSE_BUTTON_LIMIT might be via glfwSetInputMode as this is somewhat similar to how GLFW_LOCK_KEY_MODS operates. What do you think? If we want to use glfwSetInputMode I can do the work if needed.

As I'm not an active contributor, I don't know for sure when the one or the other is a better choice.

Trying to analyze it, the practical difference seems to be that init hints affect a particular window and not the whole program. The semantic difference on the other hand seems to be that input modes are used for changing preferences regarding input, where init hints are overall changes to the behavior of the library.

When taking this into account, I'd say making this an init hint makes more sense. It's not really a preference, more a difference between the old and new behaviour. Moreover, it doesn't seem useful to allow it to be set for one window but not for another.

Also, named keys were renamed to key tokens in Improve documentation relating to key tokens on 5th Dec. If we want to keep the same naming style we could move from named mouse buttons to mouse button tokens. However we can move to the required naming later but would appreciate your thoughts on the mouse button term.

I noticed that there were some discrepancies in the naming while working on #2417, so it's good that this has been taken care of. I indeed think that it'd be better for the naming to be consistent between keyboard and mouse input.

In completing this PR we will also need the steps as Contributing a feature, namely:

This was originally supposed to be classified as a bugfix, that's why I didn't do that before 😅. But yeah, now that it has documentation and new symbols it's pretty much a feature.

• Change log entry in README.md, listing all new symbols. We need to add the new GLFW_MOUSE_BUTTON_LIMIT symbol and brief explanation.

• News page entry, briefly describing the feature. TODO

• Guide documentation, with minimal examples, in the relevant guide. Need to edit the Mouse button input section of docs/input.dox. See the current Mouse button input docs

Gonna work on these now, assuming the current usage of init hints. If it's decided that input modes are better, the change can be a single commit with both code and documentation changes.

Looking at the documentation for mouse buttons to update it made me notice the version on my branch is outdated, so I took a second look at 9959dc6:

The associated term 'named mouse button' was also replaced with 'supported mouse button'.

Seems like a change is already in effect. I still think using the same term for mouse buttons and keyboard keys would be a good idea though. For now I'll rebase and update to use the term 'supported' though.

I'd like some feedback on my wording in c025d54. I'm not a native speaker and sometimes have trouble deciding if what I wrote is understandable.

While adding information to the input guide I also wanted to add that the mouse button values that are not supported (or named, or don't have tokens) are platform specific, like scancode values in case of keyboard input. But then I noticed that different platforms could also return different values for the buttons GLFW_MOUSE_BUTTON_4 and over as well, unless this was tested (I tested X11, Wayland and Windows (by testing the Win32 API calls GLFW uses, my VM can't run GLFW programs), but I don't have access to a macOS enviroment). Not sure how to handle that properly...

Saving the rest for later, I need to get some sleep.

Some feedback:

Mouse button naming

The 'supported mouse buttons' naming is misleading, we would prefer 'mouse buttons tokens' so the sentence release events for all pressed supported mouse buttons would become release events for all pressed mouse buttons with mouse buttons tokens Optionally we could be precise and add

/*
* ...
* release events for all pressed mouse buttons with mouse buttons tokens (`GLFW_MOUSE_BUTTON_1` to
* `GLFW_MOUSE_BUTTON_LAST`).
* ...
*/

GLFW_MOUSE_BUTTON_LIMIT

After communicating with elmindreda we think that it may be worth considering making it an input mode instead of an init hint. It feels a little bit like GLFW_LOCK_KEY_MODS.

So this would be part of glfwSetInputMode.

glfwSetInputMode(window,GLFW_MOUSE_BUTTON_LIMIT, GLFW_FALSE);

Done. I still think it feels out of place as an input mode though.

It might be more intuitive if it's purpose was inverted - instead of disabling the old handling, it'd enable the new one, and it'd be called GLFW_DISABLE_MOUSE_BUTTON_LIMIT.

I concur. Do you want to change to using GLFW_DISABLE_MOUSE_BUTTON_LIMIT?

Did that just now.

Thanks!

I've not done a full review but have added some comments.

I also note that we need to add GLFW_DISABLE_MOUSE_BUTTON_LIMIT to the comments above glfwSetInputMode in glfw3.h.

I can merge and make these changes if you don't have time.

I have squash merged your commits into my branch:

master...dougbinks:glfw:unlimited-mouse-buttons-merge

This way I can make any required minor changes on this branch and merge it to GLFW once complete.

Sorry I didn't manage to find the time to do it. I was planning to do it this Friday...

No problem - due to the release of GLFW 3.4 there was a merge which wasn't obvious, so I thought it best to handle this way. Hope that's OK!

After some discussions we decided that the term GLFW_UNLIMITED_MOUSE_BUTTONS was preferable and also followed the pattern of the rest of the GLFW API, so I've changed over to using that and also altered any documentation to align with this.

dougbinks/glfw@master...unlimited-mouse-buttons-merge

This has now been merged into main.

Many thanks for your work on the PR, and your patience with my reviewing!