When I wrote the code for computing a displayport rect from displayport margins I was copying the code in AsyncPanZoomController::CalculatePendingDisplayPort. However it appears I made a mistake when copying the ForceInside line http://mxr.mozilla.org/mozilla-central/source/gfx/layers/apz/src/AsyncPanZoomController.cpp?rev=86ffe5dc026c#2630 I likely assumed ForceInside was the same as intersect and didn't check the definition (just like Botond did in bug 1204136, comment 6). Bug 1191539 fixed this mistake by calling ForceInside on the screen rect. But let's just do it once at the end and get rid of the mistaken Intersect call. Also, let's rename ForceInside so we don't make this mistake again.

Open to suggestions on the name, but I think this is much better than ForceInside. No one should confuse the new name with a plain intersect.

Comment on attachment 8685741 [details] [diff] [review] call ForceInside instead of Intersect Review of attachment 8685741 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for getting to the bottom of this! (I thought, after reading bug 1204136 comment 6, that doing both ForceInside and Intersect is strange, but I didn't pursue the issue as that bug was making an orthogonal change.)

Comment on attachment 8685742 [details] [diff] [review] rename ForceInside to MoveInsideAndClamp Review of attachment 8685742 [details] [diff] [review]: ----------------------------------------------------------------- I confess I've had to read the implementation of this function to understand what it does pretty much every time I encountered it. The new name is much better!

https://hg.mozilla.org/integration/mozilla-inbound/rev/64a9ebe5f2b9 https://hg.mozilla.org/integration/mozilla-inbound/rev/c5780f2a10e8

https://hg.mozilla.org/mozilla-central/rev/64a9ebe5f2b9 https://hg.mozilla.org/mozilla-central/rev/c5780f2a10e8