Comment 26 for bug 1795135

Revision history for this message
In , Daniel Stone (daniels) wrote :

A few suggestions.

Firstly, Xorg patches are reviewed on the mailing list (<email address hidden>), with patches sent through git send-email; you might want to try that to get some more attention.

Secondly, the changes to CheckVirtualMotion() seem like they would drop fractions on the floor in too many cases. Presumably we only want to do that when the co-ordinates actually hit the limits, rather than unconditionally drop the fractional component even when it's within bounds.

One way to do that would be an explicit check against the limits, and only set ev->root_* to the sprite co-ordinates if they're out of bounds. Alternatively, you could make the sprite's HotSpot struct actually carry the fractional parts as well. I would have a slight preference for the latter, but am not involved in Xorg or review anymore, so if anyone else suggests otherwise, you're probably better off listening to them. :)

I'm curious about the changing of rounding from trunc() to floor(). Previously, -0.86 would be expressed as (0, -0.86), and this changes it to (-1, 0.14). It's not necessarily a bad change, but it is a user-visible one and I'm curious why it's necessary or better.

You might also want to put some tests for negative co-ordinates in test/xi2/protocol-eventconvert.c or similar.