Extending the wayfire-shell-unstable-v2 protocol#3019
Conversation
- Added create_positioned_hotspot request to zwf_output_v2 interface. - Added proximity_changed event to zwf_hotspot_v2 interface. - All hotspots (edge, corner, positioned) will now get proximity_changed signals
- Replaced create_positioned_hotspot(...) with create_custom_hotspot(...)
ammen99
left a comment
There was a problem hiding this comment.
lgtm overall, a few things I noticed, I trust you have tested this :)
| whenever the input pointer moves within the hotspot area. | ||
|
|
||
| Proximity indicates the current distance in pixels from the input pointer | ||
| to the trigger point of the hotspot: |
There was a problem hiding this comment.
I find this wording a bit confusing, is there a 'trigger point of the hotspot'? I thought hotspots can be rectangles.
There was a problem hiding this comment.
Yes, hotspots can be rectangles. I tried to write a general explanation, which was way too confusing. In case of the rectangle, there are simply 2(l+b) trigger points along the perimeter of the rectangle. So, in the rectangular case, the point on the perimeter closest to the mouse is the "trigger point".
I agree that it can be a bit misleading. If you have a better wording, I am happy to incorporate it.
There was a problem hiding this comment.
Proximity is defined as the length of the shortest segment connecting the input pointer to the boundary of the hotspot rectangle or something like it?
There was a problem hiding this comment.
This is what I have come up with:
Proximity is defined as the Euclidean distance in pixels from the input pointer to the hotspot's trigger geometry, clamped to the threshold:
| wf::geometry_t calculate_trigger_geometry(uint32_t edge_mask) | ||
| { | ||
| wf::geometry_t output_geom = this->output->get_layout_geometry(); | ||
| int edge_count = 0; |
There was a problem hiding this comment.
This looks quite inefficient, can't we replace it by __builtin_popcount? We already use these types of functions in other places.
|
|
||
| void send_proximity_if_needed(const wf::point_t& cursor) | ||
| { | ||
| if (wl_resource_get_version(hotspot_resource) < 2) |
There was a problem hiding this comment.
there should be a macro defined as the version in which proximity was defined (in the protocol header created by wayland-scanner).
There was a problem hiding this comment.
I think what you mean is
if (wl_resource_get_version(hotspot_resource) < ZWF_SHELL_HOTSPOT_V2_PROXIMITY_SINCE_VERSION)
Please correct me if I am wrong.
|
|
||
| uint32_t calculate_proximity(const wf::point_t& cursor) const | ||
| { | ||
| const auto& rect = trigger; |
There was a problem hiding this comment.
There is a wlroots function to do this for you :) https://gitlab.freedesktop.org/wlroots/wlroots/-/blob/master/include/wlr/util/box.h?ref_type=heads#L53
There was a problem hiding this comment.
@ammen99 Sure, I could use that. But to what end? I still have to calculate the distance to the box. Which means, I either have to do all the calculations. And I have to handle the edge cases separately: I allow the box to be empty, wlr dose not.
There was a problem hiding this comment.
@marcusbritanicus Sure but to me a code which looks like this:
auto point = closest_point(box, target)
return dist(point, target)
looks way simpler.
Added create_positioned_hotspot request to zwf_output_v2 interface.