448827 - Shift caret/cursor event coordinate transforms to event generation

Status: RESOLVED FIXED

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[Jeremy Lea]

Attachment: Shift coordinate transform to event generation

Currently the view manager does some tricks with the coordinates returned by various caret and cursor related events, which are in app units, to convert them to device pixels.  This patch moves these conversions to the point where the events are generated.

I have lightly tested this patch.  I think they are using the right underlying device context in each case, but I could be wrong.  I don't know if this fixes any actual rendering bugs (I couldn't find any), but the code is more correct this way.

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Would be great to have the patch -P and some more context (at least 8)

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Also, you should use ScaleRoundOutInverse instead of 1.0/... since division is more accurate than multiplying by the reciprocal. (E.g., 11.0 / 11.0 is always 1.0 but 11.0 * (1.0 / 11.0) may be != 1.0 which can be bad when rounding out...)

Comment 3

[Jeremy Lea]

(In reply to comment #1)
> Would be great to have the patch -P and some more context (at least 8)

The patch was straight out of mercurial MQ...  I thought I had my .hgrc set up right, but if you can tell me the right magic to make it do that, I would appreciate it.  Maybe it's the git format?

Repo is at http://www.openpave.org/cgi-bin/hgweb.cgi

Comment 4

[Jeremy Lea]

(In reply to comment #2)
> Also, you should use ScaleRoundOutInverse instead of 1.0/... since division is
> more accurate than multiplying by the reciprocal. (E.g., 11.0 / 11.0 is always
> 1.0 but 11.0 * (1.0 / 11.0) may be != 1.0 which can be bad when rounding
> out...)

Yes.  I can make the change, although you probably just got email about making it redundant with the dependency I just added.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

I have the following in the .hgrc

[diff]
git = 1

[defaults]
diff = -U 8 -p

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

IMHO you should just roll this patch into bug 448830 so we don't go through an intermediate bad state. Since I'm reviewing the patch in bug 448830 too, I'm pretty sure that won't upset anyone :-).

Comment 7

[Jeremy Lea]

Comment on attachment 331995 [details] [diff] [review]
Shift coordinate transform to event generation

OK, I will do.  I added this as a separate change since it affects things which are not just rounding...

Comment 8

[Jeremy Lea]

Mark as fixed since the patch landed as part of bug 448830.