I didn't find anywhere more appropriate to post this, so please let me know if it's not welcome here. If you're going to include the following two patches: * [PATCH 1/6] winex11: Use a bit mask for event merge actions. * [PATCH 2/6] winex11: Avoid breaking XIM XFilterEvent / XmbLookupString sequence.
then how about also adding some changes like this on top of it. [0001-winex11-In-process_events-strictly-fix-condition-gli.patch](/uploads/f484bc36c40084e0538ea8be14110674/0001-winex11-In-process_events-strictly-fix-condition-gli.patch)
This will more simply and accurately correspond to the old logic, in faster and more simple way, while retaining the extendability and flexibility you've added for setting the handling/freeing flags on old/new events separately.
### Fixing the branch logic
I saw that you extended the event merging logic so that the developer can now set different policies (keep it, process it, or discard it) for each of the old and new events independently. This I interpreted as for the purpose to make it capable for future possible cases when you need to process both old and new events at once in various patterns like "process both old and new events", "process the old event and discard the new event" and so on.
I analyzed the logic for when to perform `prev_event = event;` and found a little glitch.
The original WINE 8.10 logic would branch to either "(process and) free the old event" `HANDLE/DISCARD` flow or "(process and) free the new event" `KEEP/IGNORE`. And when choosing the "(process and) free the old event" branch by setting the event_merge_action value to `MERGE_HANDLE` or `MERGE_DISCARD`, it would do the copy.
In your new code it would do the copy when `MERGE_FREE_NEW` flag is not set. But instead, the copy should be done when both conditions are met: * The old event is set to be freed * The new event is not set to be freed
So I fixed the code to better reflect the original logic.
### Omitting the FREE flag
Now, by going through the code, setting the HANDLE flag on while not setting the FREE flag on for the same event seemed not going to happen at all in design. Otherwise the same event would be processed again later. This means if an event is set to be handled, then it can always be considered set to be freed too. So I made it when the HANDLE flag is on, the logic also implies to free the event as if the FREE flag was on, without needing to set the FREE flag explicitly on. As you can see, `MERGE_HANDLE_OR_FREE_OLD` and `MERGE_HANDLE_OR_FREE_NEW` translates to logical add of those flags. Testing `action | MERGE_HANDLE_OR_FREE_*` means run the following code (free the event) if either `HANDLE | FREE` flag for the event is set. Now those FREE flags became redundant, thus could be made optional, or further, omitted if HANDLE is on, to simplify the logic. I rewrote the code where those flags `HANDLE | FREE` to only have `HANDLE` instead. (Where only `FREE` flag is set remains as is.) You can still explicitly set both HANDLE and FREE flags on just in case, and the result will be the same.
I hope you'll like it.