qt: support non-standard mouse and trackpad wheel events in player view
This makes it possible to use trackpad in order to adjust the volume and time in player view.
Note that this does not bring support for high-precision/fine adjustment. That is handled in !5391.
Merge request reports
Activity
added MRStatus::Reviewable label
changed milestone to %4.0
added Component::Interface: Qt label
mentioned in merge request !5703
298 298 299 299 int qtWheelEventToVLCKey( const QWheelEvent& e ) 300 300 { 301 static QPoint p; this looks like a bad idea to have a static variable here, this means that if you have multiple widgets using this function, when you start scrolling in a different item you'll end up with the leftover from the previous one.
I think we should:
- make this an object that could be attached to the surface
- feed the wheel events
- when you have enough delta for a step emit a signal to notify the scroll steps ( This would not limit the function to a single step per wheel event)
- reset the scroll leftover when the direction changes
Same matter when the user switches mouses (for example, using trackpad and mouse). I was aware of the consequences for using static variable here.
In my opinion, while not ideal, it is acceptable to behave like that. I think we should have a separate merge request in the future if we want to go the way you propose. At least currently, this fixes the issue of not being able to use trackpad scroll.
Having the leftover in another widget would only affect trackpad (not standard mouse), and that is only for the initial action. That is probably not nit-picking, but more close to being "nit" than a real issue.
Follow up @chub? I really prefer this to be merged (considering it is really simple and does not cause maintenance burden and can serve as a foundation for !5782 (merged)), even if !5782 (merged) is going to be merged in the future.
Note that it is currently completely unusable without this change, "end up with the leftover from the previous one" is a significantly less concerning issue, so I don't really get the objection. We have been doing iterative development, if there is a better way to do that, then in the future it can be merged, it does not mean this should be blocked (for months).
I think !5782 (merged) is the right solution for this. Do you have follow up on that MR?
I think !5782 (merged) is the right solution for this. Do you have follow up on that MR?
My latest reply (private) was to proceed with it, because I have not had anything better to offer. I proposed to have event filter and adjust the event, but events don't seem to be possible to be adjusted (I realized that after the discussion). Please also check yourself if it can be done in a more elegant way.
added MRStatus::InReview label and removed MRStatus::Reviewable label
@chub I think I can not handle compressed events or weird mouses here (delta greater than 120). This function can only report a single key. Either the caller needs to handle that (then, it is not relevant to this merge request), or I need to return a list of keys here (I think bad idea).
In fact, if this is a problem, it is already a problem.
mentioned in merge request !5782 (merged)
added MRStatus::Stale label and removed MRStatus::InReview label
added MRStatus::InReview label and removed MRStatus::Stale label
added MRStatus::NotCompliant label and removed MRStatus::InReview label
Closing, since !5782 (merged) is merged now as a substitute.