Closed Bug 448827 Opened 12 years ago Closed 11 years ago

Shift caret/cursor event coordinate transforms to event generation

Categories

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

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: reg, Assigned: reg)

References

Details

Attachments

(1 obsolete file)

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.
Attachment #331995 - Flags: superreview?(roc)
Attachment #331995 - Flags: review?(roc)
Would be great to have the patch -P and some more context (at least 8)
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...)
Blocks: 448830
(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
(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.
I have the following in the .hgrc

[diff]
git = 1

[defaults]
diff = -U 8 -p
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 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...
Attachment #331995 - Attachment is obsolete: true
Attachment #331995 - Flags: superreview?(roc)
Attachment #331995 - Flags: review?(roc)
Mark as fixed since the patch landed as part of bug 448830.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Component: Event Handling → User events and focus handling
You need to log in before you can comment on or make changes to this bug.